Fix wildcard unknown-action redirect, isPublic scope leak and hasRole juggling (3.x)#177
Merged
Merged
Conversation
…e leak and hasRole juggling Port the 3.x (CakePHP 4) line up to parity with the master fix that the 3.2.1 release was meant to ship but landed on the wrong (cake3) branch. Wildcard fix: a `*` allow rule is expanded to the controller's concrete method list via get_class_methods() so it can be handed to the Authentication plugin, which has no wildcard support. An action that does not exist as a method was dropped from the unauthenticated set, so unauthenticated users hit the identity check and got redirected to login instead of receiving the MissingActionException (404) the missing action should produce. Keep the current request action allowed under a wildcard rule so unknown actions fall through to MissingActionException, matching the old AuthComponent behavior. Explicit denies still take precedence. isPublic scope leak: AclTrait::_isPublic() used asymmetric plugin/prefix guards that short-circuited when the request had no plugin or prefix, so a plugin- or prefix-scoped allow rule would match a request without one. Use symmetric checks so a rule is skipped whenever either side has a scope and they do not match. (AllowTrait already carries the symmetric guard here.) hasRole type juggling: AuthUserTrait::hasRole() used a loose in_array() that matched 0 against any non-numeric string and treated auto-int array keys as alias matches. Coerce both sides to string and compare strictly while still treating numeric-string and int as the same role id; only string array keys count as alias matches. Reject non-scalar inputs early.
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
1 participant
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.
Problem
The
3.2.1release was meant to ship the missing-action wildcard fix (and the isPublic/hasRole fixes) to the CakePHP 4 line, but it was tagged on thecake3branch (CakePHP 3,cakephp/cakephp ^3.7) instead ofcake4. Every other3.xtag lives oncake4(CakePHP 4), so3.2.1is an anomaly: a CakePHP 4 app cannot even install it. The fixes therefore never reached the3.xline that users of3.2.0actually run.Reported in issue 173.
What this does
Ports the fixes onto
cake4so the3.xline reaches parity with the master (5.x) release:*allow rule, an action that does not exist as a controller method is kept in the unauthenticated set, so an unknown action falls through toMissingActionException(404) for guests instead of a login redirect. Explicit denies still win.AclTrait::_isPublic()plugin/prefix guards are now symmetric, so a plugin- or prefix-scoped rule no longer matches a request without one. (AllowTraitalready carried the symmetric guard on this branch.)AuthUserTrait::hasRole()compares strictly after string coercion, preserving numeric-string/int equivalence while rejecting0 == 'admin'-style false positives and auto-int alias keys.Regression tests added for all three paths.
Gates
phpunit (103 tests), phpstan, phpcs all green locally.