In permitted_attributes, switch from database column detection to Rails attributes detection. Addresses #838
This pull request addresses #838 and makes Cancancan "see" attributes that are not backed by the database. It enables code like the following:
class Foo < ApplicationRecord
attribute :bar
end
With this pull request, cancancan's permitted_attributes will "see" and include :bar even when there is no corresponding column in the database.
Reasonning why this is a good idea: When implementing a form and checking for per-attribute authorization, there are often more fields in the form than are actually stored. An example would be a checkbox "I agree to the terms of use" which is checked by the controller and then discarded if checked, validation error otherwise. If cancancan ignores this attribute (since it's not DB backed), a form whitelisting attributes by using cancancan's permitted_attributes would miss on this field.
This becomes particularly important when using cancancan together with Gems like active_type.
Hi @coorasse, it was a pleasure meeting you IRL :-)
Thank you for your suggestions regarding testing. You've asked me to implement a failing "before" test, as well as a passing "after" test. However, it's a bit trickier than that because of the following situation:
- The current test verifying that column names are read correctly by CanCanCan creates a method
column_nameson anActiveRecord::Baseand checks that they are read back correctly. - However, this method does not affect
attribute_nameswhich is inconsistent with Rails' actual behavior: when loading the columns, Rails automatically adds each column underattribute_names, see also this StackOverflow question regarding the topic. - Therefore, while this PR works in real life, the existing test would fail.
Therefore, I modified the PR to consist of three commits:
- 1edfe71 modifies the test to actually perform testing on the DB instead of working only in-memory, enabling the needed Rails behavior of propagating column names to attributes.
- 4936716 modifies the test such that it will fail with the current implementation, demonstrating that attributes are not correctly handled in 3.5.0.
- f68d752 adds the fix that makes the test pass. This is the commit this PR is for.
I hope that these suggested code changs are to your liking and that the PR can be merged to improve CanCanCan as discussed :-)
Have a nice prolonged week-end! Best, Kalsan