Skip to content

fix(authz): keep a rotated refresh token valid for a short grace window CU-17tkuw62gnb - #2919

Merged
aliaska-varieva merged 5 commits into
mainfrom
fix/CU-17tkuw62gnb
Nov 2, 2026
Merged

aliaska-varieva merged 5 commits into
mainfrom
fix/CU-17tkuw62gnb

Conversation

@aliaska-varieva

@aliaska-varieva aliaska-varieva commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A refresh token works once. Rotation overwrites it in Mongo, and the client learns the new value only from the response. If that response is lost (reload, closed tab, dropped connection), the client keeps a token that matches nothing, and every later refresh returns 401. Reproduced on prod by aborting POST /oauth/refresh mid-flight.

Fix

  • On save, a replaced refresh token moves to retiredRefreshTokens on the same authorization.
  • findByToken accepts a retired token for 60s (openframe.authz.refresh-token-grace-seconds), so a retry rotates normally. It's a list, so a retry that rotates again doesn't kill a token another request just received.
  • The returned authorization carries the presented value, which Spring's revocation needs, and the current token's metadata, so a revoked session stays revoked.
  • The BFF resolves retired tokens for refresh and logout, and logs unknown tokens at WARN without the value.
  • New sparse index on retiredRefreshTokens.value, in MongoIndexConfig and on the document.

Testing

  • Full oss-lib suite: 4,725 run, 0 failed, 275 skipped.
  • Two repro tests failed before the fix and pass unchanged after it.
  • MongoOAuth2AuthorizationRepositoryIT covers the nested query and index on real Mongo (run with -Dintegration.tests=true).
  • Not run through Spring's real token endpoint. Verify on dev: abort a refresh, then refresh again.

Limits

  • A retry after the 60s window is still refused, so a tab closed during a refresh and reopened later is logged out.
  • A stolen rotated token stays usable for up to 60s.
  • Two rotations at the same instant still race.

Rollout

Needs an oss-lib release, then bumps in openframe-saas-shared for the auth-server and the user-gateway, which is pinned separately on prod. Deploy the auth-server first. An older auth-server ignores and drops the new field, so rollback is safe.

Change-Set: fix-cu-17tkuw62gnb

Change set flamingo-stack/fix-cu-17tkuw62gnb: this pull request is the only one in it so far. Another pull request joins by naming this one in a Depends-On line, or by carrying the same Change-Set line.

Linked work

  • …and 1 linked item not shown here

Linked by the Depends-On / Change-Set lines in these descriptions; this block is maintained by the hub.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

🦩 Flamingo Code Review

✅ Tests and checks passed on this commit (2 lanes).

4 findings: 🔴 1 action required · 🟠 1 recommended · 🔵 2 informational

Advisory: findings do not block the merge.

Rules cited: OFJAVA-002, OFJAVA-004, OFJAVA-007

Files Lines Languages Tests 🔴 Action required 🟠 Recommended 🔵 Informational
12 +677 / -2 Java (1 Maven lane) ✅ 951 / 951 green (204 skipped) 1 1 2

🔴 Action required (1)

# Where Finding Flagged by
1 MongoAuthorizationService.java:43 Method call passed directly as argument to setRetiredRefreshTokens OFJAVA-002

🟠 Recommended (1)

# Where Finding Flagged by
2 MongoAuthorizationService.java:170 Multi-line lambda body in findByRetiredRefreshToken's .map() OFJAVA-004

🔵 Informational (2)

# Where Finding Flagged by
3 MongoAuthorizationService.java:56 entity.getId() called inline as argument to findById (Minor) OFJAVA-002
4 MongoAuthorizationService.java:168 Compound condition inline in retired-token filter lambda (Minor) OFJAVA-007

✅ What this change does well

  • MongoAuthorizationServiceRefreshGraceTest.java covers the grace-window boundary, concurrent retry-rotation, and revocation-through-retired-token cases thoroughly.
  • MongoOAuth2Authorization.java pairs the new @‍CompoundIndex with a matching explicit ensureIndex in MongoIndexConfig.java, following the repo's own self-healing-index convention.
  • OAuthBffServiceRetiredTokenTest.java asserts that the unknown refresh token case never logs the raw token value, giving a concrete regression guard for leaking secrets via logs.
  • The test suite covers both the happy-path (current token match, skip retired lookup) and the retired-token/unknown-token branches of revokeRefreshTokenByLookup, exercising the new retired-token logic from multiple angles.
  • MongoAuthorizationServiceRefreshGraceTest covers the window boundary, double rotation, revocation and pruning, killing 26 of 27 mutants on the new lines of MongoAuthorizationService.java.

Inline comments: 4 new

7 possible problems checked and ruled out
  • openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:36 Inline default on new refresh-token grace @‍Value property hides environment drift: refreshTokenGraceSeconds declared with inline default ':60', exactly the pattern OFJAVA-011 forbids for an environment-sensitive security setting.
  • openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:161 Multi-line Javadoc-style block comment on findByRetiredRefreshToken: duplicate of candidate 2
  • openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:48 Multi-line Javadoc-style comment blocks added on new methods violate the no-Javadoc rule: Multi-line /** */ blocks added on retiredTokensAfterSaving and findByRetiredRefreshToken exceed the one-line-why exception that OFJAVA-008 CI-enforces.
  • openframe-data-mongo-common/src/main/java/com/openframe/data/document/oauth/RetiredRefreshToken.java:5 Multi-line Javadoc comment on RetiredRefreshToken record violates the no-Javadoc rule: Two-sentence /** */ block on the new RetiredRefreshToken record violates the same no-Javadoc CI rule, on a different file/line than candidate 2.
  • openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:55 save() computes the retired-token list with a non-atomic read-then-write, racing under concurrent rotation: Speculative concurrency claim with no evidence of concurrent save() calls on the same id actually occurring in this flow; a different, already-posted comment covers the real behavioral bug on these lines.
  • openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:56 Optional consumed via orElse(null) plus a manual null check: orElse(null)+null-check is not the isPresent()/.get() pattern the rule's example names, and the method legitimately needs the unwrapped value for several subsequent statements; stretch of the rule.
  • openframe-data-mongo-sync/src/main/java/com/openframe/data/config/MongoIndexConfig.java:15 2 new or changed definitions have no test referencing them: MongoIndexConfig is a mechanical index-declaration config class; missing-test flag on boilerplate infra config is not worth a comment.

🧪 Tests and mutation

Tests per lane

Language Project Command Status Counts Time
Java (Maven) . mvn -B test -pl openframe-authorization-service-core,openframe-data-mongo-common,openframe-data-mongo-sync,openframe-security-oauth -am ✅ passed authorization-service-core 403 (6 skipped) · core 150 · core-crypto 8 · data-mongo-sync 465 (198 skipped) · exception 4 · notification-mail 4 · security-core 38 · security-oauth 83 · 0 failures 25 s

Java tests passed on all changed modules: 1155 run, 0 failed, 204 skipped. MongoOAuth2AuthorizationRepositoryIT is among the skipped unless integration.tests=true; it passed when run by hand with real Mongo.

Mutation testing

This build has no PIT plugin, so each module is mutated from the command line with pit-adhoc.sh, changing nothing in the repository.

Module Classes Whole classes New lines only Time
authorization-service-core MongoAuthorizationService 49 mutants, 35 killed (71%), 8 survived, 6 no coverage, test strength 81%, line coverage 84% 27 mutants: 26 killed, 1 survived 20 s
data-mongo-sync MongoIndexConfig 8 mutants, 0 killed (0%), 8 no coverage, line coverage 0% none on added lines 19 s
security-oauth OAuthBffService 164 mutants, 117 killed (71%), 36 survived, 11 no coverage, test strength 76%, line coverage 94% 6 mutants: 6 killed 83 s
data-mongo-common MongoOAuth2Authorization, RetiredRefreshToken Gap: the module has no tests, so PIT was not run (every mutant would survive)

New code, its covering tests and the result

New code Covering tests Result
save in MongoAuthorizationService, line 44 MongoAuthorizationServiceRefreshGraceTest (2), MongoAuthorizationServiceRotationReproTest (1), 8 more killed
retiredTokensAfterSaving in MongoAuthorizationService, lines 57-72 MongoAuthorizationServiceRefreshGraceTest (3), 8 more killed
lambda$retiredTokensAfterSaving$0 in MongoAuthorizationService, line 64 MongoAuthorizationServiceRefreshGraceTest (3), 2 more killed
lambda$retiredTokensAfterSaving$1 in MongoAuthorizationService, line 65 MongoAuthorizationServiceRefreshGraceTest (3), 1 more killed
isWithinGrace in MongoAuthorizationService, line 76 MongoAuthorizationServiceRefreshGraceTest (2), MongoAuthorizationServiceRotationReproTest (1), 8 more killed
findByToken in MongoAuthorizationService, lines 141-149 MongoAuthorizationServiceRefreshGraceTest (3), 11 more killed
findByRetiredRefreshToken in MongoAuthorizationService, line 167 MongoAuthorizationServiceRefreshGraceTest (2), MongoAuthorizationServiceRotationReproTest (1), 7 more killed
lambda$findByRetiredRefreshToken$0 in MongoAuthorizationService, line 168 MongoAuthorizationServiceRefreshGraceTest (2), MongoAuthorizationServiceRotationReproTest (1), 6 more killed
lambda$findByRetiredRefreshToken$1 in MongoAuthorizationService, line 169 MongoAuthorizationServiceRefreshGraceTest (2), MongoAuthorizationServiceRotationReproTest (1), 6 more killed
lambda$findByRetiredRefreshToken$2 in MongoAuthorizationService, line 176 MongoAuthorizationServiceRefreshGraceTest (2), MongoAuthorizationServiceRotationReproTest (1), 5 more killed
lambda$findByRetiredRefreshToken$3 in MongoAuthorizationService, line 177 MongoAuthorizationServiceRefreshGraceTest (2), MongoAuthorizationServiceRotationReproTest (1), 5 more Survived: line 177: removed call to Map::putAll
lambda$refreshTokensByLookup$0 in OAuthBffService, line 153 OAuthBffServiceRetiredTokenTest (1), OAuthBffServiceRefreshTest (1), OAuthBffServiceFlowTest (1), 2 more killed
refreshTokensByLookup in OAuthBffService, line 153 OAuthBffServiceRetiredTokenTest (1), OAuthBffServiceRefreshTest (1), OAuthBffServiceFlowTest (1), 2 more killed
findAuthorizationByRefreshToken in OAuthBffService, line 165 OAuthBffServiceRetiredTokenTest (2), OAuthBffServiceRefreshTest (1), 6 more killed
lambda$findAuthorizationByRefreshToken$0 in OAuthBffService, line 166 OAuthBffServiceRetiredTokenTest (2), OAuthBffServiceRefreshTest (1), 2 more killed
lambda$revokeRefreshTokenByLookup$0 in OAuthBffService, line 535 OAuthBffServiceRetiredTokenTest (2), OAuthBffServiceFlowTest (1), 1 more killed
revokeRefreshTokenByLookup in OAuthBffService, line 535 OAuthBffServiceRetiredTokenTest (3), 1 more killed
The covering tests by name (17 methods)
  • MongoAuthorizationService.save: MongoAuthorizationServiceRefreshGraceTest.savingWithoutRotatingRetiresNothingAndKeepsWhatIsRetired, MongoAuthorizationServiceRefreshGraceTest.savingDropsRetiredTokensPastTheirWindow, MongoAuthorizationServiceRotationReproTest.reproducesBug_CU_17tkuw62gnb_refreshTokenSurvivesItsRotationInsideTheGraceWindow, and 8 more
  • MongoAuthorizationService.retiredTokensAfterSaving: MongoAuthorizationServiceRefreshGraceTest.savingANewAuthorizationStoresNoList, MongoAuthorizationServiceRefreshGraceTest.savingWithoutRotatingRetiresNothingAndKeepsWhatIsRetired, MongoAuthorizationServiceRefreshGraceTest.savingDropsRetiredTokensPastTheirWindow, and 8 more
  • MongoAuthorizationService.lambda$retiredTokensAfterSaving$0: MongoAuthorizationServiceRefreshGraceTest.savingWithoutRotatingRetiresNothingAndKeepsWhatIsRetired, MongoAuthorizationServiceRefreshGraceTest.revokingTheSessionAlsoEndsItsRetiredTokens, MongoAuthorizationServiceRefreshGraceTest.aRetryThatRotatesAgainKeepsEveryRetiredTokenUsable, and 2 more
  • MongoAuthorizationService.lambda$retiredTokensAfterSaving$1: MongoAuthorizationServiceRefreshGraceTest.savingWithoutRotatingRetiresNothingAndKeepsWhatIsRetired, MongoAuthorizationServiceRefreshGraceTest.revokingThroughARetiredTokenLeavesNoActiveTokenBehind, MongoAuthorizationServiceRefreshGraceTest.revokingTheSessionAlsoEndsItsRetiredTokens, and 1 more
  • MongoAuthorizationService.isWithinGrace: MongoAuthorizationServiceRefreshGraceTest.acceptsARetiredTokenJustInsideTheGraceWindow, MongoAuthorizationServiceRefreshGraceTest.savingWithoutRotatingRetiresNothingAndKeepsWhatIsRetired, MongoAuthorizationServiceRotationReproTest.reproducesBug_CU_17tkuw62gnb_refreshTokenSurvivesItsRotationInsideTheGraceWindow, and 8 more
  • MongoAuthorizationService.findByToken: MongoAuthorizationServiceRefreshGraceTest.anUntypedLookupAlsoResolvesARetiredToken, MongoAuthorizationServiceRefreshGraceTest.savingWithNothingRetiredStoresNoList, MongoAuthorizationServiceRefreshGraceTest.savingANewAuthorizationStoresNoList, and 11 more
  • MongoAuthorizationService.findByRetiredRefreshToken: MongoAuthorizationServiceRefreshGraceTest.acceptsARetiredTokenJustInsideTheGraceWindow, MongoAuthorizationServiceRotationReproTest.reproducesBug_CU_17tkuw62gnb_refreshTokenSurvivesItsRotationInsideTheGraceWindow, MongoAuthorizationServiceRefreshGraceTest.aRetiredTokenKeepsTheCurrentExpiry, and 7 more
  • MongoAuthorizationService.lambda$findByRetiredRefreshToken$0: MongoAuthorizationServiceRefreshGraceTest.acceptsARetiredTokenJustInsideTheGraceWindow, MongoAuthorizationServiceRotationReproTest.reproducesBug_CU_17tkuw62gnb_refreshTokenSurvivesItsRotationInsideTheGraceWindow, MongoAuthorizationServiceRefreshGraceTest.aRetiredTokenKeepsTheCurrentExpiry, and 6 more
  • MongoAuthorizationService.lambda$findByRetiredRefreshToken$1: MongoAuthorizationServiceRefreshGraceTest.acceptsARetiredTokenJustInsideTheGraceWindow, MongoAuthorizationServiceRotationReproTest.reproducesBug_CU_17tkuw62gnb_refreshTokenSurvivesItsRotationInsideTheGraceWindow, MongoAuthorizationServiceRefreshGraceTest.aRetiredTokenKeepsTheCurrentExpiry, and 6 more
  • MongoAuthorizationService.lambda$findByRetiredRefreshToken$2: MongoAuthorizationServiceRefreshGraceTest.acceptsARetiredTokenJustInsideTheGraceWindow, MongoAuthorizationServiceRotationReproTest.reproducesBug_CU_17tkuw62gnb_refreshTokenSurvivesItsRotationInsideTheGraceWindow, MongoAuthorizationServiceRefreshGraceTest.aRetiredTokenKeepsTheCurrentExpiry, and 5 more
  • MongoAuthorizationService.lambda$findByRetiredRefreshToken$3: MongoAuthorizationServiceRefreshGraceTest.acceptsARetiredTokenJustInsideTheGraceWindow, MongoAuthorizationServiceRotationReproTest.reproducesBug_CU_17tkuw62gnb_refreshTokenSurvivesItsRotationInsideTheGraceWindow, MongoAuthorizationServiceRefreshGraceTest.aRetiredTokenKeepsTheCurrentExpiry, and 5 more
  • OAuthBffService.lambda$refreshTokensByLookup$0: OAuthBffServiceRetiredTokenTest.logsAnUnknownRefreshTokenWithoutItsValue, OAuthBffServiceRefreshTest.reproducesBug_CU_17tkuw62gnb_lookupFindsARetiredTokensTenantSoTheRetryReachesTheAuthServer, OAuthBffServiceFlowTest.shouldRefreshAtTenantTakenFromStoredAuthorizationUri, and 2 more
  • OAuthBffService.refreshTokensByLookup: OAuthBffServiceRetiredTokenTest.logsAnUnknownRefreshTokenWithoutItsValue, OAuthBffServiceRefreshTest.reproducesBug_CU_17tkuw62gnb_lookupFindsARetiredTokensTenantSoTheRetryReachesTheAuthServer, OAuthBffServiceFlowTest.shouldRefreshAtTenantTakenFromStoredAuthorizationUri, and 2 more
  • OAuthBffService.findAuthorizationByRefreshToken: OAuthBffServiceRetiredTokenTest.doesNotCheckRetiredTokensWhenTheCurrentTokenMatches, OAuthBffServiceRetiredTokenTest.logoutWithARetiredTokenStillReachesTheAuthServer, OAuthBffServiceRefreshTest.reproducesBug_CU_17tkuw62gnb_lookupFindsARetiredTokensTenantSoTheRetryReachesTheAuthServer, and 6 more
  • OAuthBffService.lambda$findAuthorizationByRefreshToken$0: OAuthBffServiceRetiredTokenTest.logoutWithARetiredTokenStillReachesTheAuthServer, OAuthBffServiceRefreshTest.reproducesBug_CU_17tkuw62gnb_lookupFindsARetiredTokensTenantSoTheRetryReachesTheAuthServer, OAuthBffServiceRetiredTokenTest.logoutWithAnUnknownTokenCallsNothing, and 2 more
  • OAuthBffService.lambda$revokeRefreshTokenByLookup$0: OAuthBffServiceRetiredTokenTest.doesNotCheckRetiredTokensWhenTheCurrentTokenMatches, OAuthBffServiceRetiredTokenTest.logoutWithARetiredTokenStillReachesTheAuthServer, OAuthBffServiceFlowTest.shouldRevokeByLookupAtStoredTenant, and 1 more
  • OAuthBffService.revokeRefreshTokenByLookup: OAuthBffServiceRetiredTokenTest.logoutWithAnUnknownTokenCallsNothing, OAuthBffServiceRetiredTokenTest.doesNotCheckRetiredTokensWhenTheCurrentTokenMatches, OAuthBffServiceRetiredTokenTest.logoutWithARetiredTokenStillReachesTheAuthServer, and 1 more

What the surviving mutations leave unverified

  • MongoAuthorizationService.java:177: The surviving mutant removes the metadata copy. A revoked session staying revoked is asserted by tests, but those tests pass with or without this line, so they do not show that this line is what provides it.

1 mutant on lines this change added is not caught (1 survived, 0 with no test reaching it): each is annotated on its line in the Flamingo Code Review / tests and mutation check.

1 gap (not tested here):

  • Java (Maven) .: MongoOAuth2AuthorizationRepository.java changed but not mutation-tested (outside the mutation targets openframe-data-mongo-sync configures)
🤖 Prompt for AI agents: fix every finding
Address these code-review findings on pull request #2919 of flamingo-stack/openframe-oss-lib, most important first.
For each one, make the minimal change that resolves it and do not refactor unrelated code.
If you can show a finding is wrong, skip it and say why.

1. [Action required] openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:43 Method call passed directly as argument to setRetiredRefreshTokens
   entity.setRetiredRefreshTokens(retiredTokensAfterSaving(entity)) calls retiredTokensAfterSaving(entity) inline as the argument of setRetiredRefreshTokens, violating OFJAVA-002's requirement that method calls be extracted to a named local before being passed as an argument.
2. [Recommended] openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:170 Multi-line lambda body in findByRetiredRefreshToken's .map()
   The .map(entity -> { ... }) lambda in findByRetiredRefreshToken spans multiple statements (logging, variable assignment, construction of a new token, building a new OAuth2Authorization), violating OFJAVA-004's requirement that lambda bodies fit on one line and be extracted to a named method otherwise.
3. [Informational] openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:56 entity.getId() called inline as argument to findById
   repository.findById(entity.getId()) calls entity.getId() directly inside findById's argument list, violating OFJAVA-002.
4. [Informational] openframe-authorization-service-core/src/main/java/com/openframe/authz/service/auth/MongoAuthorizationService.java:168 Compound condition inline in retired-token filter lambda
   The filter predicate `retired.value().equals(token) && isWithinGrace(retired, now)` combines two conditions with && directly inside the stream lambda instead of being extracted into a named private predicate method, as OFJAVA-007 requires for any condition containing &&.

Next: commit a suggestion, or hand a comment's prompt to your coding agent. React 👍 on a finding that helped and 👎 on one that is wrong: the reviewer learns from both.

Review again. New commits are not reviewed until you ask:

  • Review the new commits: only what was pushed since this review
  • Review the whole diff again: everything, including what was already reviewed

Or comment @flamingo-review (new commits) or @flamingo-review full (everything). Add the flamingo-review-always label to review every push.

Started 2026-11-01 23:15 UTC · updated 2026-11-01 23:25 UTC · reviewed from Aliaska Varieva's machine

@aliaska-varieva
aliaska-varieva added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 0dca571 Nov 2, 2026
19 checks passed
@aliaska-varieva
aliaska-varieva deleted the fix/CU-17tkuw62gnb branch November 2, 2026 11:49
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.

3 participants