fix: prevent accessible_by from permanently overwriting shared rule conditions#892
Open
SAY-5 wants to merge 1 commit into
Open
fix: prevent accessible_by from permanently overwriting shared rule conditions#892SAY-5 wants to merge 1 commit into
SAY-5 wants to merge 1 commit into
Conversation
…ng shared conditions ConditionsNormalizer replaces rule.conditions with a newly computed hash for each model class. When rules_compressor_enabled is false (the default fallback), @compressed_rules references the same Rule objects stored in the ability's shared @rules. Calling accessible_by for one model class permanently overwrites conditions on those shared Rule objects, so subsequent can? checks for other models using the same rule fail or raise NoMethodError. Fix by mapping @compressed_rules through Rule#dup before passing to the normalizers. The dup gives each adapter its own copy of the Rule shell so that conditions= writes are local and never touch the originals. Fixes CanCanCommunity#876 Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this fix?
Calling
accessible_byfor one model class permanently overwritesrule.conditionson the sharedRuleobjects stored in the ability's@rules. A subsequentcan?check for a different model that shares the same rule then uses the already-normalized conditions, which may reference associations that do not exist on that model, causingNoMethodErroror wrong results.Reproduction (from #876)
Fix
ActiveRecordAdapter#initializenow maps@compressed_rulesthroughRule#dupbefore passing toStiNormalizerandConditionsNormalizer. The dup gives each adapter its own copy of the rule shell, soconditions=assignments from the normalizers are local and never touch the originals in@rules.Tests
Added a regression spec to
conditions_normalizer_spec.rb:rule.conditionsis unchanged afteraccessible_byis called (object identity preserved)All 305 examples pass (0 failures, 2 pending) across both defined and random order.
Closes #876