Skip to content

Commit 433d57a

Browse files
authored
Merge pull request #19992 from mozilla/FXA-12929
feat(oauth): Support session token to refresh token migration, add old device association for token exchange
2 parents 079f810 + 3c20b3c commit 433d57a

5 files changed

Lines changed: 483 additions & 24 deletions

File tree

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

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ const GRANT_TOKEN_EXCHANGE = 'urn:ietf:params:oauth:grant-type:token-exchange';
7575
const SUBJECT_TOKEN_TYPE_REFRESH =
7676
'urn:ietf:params:oauth:token-type:refresh_token';
7777

78+
// For Desktop apps migrating from using the session token to a refresh token
79+
const CREDENTIALS_REASON_MIGRATION = 'token_migration';
80+
7881
const ACCESS_TYPE_ONLINE = 'online';
7982
const ACCESS_TYPE_OFFLINE = 'offline';
8083

@@ -440,6 +443,13 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
440443
// Original scope plus requested scope, e.g. Sync + Relay
441444
const combinedScope = subjectToken.scope.union(requestedScope);
442445

446+
// Look up the device associated with the old refresh token so we can
447+
// link the new refresh token to the same device record
448+
const existingDevice = await db.deviceFromRefreshTokenId(
449+
hex(subjectToken.userId),
450+
hex(subjectToken.tokenId)
451+
);
452+
443453
return {
444454
userId: subjectToken.userId,
445455
clientId: subjectToken.clientId,
@@ -448,6 +458,7 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
448458
authAt: Math.floor(Date.now() / 1000),
449459
profileChangedAt: subjectToken.profileChangedAt,
450460
originalRefreshTokenId: subjectToken.tokenId, // for revocation after new token generation
461+
existingDeviceId: existingDevice ? existingDevice.id : undefined, // to link new token to existing device
451462
};
452463
}
453464

@@ -518,13 +529,22 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
518529
glean.oauth.tokenCreated(req, {
519530
uid,
520531
oauthClientId,
521-
reason: params.grant_type,
532+
reason: params.reason
533+
? `${params.grant_type}:${params.reason}`
534+
: params.grant_type,
522535
});
523536

524537
if (tokens.keys_jwe) {
525538
statsd.increment('oauth.rp.keys-jwe', { clientId: oauthClientId });
526539
}
527540

541+
// Include grant properties needed by the /oauth/token handler for newTokenNotification.
542+
// These are internal properties that get stripped before returning to the client.
543+
tokens._clientId = oauthClientId;
544+
if (grant.existingDeviceId) {
545+
tokens._existingDeviceId = grant.existingDeviceId;
546+
}
547+
528548
return tokens;
529549
}
530550

@@ -568,7 +588,13 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
568588
.description(DESCRIPTION.keysJweOauth),
569589
}),
570590
},
571-
handler: tokenHandler,
591+
handler: async (req) => {
592+
const result = await tokenHandler(req);
593+
// Strip internal properties that are only used by /oauth/token handler
594+
delete result._clientId;
595+
delete result._existingDeviceId;
596+
return result;
597+
},
572598
},
573599
},
574600
{
@@ -641,6 +667,12 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
641667
ttl: Joi.number().positive().optional(),
642668
resource: validators.resourceUrl.optional(),
643669
assertion: Joi.forbidden(),
670+
// 'token_migration' indicates a silent migration from session token to
671+
// refresh token for an already-authenticated user (e.g., for Relay/Sync).
672+
// This skips the new token notification email.
673+
reason: Joi.string()
674+
.valid(CREDENTIALS_REASON_MIGRATION)
675+
.optional(),
644676
}),
645677
// token exchange (RFC 8693)
646678
Joi.object({
@@ -788,18 +820,35 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
788820
grant.session_token = newSessionToken.data;
789821
}
790822

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) {
794-
// if a refresh token has
795-
// been provisioned as part of the flow
796-
// then we want to send some notifications to the user
823+
// If a refresh token has been provisioned as part of the flow,
824+
// link it to the device record and optionally notify the user.
825+
// For token exchange and token migration, skip the email notification
826+
// since these are for already-authenticated users.
827+
//
828+
// Device association:
829+
// - Token exchange: We look up the device from the old refresh token
830+
// (grant._existingDeviceId) since there's no session token auth.
831+
// - Token migration: The session token auth already includes deviceId
832+
// via the SessionWithDevice stored procedure, so it's available in
833+
// request.auth.credentials.deviceId. skipEmail is a fallback for
834+
// sessions without an associated device.
835+
if (grant.refresh_token) {
836+
const isTokenExchange =
837+
req.payload.grant_type === GRANT_TOKEN_EXCHANGE;
838+
const isTokenMigration =
839+
req.payload.reason === CREDENTIALS_REASON_MIGRATION;
840+
const skipEmail = isTokenExchange || isTokenMigration;
797841
await oauthRouteUtils.newTokenNotification(
798842
db,
799843
mailer,
800844
devices,
801845
req,
802-
grant
846+
grant,
847+
{
848+
skipEmail,
849+
existingDeviceId: grant._existingDeviceId,
850+
clientId: grant._clientId,
851+
}
803852
);
804853
}
805854

@@ -808,7 +857,9 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
808857
await db.touchSessionToken(sessionToken, {}, true);
809858
}
810859

811-
// done with 'session_token_id' at this point, do not return it.
860+
// Strip internal properties before returning to client
861+
delete grant._clientId;
862+
delete grant._existingDeviceId;
812863
delete grant.session_token_id;
813864

814865
// attempt to record metrics, but swallow the error if one is thrown.

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,16 @@ module.exports = {
2929
mailer,
3030
devices,
3131
request,
32-
grant
32+
grant,
33+
options = {}
3334
) {
35+
const { skipEmail = false, existingDeviceId, clientId: optionsClientId } = options;
3436
const fxaMailer = Container.get(FxaMailer);
3537
const oauthClientInfoService = Container.get(OAuthClientInfoServiceName);
3638

37-
const clientId = request.payload.client_id;
39+
// Use clientId from options if provided (e.g., for token exchange where
40+
// client_id is not in the request payload), otherwise fall back to payload
41+
const clientId = optionsClientId || request.payload.client_id;
3842
const scopeSet = ScopeSet.fromString(grant.scope);
3943
const credentials = (request.auth && request.auth.credentials) || {};
4044

@@ -66,6 +70,12 @@ module.exports = {
6670
credentials.id = grant.session_token_id;
6771
}
6872

73+
// Use existing device ID if provided (e.g., from token exchange where we looked
74+
// up the device associated with the old refresh token)
75+
if (!credentials.deviceId && existingDeviceId) {
76+
credentials.deviceId = existingDeviceId;
77+
}
78+
6979
// we set tokenVerified because the granted scope is part of NOTIFICATION_SCOPES
7080
credentials.tokenVerified = true;
7181
credentials.client = await client.getClientById(clientId);
@@ -78,8 +88,8 @@ module.exports = {
7888
});
7989

8090
// Email the user about their new device connection
81-
// (but don't send anything if it was an existing device or new account)
82-
if (!credentials.deviceId) {
91+
// (but don't send anything if it was an existing device, new account, or skipEmail is set)
92+
if (!credentials.deviceId && !skipEmail) {
8393
const account = await db.account(credentials.uid);
8494
const isNewAccount = Date.now() - account.createdAt < MAX_NEW_ACCOUNT_AGE;
8595

packages/fxa-auth-server/test/local/routes/utils/oauth.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,41 @@ describe('newTokenNotification', () => {
137137
assert.equal(args[1].refreshTokenId, MOCK_REFRESH_TOKEN_ID_2);
138138
assert.equal(args[2].id, MOCK_DEVICE_ID);
139139
});
140+
141+
it('creates a device but skips email when skipEmail option is true', async () => {
142+
await oauthUtils.newTokenNotification(db, mailer, devices, request, grant, {
143+
skipEmail: true,
144+
});
145+
146+
assert.equal(fxaMailer.sendNewDeviceLoginEmail.callCount, 0);
147+
assert.equal(devices.upsert.callCount, 1, 'created a device');
148+
const args = devices.upsert.args[0];
149+
assert.equal(
150+
args[1].refreshTokenId,
151+
request.auth.credentials.refreshTokenId
152+
);
153+
});
154+
155+
it('uses existingDeviceId when provided and credentials has no deviceId', async () => {
156+
const EXISTING_DEVICE_ID = 'existingdevice123456';
157+
// credentials without deviceId
158+
credentials = {
159+
uid: MOCK_UID,
160+
refreshTokenId: MOCK_REFRESH_TOKEN,
161+
};
162+
request = mocks.mockRequest({ credentials });
163+
164+
await oauthUtils.newTokenNotification(db, mailer, devices, request, grant, {
165+
existingDeviceId: EXISTING_DEVICE_ID,
166+
});
167+
168+
// Should not send email since we have an existing device
169+
assert.equal(fxaMailer.sendNewDeviceLoginEmail.callCount, 0);
170+
assert.equal(devices.upsert.callCount, 1, 'updated the device');
171+
const args = devices.upsert.args[0];
172+
// Should use the existingDeviceId for the upsert
173+
assert.equal(args[2].id, EXISTING_DEVICE_ID);
174+
// credentials.deviceId should be set
175+
assert.equal(args[1].deviceId, EXISTING_DEVICE_ID);
176+
});
140177
});

0 commit comments

Comments
 (0)