Skip to content

Commit e6d7f7e

Browse files
LZoogLZoog
authored
Merge pull request #20766 from mozilla/FXA-13784-worktree1
feat(auth): Send client_id, service, firstAuthorization to attached services
2 parents fa76606 + f74b427 commit e6d7f7e

12 files changed

Lines changed: 466 additions & 158 deletions

File tree

‎packages/fxa-auth-server/lib/log.spec.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ jest.mock('fxa-shared', () => ({
5959
}));
6060

6161
import logModule from './log';
62+
import { OAuthNativeClients, OAuthNativeServices } from '@fxa/accounts/oauth';
6263

6364
const validEvent = {
6465
op: 'amplitudeEvent',
@@ -859,4 +860,58 @@ describe('log', () => {
859860
},
860861
});
861862
});
863+
864+
it('.notifyAttachedServices preserves an explicit clientId and firstAuthorization alongside an OAuthNative service', async () => {
865+
const now = 1600000000000;
866+
jest.spyOn(Date, 'now').mockReturnValue(now);
867+
868+
const metricsContext = {
869+
time: now,
870+
entrypoint: 'wibble',
871+
entrypoint_experiment: undefined,
872+
entrypoint_variation: undefined,
873+
flow_id:
874+
'F1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF103',
875+
flow_time: 23,
876+
flowBeginTime: now - 23,
877+
flowCompleteSignal: undefined,
878+
flowType: undefined,
879+
plan_id: undefined,
880+
product_id: undefined,
881+
utm_campaign: 'utm campaign',
882+
utm_content: 'utm content',
883+
utm_medium: 'utm medium',
884+
utm_source: 'utm source',
885+
utm_term: 'utm term',
886+
};
887+
const mockGatherMetricsContext = jest
888+
.fn()
889+
.mockResolvedValue(metricsContext);
890+
const request = { gatherMetricsContext: mockGatherMetricsContext };
891+
892+
// Browser-flow login: service is an OAuthNative service name (the `service`
893+
// query param), not a hex client id, so the hex->clientId conversion must not
894+
// fire and the explicit clientId/firstAuthorization pass through.
895+
await log.notifyAttachedServices('login', request, {
896+
service: OAuthNativeServices.SmartWindow,
897+
clientId: OAuthNativeClients.FirefoxDesktop,
898+
firstAuthorization: true,
899+
ts: now,
900+
});
901+
902+
expect(mockGatherMetricsContext).toHaveBeenCalledTimes(1);
903+
expect(mockNotifierSend).toHaveBeenCalledTimes(1);
904+
expect(mockNotifierSend).toHaveBeenCalledWith({
905+
event: 'login',
906+
data: {
907+
service: OAuthNativeServices.SmartWindow,
908+
clientId: OAuthNativeClients.FirefoxDesktop,
909+
firstAuthorization: true,
910+
timestamp: now,
911+
ts: now,
912+
iss: 'example.com',
913+
metricsContext,
914+
},
915+
});
916+
});
862917
});

‎packages/fxa-auth-server/lib/oauth/db/index.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,20 @@ class OauthDB extends ConnectedServicesDb {
384384
};
385385
}
386386

387+
// True iff the user has any prior consent for this service (any scope/client).
388+
// Used for the browser-service grain of the first-authorization signal.
389+
async hasConsentForService(uid, service) {
390+
await this.ready();
391+
return this.mysql._hasConsentForService(uid, service);
392+
}
393+
394+
// True iff the user has any prior consent for this client (any scope/service).
395+
// Used for the web-RP grain of the first-authorization signal.
396+
async hasConsentForClient(uid, clientId) {
397+
await this.ready();
398+
return this.mysql._hasConsentForClient(uid, clientId);
399+
}
400+
387401
async deleteAllConsentsForUser(uid) {
388402
await this.ready();
389403
return this.mysql._deleteAllAccountConsentsForUser(uid);

‎packages/fxa-auth-server/lib/oauth/db/mysql/index.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,13 @@ const QUERY_ACCOUNT_CONSENT_FIND_SIGNIN =
220220
// same service.
221221
const QUERY_HAS_CONSENT_FOR_SCOPE =
222222
'SELECT 1 FROM accountAuthorizations WHERE uid=? AND scope=? AND service=? LIMIT 1';
223+
// Existence checks for the first-authorization signal (FXA-13784). Bounded by
224+
// the user's rows (PK prefix on uid) with a LIMIT 1 early-out — cheaper than
225+
// fetching all of a user's consents and filtering in JS.
226+
const QUERY_HAS_CONSENT_FOR_SERVICE =
227+
'SELECT 1 FROM accountAuthorizations WHERE uid=? AND service=? LIMIT 1';
228+
const QUERY_HAS_CONSENT_FOR_CLIENT =
229+
'SELECT 1 FROM accountAuthorizations WHERE uid=? AND clientId=? LIMIT 1';
223230
const QUERY_ACCOUNT_CONSENT_DELETE_BY_UID =
224231
'DELETE FROM accountAuthorizations WHERE uid=?';
225232
const QUERY_ACCOUNT_CONSENT_LIST_BY_UID =
@@ -689,6 +696,26 @@ class MysqlStore extends MysqlOAuthShared {
689696
return rows.length > 0;
690697
}
691698

699+
// True iff the user has any consent row for this service (any scope/client).
700+
// Drives the browser-service grain of the first-authorization signal.
701+
async _hasConsentForService(uid, service) {
702+
const rows = await this._read(QUERY_HAS_CONSENT_FOR_SERVICE, [
703+
buf(uid),
704+
service,
705+
]);
706+
return rows.length > 0;
707+
}
708+
709+
// True iff the user has any consent row for this client (any scope/service).
710+
// Drives the web-RP grain of the first-authorization signal.
711+
async _hasConsentForClient(uid, clientId) {
712+
const rows = await this._read(QUERY_HAS_CONSENT_FOR_CLIENT, [
713+
buf(uid),
714+
buf(clientId),
715+
]);
716+
return rows.length > 0;
717+
}
718+
692719
_deleteAllAccountConsentsForUser(uid) {
693720
return this._write(QUERY_ACCOUNT_CONSENT_DELETE_BY_UID, [buf(uid)]);
694721
}

‎packages/fxa-auth-server/lib/oauth/first-authorization.spec.ts‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import { OAuthNativeClients, OAuthNativeServices } from '@fxa/accounts/oauth';
6+
7+
import {
8+
isFirstAuthorization,
9+
FirstAuthorizationDb,
10+
} from './first-authorization';
11+
12+
const UID = 'a'.repeat(32);
13+
const DESKTOP = OAuthNativeClients.FirefoxDesktop; // native
14+
const WEB_RP = '98e6508e88680e1b'; // arbitrary non-native web RP (no enum)
15+
16+
function mockDb(
17+
overrides: Partial<jest.Mocked<FirstAuthorizationDb>> = {}
18+
): jest.Mocked<FirstAuthorizationDb> {
19+
return {
20+
hasConsentForService: jest.fn().mockResolvedValue(false),
21+
hasConsentForClient: jest.fn().mockResolvedValue(false),
22+
...overrides,
23+
};
24+
}
25+
26+
describe('isFirstAuthorization', () => {
27+
describe('native (browser) client with a resolved service', () => {
28+
it('is true when the user has no prior consent for the service', async () => {
29+
const db = mockDb();
30+
await expect(
31+
isFirstAuthorization(db, {
32+
uid: UID,
33+
serviceValue: OAuthNativeServices.SmartWindow,
34+
clientIdHex: DESKTOP,
35+
isNativeClient: true,
36+
})
37+
).resolves.toBe(true);
38+
expect(db.hasConsentForService).toHaveBeenCalledWith(
39+
UID,
40+
OAuthNativeServices.SmartWindow
41+
);
42+
expect(db.hasConsentForClient).not.toHaveBeenCalled();
43+
});
44+
45+
it('is false when the user already has consent for the service', async () => {
46+
const db = mockDb({
47+
hasConsentForService: jest.fn().mockResolvedValue(true),
48+
});
49+
await expect(
50+
isFirstAuthorization(db, {
51+
uid: UID,
52+
serviceValue: OAuthNativeServices.Vpn,
53+
clientIdHex: DESKTOP,
54+
isNativeClient: true,
55+
})
56+
).resolves.toBe(false);
57+
});
58+
59+
it('is true on the first sync authorization (sync is included, though unreliable)', async () => {
60+
const db = mockDb();
61+
await expect(
62+
isFirstAuthorization(db, {
63+
uid: UID,
64+
serviceValue: OAuthNativeServices.Sync,
65+
clientIdHex: DESKTOP,
66+
isNativeClient: true,
67+
})
68+
).resolves.toBe(true);
69+
expect(db.hasConsentForService).toHaveBeenCalledWith(
70+
UID,
71+
OAuthNativeServices.Sync
72+
);
73+
});
74+
});
75+
76+
describe('web RP (non-native client)', () => {
77+
it('is true when the user has no prior consent for the client', async () => {
78+
const db = mockDb();
79+
await expect(
80+
isFirstAuthorization(db, {
81+
uid: UID,
82+
serviceValue: '',
83+
clientIdHex: WEB_RP,
84+
isNativeClient: false,
85+
})
86+
).resolves.toBe(true);
87+
expect(db.hasConsentForClient).toHaveBeenCalledWith(UID, WEB_RP);
88+
expect(db.hasConsentForService).not.toHaveBeenCalled();
89+
});
90+
91+
it('is false when the user already authorized the client', async () => {
92+
const db = mockDb({
93+
hasConsentForClient: jest.fn().mockResolvedValue(true),
94+
});
95+
await expect(
96+
isFirstAuthorization(db, {
97+
uid: UID,
98+
serviceValue: '',
99+
clientIdHex: WEB_RP,
100+
isNativeClient: false,
101+
})
102+
).resolves.toBe(false);
103+
});
104+
105+
it('ignores a service= sent by a web RP and keys on clientId (no spoofing a browser service)', async () => {
106+
const db = mockDb();
107+
await isFirstAuthorization(db, {
108+
uid: UID,
109+
serviceValue: OAuthNativeServices.Sync,
110+
clientIdHex: WEB_RP,
111+
isNativeClient: false,
112+
});
113+
expect(db.hasConsentForClient).toHaveBeenCalledWith(UID, WEB_RP);
114+
expect(db.hasConsentForService).not.toHaveBeenCalled();
115+
});
116+
});
117+
118+
describe('native client with no resolved service', () => {
119+
it('is false without any DB query (ambiguous)', async () => {
120+
const db = mockDb();
121+
await expect(
122+
isFirstAuthorization(db, {
123+
uid: UID,
124+
serviceValue: '',
125+
clientIdHex: DESKTOP,
126+
isNativeClient: true,
127+
})
128+
).resolves.toBe(false);
129+
expect(db.hasConsentForService).not.toHaveBeenCalled();
130+
expect(db.hasConsentForClient).not.toHaveBeenCalled();
131+
});
132+
});
133+
});

‎packages/fxa-auth-server/lib/oauth/first-authorization.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
// Whether an OAuth signin is the user's *first* use of a service / relying
6+
// party, used to set `firstAuthorization` on the `login` event (FXA-13784).
7+
// Two grains, since the accountAuthorizations ledger keys on
8+
// (uid, scope, service, clientId):
9+
// - Native (browser) client with a resolved service: "new to the service",
10+
// keyed on `service` (ignoring clientId), so it's correct across the user's
11+
// devices/apps. NOT currently reliable for `sync` — Desktop always creates a
12+
// sync-scoped access token even when signing into another service, so a first
13+
// `sync` can't be told apart; emitted anyway (flagged in the docs) pending a
14+
// desktop fix.
15+
// - Web RP (non-native client): "new to the RP", keyed on `clientId`. Any
16+
// `service=` a web RP sends is intentionally ignored — `service` is only
17+
// meaningful for native clients, so a web RP cannot spoof a browser service.
18+
// - Native client with no resolved service: ambiguous; returns false without a
19+
// DB query.
20+
21+
export interface FirstAuthorizationDb {
22+
/** True iff the user has any prior consent row for this service. */
23+
hasConsentForService(uid: string, service: string): Promise<boolean>;
24+
/** True iff the user has any prior consent row for this client. */
25+
hasConsentForClient(uid: string, clientId: string): Promise<boolean>;
26+
}
27+
28+
export async function isFirstAuthorization(
29+
db: FirstAuthorizationDb,
30+
params: {
31+
uid: string;
32+
/** Resolved native service for this authorization ('' for web RPs). */
33+
serviceValue: string;
34+
/** Hex OAuth client id for this authorization. */
35+
clientIdHex: string;
36+
/** Whether clientIdHex is a native (browser) client. */
37+
isNativeClient: boolean;
38+
}
39+
): Promise<boolean> {
40+
const { uid, serviceValue, clientIdHex, isNativeClient } = params;
41+
42+
if (isNativeClient && serviceValue) {
43+
return !(await db.hasConsentForService(uid, serviceValue));
44+
}
45+
46+
if (!isNativeClient) {
47+
return !(await db.hasConsentForClient(uid, clientIdHex));
48+
}
49+
50+
// Native client with no resolved service: ambiguous, no query needed.
51+
return false;
52+
}

‎packages/fxa-auth-server/lib/routes/account.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ export class AccountHandler {
255255
const country = geoData.location && geoData.location.country;
256256
const countryCode = geoData.location && geoData.location.countryCode;
257257
if (account.emailVerified) {
258+
const { clientId } = getClientServiceTags(request);
258259
await this.log.notifyAttachedServices('verified', request, {
259260
email: account.email,
260261
locale: account.locale,
@@ -263,6 +264,7 @@ export class AccountHandler {
263264
userAgent: userAgentString,
264265
country,
265266
countryCode,
267+
...(clientId && { clientId }),
266268
});
267269
}
268270

0 commit comments

Comments
 (0)