Repository navigation
[#1090] Treated the 'anonymous' role as a logout in 'I am logged in as a user with the role(s) :roles'. - #1094
Conversation
…s a user with the role(s) :roles'.
…role is assigned.
…step, not only denied access.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe Drupal user trait now detects anonymous-only role lists, logs out instead of creating an account for those requests, and rejects anonymous roles when they appear with other roles or during account creation. Tests and documentation cover the updated role handling. ChangesAnonymous role handling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant BehatStep
participant UserTrait
BehatStep->>UserTrait: userLoginWithRoles(roles)
UserTrait->>UserTrait: userIsAnonymousRole(roles)
alt role list contains only anonymous
UserTrait->>UserTrait: userLogoutSession()
else other role list
UserTrait->>UserTrait: userCreateAndLogin(roles)
end
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 2 files. (5 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the primary change: treating the "anonymous" role as a logout in the specified user-login step. It is specific and relevant to the changeset. Linked Issues check Issue #1090requires the role step to treat exactlyanonymousoranonymous useras an anonymous session, including case variants, and to reject mixed lists.userLoginWithRoles()calls `authLogou…Out of Scope Changes check The changes stay within issue #1090. The parser and assignment changes prevent invalid anonymous roles in user creation paths. The PHPUnit tests, Behat scenarios, and documentation support the request…Full details: Linked Issues check
-
Explanation
Issue
#1090requires the role step to treat exactlyanonymousoranonymous useras an anonymous session, including case variants, and to reject mixed lists.userLoginWithRoles()callsauthLogout(TRUE)and returns whenuserIsAnonymousRole()matches.userIsAnonymousRole()handles both names case-insensitively.userParseRoles()rejects anonymous roles before account creation or role assignment. PHPUnit and Behat coverage tests these cases. The migration and step documentation also describe the behavior.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#1090. The parser and assignment changes prevent invalid anonymous roles in user creation paths. The PHPUnit tests, Behat scenarios, and documentation support the requested role and logout behavior. No unrelated product behavior is demonstrated.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 2 files. (5 skipped: 5 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checked the role list twice,
“Anonymous means logout,” said it nice.
Empty commas hop away,
Authenticated roles don’t stay,
And mixed roles earn a parsing slice.
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1094 +/- ##
==========================================
+ Coverage 99.22% 99.23% +0.01%
==========================================
Files 180 180
Lines 9570 9583 +13
==========================================
+ Hits 9496 9510 +14
+ Misses 74 73 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
added
the
Needs review
Closes #1090
Summary
Given I am logged in as a user with the role "anonymous"now logs the current user out, the same wayGiven I am an anonymous userdoes.anonymous userand any capitalization of either work too, so a Scenario Outline can listanonymousin its Examples table beside the real roles.On 4.0.0-rc2 that row failed at the login step with Drupal's
Anonymous or authenticated role ID must not be assigned manually.(InvalidArgumentException), becauseUserTrait::userLoginWithRoles()created an account and tried to assignanonymousto it.Given I am an anonymous usertakes no argument, so an Examples row couldn't pick it, and every access-matrix outline had to be split up or the step overridden inFeatureContext.When
anonymoussits beside other roles ("anonymous, editor"), or shows up in the... and I have the following fields:step or arolescolumn ofGiven the following users exist:, the step fails withThe role "anonymous" cannot be assigned to a user.(\RuntimeException). It fails before any account is saved or any role is assigned, on both the Drupal and Drush backends. The step text and every other role stay as they were.Before / After
Changes
Step and helpers (
src/Steps/Drupal/UserTrait.php)userLoginWithRoles()callsauthLogout(TRUE)and returns whenuserIsAnonymousRole()holds; otherwise it creates and logs in a user as before.userIsAnonymousRole()(new, public) is TRUE when the list holds exactly 1 role,anonymousoranonymous user, in any case.userParseRoles()(new, public) turns a role list into the roles an account can be assigned: it dropsauthenticatedand empty entries, and throws onanonymous.userAssignRoles()assigns the parsed roles, so a list naminganonymousassigns none of them.userCreate()anduserCreateAndLogin()parse the list before saving the account.Tests
tests/phpunit/src/Unit/Steps/Drupal/UserTraitTest.php(new) coversuserIsAnonymousRole(),userParseRoles()anduserAssignRoles(), includinganonymousbefore and after another role.tests/behat/features/drupal_user.featureadds the access-matrix outline from the issue (administrator,authenticated,anonymous,anonymous user,Anonymous). It also adds an outline that logs in asadministrator, switches to each anonymous spelling and asserts thatuser/loginstays on/user/login. A logged-in user gets redirected to their profile there, so this tells a real logout apart from a non-admin account. 2@test-traitscenarios cover the mixed list and arolescolumn ofanonymous user.Docs
MIGRATION.mddescribes theanonymousrole as logging out again in the 3.14 step table and theOverrideTraitmethod table.STEPS.md,HELPERS.mdanddocs/content/drupal-steps/user.mdare regenerated withahoy update-docs.Summary
UserTraitnow treatsanonymousandanonymous useras a request to log out when either is the only role specified, regardless of capitalization.Step-definition review
No step-definition rule violations were identified in the inspected definitions. The change adds helper methods and tests; it does not add a step definition.