Skip to content

fix: verify nonce against original after refresh instead of overwriting it #2211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FabianGosebrink wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: verify nonce against original after refresh instead of overwriting it #2211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44a20a0
fix: verify nonce against original after refresh instead of overwriti…
FabianGosebrink May 28, 2026
b0ea19a
Merge branch 'main' into fix/silent-renew-nonce-placeholder
FabianGosebrink May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions ...c-client/src/lib/flows/callback-handling/refresh-session-callback-handler.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,23 @@ describe('RefreshSessionCallbackHandlerService', () => {
},
});
}));

it('does not overwrite the stored nonce when a refresh token exists', waitForAsync(() => {
spyOn(
flowsDataService,
'getExistingOrCreateAuthStateControl'
).and.returnValue('state-data');
spyOn(authStateService, 'getRefreshToken').and.returnValue(
'henlo-furiend'
);
spyOn(authStateService, 'getIdToken').and.returnValue('henlo-legger');
const setNonceSpy = spyOn(flowsDataService, 'setNonce');

service
.refreshSessionWithRefreshTokens({ configId: 'configId1' })
.subscribe(() => {
expect(setNonceSpy).not.toHaveBeenCalled();
});
}));
});
});
6 changes: 0 additions & 6 deletions ...h-oidc-client/src/lib/flows/callback-handling/refresh-session-callback-handler.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { Observable, of, throwError } from 'rxjs';
import { AuthStateService } from '../../auth-state/auth-state.service';
import { OpenIdConfiguration } from '../../config/openid-configuration';
import { LoggerService } from '../../logging/logger.service';
import { TokenValidationService } from '../../validation/token-validation.service';
import { CallbackContext } from '../callback-context';
import { FlowsDataService } from '../flows-data.service';

Expand Down Expand Up @@ -44,11 +43,6 @@ export class RefreshSessionCallbackHandlerService {
config,
'found refresh code, obtaining new credentials with refresh code'
);
// Nonce is not used with refresh tokens; but Key cloak may send it anyway
this.flowsDataService.setNonce(
TokenValidationService.refreshTokenNoncePlaceholder,
config
);

return of(callbackContext);
} else {
Expand Down
3 changes: 2 additions & 1 deletion projects/angular-auth-oidc-client/src/lib/validation/state-validation.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ export class StateValidationService {
toReturn.decodedIdToken,
authNonce,
Boolean(ignoreNonceAfterRefresh),
configuration
configuration,
isInRefreshTokenFlow
)
) {
this.loggerService.logWarning(
Expand Down
52 changes: 24 additions & 28 deletions projects/angular-auth-oidc-client/src/lib/validation/token-validation.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,56 +111,52 @@ describe('TokenValidationService', () => {
).toBe(false);
});

it('should validate id token nonce after refresh token grant when undefined and no ignore', () => {
it('validates refresh-token flow when id_token contains no nonce (spec-compliant IdP)', () => {
expect(
tokenValidationService.validateIdTokenNonce(
{ nonce: undefined },
TokenValidationService.refreshTokenNoncePlaceholder,
'realNonce',
false,
{
configId: 'configId1',
}
{ configId: 'configId1' },
true
)
).toBe(true);
});

it('should validate id token nonce after refresh token grant when undefined and ignore', () => {
it('validates refresh-token flow when id_token nonce matches the original nonce (Keycloak case)', () => {
expect(
tokenValidationService.validateIdTokenNonce(
{ nonce: undefined },
TokenValidationService.refreshTokenNoncePlaceholder,
true,
{
configId: 'configId1',
}
{ nonce: 'realNonce' },
'realNonce',
false,
{ configId: 'configId1' },
true
)
).toBe(true);
});

it('should validate id token nonce after refresh token grant when defined and ignore', () => {
it('rejects refresh-token flow when id_token nonce differs from original and ignoreNonceAfterRefresh is false', () => {
expect(
tokenValidationService.validateIdTokenNonce(
{ nonce: 'test1' },
TokenValidationService.refreshTokenNoncePlaceholder,
true,
{
configId: 'configId1',
}
{ nonce: 'tamperedNonce' },
'realNonce',
false,
{ configId: 'configId1' },
true
)
).toBe(true);
).toBe(false);
});

it('should not validate id token nonce after refresh token grant when defined and no ignore', () => {
it('validates refresh-token flow when ignoreNonceAfterRefresh is true regardless of returned nonce', () => {
expect(
tokenValidationService.validateIdTokenNonce(
{ nonce: 'test1' },
TokenValidationService.refreshTokenNoncePlaceholder,
false,
{
configId: 'configId1',
}
{ nonce: 'anythingFromIdP' },
'realNonce',
true,
{ configId: 'configId1' },
true
)
).toBe(false);
).toBe(true);
});
});

Expand Down
36 changes: 31 additions & 5 deletions projects/angular-auth-oidc-client/src/lib/validation/token-validation.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ import { alg2kty, getImportAlg, getVerifyAlg } from './token-validation.helper';

@Injectable({ providedIn: 'root' })
export class TokenValidationService {
/**
* @deprecated No longer written to storage. Kept only so external
* consumers that previously imported the symbol still compile.
* Will be removed in a future major release.
*/
static refreshTokenNoncePlaceholder = '--RefreshToken--';

@damienbod damienbod May 28, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why is this not stored on the storage, what happens after a refresh?

Not saying this is bad, just want to understand this better. Also wondering if this is a behavior change

@FabianGosebrink FabianGosebrink May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

All Good. It is good that you double check this.

In the old code the RefreshSessionCallbackHandlerService overwrote authNonce in storage with '--RefreshToken--' before each refresh. validateIdTokenNonce then detected refresh flow by checking if localNonce === refreshTokenNoncePlaceholder. So the storage was being used as a flag and the real initial nonce was getting overwritten.

Now it is not stored in the storage anymore. authNonce in storage now holds the original nonce from the initial authentication, for the whole session. It's never overwritten during refresh. state-validation.service.ts already computes isInRefreshTokenFlow at line 85 from the callback context (isRenewProcess && !!refreshToken), and that boolean is now passed directly into validateIdTokenNonce instead of being used via storage.

It is a change, yes.

IdP returns in the refreshed id_token Old behavior New behavior
No nonce (spec-compliant) ✅ accepted ✅ accepted
Original nonce (Keycloak) ❌ rejected (placeholder ≠ returned nonce) ✅ accepted (returned nonce == original in storage)
Some other nonce ❌ rejected ❌ rejected
ignoreNonceAfterRefresh: true ✅ accepted ✅ accepted (unchanged)

So yes — this is a behavior change, and it's intentional. Two improvements:

  1. Keycloak (and other IdPs that echo the nonce on refresh) now work with the default ignoreNonceAfterRefresh: false. Previously they required users to flip the flag, which weakens the check.
  2. Tampering is now caught. If a man-in-the-middle returns a nonce that isn't the original, validation fails. The old code couldn't tell the difference between "spec-compliant IdP" and "tampered refresh" — both produced a mismatch with the placeholder.

Net security posture is stricter, not looser. ignoreNonceAfterRefresh: true still exists as an escape hatch for IdPs that return arbitrary nonces.

On the deprecated static

The refreshTokenNoncePlaceholder constant is still exported (just unused internally) so any external consumer that imported it still compiles. Plan is to drop it in the next major. Happy to remove it now instead if you'd prefer a cleaner break — it's only useful as a transition aid.

keyAlgorithms: string[] = [
Expand Down Expand Up @@ -282,13 +287,34 @@ export class TokenValidationService {
dataIdToken: any,
localNonce: any,
ignoreNonceAfterRefresh: boolean,
configuration: OpenIdConfiguration
configuration: OpenIdConfiguration,
isRefreshTokenFlow = false
): boolean {
const isFromRefreshToken =
(dataIdToken.nonce === undefined || ignoreNonceAfterRefresh) &&
localNonce === TokenValidationService.refreshTokenNoncePlaceholder;
if (isRefreshTokenFlow) {
if (dataIdToken.nonce === undefined) {
return true;
}

if (ignoreNonceAfterRefresh) {
return true;
}

if (dataIdToken.nonce === localNonce) {
return true;
}

this.loggerService.logDebug(
configuration,
'Validate_id_token_nonce failed in refresh-token flow, dataIdToken.nonce: ' +
dataIdToken.nonce +
' local_nonce:' +
localNonce
);

return false;
}

if (!isFromRefreshToken && dataIdToken.nonce !== localNonce) {
if (dataIdToken.nonce !== localNonce) {
this.loggerService.logDebug(
configuration,
'Validate_id_token_nonce failed, dataIdToken.nonce: ' +
Expand Down
Loading