Skip to content

Commit 330a6f7

Browse files
authored
Merge pull request #19535 from mozilla/FXA-12432
task(auth): Add verified session checks to mfa strategy
2 parents 27e5133 + 8a8b075 commit 330a6f7

5 files changed

Lines changed: 375 additions & 49 deletions

File tree

packages/fxa-auth-server/lib/routes/auth-schemes/mfa.ts

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import { Request, ResponseToolkit } from '@hapi/hapi';
66
import * as AppError from '../../error';
77
import { ConfigType } from '../../../config/index';
88
import * as jwt from 'jsonwebtoken';
9+
import { Account } from 'fxa-shared/db/models/auth';
10+
import { StatsD } from 'hot-shots';
11+
import * as authMethods from '../../authMethods';
912

1013
export type Credentials = {
1114
data?: string | null;
@@ -40,19 +43,54 @@ export type Credentials = {
4043
locale?: string | null;
4144
mustVerify?: string | null;
4245
tokenVerificationId?: string | null;
43-
tokenVerified?: string | null;
46+
tokenVerified?: boolean | null;
4447
verificationMethod?: string | null;
4548
verificationMethodValue?: string | null;
4649
verifiedAt?: string | null;
4750
metricsOptOutAt?: string | null;
4851
providerId?: string | null;
4952
scope?: string[];
53+
authenticatorAssuranceLevel: number;
5054
};
5155

56+
export interface VerifiedSessionTokenStrategyDb {
57+
account(uid: string): Promise<Account>;
58+
totpToken(uid: string): Promise<{
59+
verified?: boolean;
60+
enabled?: boolean;
61+
}>;
62+
}
63+
5264
export const strategy = (
5365
config: ConfigType,
54-
getCredentialsFunc: (sessionTokenId: string) => Credentials
66+
getCredentialsFunc: (sessionTokenId: string) => Credentials,
67+
db: VerifiedSessionTokenStrategyDb,
68+
statsd: StatsD
5569
) => {
70+
// TODO: FXA-12494 - This was copied from verified-session-token.js. We should
71+
// convert verified-session-token.js to typescript and make the following logic reusable.
72+
73+
// Extract regular expressions to allow for optional skipping of certain routes for certain checks.
74+
// We reuse the verified session token configuration here, since it's a single point of config
75+
// to control which routes deviate from the default set of checks.
76+
const verifiedSessionTokenConfig =
77+
config?.authStrategies?.verifiedSessionToken;
78+
79+
const skipEmailVerifiedCheckForRoutes =
80+
verifiedSessionTokenConfig?.skipEmailVerifiedCheckForRoutes
81+
? new RegExp(verifiedSessionTokenConfig.skipEmailVerifiedCheckForRoutes)
82+
: null;
83+
84+
const skipTokenVerifiedCheckForRoutes =
85+
verifiedSessionTokenConfig?.skipTokenVerifiedCheckForRoutes
86+
? new RegExp(verifiedSessionTokenConfig.skipTokenVerifiedCheckForRoutes)
87+
: null;
88+
89+
const skipAalCheckForRoutes =
90+
verifiedSessionTokenConfig?.skipAalCheckForRoutes
91+
? new RegExp(verifiedSessionTokenConfig.skipAalCheckForRoutes)
92+
: null;
93+
5694
return () => ({
5795
async authenticate(req: Request, h: ResponseToolkit) {
5896
const auth = req.headers.authorization;
@@ -101,10 +139,69 @@ export const strategy = (
101139
throw AppError.unauthorized('Token not found');
102140
}
103141

104-
if (sessionToken.uid !== decoded.sub) {
142+
if (sessionToken.uid == null || sessionToken.uid !== decoded.sub) {
105143
throw AppError.unauthorized('Token invalid');
106144
}
107145

146+
// TODO: FXA-12494 - This was copied from verified-session-token.js. We should
147+
// convert verified-session-token.js to typescript and make the following logic reusable.
148+
const account = await db.account(sessionToken.uid);
149+
150+
// 1) account email is verified
151+
if (!account?.primaryEmail?.isVerified) {
152+
if (skipEmailVerifiedCheckForRoutes?.test(req.route.path)) {
153+
// Important! Using req.route.path which has much lower cardinality than req.path
154+
statsd?.increment(
155+
'verified_session_token.primary_email_not_verified.skipped',
156+
[`path:${req.route.path}`]
157+
);
158+
} else {
159+
statsd?.increment(
160+
'verified_session_token.primary_email_not_verified.error',
161+
[`path:${req.route.path}`]
162+
);
163+
throw AppError.unverifiedAccount();
164+
}
165+
}
166+
167+
// 2) session token is verified
168+
if (
169+
sessionToken.tokenVerificationId ||
170+
sessionToken.tokenVerified === false
171+
) {
172+
if (skipTokenVerifiedCheckForRoutes?.test(req.route.path)) {
173+
statsd?.increment('verified_session_token.token_verified.skipped', [
174+
`path:${req.route.path}`,
175+
]);
176+
} else {
177+
statsd?.increment('verified_session_token.token_verified.error', [
178+
`path:${req.route.path}`,
179+
]);
180+
throw AppError.unverifiedSession();
181+
}
182+
}
183+
184+
// 3) account AAL and session AAL match
185+
const accountAmr = await authMethods.availableAuthenticationMethods(
186+
db,
187+
account
188+
);
189+
const accountAal = authMethods.maximumAssuranceLevel(accountAmr);
190+
const sessionAal = sessionToken.authenticatorAssuranceLevel;
191+
192+
if (sessionAal < accountAal) {
193+
if (skipAalCheckForRoutes?.test(req.route.path)) {
194+
statsd?.increment('verified_session_token.aal.skipped', [
195+
`path:${req.route.path}`,
196+
]);
197+
} else {
198+
statsd?.increment('verified_session_token.aal.error', [
199+
`path:${req.route.path}`,
200+
]);
201+
throw AppError.unauthorized('AAL mismatch');
202+
}
203+
}
204+
108205
// Decorate session token with scope
109206
sessionToken.scope = decoded.scope;
110207

packages/fxa-auth-server/lib/server.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ async function create(log, error, config, routes, db, statsd, glean, customs) {
494494

495495
server.auth.scheme(
496496
'mfa',
497-
mfa.strategy(config, makeCredentialFn(db.sessionToken.bind(db)))
497+
mfa.strategy(config, makeCredentialFn(db.sessionToken.bind(db)), db, statsd)
498498
);
499499
server.auth.strategy('mfa', 'mfa');
500500

0 commit comments

Comments
 (0)