Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 55 additions & 0 deletions packages/fxa-auth-server/lib/log.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ jest.mock('fxa-shared', () => ({
}));

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

const validEvent = {
op: 'amplitudeEvent',
Expand Down Expand Up @@ -859,4 +860,58 @@ describe('log', () => {
},
});
});

it('.notifyAttachedServices preserves an explicit clientId and firstAuthorization alongside an OAuthNative service', async () => {
const now = 1600000000000;
jest.spyOn(Date, 'now').mockReturnValue(now);

const metricsContext = {
time: now,
entrypoint: 'wibble',
entrypoint_experiment: undefined,
entrypoint_variation: undefined,
flow_id:
'F1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF103',
flow_time: 23,
flowBeginTime: now - 23,
flowCompleteSignal: undefined,
flowType: undefined,
plan_id: undefined,
product_id: undefined,
utm_campaign: 'utm campaign',
utm_content: 'utm content',
utm_medium: 'utm medium',
utm_source: 'utm source',
utm_term: 'utm term',
};
const mockGatherMetricsContext = jest
.fn()
.mockResolvedValue(metricsContext);
const request = { gatherMetricsContext: mockGatherMetricsContext };

// Browser-flow login: service is an OAuthNative service name (the `service`
// query param), not a hex client id, so the hex->clientId conversion must not
// fire and the explicit clientId/firstAuthorization pass through.
await log.notifyAttachedServices('login', request, {
service: OAuthNativeServices.SmartWindow,
clientId: OAuthNativeClients.FirefoxDesktop,
firstAuthorization: true,
ts: now,
});

expect(mockGatherMetricsContext).toHaveBeenCalledTimes(1);
expect(mockNotifierSend).toHaveBeenCalledTimes(1);
expect(mockNotifierSend).toHaveBeenCalledWith({
event: 'login',
data: {
service: OAuthNativeServices.SmartWindow,
clientId: OAuthNativeClients.FirefoxDesktop,
firstAuthorization: true,
timestamp: now,
ts: now,
iss: 'example.com',
metricsContext,
},
});
});
});
114 changes: 114 additions & 0 deletions packages/fxa-auth-server/lib/oauth/first-authorization.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { OAuthNativeClients, OAuthNativeServices } from '@fxa/accounts/oauth';

import {
deriveFirstAuthorization,
ConsentRowLike,
} from './first-authorization';

const DESKTOP = OAuthNativeClients.FirefoxDesktop; // native
const IOS = OAuthNativeClients.FirefoxIOS; // native, different app
const WEB_RP = '98e6508e88680e1b'; // arbitrary non-native web RP (no enum)

function row(service: string, clientIdHex: string): ConsentRowLike {
return { service, clientId: clientIdHex };
}

describe('deriveFirstAuthorization', () => {
describe('browser service (native client)', () => {
it('is true on the first authorization of the service', () => {
expect(
deriveFirstAuthorization({
serviceValue: OAuthNativeServices.SmartWindow,
clientIdHex: DESKTOP,
isNativeClient: true,
existingConsents: [],
})
).toBe(true);
});

it('is true even when the user already used a different service on the same client', () => {
expect(
deriveFirstAuthorization({
serviceValue: OAuthNativeServices.SmartWindow,
clientIdHex: DESKTOP,
isNativeClient: true,
existingConsents: [row(OAuthNativeServices.Sync, DESKTOP)],
})
).toBe(true);
});

it('is false on a repeat authorization of the service', () => {
expect(
deriveFirstAuthorization({
serviceValue: OAuthNativeServices.SmartWindow,
clientIdHex: DESKTOP,
isNativeClient: true,
existingConsents: [row(OAuthNativeServices.SmartWindow, DESKTOP)],
})
).toBe(false);
});

it('is false when the prior consent for the service came from a different client (cross-device)', () => {
expect(
deriveFirstAuthorization({
serviceValue: OAuthNativeServices.Vpn,
clientIdHex: DESKTOP,
isNativeClient: true,
existingConsents: [row(OAuthNativeServices.Vpn, IOS)],
})
).toBe(false);
});
});

describe('web RP (non-native client, no resolved service)', () => {
it('is true on the first authorization of the RP', () => {
expect(
deriveFirstAuthorization({
serviceValue: '',
clientIdHex: WEB_RP,
isNativeClient: false,
existingConsents: [],
})
).toBe(true);
});

it('is true when the user has used a different RP before', () => {
expect(
deriveFirstAuthorization({
serviceValue: '',
clientIdHex: WEB_RP,
isNativeClient: false,
existingConsents: [row('', 'aaaaaaaaaaaaaaaa')],
})
).toBe(true);
});

it('is false on a repeat authorization of the RP, regardless of scope', () => {
expect(
deriveFirstAuthorization({
serviceValue: '',
clientIdHex: WEB_RP,
isNativeClient: false,
existingConsents: [row('', WEB_RP)],
})
).toBe(false);
});
});

describe('ambiguous native client with no resolved service', () => {
it('is false (not a marketing RP, service unknown)', () => {
expect(
deriveFirstAuthorization({
serviceValue: '',
clientIdHex: DESKTOP,
isNativeClient: true,
existingConsents: [],
})
).toBe(false);
});
});
});
50 changes: 50 additions & 0 deletions packages/fxa-auth-server/lib/oauth/first-authorization.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

// Decide whether an OAuth signin is the *first* time a user has used a given
// service / relying party, from the existing accountAuthorizations consent rows
// read just before this authorization's rows are written.
//
// Two grains, because the ledger keys on (uid, scope, service, clientId):
// - Browser service (an OAuthNative service): Firefox shares a client ID across all
// OAuth Native services for that client, so "new to the service" must key on `service`.
// - Web RP (service=''): the clientId *is* the RP, so key on clientId.
//
// `sync` is included but NOT currently reliable: Desktop always creates a sync-scoped
// access token even when signing into another service, so a first sync can't be told apart
// from a first use of that other service. Native clients with no resolved service are
// ambiguous and excluded (return false).

export type ConsentRowLike = {
service: string;
/** Hex-encoded OAuth client id (the caller normalizes Buffer rows to hex). */
clientId: string;
};

export function deriveFirstAuthorization(params: {
/** Resolved native service for this authorization ('' for web RPs). */
serviceValue: string;
/** Hex OAuth client id for this authorization. */
clientIdHex: string;
/** Whether clientIdHex is a native (browser) client. */
isNativeClient: boolean;
/** The user's accountAuthorizations rows read *before* this auth's writes. */
existingConsents: ConsentRowLike[];
}): boolean {
const { serviceValue, clientIdHex, isNativeClient, existingConsents } =
params;

if (serviceValue) {
// OAuthNative: new to the service= query param passed in
return !existingConsents.some((r) => r.service === serviceValue);
}

if (!serviceValue && !isNativeClient) {
// Web RP: new to the RP, identified by clientId.
return !existingConsents.some((r) => r.clientId === clientIdHex);
}

// Native client with no resolved service: ambiguous
return false;
}
2 changes: 2 additions & 0 deletions packages/fxa-auth-server/lib/routes/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export class AccountHandler {
const country = geoData.location && geoData.location.country;
const countryCode = geoData.location && geoData.location.countryCode;
if (account.emailVerified) {
const { clientId } = getClientServiceTags(request);
await this.log.notifyAttachedServices('verified', request, {
email: account.email,
locale: account.locale,
Expand All @@ -263,6 +264,7 @@ export class AccountHandler {
userAgent: userAgentString,
country,
countryCode,
...(clientId && { clientId }),
});
}

Expand Down
40 changes: 39 additions & 1 deletion packages/fxa-auth-server/lib/routes/oauth/authorization.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ const validators = require('../../oauth/validators');
const { validateRequestedGrant, generateTokens } = require('../../oauth/grant');
const { makeAssertionJWT } = require('../../oauth/util');
const verifyAssertion = require('../../oauth/assertion');
const {
deriveFirstAuthorization,
} = require('../../oauth/first-authorization');
const OAUTH_DOCS = require('../../../docs/swagger/oauth-api').default;
const OAUTH_SERVER_DOCS =
require('../../../docs/swagger/oauth-server-api').default;
Expand Down Expand Up @@ -205,6 +208,10 @@ module.exports = ({ log, oauthDB, config, statsd }) => {
serviceValue = inferred[0];
}
}
// Expose the resolved service so the `login` event reports `service` at the
// same grain as `firstAuthorization` (serviceTag alone misses scope-only
// flows, e.g. VPN cached sign-in that sends scope but no service=).
req.app.oauthService = serviceValue;
Comment on lines +211 to +214
if (!oauthDB.isClientAllowedForService(serviceValue, clientIdHex)) {
statsd?.increment('accountAuthorization.skipped', {
reason: 'client_not_allowed',
Expand All @@ -221,6 +228,28 @@ module.exports = ({ log, oauthDB, config, statsd }) => {
}
const now = Date.now();
const uidHex = hex(grant.userId);
// Read existing consents *before* the writes to detect the user's first use
// of this service / RP (drives `firstAuthorization` on the `login` event).
// Best-effort, so its own try/catch keeps a read failure from suppressing
// the load-bearing consent writes below.
let firstAuthorization = false;
try {
const existingConsents = await oauthDB.listAccountConsentsByUid(uidHex);
firstAuthorization = deriveFirstAuthorization({
serviceValue,
clientIdHex,
isNativeClient: OAUTH_NATIVE_CLIENT_IDS.has(clientIdHex),
existingConsents: existingConsents.map((r) => ({
service: r.service,
clientId: hex(r.clientId),
})),
});
} catch (err) {
statsd?.increment('accountAuthorization.first_auth_read_failed');
log.warn('accountAuthorization.first_auth_read_failed', {
err: err.message,
});
}
// allSettled so a second sibling rejection does not become an
// unhandled rejection after the first failure has been caught upstream.
const results = await Promise.allSettled(
Expand All @@ -238,6 +267,10 @@ module.exports = ({ log, oauthDB, config, statsd }) => {
if (failure) {
throw failure.reason;
}
// Only flag once the write succeeded, so the signal matches what landed.
if (firstAuthorization) {
req.app.firstAuthorization = true;
}
statsd?.increment('accountAuthorization.recorded', {
service: serviceValue || 'unset',
access_type: grant.offline ? 'offline' : 'online',
Expand Down Expand Up @@ -573,10 +606,15 @@ module.exports = ({ log, oauthDB, config, statsd }) => {
countryCode,
deviceCount: devices.length,
email,
service: clientId,
// Resolved browser service (an OAuthNative service) when there is
// one, so consumers can distinguish services that share a clientId
// (Smart Window vs Sync on Desktop); else serviceTag, else the clientId
// (mapped by log.js) for web RPs.
service: req.app.oauthService || req.app.serviceTag || clientId,
clientId,
uid,
userAgent: req.headers['user-agent'],
firstAuthorization: !!req.app.firstAuthorization,
});
return result;
},
Expand Down
Loading