refactor(authn)!: use a single principal and pass all claims through

[mushrafmim]

Summary

Replaces authn's user/client principal split with a single Principal. email, ouId and ouHandle become standard claims, each configurable as required. Every other claim passes through in ExtraClaims with no declaration needed.

Before, every token was split into a user or a client principal by its grant_type claim. That claim isn't part of RFC 9068, so IdPs that don't emit it had every token rejected. The split also invited authorization logic that branches on how a token was obtained rather than on roles and scopes. Within authn, the only behaviour that depended on it was user provisioning, which isn't authentication.

A token is now accepted when its signature, iss, aud and exp check out, it carries sub and client_id, and it carries every standard claim configured as required. The resulting Principal is the same however the token was obtained. Anything the application does with it, such as provisioning a user record, moves to application middleware chained after authn.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Other (please describe):

Changes Made

• authn/token_parser.go: one Principal with these fields:

  • Subject and ClientID, both always required;
  • Roles and Scopes;
  • Email, OUID and OUHandle, trimmed, "" when absent;
  • ExtraClaims.

  Removed UserPrincipal, ClientPrincipal, PrincipalType, AllowedGrantType and the grant_type dispatch, along with its unsupported grant type rejection. A standard claim that is configured as required, but missing or blank, rejects the token.

• authn/config.go, authn/options.go: new Config.Claims (ClaimsConfig), with one ClaimConfig{Required} entry per standard claim. Code that builds an extractor directly uses WithClaims. Removed ClaimSpec, UserClaims / ClientClaims, WithUserClaims / WithClientClaims, and RolesClaim / WithRolesClaim. Roles are always read from roles.

  claims:
    ouId:
      required: true    # email / ouHandle omitted = optional

• authn/context.go: AuthContext is now AuthContext{Principal Principal}, with nil-safe accessors Subject(), ClientID(), Roles(), Scopes(), Email(), OUID(), OUHandle() and ExtraClaims(). It still satisfies authz.Principal. Removed UserContext, ClientContext, Type() and the UserProfileService interface.

• authn/extra_claims.go, authn/fixed_schema.go: ExtraClaims holds every claim outside the fixed schema. The fixed schema is sub, client_id, scope, roles, email, ouId, ouHandle and the JWT registered claims; case variants of those names are excluded too. String values are trimmed, and null or blank values are dropped. grant_type is no longer reserved.

• authn/middleware.go, authn/manager.go: the new signatures are Middleware(extractor), RequireAuth(extractor) and NewManager(cfg).

• authn/README.md: rewritten for the new model. It now includes:

  • a "Standard identity claims" section;
  • a Principal fields table showing which claims are required;
  • an example of application-owned provisioning middleware;
  • a migration guide.

• Tests: updated for the new model. New tests cover:

  • grant_type being ignored, and passing through as an ordinary claim;
  • each standard claim's required and optional behaviour, including client tokens;
  • claim pass-through and fixed-schema exclusion;
  • an application middleware chained after RequireAuth seeing the validated principal.

Testing

• I have tested this change locally
• I have added tests that prove my fix is effective or that my feature works
• I have tested edge cases
• All existing tests pass

Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have checked that there are no merge conflicts

Related Issues

Closes #291

This departs from the issue's proposed claims: [{name, required}] list. Only email, ouId and ouHandle are configurable, as typed per-claim entries. Every other claim passes through without being declared; see this comment. The per-claim entries leave room for a claim-name mapping setting later, next to required.

Screenshots/Demo

N/A

Additional Notes

Breaking changes:

• NewManager(userProfileSvc, cfg) becomes NewManager(cfg). Middleware and RequireAuth lose their UserProfileService argument. Provisioning moves to application middleware chained after authn (see the README).
• authCtx.User / authCtx.Client become the accessors or authCtx.Principal. authCtx.Type() and the principal-type constants are removed.
• UserClaims / ClientClaims (userClaims / clientClaims in yaml) and WithUserClaims / WithClientClaims are removed. Use claims / WithClaims for email, ouId and ouHandle; every other claim is in ExtraClaims.
• RolesClaim (rolesClaim in yaml) and WithRolesClaim are removed.

Behaviour changes the compiler won't catch:

• Subject() now returns the JWT sub. For user tokens it used to return the persisted user ID from UserProfileService; for client tokens it returned client_id. Check anything keyed by Subject().
• Tokens without sub are rejected. Client-credentials tokens without it used to be accepted.
• Required standard claims apply to every token. Requiring email also rejects client-credentials tokens that don't carry one.
• Only email, ouId and ouHandle can be required. Move checks for any other claim into application middleware.
• email, ouId and ouHandle are no longer in ExtraClaims, so ExtraClaims().String("email") returns "". Use Email().
• A leftover rolesClaim key in a yaml config is silently ignored. An IdP emitting roles under another name must map them to roles.
• The JSON shape of AuthContext has changed.

Follow-ups:

• storage pins authn v0.3.0. When it is bumped, storage/http_handler_test.go needs User: &authn.UserContext{ID: ...} changed to Principal: authn.Principal{Subject: ...}.
• Out of scope: IdP-specific accommodations, such as a claim-name mapping (for example client_id from azp, or roles from a differently-named claim) or EC (ES256) signing keys.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: ac3e1874-ba3b-4608-ba7a-6eb110379c7e

---

📥 Commits

Reviewing files that changed from the base of the PR and between 7e5e0b2 and b2d9cf2.

---

📒 Files selected for processing (19)

• authn/README.md
• authn/config.go
• authn/config_test.go
• authn/context.go
• authn/context_test.go
• authn/extra_claims.go
• authn/extra_claims_test.go
• authn/fixed_schema.go
• authn/fixed_schema_test.go
• authn/manager.go
• authn/manager_test.go
• authn/middleware.go
• authn/middleware_test.go
• authn/options.go
• authn/scopes_test.go
• authn/test_helpers_test.go
• authn/token_parser.go
• authn/token_parser_test.go
• authn/utils.go

---

💤 Files with no reviewable changes (1)

• authn/utils.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

📝 Walkthrough

📝 Walkthrough

Walkthrough

The authn package now uses one Principal for user and client tokens. It ignores grant_type when accepting tokens, exposes eligible claims through ExtraClaims, and removes user-profile provisioning from its authentication middleware.

Changes

Unified principal and token parsing

Layer / File(s)	Summary
Token parsing and claim extraction authn/token_parser.go, authn/extra_claims.go, authn/fixed_schema.go, authn/config.go, authn/options.go, authn/*_test.go	Token parsing requires sub and client_id and no longer dispatches on grant_type. The parser returns one Principal; eligible non-fixed-schema claims are exposed through ExtraClaims. Claim declaration APIs and their validation are removed.

Authentication context and middleware

Layer / File(s)	Summary
Principal access and middleware integration authn/context.go, authn/manager.go, authn/middleware.go, authn/utils.go, authn/*_test.go, authn/README.md	AuthContext exposes principal fields through accessors. Manager and middleware constructors no longer accept a user-profile service. The README shows application middleware chained after authentication for provisioning and request-context enrichment.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant TokenExtractor
  participant AuthMiddleware
  participant ApplicationMiddleware
  participant Handler
  TokenExtractor->>AuthMiddleware: Validated Principal
  AuthMiddleware->>ApplicationMiddleware: Request with AuthContext
  ApplicationMiddleware->>Handler: Request with application context

Loading

---

Merge Risk: ⚪ Minimal · up to b2d9c

The unified principal and middleware changes appear mergeable after normal checks; callers must follow the documented API migration.

Pre-merge checks | 3 | 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check		The PR implements major #291 objectives. It replaces grant_type dispatch with one Principal, removes UserProfileService, updates middleware APIs, and documents application-owned provisioning. However,…	Implement the #291 claim declaration model, including Config.Claims, Claim, WithClaims, duplicate-name validation, and required-claim enforcement with tests. Implement client_id resolution with azp fallback and exclude azp from ExtraClaims.…
Docstring Coverage		Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 17 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check		The listed changes stay within #291. They modify authn parsing, principal and context types, middleware, configuration, documentation, and related authn tests. Extra-claim pass-through and application…
Title check		The title clearly and concisely describes the primary changes: replacing the user/client split with a single principal and passing claims through.
Description check		The description is detailed and follows the template. It explains the purpose, breaking changes, implementation details, tests, checklist status, related issue, migration impact, and follow-ups.

---

Full details: Linked Issues check

Explanation

The PR implements major #291 objectives. It replaces grant_type dispatch with one Principal, removes UserProfileService, updates middleware APIs, and documents application-owned provisioning. However, #291 requires one configurable claim declaration list with optional/required semantics and duplicate-name validation. The PR removes claim declarations completely, as shown by authn/config.go and authn/options.go. #291 also requires client_id with an azp fallback and reserves azp. authn/token_parser.go requires client_id and does not parse or exclude azp. These are concrete unmet coding requirements.

Resolution

Implement the #291 claim declaration model, including Config.Claims, Claim, WithClaims, duplicate-name validation, and required-claim enforcement with tests. Implement client_id resolution with azp fallback and exclude azp from ExtraClaims. If the intended design remains claim pass-through and mandatory client_id, update or close #291 instead of claiming that this PR closes it.

---

Full details: Docstring Coverage

Explanation

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

---

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

---

🧪 Generate unit tests (beta)

• 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

• X
• Mastodon
• Reddit
• LinkedIn

---

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