Skip to content

Commit

Permalink
Merge pull request #17636 from mozilla/FXA-9398-forgot-token-auth
Browse files Browse the repository at this point in the history 
feat(recovery): fetch recovery key status with passwordForgotToken
  • Loading branch information
@chenba
chenba committed Sep 19, 2024
2 parents 3b185ed + 4958413 commit 794bf57
Show file tree
Hide file tree
Showing 15 changed files with 518 additions and 830 deletions.
25 changes: 14 additions & 11 deletions packages/fxa-auth-client/lib/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,20 @@ export default class AuthClient {
);
}

async passwordForgotRecoveryKeyStatus(
passwordForgotToken: hexstring,
headers?: Headers
) {
return this.hawkRequest(
'POST',
'/recoveryKey/exists',
passwordForgotToken,
tokenType.passwordForgotToken,
null,
headers
);
}

// TODO: Once password reset react is 100% and stable in production
// we can remove this.
async accountReset(
Expand Down Expand Up @@ -1904,17 +1918,6 @@ export default class AuthClient {
);
}

// TODO: Review in FXA-7400 - possibly convert to POST to pass payload instead of using param, and enforce rate limiting
// async getRecoveryKeyHint(
// sessionToken: hexstring | undefined,
// email?: string
// ): Promise<{ hint: string | null }> {
// if (sessionToken) {
// return this.sessionGet('/recoveryKey/hint', sessionToken);
// }
// return this.request('GET', `/recoveryKey/hint?email=${email}`);
// }

async updateRecoveryKeyHint(
sessionToken: hexstring,
hint: string,
Expand Down
4 changes: 2 additions & 2 deletions packages/fxa-auth-server/docs/swagger/recovery-key-api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ const RECOVERYKEY_EXISTS_POST = {
...TAGS_RECOVERY_KEY,
description: '/recoveryKey/exists',
notes: [
'🔒🔓 Optionally authenticated with session token',
'This route checks to see if given user has setup an account recovery key. When used during the password reset flow, an email can be provided (instead of a sessionToken) to check for the status. However, when using an email, the request is rate limited.',
'🔒🔓 Authenticated with session token or password-forgot token',
'This route checks to see if given user has setup an account recovery key. When used during the password reset flow, a password-forgot token to check for the status.',
],
};

Expand Down
10 changes: 3 additions & 7 deletions packages/fxa-auth-server/lib/db/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1087,13 +1087,9 @@ module.exports = (config, log, Token, UnblockCode = null) => {
return RecoveryKey.update({ uid, recoveryKeyId, enabled });
};

DB.prototype.getRecoveryKeyHint = async function (uid) {
log.trace('DB.getRecoveryKeyHint', { uid });
const data = await RecoveryKey.findByUid(uid);
if (!data) {
throw error.recoveryKeyNotFound();
}
return { hint: await RecoveryKey.findHintByUid(uid) };
DB.prototype.getRecoveryKeyRecordWithHint = async function (uid) {
log.trace('DB.getRecoveryKeyRecordWithHint', { uid });
return await RecoveryKey.findRecordWithHintByUid(uid);
};

DB.prototype.updateRecoveryKeyHint = async function (uid, hint) {
Expand Down
39 changes: 28 additions & 11 deletions packages/fxa-auth-server/lib/routes/auth-schemes/hawk-fxa-token.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,16 @@ function parseAuthorizationHeader(header, keys) {
* @param {Function} getCredentialsFunc - The function to get the credentials.
* @returns {Function}
*/
function strategy(getCredentialsFunc) {
function strategy(
getCredentialsFunc,
authStrategyOptions = { throwOnFailure: true }
) {
const tokenNotFoundError = () => {
const error = AppError.unauthorized('Token not found');
error.isMissing = true;
return error;
};

return function (server, options) {
return {
authenticate: async function (req, h) {
Expand All @@ -89,22 +98,28 @@ function strategy(getCredentialsFunc) {
return h.continue;
}

const error = AppError.unauthorized('Token not found');
error.isMissing = true;
throw error;
throw tokenNotFoundError();
}

if (auth.toLowerCase().indexOf('hawk') > -1) {
// If a Hawk token is found, lets parse it and get the token's id
const parsedHeader = parseAuthorizationHeader(auth);
const token = await getCredentialsFunc(parsedHeader.id);
let token;

try {
token = await getCredentialsFunc(parsedHeader.id);
} catch (_) {
// we'll handle the empty token case below
}

// If a token isn't found, this means it doesn't exist or expired and
// was removed from database
if (!token) {
const error = AppError.unauthorized('Token not found');
error.isMissing = true;
throw error;
if (authStrategyOptions.throwOnFailure) {
throw tokenNotFoundError();
} else {
return Boom.unauthorized(null, 'hawkFxaToken');
}
}

return h.authenticated({
Expand All @@ -125,9 +140,11 @@ function strategy(getCredentialsFunc) {
// } catch (err) {}
// }

const error = AppError.unauthorized('Token not found');
error.isMissing = true;
throw error;
if (authStrategyOptions.throwOnFailure) {
throw tokenNotFoundError();
}

return Boom.unauthorized(null, 'hawkFxaToken');
},
payload: async function (req, h) {
// Since we skip Hawk header validation, we don't need to perform payload validation either...
Expand Down
102 changes: 17 additions & 85 deletions packages/fxa-auth-server/lib/routes/recovery-key.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,44 +296,34 @@ module.exports = (
options: {
...RECOVERY_KEY_DOCS.RECOVERYKEY_EXISTS_POST,
auth: {
mode: 'optional',
strategy: 'sessionToken',
strategies: [
'multiStrategySessionToken',
'multiStrategyPasswordForgotToken',
],
},
validate: {
payload: isA.object({
email: validators.email().optional(),
}),
payload: isA
.object({
email: validators.email().optional(),
})
.optional()
.allow(null),
},
response: {
schema: isA.object({
exists: isA.boolean().required(),
hint: isA.string().optional().allow(null),
estimatedSyncDeviceCount: isA.number().optional(),
}),
},
},
async handler(request) {
log.begin('recoveryKeyExists', request);

const { email } = request.payload;

let uid;
if (request.auth.credentials) {
uid = request.auth.credentials.uid;
}

if (!uid) {
// If not using a sessionToken, an email is required to check
// for an account recovery key. This occurs when checking from the
// password reset page and allows us to redirect the user to either
// the regular password reset or account recovery password reset.
if (!email) {
throw errors.missingRequestParameter('email');
}

await customs.check(request, email, 'recoveryKeyExists');
({ uid } = await db.accountRecord(email));
}
const exists = await db.recoveryKeyExists(uid);
const uid = request.auth.credentials.uid;
const recoveryKeyRecordWithHint = await db.getRecoveryKeyRecordWithHint(
uid
);

// We try our best to estimate the number of Sync devices that could be impacted by using a recovery key.
// It is an estimate because we currently can't query the Sync service to get the actual number of items
Expand All @@ -347,70 +337,12 @@ module.exports = (
);

return {
exists: exists.exists,
exists: !!recoveryKeyRecordWithHint,
hint: recoveryKeyRecordWithHint?.hint,
estimatedSyncDeviceCount,
};
},
},
// This method is not yet in use
// Disabled until we are ready to enable as part of FXA-6670
// GET request for authenticated user should use the sessionToken to display the hint in /settings
// To display the hint during password reset, this method will need to be usable without authenticating,
// but unauthenticated requests should be rate limited by IP and email to prevent checking multiple emails for hints
// TODO : Review in FXA-7400 - possibly convert to POST to pass payload instead of using param, and enforce rate limiting
// {
// method: 'GET',
// path: '/recoveryKey/hint',
// options: {
// ...RECOVERY_KEY_DOCS.RECOVERYKEY_HINT_GET,
// auth: {
// mode: 'optional',
// strategy: 'sessionToken',
// },
// validate: {
// query: {
// email: validators.email().optional(),
// },
// },
// },
// handler: async function (request) {
// log.begin('getRecoveryKeyHint', request);

// const { email } = request.query;

// let uid;
// if (request.auth.credentials) {
// uid = request.auth.credentials.uid;
// }

// if (!uid) {
// // If not using a sessionToken, an email is required to check
// // for an account recovery key.
// if (!email) {
// throw errors.missingRequestParameter('email');
// }

// // When this request is unauthenticated, we must rate-limit by IP and email
// // to review in FXA-7400, rate limiting does not seem to be working as expected
// await customs.check(request, email, 'recoveryKeyExists');
// try {
// const result = await db.accountRecord(email);
// uid = result.uid;
// } catch (err) {
// throw errors.unknownAccount();
// }
// }

// const result = await db.recoveryKeyExists(uid);
// if (!result.exists) {
// throw errors.recoveryKeyNotFound();
// }

// const hint = await db.getRecoveryKeyHint(uid);

// return hint;
// },
// },
{
method: 'POST',
path: '/recoveryKey/hint',
Expand Down
27 changes: 27 additions & 0 deletions packages/fxa-auth-server/lib/server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,24 @@ async function create(log, error, config, routes, db, statsd, glean) {
hawkFxAToken.strategy(makeCredentialFn(db.passwordChangeToken.bind(db)))
);

// the recoveryKey/exists route accepts a session token or a password-forgot
// token. in order for Hapi to try all the strategies (until auth succeeds),
// the strategy's authenticate function _must not_ throw on failed
// authentication. otherwise, Hapi will stop at the first strategy that
// throws.
server.auth.scheme(
'multi-strategy-fxa-hawk-session-token',
hawkFxAToken.strategy(makeCredentialFn(db.sessionToken.bind(db)), {
throwOnFailure: false,
})
);
server.auth.scheme(
'multi-strategy-fxa-hawk-passwordForgot-token',
hawkFxAToken.strategy(makeCredentialFn(db.passwordForgotToken.bind(db)), {
throwOnFailure: false,
})
);

server.auth.strategy('sessionToken', 'fxa-hawk-session-token');
server.auth.strategy('keyFetchToken', 'fxa-hawk-keyFetch-token');
server.auth.strategy(
Expand All @@ -413,6 +431,15 @@ async function create(log, error, config, routes, db, statsd, glean) {
server.auth.strategy('passwordForgotToken', 'fxa-hawk-passwordForgot-token');
server.auth.strategy('passwordChangeToken', 'fxa-hawk-passwordChange-token');

server.auth.strategy(
'multiStrategySessionToken',
'multi-strategy-fxa-hawk-session-token'
);
server.auth.strategy(
'multiStrategyPasswordForgotToken',
'multi-strategy-fxa-hawk-passwordForgot-token'
);

server.auth.scheme(authOauth.AUTH_SCHEME, authOauth.strategy);
server.auth.strategy('oauthToken', authOauth.AUTH_SCHEME, config.oauth);

Expand Down
8 changes: 2 additions & 6 deletions packages/fxa-auth-server/test/client/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,12 +846,8 @@ module.exports = (config) => {
return this.api.getRecoveryKey(this.accountResetToken, recoveryKeyId);
};

Client.prototype.getRecoveryKeyExists = function (email) {
if (!email) {
return this.api.getRecoveryKeyExistsWithSession(this.sessionToken);
} else {
return this.api.getRecoveryKeyExistsWithEmail(email);
}
Client.prototype.getRecoveryKeyExists = function () {
return this.api.getRecoveryKeyExistsWithSession(this.sessionToken);
};

Client.prototype.deleteRecoveryKey = function () {
Expand Down
Loading

0 comments on commit 794bf57

Please sign in to comment.