Add token family id (tfid) for grant-scoped revocation - #4300
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds token-family identifiers to OAuth tokens and SSO sessions, persists family revocation criteria, enforces revocation by JTI or family ID, and adds revocation triggers for refresh reuse, explicit revoke, authorization-code replay, and session logout. ChangesToken family propagation and SSO sessions
Criteria revocation and OAuth flows
Validation and integration coverage
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
backend/internal/flow/executor/session_executor.go (1)
127-177: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCheck save-path token-family-id coverage.
TestFreshSavecapturesSaveCheckpointInput, but it does not assertin.TokenFamilyIDis non-empty when the SSO call mints/participates in the session. A missing token family id can make these grant tokens unrevocable as a family, so add a focused assertion/check expectation for the save path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/flow/executor/session_executor.go` around lines 127 - 177, Extend TestFreshSave to assert that the captured SaveCheckpointInput has a non-empty TokenFamilyID when the SSO session mints or participates in the token family. Keep the assertion focused on the save-path input and preserve the existing checkpoint expectations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cmd/server/config/default.json`:
- Around line 149-158: Create or update
docs/content/guides/token-family-revocation.mdx to document the
oauth.revocation.token_family.* triggers, oauth.token_exchange.token_family
values none and inherit, RP-initiated logout revoking associated token families,
and the default oauth.refresh_token.renew_on_grant: true behavior.
In `@backend/internal/flow/session/service.go`:
- Around line 226-237: Update recordParticipant and its persistence path so an
empty tokenFamilyID does not overwrite an existing Participant.TokenFamilyID;
only write the field when a non-empty ID is supplied for the current grant.
Preserve creation and updates for non-empty tokenFamilyID values, and keep the
LoadCheckpoint caller behavior unchanged.
In `@backend/internal/flow/session/store_constants.go`:
- Around line 102-111: Update queryUpsertParticipant in
backend/internal/flow/session/store_constants.go:102-111 to retain existing
TFIDs instead of overwriting them, using the token-family association model.
Define the one-to-many participant-to-token-family schema in
backend/dbscripts/runtime_persistent/postgres.sql:106-106 and mirror it in
backend/dbscripts/runtime_persistent/sqlite.sql:106-106, preserving all families
for logout revocation.
In `@backend/internal/oauth/oauth2/constants/constants.go`:
- Around line 298-304: Update the relevant OAuth guides under
docs/content/guides/ to document oauth.revocation.token_family.*,
oauth.token_exchange.token_family with none and inherit behavior, and family
invalidation on refresh-token reuse, authorization-code replay, RFC 7009
revocation, and SSO logout; update docs/content/apis.mdx with the RFC 7009
revocation behavior. Cover the configuration introduced near
TokenExchangeTokenFamilyNone and the revocation flow in
backend/internal/oauth/oauth2/granthandlers/refresh_token.go:132-136; no direct
code change is required at either source site.
In `@backend/internal/oauth/oauth2/granthandlers/refresh_token_test.go`:
- Around line 314-316: Strengthen the positive tfid propagation tests: in
backend/internal/oauth/oauth2/granthandlers/refresh_token_test.go lines 314-316,
pass a non-empty family ID to IssueRefreshToken and assert that
RefreshTokenBuildContext.TokenFamilyID matches it; in
backend/internal/oauth/oauth2/tokenservice/validator_test.go lines 2012-2016,
validate a token containing tfid and assert EnsureNotRevoked receives that exact
value.
In `@backend/internal/oauth/oauth2/granthandlers/token_exchange.go`:
- Around line 262-274: Update the relevant documentation under
docs/content/guides/ and docs/content/apis.mdx to cover token-family revocation
triggers, including refresh-token reuse, authorization-code replay, RFC 7009
revocation, and SSO logout; document oauth.revocation.token_family.* and
oauth.token_exchange.token_family, including inherit, none, defaults, and
invalid-value handling; and describe tfid propagation and revocation effects for
token-exchange consumers.
In `@backend/internal/oauth/oauth2/revocation/criteria_revoker.go`:
- Around line 54-65: The newCriteriaRevoker initialization must ensure
token-family revocation criteria remain valid through the longest family-bearing
token lifetime, not just the refresh-token TTL. Replace the current fixed
tokenFamilyLifetime fallback with the maximum configured family token lifetime,
including access-token validity resolved through ResolveTokenConfig, or persist
and compare against each issued token’s actual expiry; preserve the existing
default only when no longer lifetime is configured.
In `@backend/internal/oauth/oauth2/token/service_test.go`:
- Around line 529-532: Update the refresh-token issuance tests around the
IssueRefreshToken expectations to assign a non-empty value to
tokenRespDTO.AccessToken.TokenFamilyID and use that same value for the new
argument in at least one expectation, including the corresponding expectations
at the other noted locations where appropriate. Preserve the existing test setup
and assertions while ensuring TFID propagation is verified.
In `@backend/pkg/thunderidengine/config/config.go`:
- Around line 214-243: Add documentation under docs/content/guides/ for
oauth.revocation.token_family, covering refresh-token reuse, RFC 7009 explicit
revocation, authorization-code replay, defaults, and full token-family
invalidation; document oauth.token_exchange.token_family’s none and inherit
modes and revocation effects in the token-exchange guide; and update
docs/content/apis.mdx to describe how /oauth2/revoke can invalidate an entire
token family.
---
Nitpick comments:
In `@backend/internal/flow/executor/session_executor.go`:
- Around line 127-177: Extend TestFreshSave to assert that the captured
SaveCheckpointInput has a non-empty TokenFamilyID when the SSO session mints or
participates in the token family. Keep the assertion focused on the save-path
input and preserve the existing checkpoint expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f6a5b19-bdbe-4946-8b73-ea552fcc1059
⛔ Files ignored due to path filters (4)
backend/tests/mocks/flow/sessionmock/Service_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/granthandlersmock/RefreshTokenGrantHandlerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/CriteriaRevokerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/EnforcementServiceInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (81)
backend/.mockery.public.ymlbackend/cmd/server/config/default.jsonbackend/cmd/server/servicemanager.gobackend/dbscripts/runtime_persistent/postgres-cleanup.sqlbackend/dbscripts/runtime_persistent/postgres.sqlbackend/dbscripts/runtime_persistent/sqlite.sqlbackend/internal/flow/common/constants.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/session_executor.gobackend/internal/flow/executor/session_executor_test.gobackend/internal/flow/session/CriteriaRevoker_mock_test.gobackend/internal/flow/session/Service_mock_test.gobackend/internal/flow/session/init.gobackend/internal/flow/session/model.gobackend/internal/flow/session/participant_store_test.gobackend/internal/flow/session/service.gobackend/internal/flow/session/service_test.gobackend/internal/flow/session/store.gobackend/internal/flow/session/store_constants.gobackend/internal/oauth/init.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/oauth/oauth2/authz/init_test.gobackend/internal/oauth/oauth2/authz/model.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/oauth/oauth2/granthandlers/authorization_code.gobackend/internal/oauth/oauth2/granthandlers/grant_handler.gobackend/internal/oauth/oauth2/granthandlers/init.gobackend/internal/oauth/oauth2/granthandlers/provider.gobackend/internal/oauth/oauth2/granthandlers/provider_test.gobackend/internal/oauth/oauth2/granthandlers/refresh_token.gobackend/internal/oauth/oauth2/granthandlers/refresh_token_test.gobackend/internal/oauth/oauth2/granthandlers/token_exchange.gobackend/internal/oauth/oauth2/granthandlers/token_exchange_test.gobackend/internal/oauth/oauth2/model/token.gobackend/internal/oauth/oauth2/revocation/CriteriaRevokerInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/EnforcementServiceInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/criteriaStoreInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/criteria_revoker.gobackend/internal/oauth/oauth2/revocation/criteria_revoker_test.gobackend/internal/oauth/oauth2/revocation/criteria_store.gobackend/internal/oauth/oauth2/revocation/criteria_store_test.gobackend/internal/oauth/oauth2/revocation/enforcement_service.gobackend/internal/oauth/oauth2/revocation/enforcement_service_test.gobackend/internal/oauth/oauth2/revocation/init.gobackend/internal/oauth/oauth2/revocation/init_test.gobackend/internal/oauth/oauth2/revocation/model.gobackend/internal/oauth/oauth2/revocation/service.gobackend/internal/oauth/oauth2/revocation/service_test.gobackend/internal/oauth/oauth2/revocation/store_constants.gobackend/internal/oauth/oauth2/token/TokenServiceInterface_mock_test.gobackend/internal/oauth/oauth2/token/service.gobackend/internal/oauth/oauth2/token/service_test.gobackend/internal/oauth/oauth2/tokenservice/builder.gobackend/internal/oauth/oauth2/tokenservice/model.gobackend/internal/oauth/oauth2/tokenservice/validator.gobackend/internal/oauth/oauth2/tokenservice/validator_test.gobackend/internal/system/config/config.gobackend/internal/system/revocationcache/cache.gobackend/internal/system/revocationcache/cache_test.gobackend/internal/system/revocationcache/enforcer.gobackend/internal/system/revocationcache/enforcer_test.gobackend/internal/system/revocationcache/init_test.gobackend/internal/system/revocationcache/model.gobackend/internal/system/revocationcache/query_constants.gobackend/internal/system/revocationcache/source.gobackend/internal/system/revocationcache/source_db.gobackend/internal/system/revocationcache/source_db_test.gobackend/internal/system/revocationcache/syncer.gobackend/internal/system/revocationcache/syncer_test.gobackend/internal/system/security/RevocationEnforcerInterface_mock_test.gobackend/internal/system/security/context.gobackend/internal/system/security/jwt_authenticator.gobackend/internal/system/security/service.gobackend/internal/system/security/service_test.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/config/validate.gotests/integration/oauth/sso/rp_logout_test.gotests/integration/oauth/sso/suite_test.gotests/integration/oauth/token/tfid_test.go
| "revocation": { | ||
| "token_family": { | ||
| "on_refresh_reuse": true, | ||
| "on_explicit_revoke": true, | ||
| "on_code_replay": true | ||
| } | ||
| }, | ||
| "token_exchange": { | ||
| "token_family": "none" | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
- Token-family revocation configuration: create or update
docs/content/guides/token-family-revocation.mdxforoauth.revocation.token_family.*triggers. - Token-exchange family behavior: document
oauth.token_exchange.token_familyvalues (noneandinherit) indocs/content/guides/token-family-revocation.mdx. - SSO logout behavior: document that RP-initiated logout revokes associated token families in
docs/content/guides/token-family-revocation.mdx. - Refresh-token renewal default: document the default
oauth.refresh_token.renew_on_grant: truebehavior in the same configuration guide.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/cmd/server/config/default.json` around lines 149 - 158, Create or
update docs/content/guides/token-family-revocation.mdx to document the
oauth.revocation.token_family.* triggers, oauth.token_exchange.token_family
values none and inherit, RP-initiated logout revoking associated token families,
and the default oauth.refresh_token.renew_on_grant: true behavior.
Source: Path instructions
| // LAST_ACTIVE_AT and the current-grant TFID (but preserving FIRST_JOINED_AT) when the application | ||
| // has already joined. TFID moves to the latest grant so logout revokes the most recent family. | ||
| // The ON CONFLICT ... DO UPDATE form is valid in both PostgreSQL and SQLite. | ||
| queryUpsertParticipant = model.DBQuery{ | ||
| ID: "SSO-SESS-09", | ||
| Query: `INSERT INTO "SSO_SESSION_PARTICIPANT" ` + | ||
| `(SESSION_ID, DEPLOYMENT_ID, APP_ID, FIRST_JOINED_AT, LAST_ACTIVE_AT) ` + | ||
| `VALUES ($1, $2, $3, $4, $5) ` + | ||
| `ON CONFLICT (SESSION_ID, DEPLOYMENT_ID, APP_ID) DO UPDATE SET LAST_ACTIVE_AT = excluded.LAST_ACTIVE_AT`, | ||
| `(SESSION_ID, DEPLOYMENT_ID, APP_ID, FIRST_JOINED_AT, LAST_ACTIVE_AT, TFID) ` + | ||
| `VALUES ($1, $2, $3, $4, $5, $6) ` + | ||
| `ON CONFLICT (SESSION_ID, DEPLOYMENT_ID, APP_ID) DO UPDATE SET ` + | ||
| `LAST_ACTIVE_AT = excluded.LAST_ACTIVE_AT, TFID = excluded.TFID`, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Retain every token family associated with an SSO session.
Overwriting TFID for an existing (SESSION_ID, DEPLOYMENT_ID, APP_ID) loses earlier login-flow families. Logout can then revoke only the most recent family, leaving tokens from earlier families active.
backend/internal/flow/session/store_constants.go#L102-L111: record token-family IDs without replacing prior families.backend/dbscripts/runtime_persistent/postgres.sql#L106-L106: model a one-to-many participant-to-token-family association.backend/dbscripts/runtime_persistent/sqlite.sql#L106-L106: mirror the PostgreSQL association model.
📍 Affects 3 files
backend/internal/flow/session/store_constants.go#L102-L111(this comment)backend/dbscripts/runtime_persistent/postgres.sql#L106-L106backend/dbscripts/runtime_persistent/sqlite.sql#L106-L106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/flow/session/store_constants.go` around lines 102 - 111,
Update queryUpsertParticipant in
backend/internal/flow/session/store_constants.go:102-111 to retain existing
TFIDs instead of overwriting them, using the token-family association model.
Define the one-to-many participant-to-token-family schema in
backend/dbscripts/runtime_persistent/postgres.sql:106-106 and mirror it in
backend/dbscripts/runtime_persistent/sqlite.sql:106-106, preserving all families
for logout revocation.
| Revocation RevocationConfig `yaml:"revocation" json:"revocation"` | ||
| TokenExchange TokenExchangeConfig `yaml:"token_exchange" json:"token_exchange"` | ||
| // AllowWildcardRedirectURI enables wildcard pattern matching for redirect URIs. | ||
| // When false (default), only exact redirect URI matching is performed. | ||
| AllowWildcardRedirectURI bool `yaml:"allow_wildcard_redirect_uri" json:"allow_wildcard_redirect_uri"` | ||
| } | ||
|
|
||
| // RevocationConfig holds grant-scoped (token family) revocation settings. | ||
| type RevocationConfig struct { | ||
| TokenFamily TokenFamilyRevocationConfig `yaml:"token_family" json:"token_family"` | ||
| } | ||
|
|
||
| // TokenFamilyRevocationConfig toggles the triggers that revoke a whole token family (one authorization | ||
| // grant). Each defaults to on (set in default.json), matching the fail-closed security posture. | ||
| type TokenFamilyRevocationConfig struct { | ||
| // OnRefreshReuse revokes the family when a rotated (already-revoked) refresh token is replayed. | ||
| OnRefreshReuse bool `yaml:"on_refresh_reuse" json:"on_refresh_reuse"` | ||
| // OnExplicitRevoke revokes the family when a token carrying a tfid is revoked via RFC 7009, so a | ||
| // login's access tokens drop with its refresh token. | ||
| OnExplicitRevoke bool `yaml:"on_explicit_revoke" json:"on_explicit_revoke"` | ||
| // OnCodeReplay revokes the family when an authorization code is redeemed twice (replay). | ||
| OnCodeReplay bool `yaml:"on_code_replay" json:"on_code_replay"` | ||
| } | ||
|
|
||
| // TokenExchangeConfig holds RFC 8693 token-exchange settings. | ||
| type TokenExchangeConfig struct { | ||
| // TokenFamily selects how an exchanged token relates to the subject token's token family: | ||
| // "none" (default) issues an independent token with no tfid; "inherit" copies the subject | ||
| // token's tfid so the exchanged token is revoked with that token family. | ||
| TokenFamily string `yaml:"token_family" json:"token_family"` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
oauth.revocation.token_family: document the refresh-reuse, explicit RFC 7009 revocation, and authorization-code replay switches, defaults, and token-family invalidation behavior in the relevantdocs/content/guides/OAuth configuration guide.oauth.token_exchange.token_family: document thenoneandinheritmodes and their revocation consequences in the token-exchange guide underdocs/content/guides/./oauth2/revoke: document that explicit revocation can invalidate the full token family indocs/content/apis.mdx.
As per path instructions, “If ANY of the above are detected and the PR does NOT include corresponding updates under docs/, post a single consolidated PR-level comment.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/pkg/thunderidengine/config/config.go` around lines 214 - 243, Add
documentation under docs/content/guides/ for oauth.revocation.token_family,
covering refresh-token reuse, RFC 7009 explicit revocation, authorization-code
replay, defaults, and full token-family invalidation; document
oauth.token_exchange.token_family’s none and inherit modes and revocation
effects in the token-exchange guide; and update docs/content/apis.mdx to
describe how /oauth2/revoke can invalidate an entire token family.
Source: Path instructions
38e6432 to
61b88ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/pkg/thunderidengine/config/validate.go`:
- Around line 178-188: Update the default error branch in
TokenExchangeConfig.Validate so its message lists the empty value alongside
"none" and "inherit" as accepted token_family values, keeping the validation
behavior unchanged.
In `@tests/integration/oauth/token/tfid_test.go`:
- Around line 361-373: Update TestTfidPreservedOnRefresh to enable refresh-token
rotation for this test, then assert the rotated refreshed.RefreshToken is
non-empty and its tfid matches originalTfid, alongside the existing access-token
assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e109636b-caaf-4368-8f34-8f986f31bf5a
⛔ Files ignored due to path filters (4)
backend/tests/mocks/flow/sessionmock/Service_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/granthandlersmock/RefreshTokenGrantHandlerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/CriteriaRevokerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/EnforcementServiceInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (86)
backend/.mockery.public.ymlbackend/cmd/server/config/default.jsonbackend/cmd/server/servicemanager.gobackend/dbscripts/runtime_persistent/postgres-cleanup.sqlbackend/dbscripts/runtime_persistent/postgres.sqlbackend/dbscripts/runtime_persistent/sqlite.sqlbackend/internal/flow/common/constants.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/session_executor.gobackend/internal/flow/executor/session_executor_test.gobackend/internal/flow/session/CriteriaRevoker_mock_test.gobackend/internal/flow/session/Service_mock_test.gobackend/internal/flow/session/init.gobackend/internal/flow/session/model.gobackend/internal/flow/session/participant_store_test.gobackend/internal/flow/session/service.gobackend/internal/flow/session/service_test.gobackend/internal/flow/session/store.gobackend/internal/flow/session/store_constants.gobackend/internal/oauth/init.gobackend/internal/oauth/oauth2/authz/AuthorizationCodeStoreInterface_mock_test.gobackend/internal/oauth/oauth2/authz/auth_code_store.gobackend/internal/oauth/oauth2/authz/auth_code_store_test.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/oauth/oauth2/authz/init_test.gobackend/internal/oauth/oauth2/authz/model.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/oauth/oauth2/granthandlers/authorization_code.gobackend/internal/oauth/oauth2/granthandlers/grant_handler.gobackend/internal/oauth/oauth2/granthandlers/init.gobackend/internal/oauth/oauth2/granthandlers/provider.gobackend/internal/oauth/oauth2/granthandlers/provider_test.gobackend/internal/oauth/oauth2/granthandlers/refresh_token.gobackend/internal/oauth/oauth2/granthandlers/refresh_token_test.gobackend/internal/oauth/oauth2/granthandlers/token_exchange.gobackend/internal/oauth/oauth2/granthandlers/token_exchange_test.gobackend/internal/oauth/oauth2/model/token.gobackend/internal/oauth/oauth2/revocation/CriteriaRevokerInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/EnforcementServiceInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/RevocationServiceInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/RevokedTokenStoreInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/criteria_revoker.gobackend/internal/oauth/oauth2/revocation/criteria_revoker_test.gobackend/internal/oauth/oauth2/revocation/enforcement_service.gobackend/internal/oauth/oauth2/revocation/enforcement_service_test.gobackend/internal/oauth/oauth2/revocation/init.gobackend/internal/oauth/oauth2/revocation/init_test.gobackend/internal/oauth/oauth2/revocation/model.gobackend/internal/oauth/oauth2/revocation/revocationStoreInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/service.gobackend/internal/oauth/oauth2/revocation/service_test.gobackend/internal/oauth/oauth2/revocation/store.gobackend/internal/oauth/oauth2/revocation/store_constants.gobackend/internal/oauth/oauth2/revocation/store_test.gobackend/internal/oauth/oauth2/token/TokenServiceInterface_mock_test.gobackend/internal/oauth/oauth2/token/service.gobackend/internal/oauth/oauth2/token/service_test.gobackend/internal/oauth/oauth2/tokenservice/builder.gobackend/internal/oauth/oauth2/tokenservice/model.gobackend/internal/oauth/oauth2/tokenservice/validator.gobackend/internal/oauth/oauth2/tokenservice/validator_test.gobackend/internal/system/config/config.gobackend/internal/system/revocationcache/cache.gobackend/internal/system/revocationcache/cache_test.gobackend/internal/system/revocationcache/enforcer.gobackend/internal/system/revocationcache/enforcer_test.gobackend/internal/system/revocationcache/init_test.gobackend/internal/system/revocationcache/model.gobackend/internal/system/revocationcache/query_constants.gobackend/internal/system/revocationcache/source.gobackend/internal/system/revocationcache/source_db.gobackend/internal/system/revocationcache/source_db_test.gobackend/internal/system/revocationcache/syncer.gobackend/internal/system/revocationcache/syncer_test.gobackend/internal/system/security/RevocationEnforcerInterface_mock_test.gobackend/internal/system/security/context.gobackend/internal/system/security/jwt_authenticator.gobackend/internal/system/security/service.gobackend/internal/system/security/service_test.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/config/validate.gotests/integration/oauth/sso/rp_logout_test.gotests/integration/oauth/sso/suite_test.gotests/integration/oauth/token/tfid_test.go
💤 Files with no reviewable changes (1)
- backend/internal/oauth/oauth2/revocation/RevokedTokenStoreInterface_mock_test.go
🚧 Files skipped from review as they are similar to previous changes (65)
- backend/internal/flow/session/init.go
- backend/internal/oauth/oauth2/granthandlers/grant_handler.go
- backend/internal/oauth/oauth2/granthandlers/init.go
- backend/internal/system/security/context.go
- backend/internal/system/revocationcache/syncer.go
- backend/internal/oauth/oauth2/granthandlers/authorization_code.go
- backend/dbscripts/runtime_persistent/postgres-cleanup.sql
- backend/internal/oauth/oauth2/authz/init.go
- backend/internal/oauth/oauth2/granthandlers/provider.go
- backend/internal/oauth/oauth2/revocation/store_constants.go
- backend/internal/oauth/oauth2/authz/model.go
- backend/cmd/server/config/default.json
- backend/internal/oauth/oauth2/revocation/model.go
- backend/internal/oauth/oauth2/tokenservice/model.go
- backend/internal/system/revocationcache/query_constants.go
- backend/internal/oauth/oauth2/model/token.go
- backend/.mockery.public.yml
- backend/internal/flow/session/CriteriaRevoker_mock_test.go
- backend/internal/oauth/oauth2/revocation/criteria_revoker_test.go
- backend/internal/system/revocationcache/source.go
- backend/internal/system/config/config.go
- backend/dbscripts/runtime_persistent/sqlite.sql
- backend/internal/system/revocationcache/cache_test.go
- backend/internal/flow/session/store.go
- backend/internal/oauth/oauth2/authz/init_test.go
- backend/internal/system/revocationcache/model.go
- backend/internal/system/revocationcache/init_test.go
- tests/integration/oauth/sso/rp_logout_test.go
- backend/internal/oauth/oauth2/token/service.go
- backend/internal/flow/executor/auth_assert_executor.go
- backend/internal/flow/session/store_constants.go
- backend/internal/system/revocationcache/enforcer_test.go
- backend/internal/system/security/service.go
- backend/internal/system/revocationcache/enforcer.go
- backend/internal/flow/common/constants.go
- backend/internal/oauth/oauth2/granthandlers/token_exchange.go
- backend/internal/oauth/oauth2/revocation/EnforcementServiceInterface_mock_test.go
- backend/internal/system/security/jwt_authenticator.go
- backend/internal/oauth/oauth2/tokenservice/validator.go
- backend/internal/oauth/init.go
- backend/internal/oauth/oauth2/constants/constants.go
- backend/internal/flow/session/participant_store_test.go
- backend/internal/oauth/oauth2/revocation/criteria_revoker.go
- backend/internal/oauth/oauth2/granthandlers/provider_test.go
- backend/internal/oauth/oauth2/token/service_test.go
- backend/internal/system/security/RevocationEnforcerInterface_mock_test.go
- backend/pkg/thunderidengine/config/config.go
- backend/internal/oauth/oauth2/revocation/init_test.go
- backend/internal/flow/executor/session_executor_test.go
- backend/cmd/server/servicemanager.go
- backend/internal/system/revocationcache/source_db.go
- backend/internal/oauth/oauth2/revocation/CriteriaRevokerInterface_mock_test.go
- backend/internal/system/revocationcache/source_db_test.go
- backend/internal/oauth/oauth2/granthandlers/token_exchange_test.go
- backend/internal/flow/executor/session_executor.go
- tests/integration/oauth/sso/suite_test.go
- backend/internal/oauth/oauth2/revocation/service_test.go
- backend/internal/oauth/oauth2/tokenservice/validator_test.go
- backend/internal/system/revocationcache/syncer_test.go
- backend/internal/flow/session/Service_mock_test.go
- backend/internal/oauth/oauth2/tokenservice/builder.go
- backend/internal/oauth/oauth2/granthandlers/refresh_token.go
- backend/internal/flow/session/service.go
- backend/internal/oauth/oauth2/authz/service.go
- backend/internal/flow/session/service_test.go
ec90868 to
dd724ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/internal/flow/session/store_constants.go (1)
102-111: 🔒 Security & Privacy | 🟠 MajorRetain every active token family for an SSO participant.
Replacing
TFIDmeans termination can revoke only the most recent grant, while tokens issued by earlier re-authorizations remain valid. Store all active families per session/application, or revoke the displaced family immediately when replacing it.
backend/internal/flow/session/store_constants.go#L102-L111: do not overwrite the prior family ID for an existing participant.backend/internal/flow/session/model.go#L89-L93: model the participant-to-family relationship so termination can enumerate every active family.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/flow/session/store_constants.go` around lines 102 - 111, Update queryUpsertParticipant in backend/internal/flow/session/store_constants.go:102-111 so an existing participant’s TFID is not overwritten; preserve each active token family or immediately revoke the displaced family. Update the participant model relationship in backend/internal/flow/session/model.go:89-93 so termination can enumerate and revoke every active family for the session/application.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/internal/oauth/oauth2/authz/auth_code_store.go`:
- Around line 41-47: Make authorization-code redemption persist the
consumed-code state and its token-family replay marker as one atomic operation,
updating the flow around MarkConsumedTokenFamily and ConsumedTokenFamily and the
underlying store transaction as needed. Propagate marker-persistence failures
from redemption instead of only logging them, and ensure concurrent replays
cannot observe a consumed code without its family ID. Preserve the empty
tokenFamilyID no-op behavior and align any identity handling with the applicable
RFC requirements.
---
Duplicate comments:
In `@backend/internal/flow/session/store_constants.go`:
- Around line 102-111: Update queryUpsertParticipant in
backend/internal/flow/session/store_constants.go:102-111 so an existing
participant’s TFID is not overwritten; preserve each active token family or
immediately revoke the displaced family. Update the participant model
relationship in backend/internal/flow/session/model.go:89-93 so termination can
enumerate and revoke every active family for the session/application.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 49d26c93-cc29-4d59-a056-ef673a779150
⛔ Files ignored due to path filters (4)
backend/tests/mocks/flow/sessionmock/Service_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/granthandlersmock/RefreshTokenGrantHandlerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/CriteriaRevokerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/EnforcementServiceInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (86)
backend/.mockery.public.ymlbackend/cmd/server/config/default.jsonbackend/cmd/server/servicemanager.gobackend/dbscripts/runtime_persistent/postgres-cleanup.sqlbackend/dbscripts/runtime_persistent/postgres.sqlbackend/dbscripts/runtime_persistent/sqlite.sqlbackend/internal/flow/common/constants.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/session_executor.gobackend/internal/flow/executor/session_executor_test.gobackend/internal/flow/session/CriteriaRevoker_mock_test.gobackend/internal/flow/session/Service_mock_test.gobackend/internal/flow/session/init.gobackend/internal/flow/session/model.gobackend/internal/flow/session/participant_store_test.gobackend/internal/flow/session/service.gobackend/internal/flow/session/service_test.gobackend/internal/flow/session/store.gobackend/internal/flow/session/store_constants.gobackend/internal/oauth/init.gobackend/internal/oauth/oauth2/authz/AuthorizationCodeStoreInterface_mock_test.gobackend/internal/oauth/oauth2/authz/auth_code_store.gobackend/internal/oauth/oauth2/authz/auth_code_store_test.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/oauth/oauth2/authz/init_test.gobackend/internal/oauth/oauth2/authz/model.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/oauth/oauth2/granthandlers/authorization_code.gobackend/internal/oauth/oauth2/granthandlers/grant_handler.gobackend/internal/oauth/oauth2/granthandlers/init.gobackend/internal/oauth/oauth2/granthandlers/provider.gobackend/internal/oauth/oauth2/granthandlers/provider_test.gobackend/internal/oauth/oauth2/granthandlers/refresh_token.gobackend/internal/oauth/oauth2/granthandlers/refresh_token_test.gobackend/internal/oauth/oauth2/granthandlers/token_exchange.gobackend/internal/oauth/oauth2/granthandlers/token_exchange_test.gobackend/internal/oauth/oauth2/model/token.gobackend/internal/oauth/oauth2/revocation/CriteriaRevokerInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/EnforcementServiceInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/RevocationServiceInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/RevokedTokenStoreInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/criteria_revoker.gobackend/internal/oauth/oauth2/revocation/criteria_revoker_test.gobackend/internal/oauth/oauth2/revocation/enforcement_service.gobackend/internal/oauth/oauth2/revocation/enforcement_service_test.gobackend/internal/oauth/oauth2/revocation/init.gobackend/internal/oauth/oauth2/revocation/init_test.gobackend/internal/oauth/oauth2/revocation/model.gobackend/internal/oauth/oauth2/revocation/revocationStoreInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/service.gobackend/internal/oauth/oauth2/revocation/service_test.gobackend/internal/oauth/oauth2/revocation/store.gobackend/internal/oauth/oauth2/revocation/store_constants.gobackend/internal/oauth/oauth2/revocation/store_test.gobackend/internal/oauth/oauth2/token/TokenServiceInterface_mock_test.gobackend/internal/oauth/oauth2/token/service.gobackend/internal/oauth/oauth2/token/service_test.gobackend/internal/oauth/oauth2/tokenservice/builder.gobackend/internal/oauth/oauth2/tokenservice/model.gobackend/internal/oauth/oauth2/tokenservice/validator.gobackend/internal/oauth/oauth2/tokenservice/validator_test.gobackend/internal/system/config/config.gobackend/internal/system/revocationcache/cache.gobackend/internal/system/revocationcache/cache_test.gobackend/internal/system/revocationcache/enforcer.gobackend/internal/system/revocationcache/enforcer_test.gobackend/internal/system/revocationcache/init_test.gobackend/internal/system/revocationcache/model.gobackend/internal/system/revocationcache/query_constants.gobackend/internal/system/revocationcache/source.gobackend/internal/system/revocationcache/source_db.gobackend/internal/system/revocationcache/source_db_test.gobackend/internal/system/revocationcache/syncer.gobackend/internal/system/revocationcache/syncer_test.gobackend/internal/system/security/RevocationEnforcerInterface_mock_test.gobackend/internal/system/security/context.gobackend/internal/system/security/jwt_authenticator.gobackend/internal/system/security/service.gobackend/internal/system/security/service_test.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/config/validate.gotests/integration/oauth/sso/rp_logout_test.gotests/integration/oauth/sso/suite_test.gotests/integration/oauth/token/tfid_test.go
💤 Files with no reviewable changes (1)
- backend/internal/oauth/oauth2/revocation/RevokedTokenStoreInterface_mock_test.go
🚧 Files skipped from review as they are similar to previous changes (74)
- backend/.mockery.public.yml
- backend/internal/oauth/oauth2/model/token.go
- backend/dbscripts/runtime_persistent/postgres-cleanup.sql
- backend/pkg/thunderidengine/config/validate.go
- backend/internal/oauth/oauth2/authz/auth_code_store_test.go
- backend/internal/flow/session/CriteriaRevoker_mock_test.go
- backend/internal/oauth/oauth2/token/service.go
- backend/internal/system/security/context.go
- backend/internal/oauth/oauth2/granthandlers/grant_handler.go
- backend/internal/flow/session/init.go
- backend/internal/oauth/oauth2/granthandlers/authorization_code.go
- tests/integration/oauth/sso/rp_logout_test.go
- backend/internal/oauth/oauth2/revocation/model.go
- backend/internal/flow/common/constants.go
- backend/internal/system/revocationcache/enforcer_test.go
- backend/dbscripts/runtime_persistent/postgres.sql
- backend/internal/oauth/oauth2/revocation/criteria_revoker_test.go
- backend/internal/oauth/oauth2/authz/model.go
- backend/internal/oauth/oauth2/authz/init.go
- backend/internal/oauth/oauth2/token/TokenServiceInterface_mock_test.go
- backend/internal/flow/executor/auth_assert_executor.go
- backend/internal/oauth/oauth2/authz/init_test.go
- backend/internal/system/revocationcache/init_test.go
- backend/internal/flow/session/store.go
- backend/internal/system/revocationcache/model.go
- backend/internal/oauth/oauth2/tokenservice/builder.go
- backend/internal/oauth/oauth2/granthandlers/provider.go
- backend/internal/oauth/oauth2/revocation/RevocationServiceInterface_mock_test.go
- backend/internal/flow/session/participant_store_test.go
- backend/internal/oauth/oauth2/revocation/CriteriaRevokerInterface_mock_test.go
- backend/internal/system/revocationcache/query_constants.go
- backend/internal/system/revocationcache/cache_test.go
- backend/internal/oauth/oauth2/granthandlers/token_exchange.go
- backend/internal/oauth/oauth2/revocation/init_test.go
- backend/internal/oauth/oauth2/revocation/EnforcementServiceInterface_mock_test.go
- backend/internal/flow/executor/session_executor_test.go
- backend/internal/flow/session/Service_mock_test.go
- backend/internal/system/revocationcache/cache.go
- backend/internal/oauth/oauth2/tokenservice/validator_test.go
- backend/cmd/server/servicemanager.go
- backend/internal/oauth/init.go
- backend/internal/oauth/oauth2/authz/AuthorizationCodeStoreInterface_mock_test.go
- tests/integration/oauth/sso/suite_test.go
- backend/internal/system/revocationcache/source_db_test.go
- backend/internal/oauth/oauth2/granthandlers/token_exchange_test.go
- backend/internal/oauth/oauth2/authz/service_test.go
- backend/internal/oauth/oauth2/tokenservice/model.go
- backend/pkg/thunderidengine/config/config.go
- backend/internal/system/revocationcache/syncer_test.go
- backend/internal/system/config/config.go
- backend/internal/system/security/service_test.go
- backend/internal/oauth/oauth2/granthandlers/init.go
- backend/internal/flow/session/service_test.go
- backend/internal/oauth/oauth2/revocation/store.go
- backend/internal/system/security/jwt_authenticator.go
- backend/internal/oauth/oauth2/revocation/service.go
- backend/internal/oauth/oauth2/revocation/revocationStoreInterface_mock_test.go
- backend/internal/system/security/RevocationEnforcerInterface_mock_test.go
- backend/internal/flow/executor/session_executor.go
- backend/internal/oauth/oauth2/revocation/store_test.go
- backend/cmd/server/config/default.json
- backend/internal/oauth/oauth2/granthandlers/refresh_token.go
- backend/internal/system/revocationcache/source_db.go
- backend/internal/oauth/oauth2/revocation/service_test.go
- backend/internal/oauth/oauth2/authz/service.go
- backend/internal/oauth/oauth2/tokenservice/validator.go
- backend/internal/system/revocationcache/enforcer.go
- backend/internal/oauth/oauth2/revocation/criteria_revoker.go
- backend/internal/system/security/service.go
- backend/internal/oauth/oauth2/constants/constants.go
- backend/internal/oauth/oauth2/revocation/enforcement_service.go
- backend/internal/flow/session/service.go
- backend/internal/oauth/oauth2/revocation/enforcement_service_test.go
- backend/dbscripts/runtime_persistent/sqlite.sql
| // MarkConsumedTokenFamily records tokenFamilyID under a replay-lookup key for a just-consumed code, | ||
| // bounded by ttl, so a later replay of the removed code can recover the tfid. An empty | ||
| // tokenFamilyID is a no-op. | ||
| MarkConsumedTokenFamily(ctx context.Context, authCode, tokenFamilyID string, ttl time.Duration) error | ||
| // ConsumedTokenFamily returns the token family id recorded for a consumed authorization code, and | ||
| // whether such a marker exists. | ||
| ConsumedTokenFamily(ctx context.Context, authCode string) (string, bool, error) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Make code consumption and replay-marker persistence atomic.
The supplied redemption flow consumes the code before writing this marker and only logs marker-write failures. A concurrent replay in that gap—or any failed Put—finds no family ID, so the issued family is not revoked. Persist the marker atomically with consumption, and fail redemption when that guarantee cannot be established.
As per coding guidelines, identity-related code must align with relevant RFC specifications.
Also applies to: 94-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/oauth/oauth2/authz/auth_code_store.go` around lines 41 - 47,
Make authorization-code redemption persist the consumed-code state and its
token-family replay marker as one atomic operation, updating the flow around
MarkConsumedTokenFamily and ConsumedTokenFamily and the underlying store
transaction as needed. Propagate marker-persistence failures from redemption
instead of only logging them, and ensure concurrent replays cannot observe a
consumed code without its family ID. Preserve the empty tokenFamilyID no-op
behavior and align any identity handling with the applicable RFC requirements.
Source: Coding guidelines
dd724ea to
6820ab5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/oauth/token/tfid_test.go`:
- Around line 349-419: Update the relevant authentication and configuration
documentation in docs/content/guides/ and docs/content/apis.mdx to describe the
tfid token claim, its propagation across authorization-code and refresh grants,
and grant-scoped revocation triggered by RFC 7009 revocation, refresh-token
reuse, authorization-code replay, and SSO sign-out. Also document
oauth.revocation.token_family.* and oauth.token_exchange.token_family, including
their defaults and behavioral effects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ebc7b9d-161d-4b44-9622-c46477edcc10
⛔ Files ignored due to path filters (4)
backend/tests/mocks/flow/sessionmock/Service_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/granthandlersmock/RefreshTokenGrantHandlerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/CriteriaRevokerInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oauth/oauth2/revocationmock/EnforcementServiceInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (86)
backend/.mockery.public.ymlbackend/cmd/server/config/default.jsonbackend/cmd/server/servicemanager.gobackend/dbscripts/runtime_persistent/postgres-cleanup.sqlbackend/dbscripts/runtime_persistent/postgres.sqlbackend/dbscripts/runtime_persistent/sqlite.sqlbackend/internal/flow/common/constants.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/session_executor.gobackend/internal/flow/executor/session_executor_test.gobackend/internal/flow/session/CriteriaRevoker_mock_test.gobackend/internal/flow/session/Service_mock_test.gobackend/internal/flow/session/init.gobackend/internal/flow/session/model.gobackend/internal/flow/session/participant_store_test.gobackend/internal/flow/session/service.gobackend/internal/flow/session/service_test.gobackend/internal/flow/session/store.gobackend/internal/flow/session/store_constants.gobackend/internal/oauth/init.gobackend/internal/oauth/oauth2/authz/AuthorizationCodeStoreInterface_mock_test.gobackend/internal/oauth/oauth2/authz/auth_code_store.gobackend/internal/oauth/oauth2/authz/auth_code_store_test.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/oauth/oauth2/authz/init_test.gobackend/internal/oauth/oauth2/authz/model.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/oauth/oauth2/granthandlers/authorization_code.gobackend/internal/oauth/oauth2/granthandlers/grant_handler.gobackend/internal/oauth/oauth2/granthandlers/init.gobackend/internal/oauth/oauth2/granthandlers/provider.gobackend/internal/oauth/oauth2/granthandlers/provider_test.gobackend/internal/oauth/oauth2/granthandlers/refresh_token.gobackend/internal/oauth/oauth2/granthandlers/refresh_token_test.gobackend/internal/oauth/oauth2/granthandlers/token_exchange.gobackend/internal/oauth/oauth2/granthandlers/token_exchange_test.gobackend/internal/oauth/oauth2/model/token.gobackend/internal/oauth/oauth2/revocation/CriteriaRevokerInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/EnforcementServiceInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/RevocationServiceInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/RevokedTokenStoreInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/criteria_revoker.gobackend/internal/oauth/oauth2/revocation/criteria_revoker_test.gobackend/internal/oauth/oauth2/revocation/enforcement_service.gobackend/internal/oauth/oauth2/revocation/enforcement_service_test.gobackend/internal/oauth/oauth2/revocation/init.gobackend/internal/oauth/oauth2/revocation/init_test.gobackend/internal/oauth/oauth2/revocation/model.gobackend/internal/oauth/oauth2/revocation/revocationStoreInterface_mock_test.gobackend/internal/oauth/oauth2/revocation/service.gobackend/internal/oauth/oauth2/revocation/service_test.gobackend/internal/oauth/oauth2/revocation/store.gobackend/internal/oauth/oauth2/revocation/store_constants.gobackend/internal/oauth/oauth2/revocation/store_test.gobackend/internal/oauth/oauth2/token/TokenServiceInterface_mock_test.gobackend/internal/oauth/oauth2/token/service.gobackend/internal/oauth/oauth2/token/service_test.gobackend/internal/oauth/oauth2/tokenservice/builder.gobackend/internal/oauth/oauth2/tokenservice/model.gobackend/internal/oauth/oauth2/tokenservice/validator.gobackend/internal/oauth/oauth2/tokenservice/validator_test.gobackend/internal/system/config/config.gobackend/internal/system/revocationcache/cache.gobackend/internal/system/revocationcache/cache_test.gobackend/internal/system/revocationcache/enforcer.gobackend/internal/system/revocationcache/enforcer_test.gobackend/internal/system/revocationcache/init_test.gobackend/internal/system/revocationcache/model.gobackend/internal/system/revocationcache/query_constants.gobackend/internal/system/revocationcache/source.gobackend/internal/system/revocationcache/source_db.gobackend/internal/system/revocationcache/source_db_test.gobackend/internal/system/revocationcache/syncer.gobackend/internal/system/revocationcache/syncer_test.gobackend/internal/system/security/RevocationEnforcerInterface_mock_test.gobackend/internal/system/security/context.gobackend/internal/system/security/jwt_authenticator.gobackend/internal/system/security/service.gobackend/internal/system/security/service_test.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/config/validate.gotests/integration/oauth/sso/rp_logout_test.gotests/integration/oauth/sso/suite_test.gotests/integration/oauth/token/tfid_test.go
💤 Files with no reviewable changes (1)
- backend/internal/oauth/oauth2/revocation/RevokedTokenStoreInterface_mock_test.go
🚧 Files skipped from review as they are similar to previous changes (71)
- backend/internal/oauth/oauth2/token/service.go
- backend/internal/oauth/oauth2/authz/model.go
- backend/internal/oauth/oauth2/authz/init_test.go
- backend/internal/oauth/oauth2/revocation/model.go
- backend/internal/oauth/oauth2/granthandlers/init.go
- backend/internal/system/revocationcache/enforcer_test.go
- backend/internal/system/security/jwt_authenticator.go
- backend/internal/flow/session/store_constants.go
- backend/internal/system/security/service.go
- backend/internal/flow/session/init.go
- backend/internal/oauth/init.go
- backend/internal/oauth/oauth2/revocation/store_constants.go
- backend/internal/oauth/oauth2/revocation/criteria_revoker.go
- backend/internal/flow/session/store.go
- backend/internal/flow/session/CriteriaRevoker_mock_test.go
- backend/internal/oauth/oauth2/authz/auth_code_store_test.go
- backend/internal/system/revocationcache/cache_test.go
- backend/internal/oauth/oauth2/revocation/init_test.go
- backend/internal/oauth/oauth2/model/token.go
- backend/internal/flow/executor/session_executor_test.go
- backend/internal/oauth/oauth2/tokenservice/builder.go
- backend/internal/system/config/config.go
- backend/internal/system/revocationcache/query_constants.go
- backend/internal/oauth/oauth2/constants/constants.go
- backend/internal/flow/executor/auth_assert_executor.go
- backend/.mockery.public.yml
- backend/cmd/server/config/default.json
- backend/internal/system/revocationcache/source_db.go
- backend/internal/oauth/oauth2/authz/init.go
- backend/internal/flow/common/constants.go
- backend/internal/system/revocationcache/model.go
- backend/internal/system/security/context.go
- backend/internal/flow/session/participant_store_test.go
- backend/internal/oauth/oauth2/revocation/criteria_revoker_test.go
- backend/internal/system/revocationcache/init_test.go
- backend/internal/oauth/oauth2/authz/auth_code_store.go
- backend/internal/oauth/oauth2/revocation/RevocationServiceInterface_mock_test.go
- backend/internal/system/revocationcache/cache.go
- backend/cmd/server/servicemanager.go
- backend/internal/oauth/oauth2/revocation/revocationStoreInterface_mock_test.go
- backend/internal/oauth/oauth2/token/service_test.go
- backend/internal/oauth/oauth2/granthandlers/token_exchange_test.go
- backend/internal/oauth/oauth2/granthandlers/provider_test.go
- backend/internal/system/revocationcache/source.go
- backend/internal/oauth/oauth2/granthandlers/provider.go
- backend/internal/oauth/oauth2/revocation/enforcement_service.go
- backend/internal/flow/session/service.go
- backend/dbscripts/runtime_persistent/postgres-cleanup.sql
- backend/internal/oauth/oauth2/granthandlers/grant_handler.go
- backend/internal/oauth/oauth2/revocation/service.go
- backend/internal/system/revocationcache/syncer_test.go
- backend/pkg/thunderidengine/config/validate.go
- backend/internal/oauth/oauth2/tokenservice/validator_test.go
- backend/internal/oauth/oauth2/authz/AuthorizationCodeStoreInterface_mock_test.go
- backend/internal/flow/session/Service_mock_test.go
- backend/internal/oauth/oauth2/authz/service_test.go
- tests/integration/oauth/sso/rp_logout_test.go
- backend/internal/oauth/oauth2/tokenservice/model.go
- backend/internal/system/security/service_test.go
- backend/dbscripts/runtime_persistent/postgres.sql
- backend/internal/oauth/oauth2/granthandlers/token_exchange.go
- backend/dbscripts/runtime_persistent/sqlite.sql
- backend/internal/flow/session/service_test.go
- backend/internal/oauth/oauth2/revocation/CriteriaRevokerInterface_mock_test.go
- tests/integration/oauth/sso/suite_test.go
- backend/internal/system/revocationcache/enforcer.go
- backend/internal/oauth/oauth2/revocation/EnforcementServiceInterface_mock_test.go
- backend/internal/oauth/oauth2/revocation/enforcement_service_test.go
- backend/internal/oauth/oauth2/tokenservice/validator.go
- backend/internal/oauth/oauth2/granthandlers/refresh_token.go
- backend/internal/flow/executor/session_executor.go
| // The access and refresh tokens of one login share a single, non-empty tfid. | ||
| func (ts *TfidTestSuite) TestAccessAndRefreshTokensShareTfid() { | ||
| tokens := ts.obtainTokens() | ||
|
|
||
| atTfid := ts.tfidClaim(tokens.AccessToken) | ||
| rtTfid := ts.tfidClaim(tokens.RefreshToken) | ||
|
|
||
| ts.NotEmpty(atTfid, "Access token should carry a tfid") | ||
| ts.NotEmpty(rtTfid, "Refresh token should carry a tfid") | ||
| ts.Equal(atTfid, rtTfid, "Access and refresh tokens of one grant share a tfid") | ||
| } | ||
|
|
||
| // The tfid is copied onto both tokens minted during a refresh: the new access token and, since | ||
| // refresh-token rotation is enabled by default, the rotated refresh token. | ||
| func (ts *TfidTestSuite) TestTfidPreservedOnRefresh() { | ||
| tokens := ts.obtainTokens() | ||
| originalTfid := ts.tfidClaim(tokens.AccessToken) | ||
| ts.Require().NotEmpty(originalTfid) | ||
|
|
||
| refreshed, err := testutils.RefreshAccessToken(tfidTestClientID, tfidTestClientSecret, tokens.RefreshToken) | ||
| ts.Require().NoError(err, "Refresh should succeed") | ||
| ts.Require().NotEmpty(refreshed.AccessToken, "Refreshed access token should not be empty") | ||
| ts.Require().NotEmpty(refreshed.RefreshToken, "Rotation is enabled, so a new refresh token should be issued") | ||
|
|
||
| ts.Equal(originalTfid, ts.tfidClaim(refreshed.AccessToken), | ||
| "The refreshed access token keeps the grant's tfid") | ||
| ts.Equal(originalTfid, ts.tfidClaim(refreshed.RefreshToken), | ||
| "The rotated refresh token keeps the grant's tfid") | ||
| } | ||
|
|
||
| // Explicitly revoking a login's refresh token also drops its access token (grant-scoped revocation). | ||
| func (ts *TfidTestSuite) TestExplicitRefreshRevokeDropsAccessToken() { | ||
| tokens := ts.obtainTokens() | ||
| ts.Require().True(ts.introspectActive(tokens.AccessToken), "Access token should start active") | ||
|
|
||
| ts.revokeRefreshToken(tokens.RefreshToken) | ||
|
|
||
| ts.False(ts.introspectActive(tokens.AccessToken), | ||
| "Revoking the refresh token must drop the login's access token via its tfid") | ||
| } | ||
|
|
||
| // Redeeming an authorization code twice (replay) revokes the whole grant issued from the first redemption. | ||
| func (ts *TfidTestSuite) TestAuthCodeReplayRevokesGrant() { | ||
| code, tokens := ts.obtainCodeAndTokens() | ||
| ts.Require().True(ts.introspectActive(tokens.AccessToken), "Access token should start active") | ||
|
|
||
| // Replay the already-consumed code. | ||
| replay, err := testutils.RequestTokenWithResource( | ||
| tfidTestClientID, tfidTestClientSecret, code, tfidTestRedirectURI, "authorization_code", tfidTestResource) | ||
| ts.Require().NoError(err, "Replay request should complete") | ||
| ts.NotEqual(http.StatusOK, replay.StatusCode, "Replaying a consumed code must not issue tokens") | ||
|
|
||
| ts.False(ts.introspectActive(tokens.AccessToken), | ||
| "An authorization-code replay must revoke the grant issued from the first redemption") | ||
| } | ||
|
|
||
| // Independent logins get distinct tfids, and revoking one family leaves the other untouched. | ||
| func (ts *TfidTestSuite) TestIndependentGrantsAreIsolated() { | ||
| first := ts.obtainTokens() | ||
| second := ts.obtainTokens() | ||
|
|
||
| ts.NotEqual(ts.tfidClaim(first.AccessToken), ts.tfidClaim(second.AccessToken), | ||
| "Two independent logins must mint different tfids") | ||
|
|
||
| // Revoke the first login's family; the second must remain active. | ||
| ts.revokeRefreshToken(first.RefreshToken) | ||
|
|
||
| ts.False(ts.introspectActive(first.AccessToken), "The revoked login's access token is inactive") | ||
| ts.True(ts.introspectActive(second.AccessToken), | ||
| "An independent login must be unaffected by another login's revocation") | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
- Token-family (
tfid) token claim and grant-scoped revocation behavior: document propagation across authorization-code and refresh grants, plus revocation caused by RFC 7009 revocation, refresh-token reuse, authorization-code replay, and SSO sign-out indocs/content/guides/anddocs/content/apis.mdx. - Token-family configuration: document
oauth.revocation.token_family.*andoauth.token_exchange.token_familydefaults and effects indocs/content/guides/.
As per path instructions, authentication, configuration, user-facing behavior, and SDK-impacting changes without docs/ updates require one consolidated documentation comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/oauth/token/tfid_test.go` around lines 349 - 419, Update
the relevant authentication and configuration documentation in
docs/content/guides/ and docs/content/apis.mdx to describe the tfid token claim,
its propagation across authorization-code and refresh grants, and grant-scoped
revocation triggered by RFC 7009 revocation, refresh-token reuse,
authorization-code replay, and SSO sign-out. Also document
oauth.revocation.token_family.* and oauth.token_exchange.token_family, including
their defaults and behavioral effects.
Source: Path instructions
6820ab5 to
3608e7c
Compare
ed850e4 to
a40b408
Compare
Mint a token family id (tfid) per login flow and carry it across the authorization grant (authorization code, access and refresh tokens, preserved across refresh rotation) so a whole grant can be revoked at once. Add a criteria-based revocation deny list (REVOCATION_CRITERIA) with a token_family criterion and a CriteriaRevoker write seam, enforced on both the authorization-server hot path and the resource-server cache. Revocation triggers: refresh-token reuse, RFC 7009 explicit revoke, authorization-code replay, and SSO sign-out. Configurable via oauth.revocation.token_family.* and oauth.token_exchange.token_family. Refs thunder-id#3321
a40b408 to
1589867
Compare
Add a token revocation reference under the OAuth/OIDC protocol guides covering the RFC 7009 endpoint, grant-scoped revocation through the token family, and user-scoped revocation, along with where each is enforced and how fresh that enforcement is. Fill the matching gaps in the configuration reference: the revocation settings under oauth.* and server.security.token_revocation.*, and userDeletionFlow in the supported flow types. Refs thunder-id#4300 thunder-id#4591 thunder-id#4669
Add a token revocation reference under the OAuth/OIDC protocol guides covering the RFC 7009 endpoint, grant-scoped revocation through the token family, and user-scoped revocation, along with where each is enforced and how fresh that enforcement is. Fill the matching gaps in the configuration reference: the revocation settings under oauth.* and server.security.token_revocation.*, and userDeletionFlow in the supported flow types. Refs thunder-id#4300 thunder-id#4591 thunder-id#4669
Purpose
Add a token family id (tfid) for grant-scoped revocation, so a whole authorization grant (its access and refresh tokens) can be revoked at once, closing the refresh-token-reuse and authorization-code-replay gaps called out in RFC 9700.
A tfid is minted per login flow and carried across the grant: authorization code, access token, and refresh token, and preserved across refresh rotation. A new criteria-based revocation deny list records a revoked tfid, and both the authorization server (hot path) and resource server (cache) reject any token whose tfid is revoked.
Approach
tfidclaim during the login flow (SessionExecutor), reused across a flow execution, and carry it assertion to authorization code to access/refresh tokens. Non-SSO auth-code flows anchor a fallback tfid at code creation so grant-scoped revocation always applies.REVOCATION_CRITERIAtable (runtime-persistent) with atoken_familycriterion and aCriteriaRevokerwrite seam. The revoker type is intentionally general (criteria-based); its current method,RevokeTokenFamily, names the one criterion that exists today, so future criteria add methods rather than new types.oauth.revocation.token_family.{on_refresh_reuse,on_explicit_revoke,on_code_replay}andoauth.token_exchange.token_family(none|inherit).Related Issues
Related PRs
Checklist
Security checks
Summary by CodeRabbit
none/inherit).