Skip to content

feat(gonsole/auth): roles hook from a role registry, and the lost role rule - #102

Merged
SirLouen merged 2 commits into
mainfrom
feat/99
Oct 18, 2026
Merged

SirLouen merged 2 commits into
mainfrom
feat/99

Conversation

@SirLouen

@SirLouen SirLouen commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Closes #99

What

  • DeclaredRoles returns the account commands' roles hook from a role registry: it loads the call's plugins, refuses a failed load, then answers the registry's roles with the holders of one capability privileged. It takes any registry answering Roles, CapabilitiesOf and HoldersOf, so gonsole/auth imports no registry module.
  • DeclaredRoles fails when no role carries the capability, so a misspelled capability can never switch the last privileged account guard off.
  • The role and disable commands follow authkit's reach rule for an account whose stored role nobody declares: its role may change and it may be disabled, but re-enabling it is refused until it holds a declared role.
  • Both commands write through SetUserRoleFrom and SetUserDisabledFrom, so a role that moved between the check and the write is refused with a clear message.

Why

Applications built the roles hook by hand with the same lines, and the commands answered a lost role and a moved role differently from the HTTP admin.

Testing Instructions

  1. GOWORK=off go test -race ./... in gonsole/auth: every package passes.
  2. declaredroles_test.go: the hook after the plugins, a failed plugin load, a capability no role carries refused and the last privileged account kept.
  3. lostrole_test.go and checkedwrite_test.go: the lost role rule and the moved role refusal through the commands.
  4. The two engine modules pass against this change.

Summary by CodeRabbit

  • New Features

    • Role-based authorization can now use roles declared by registered plugins and identify roles with specific capabilities.
  • Bug Fixes

    • Accounts with no role or an undeclared role can no longer be enabled until assigned a declared role.
    • Account role changes, enablement, and disablement are refused if the account’s role changes after it has been checked.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b57b9476-1692-424f-b4b3-06dcb9775c5d
📥 Commits

Reviewing files that changed from the base of the PR and between 42aa9be and 431e551.

📒 Files selected for processing: 10
  • gonsole/auth/CHANGELOG.md
  • gonsole/auth/checkedwrite_test.go
  • gonsole/auth/declaredroles.go
  • gonsole/auth/declaredroles_test.go
  • gonsole/auth/lostrole_test.go
  • gonsole/auth/reach.go
  • gonsole/auth/reach_test.go
  • gonsole/auth/role.go
  • gonsole/auth/rolesfrom_test.go
  • gonsole/auth/standing.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The change adds a plugin-aware roles hook and updates account commands to validate declared roles. Enabling is refused for accounts with no declared role. Role and standing writes also fail if the account’s role changes after it is read.

Changes: Authentication roles and account commands

Layer / File(s) Summary
Build roles from the plugin registry
gonsole/auth/declaredroles.go, gonsole/auth/declaredroles_test.go, gonsole/auth/rolesfrom_test.go
DeclaredRoles loads plugins, derives roles from registry capability holders, and reports errors for plugin failures or capabilities with no holders. Tests cover declared roles and use of the hook in account role commands.
Validate account roles and guard writes
gonsole/auth/reach.go, gonsole/auth/role.go, gonsole/auth/standing.go, gonsole/auth/reach_test.go, gonsole/auth/lostrole_test.go, gonsole/auth/checkedwrite_test.go, gonsole/auth/CHANGELOG.md
Enabling fails when an account has no role or an undeclared role. Role and standing writes use the previously read role and fail when it changes before the write. Tests cover these checks and preserve account state when writes are refused.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant AccountCommands
  participant DeclaredRoles
  participant Plugins
  participant RoleRegistry
  AccountCommands->>DeclaredRoles: request roles
  DeclaredRoles->>Plugins: load and register plugins
  Plugins->>RoleRegistry: register roles
  DeclaredRoles->>RoleRegistry: read roles and capability holders
  RoleRegistry-->>DeclaredRoles: registry roles and holders
  DeclaredRoles-->>AccountCommands: return Roles
Loading
Merge Risk: ⚪ Minimal · up to 431e5

No confirmed issue blocks merging after normal checks.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Linked Issues check Warning The implementation satisfies the main coding requirements in #99. DeclaredRoles accepts the required registry shape, loads plugins, rejects load failures and missing capabilities, and returns `Roles… Update the account commands documentation to show how to use DeclaredRoles, including the registry interface and capability argument. Add or update a documentation test or example if the repository uses such checks.
Docstring Coverage Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 9 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the two main changes: a roles hook from a role registry and the lost role rule.
Out of Scope Changes check Passed The changed production files implement the helper, lost-role behavior, and checked writes required by #99. The tests cover those behaviors. The changelog records the same changes. No unrelated change …
Full details: Out of Scope Changes check

Explanation

The changed production files implement the helper, lost-role behavior, and checked writes required by #99. The tests cover those behaviors. The changelog records the same changes. No unrelated change is demonstrated.

Full details: Linked Issues check

Explanation

The implementation satisfies the main coding requirements in #99. DeclaredRoles accepts the required registry shape, loads plugins, rejects load failures and missing capabilities, and returns RolesFrom results. The role and standing commands use the lost-role rule and checked writes. The added tests cover these paths. However, #99 notes that the account commands documentation must show the helper. The whole-PR change summary lists no account commands documentation update; it lists only the changelog for documentation-related changes.

Resolution

Update the account commands documentation to show how to use DeclaredRoles, including the registry interface and capability argument. Add or update a documentation test or example if the repository uses such checks.

Full details: Docstring Coverage

Explanation

Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 9 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ 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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@greptile-apps

greptile-apps Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] Safe to merge based on the verified scope; no blocking or non-blocking findings remain.

Summary

Adds DeclaredRoles to load plugins before reading roles and to reject a capability that no role carries.

  • Account commands can build their role list from registered plugins.
  • Enabling an account waits until it has a declared role.
  • Account writes stop when the checked role has changed.

No actionable issues were found. The verified changes are safe to merge.

Reviews (1) · Last reviewed commit: "feat(gonsole/auth): refuse a lost role r..." · Reviewed by Greptile

@SirLouen
SirLouen merged commit fea3928 into main Oct 18, 2026
41 checks passed
@SirLouen
SirLouen deleted the feat/99 branch October 18, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gonsole/auth: roles hook from a role registry, and the lost role rule

1 participant