Skip to content

Commit

Permalink
Prevent write operations based on instance and column's readonly status
Browse files Browse the repository at this point in the history
Closes #1684
  • Loading branch information
mshibuya committed Jan 16, 2022
1 parent 0c7bc61 commit 9cd7541
Show file tree
Hide file tree
Showing 16 changed files with 122 additions and 12 deletions.
8 changes: 4 additions & 4 deletions app/views/rails_admin/main/_submit_buttons.html.erb
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
<div class="form-group form-actions">
<div class="col-sm-offset-2 col-sm-10">
<input name="return_to" type="<%= :hidden %>" value="<%= (params[:return_to].presence || request.referer) %>"></input>
<button class="btn btn-primary" data-disable-with="<%= t("admin.form.save") %>" name="_save" type="submit">
<button class="btn btn-primary" data-disable-with="<%= t("admin.form.save") %>" name="_save" type="submit"<%= ' disabled' unless @action.enabled? %>>
<i class="fas fa-check"></i>
<%= t("admin.form.save") %>
</button>
<span class="extra_buttons">
<% if authorized? :new, @abstract_model %>
<% if @action.enabled? && authorized?(:new, @abstract_model) %>
<button class="btn btn-info" data-disable-with="<%= t("admin.form.save_and_add_another") %>" name="_add_another" type="submit">
<%= t("admin.form.save_and_add_another") %>
</button>
<% end %>
<% if authorized? :edit, @abstract_model %>
<button class="btn btn-info" data-disable-with="<%= t("admin.form.save_and_edit") %>" name="_add_edit" type="submit">
<% if @action.enabled? && authorized?(:edit, @abstract_model) %>
<button class="btn btn-info" data-disable-with="<%= t("admin.form.save_and_edit") %>" name="_add_edit" type="submit"<%= ' disabled' unless @action.enabled? %>>
<%= t("admin.form.save_and_edit") %>
</button>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_admin/adapters/active_record/property.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def association?
end

def read_only?
false
model.readonly_attributes.include? property.name.to_s
end

private
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_admin/adapters/mongoid/property.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def association?
end

def read_only?
false
model.readonly_attributes.include? property.name.to_s
end

private
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_admin/config/actions/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Base
(only.nil? || [only].flatten.collect(&:to_s).include?(bindings[:abstract_model].to_s)) &&
![except].flatten.collect(&:to_s).include?(bindings[:abstract_model].to_s) &&
!bindings[:abstract_model].config.excluded?
)
) && (!respond_to?(:writable?) || writable?)
end

register_instance_option :authorized? do
Expand Down
4 changes: 4 additions & 0 deletions lib/rails_admin/config/actions/delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ class Delete < RailsAdmin::Config::Actions::Base
register_instance_option :link_icon do
'fas fa-times'
end

register_instance_option :writable? do
!(bindings[:object] && bindings[:object].readonly?)
end
end
end
end
Expand Down
4 changes: 4 additions & 0 deletions lib/rails_admin/config/actions/edit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ class Edit < RailsAdmin::Config::Actions::Base
register_instance_option :link_icon do
'fas fa-pencil-alt'
end

register_instance_option :writable? do
!(bindings[:object] && bindings[:object].readonly?)
end
end
end
end
Expand Down
5 changes: 5 additions & 0 deletions lib/rails_admin/config/actions/new.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class New < RailsAdmin::Config::Actions::Base
if request.get? # NEW

@object = @abstract_model.new
@action = @action.with(@action.bindings.merge(object: @object))
@authorization_adapter&.attributes_for(:new, @abstract_model)&.each do |name, value|
@object.send("#{name}=", value)
end
Expand Down Expand Up @@ -55,6 +56,10 @@ class New < RailsAdmin::Config::Actions::Base
register_instance_option :link_icon do
'fas fa-plus'
end

register_instance_option :writable? do
!(bindings[:object] && bindings[:object].readonly?)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_admin/config/fields/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def eager_load_values
end

def editable?
!(@properties && @properties.read_only?)
!((@properties && @properties.read_only?) || (bindings[:object] && bindings[:object].readonly?))
end

# Is this an association
Expand Down
5 changes: 5 additions & 0 deletions spec/dummy_app/app/active_record/read_only_comment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class ReadOnlyComment < Comment
def readonly?
true
end
end
5 changes: 5 additions & 0 deletions spec/dummy_app/app/mongoid/read_only_comment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class ReadOnlyComment < Comment
def readonly?
true
end
end
3 changes: 3 additions & 0 deletions spec/helpers/rails_admin/form_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
(@object = Player.new).save
@builder = RailsAdmin::FormBuilder.new(:player, @object, helper, {})
allow(@builder).to receive(:field_for).and_return('field')
action = double
allow(action).to receive(:enabled?).and_return true
helper.instance_variable_set :@action, action
end

it 'does not add additional error div from default ActionView::Base.field_error_proc' do
Expand Down
8 changes: 8 additions & 0 deletions spec/integration/actions/edit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,14 @@ class HelpTest < Tableless
end
end

context 'with a readonly object' do
let(:comment) { FactoryBot.create :comment, (CI_ORM == :mongoid ? {_type: 'ReadOnlyComment'} : {}) }

it 'raises ActionNotAllowed' do
expect { visit edit_path(model_name: 'read_only_comment', id: comment.id) }.to raise_error 'RailsAdmin::ActionNotAllowed'
end
end

context 'with missing label', given: ['a player exists', 'three teams with no name exist'] do
before do
@player = FactoryBot.create :player
Expand Down
15 changes: 15 additions & 0 deletions spec/integration/actions/new_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,19 @@
is_expected.to have_content('Player is cheating')
end
end

context 'with a readonly object' do
it 'shows non-editable form' do
RailsAdmin.config do |config|
config.model ReadOnlyComment do
edit do
field :content
end
end
end
visit new_path(model_name: 'read_only_comment')
is_expected.not_to have_css('textarea[name="read_only_comment[content]"]')
is_expected.to have_css('button[name="_save"]:disabled')
end
end
end
14 changes: 14 additions & 0 deletions spec/rails_admin/adapters/active_record/property_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,18 @@
expect(subject.serial?).to be_falsey
end
end

describe '#read_only?' do
before do
class HasReadOnlyColumn < Tableless
column :name, :varchar
attr_readonly :name
end
end

it 'returns correct values' do
expect(RailsAdmin::AbstractModel.new('Player').properties.detect { |f| f.name == :name }).not_to be_read_only
expect(RailsAdmin::AbstractModel.new('HasReadOnlyColumn').properties.detect { |f| f.name == :name }).to be_read_only
end
end
end
15 changes: 15 additions & 0 deletions spec/rails_admin/adapters/mongoid/property_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,19 @@ class CustomValiated
expect { RailsAdmin::AbstractModel.new('CustomValiated').properties.last.send(:length_validation_lookup) }.not_to raise_error
end
end

describe '#read_only?' do
before do
class HasReadOnlyColumn
include Mongoid::Document
field :name, type: String
attr_readonly :name
end
end

it 'returns correct values' do
expect(RailsAdmin::AbstractModel.new('Player').properties.detect { |f| f.name == :name }).not_to be_read_only
expect(RailsAdmin::AbstractModel.new('HasReadOnlyColumn').properties.detect { |f| f.name == :name }).to be_read_only
end
end
end
40 changes: 36 additions & 4 deletions spec/rails_admin/config/actions/base_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

RSpec.describe RailsAdmin::Config::Actions::Base do
describe '#visible?' do
describe '#enabled?' do
it 'excludes models not referenced in the only array' do
RailsAdmin.config do |config|
config.actions do
Expand All @@ -10,9 +10,9 @@
end
end
end
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Player))).to be_visible
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Player))).to be_enabled
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Team))).to be_nil
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Cms::BasicPage))).to be_visible
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Cms::BasicPage))).to be_enabled
end

it 'excludes models referenced in the except array' do
Expand All @@ -24,8 +24,40 @@
end
end
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Player))).to be_nil
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Team))).to be_visible
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Team))).to be_enabled
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Cms::BasicPage))).to be_nil
end

it 'is always true for a writable model' do
RailsAdmin.config do |config|
config.actions do
index
show
new
edit
delete
end
end
%i[index show new edit delete].each do |action|
expect(RailsAdmin::Config::Actions.find(action, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(Player), object: Player.new)).to be_enabled
end
end

it 'is false for write operations of a read-only model' do
RailsAdmin.config do |config|
config.actions do
index
show
new
edit
delete
end
end
expect(RailsAdmin::Config::Actions.find(:index, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(ReadOnlyComment))).to be_enabled
expect(RailsAdmin::Config::Actions.find(:show, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(ReadOnlyComment), object: ReadOnlyComment.new)).to be_enabled
expect(RailsAdmin::Config::Actions.find(:new, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(ReadOnlyComment))).to be_enabled
expect(RailsAdmin::Config::Actions.find(:edit, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(ReadOnlyComment), object: ReadOnlyComment.new)).to be_nil
expect(RailsAdmin::Config::Actions.find(:delete, controller: double(authorized?: true), abstract_model: RailsAdmin::AbstractModel.new(ReadOnlyComment), object: ReadOnlyComment.new)).to be_nil
end
end
end

0 comments on commit 9cd7541

Please sign in to comment.