refactor: scope-based authorization (roles are scope bundles)#56
Open
PenguinzTech wants to merge 1 commit into
Open
refactor: scope-based authorization (roles are scope bundles)#56PenguinzTech wants to merge 1 commit into
PenguinzTech wants to merge 1 commit into
Conversation
Authorization decisions now use scopes only, never role names (security standard). Roles are expanded into concrete resource:action scope bundles at token issuance and carried in the token's scope claim. - app/services/scopes.py: central ROLE_SCOPES bundles + expand_scopes()/ scope_string(); SUPERADMIN_SCOPE sentinel. Bundles reproduce the prior access matrix exactly (SystemAdmin ⊇ OrgAdmin/UserManager; Viewer reads). - auth_service.create_access_token: emits the scope claim. - middleware/auth.py: get_current_user exposes scope. - middleware/rbac.py: new requires_scope decorator; SystemAdmin bypasses (team/zone helpers) now check admin:super scope; can_manage_users/teams check scopes; requires_system_admin/requires_admin repointed to scopes. - Migrated every endpoint decorator across users/dns_servers/teams/time/ dhcp/ioc_feeds from @requires_role(...) to @requires_scope(...). - global_role/team_roles retained for audit + per-team membership only. Tests: new test_scope_authz.py (15) — expansion, matrix preservation, scope emission, decorator 401/403/200, scope-based helpers. conftest token factory emits scope. Full manager suite: 108 passing. flake8 clean. Documented in docs/ENTERPRISE_SECURITY.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reviewer's GuideRefactors manager authorization to be scope-based instead of role-name–based by introducing centralized scope bundles, emitting scopes in tokens, wiring middleware and helpers to read scopes, and migrating endpoint decorators and docs/tests accordingly, while preserving the existing access matrix. Sequence diagram for scope-based authorization during request handlingsequenceDiagram
actor User
participant AuthService
participant scopes_service as scopes_py
participant AuthMiddleware as auth_token_required
participant RBACMiddleware as rbac_requires_scope
participant Endpoint
User->>AuthService: create_access_token(user_id, username, global_role, team_roles)
AuthService->>scopes_service: scope_string(global_role, team_roles)
scopes_service-->>AuthService: "users:write servers:write ..."
AuthService-->>User: JWT { scope, global_role, team_roles }
User->>AuthMiddleware: HTTP request with Authorization header
AuthMiddleware->>AuthMiddleware: decode JWT
AuthMiddleware->>AuthMiddleware: build current_user with scope
AuthMiddleware-->>RBACMiddleware: current_user available via get_current_user
RBACMiddleware->>RBACMiddleware: requires_scope(required_scopes)
RBACMiddleware->>RBACMiddleware: _user_scopes(current_user)
alt no intersection
RBACMiddleware-->>User: 403 Insufficient permissions
else has required scope
RBACMiddleware->>Endpoint: call protected handler
Endpoint-->>User: 200 OK
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
expand_scopes/scope_string, theteam_rolesparameter is accepted but never used; consider removing it or adding a brief comment at the call sites to avoid confusion about whether per-team roles should affect global scopes. - The repeated inline imports of
scope_stringinauth_service.create_access_tokenand the test token factory add indirection and could mask import-time issues; it would be clearer to move these imports to the module top level if circular dependencies are not a concern. - The
_user_scopeshelper assumes a space-delimited string and silently treats malformedscopevalues as an empty set; if tokens may be manipulated or evolve, consider adding minimal validation or logging when unexpected formats are encountered to ease debugging of authorization issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `expand_scopes`/`scope_string`, the `team_roles` parameter is accepted but never used; consider removing it or adding a brief comment at the call sites to avoid confusion about whether per-team roles should affect global scopes.
- The repeated inline imports of `scope_string` in `auth_service.create_access_token` and the test token factory add indirection and could mask import-time issues; it would be clearer to move these imports to the module top level if circular dependencies are not a concern.
- The `_user_scopes` helper assumes a space-delimited string and silently treats malformed `scope` values as an empty set; if tokens may be manipulated or evolve, consider adding minimal validation or logging when unexpected formats are encountered to ease debugging of authorization issues.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Stacked on #55. Fourth branch in the chain.
Brings the manager API into line with the security standard: authorization decisions on scopes only, never role names. Roles are expanded into concrete
resource:actionscope bundles at token issuance.Changes
app/services/scopes.py(new) — centralROLE_SCOPESbundles,expand_scopes()/scope_string(),SUPERADMIN_SCOPEsentinel. Bundles reproduce the pre-refactor access matrix exactly (SystemAdmin ⊇ OrgAdmin/UserManager privileges; Viewer read-only).auth_service.create_access_token— emits thescopeclaim (external verifiers already readscope, so they now receive meaningful scopes).middleware/auth.py—get_current_userexposesscope.middleware/rbac.py— newrequires_scopedecorator; SystemAdmin bypasses in team/zone helpers now check theadmin:superscope;can_manage_users/can_manage_teamscheck scopes;requires_system_admin/requires_adminrepointed to scopes.users,dns_servers,teams,time,dhcp,ioc_feeds) from@requires_role(...)→@requires_scope(...).global_role/team_rolesretained for audit + per-team membership only.Access matrix (preserved)
*:write/*:admin+admin:super+ readsservers/teams/time/dhcp:write+ readsusers:write+ readsTests
New
test_scope_authz.py(15): expansion, matrix preservation, scope emission on issued tokens,requires_scope401/403/200 + any-of semantics, scope-based helpers. conftest token factory emitsscope. Full manager suite: 108 passing. flake8 clean.Documented in
docs/ENTERPRISE_SECURITY.md.🤖 Generated with Claude Code
Summary by Sourcery
Switch manager API authorization from role-based checks to scope-based bundles while preserving the existing access matrix.
New Features:
Enhancements:
Documentation:
Tests: