Skip to content

Commit b585fe2

Browse files
committed
fix(auth): Skip newTokenNotification for token-exchange grant type
Because: * This function errors out because it looks for client_id in the request. We could pull the client_id from subject_token to use here, however, we don't want users to be notified about the token exchange since it occurs silently and should not be counted as a new login This commit: * Skips newTokenNotification when the requested grant type results in a refresh token, and is for token-exchange grant type fixes FXA-12963
1 parent a116e09 commit b585fe2

2 files changed

Lines changed: 8 additions & 3 deletions

File tree

packages/fxa-auth-server/lib/routes/oauth/token.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,9 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
788788
grant.session_token = newSessionToken.data;
789789
}
790790

791-
if (grant.refresh_token) {
791+
// Token exchange is swapping tokens for an already-authenticated user,
792+
// so we skip the new token notification.
793+
if (grant.refresh_token && req.payload.grant_type !== GRANT_TOKEN_EXCHANGE) {
792794
// if a refresh token has
793795
// been provisioned as part of the flow
794796
// then we want to send some notifications to the user

packages/fxa-auth-server/test/oauth/routes/token.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,8 +701,9 @@ describe('/oauth/token POST', function () {
701701
describe('token exchange via /oauth/token', () => {
702702
const ScopeSet = require('fxa-shared').oauth.scopes;
703703

704-
it('handles token exchange with multiple existing scopes (sync + profile)', async () => {
704+
it('handles token exchange with multiple existing scopes and skips newTokenNotification', async () => {
705705
const PROFILE_SCOPE = 'profile';
706+
const newTokenNotificationStub = sinon.stub().resolves();
706707
const routes = proxyquire(tokenRoutePath, {
707708
...tokenRoutesDepMocks,
708709
'../../oauth/grant': {
@@ -716,7 +717,7 @@ describe('/oauth/token POST', function () {
716717
validateRequestedGrant: () => ({ offline: true, scope: 'testo' }),
717718
},
718719
'../utils/oauth': {
719-
newTokenNotification: async () => {},
720+
newTokenNotification: newTokenNotificationStub,
720721
},
721722
})({
722723
...tokenRoutesArgMocks,
@@ -761,6 +762,8 @@ describe('/oauth/token POST', function () {
761762
assert.include(result.scope, SYNC_SCOPE);
762763
assert.include(result.scope, PROFILE_SCOPE);
763764
assert.include(result.scope, RELAY_SCOPE);
765+
// Token exchange should NOT call newTokenNotification
766+
sinon.assert.notCalled(newTokenNotificationStub);
764767
});
765768
});
766769
});

0 commit comments

Comments
 (0)