diff --git a/packages/fxa-auth-server/bin/key_server.js b/packages/fxa-auth-server/bin/key_server.js index 883ddd03643..22ceb673918 100755 --- a/packages/fxa-auth-server/bin/key_server.js +++ b/packages/fxa-auth-server/bin/key_server.js @@ -241,7 +241,7 @@ async function run(config) { otpCodeManager, config.recoveryPhone ); - Container.set('RecoveryPhoneService', recoveryPhoneService); + Container.set(RecoveryPhoneService, recoveryPhoneService); // The AccountDeleteManager is dependent on some of the object set into // Container above. diff --git a/packages/fxa-auth-server/lib/metrics/glean/index.ts b/packages/fxa-auth-server/lib/metrics/glean/index.ts index f703a46c7de..a12953b92d0 100644 --- a/packages/fxa-auth-server/lib/metrics/glean/index.ts +++ b/packages/fxa-auth-server/lib/metrics/glean/index.ts @@ -307,7 +307,9 @@ export function gleanMetrics(config: ConfigType) { sendError: createEventFn('two_step_auth_phone_code_send_error'), complete: createEventFn('two_step_auth_phone_code_complete'), }, - + twoStepAuthRemove: { + success: createEventFn('two_step_auth_remove_success'), + }, inactiveAccountDeletion: { statusChecked: createEventFn('inactive_account_deletion_status_checked'), firstEmailTaskRequest: createEventFn( @@ -363,6 +365,7 @@ export const logErrorWithGlean = ({ | 'twoFactorAuthSetup' | 'inactiveAccountDeletion' | 'twoStepAuthPhoneRemove' + | 'twoStepAuthRemove' > ]; funnelFns[event as keyof typeof funnelFns](request, { diff --git a/packages/fxa-auth-server/lib/metrics/glean/server_events.ts b/packages/fxa-auth-server/lib/metrics/glean/server_events.ts index 63f914864f0..942d28ea91b 100644 --- a/packages/fxa-auth-server/lib/metrics/glean/server_events.ts +++ b/packages/fxa-auth-server/lib/metrics/glean/server_events.ts @@ -3626,6 +3626,88 @@ class EventsServerEventLogger { event, }); } + /** + * Record and submit a two_step_auth_remove_success event: + * Event that indicates the user successfully remove two-step authentication + * Event is logged using internal mozlog logger. + * + * @param {string} user_agent - The user agent. + * @param {string} ip_address - The IP address. Will be used to decode Geo + * information and scrubbed at ingestion. + * @param {string} account_user_id - The firefox/mozilla account id. + * @param {string} account_user_id_sha256 - A hex string of a sha256 hash of the account's uid. + * @param {string} relying_party_oauth_client_id - The client id of the relying party. + * @param {string} relying_party_service - The service name of the relying party. + * @param {string} session_device_type - one of 'mobile', 'tablet', or ''. + * @param {string} session_entrypoint - Entrypoint to the service. + * @param {string} session_entrypoint_experiment - Identifier for the experiment the user is part of at the entrypoint. + * @param {string} session_entrypoint_variation - Identifier for the experiment variation the user is part of at the entrypoint. + * @param {string} session_flow_id - an ID generated by FxA for its flow metrics. + * @param {string} utm_campaign - A marketing campaign. For example, if a user signs into FxA from selecting a Mozilla VPN plan on Mozilla VPN's product site, then the value of this metric could be 'vpn-product-page'. The value has a max length of 128 characters with the alphanumeric characters, _ (underscore), forward slash (/), . (period), % (percentage sign), and - (hyphen) in the allowed set of characters. The special value of 'page+referral+-+not+part+of+a+campaign' is also allowed.. + * @param {string} utm_content - The content on which the user acted. For example, if the user clicked on the (previously available) "Get started here" link in "Looking for Firefox Sync? Get started here", then the value for this metric would be 'fx-sync-get-started'. The value has a max length of 128 characters with the alphanumeric characters, _ (underscore), forward slash (/), . (period), % (percentage sign), and - (hyphen) in the allowed set of characters.. + * @param {string} utm_medium - The "medium" on which the user acted. For example, if the user clicked on a link in an email, then the value of this metric would be 'email'. The value has a max length of 128 characters with the alphanumeric characters, _ (underscore), forward slash (/), . (period), % (percentage sign), and - (hyphen) in the allowed set of characters.. + * @param {string} utm_source - The source from where the user started. For example, if the user clicked on a link on the Mozilla accounts web site, this value could be 'fx-website'. The value has a max length of 128 characters with the alphanumeric characters, _ (underscore), forward slash (/), . (period), % (percentage sign), and - (hyphen) in the allowed set of characters.. + * @param {string} utm_term - This metric is similar to the `utm.source`; it is used in the Firefox browser. For example, if the user started from about:welcome, then the value could be 'aboutwelcome-default-screen'. The value has a max length of 128 characters with the alphanumeric characters, _ (underscore), forward slash (/), . (period), % (percentage sign), and - (hyphen) in the allowed set of characters.. + */ + recordTwoStepAuthRemoveSuccess({ + user_agent, + ip_address, + account_user_id, + account_user_id_sha256, + relying_party_oauth_client_id, + relying_party_service, + session_device_type, + session_entrypoint, + session_entrypoint_experiment, + session_entrypoint_variation, + session_flow_id, + utm_campaign, + utm_content, + utm_medium, + utm_source, + utm_term, + }: { + user_agent: string; + ip_address: string; + account_user_id: string; + account_user_id_sha256: string; + relying_party_oauth_client_id: string; + relying_party_service: string; + session_device_type: string; + session_entrypoint: string; + session_entrypoint_experiment: string; + session_entrypoint_variation: string; + session_flow_id: string; + utm_campaign: string; + utm_content: string; + utm_medium: string; + utm_source: string; + utm_term: string; + }) { + const event = { + category: 'two_step_auth_remove', + name: 'success', + }; + this.#record({ + user_agent, + ip_address, + account_user_id, + account_user_id_sha256, + relying_party_oauth_client_id, + relying_party_service, + session_device_type, + session_entrypoint, + session_entrypoint_experiment, + session_entrypoint_variation, + session_flow_id, + utm_campaign, + utm_content, + utm_medium, + utm_source, + utm_term, + event, + }); + } } /** diff --git a/packages/fxa-auth-server/lib/routes/recovery-phone.ts b/packages/fxa-auth-server/lib/routes/recovery-phone.ts index 5421bfad83a..896c1f8eb25 100644 --- a/packages/fxa-auth-server/lib/routes/recovery-phone.ts +++ b/packages/fxa-auth-server/lib/routes/recovery-phone.ts @@ -28,7 +28,7 @@ class RecoveryPhoneHandler { private readonly log: AuthLogger, private readonly glean: GleanMetricsType ) { - this.recoveryPhoneService = Container.get('RecoveryPhoneService'); + this.recoveryPhoneService = Container.get(RecoveryPhoneService); } async sendCode(request: AuthRequest) { diff --git a/packages/fxa-auth-server/lib/routes/totp.js b/packages/fxa-auth-server/lib/routes/totp.js index 289619df730..5584dedbb2f 100644 --- a/packages/fxa-auth-server/lib/routes/totp.js +++ b/packages/fxa-auth-server/lib/routes/totp.js @@ -15,6 +15,7 @@ const TOTP_DOCS = require('../../docs/swagger/totp-api').default; const DESCRIPTION = require('../../docs/swagger/shared/descriptions').default; const { Container } = require('typedi'); const { AccountEventsManager } = require('../account-events'); +const { RecoveryPhoneService } = require('@fxa/accounts/recovery-phone'); const RECOVERY_CODE_SANE_MAX_LENGTH = 20; @@ -35,6 +36,7 @@ module.exports = (log, db, mailer, customs, config, glean, profileClient) => { promisify(qrcode.toDataURL); const accountEventsManager = Container.get(AccountEventsManager); + const recoveryPhoneService = Container.get(RecoveryPhoneService); return [ { @@ -130,6 +132,16 @@ module.exports = (log, db, mailer, customs, config, glean, profileClient) => { auth: { strategy: 'sessionToken', }, + validate: { + payload: isA.object({ + code: isA + .string() + .max(32) + .regex(validators.DIGITS) + .optional() + .description(DESCRIPTION.codeTotp), + }), + }, response: {}, }, handler: async function (request) { @@ -151,6 +163,31 @@ module.exports = (log, db, mailer, customs, config, glean, profileClient) => { throw errors.unverifiedSession(); } + // Will also use the recovery phone to validate the code. + const code = request.payload.code; + if (code) { + // Client has posted a code, but we need make it was a code that was actually + // sent to the client + const success = await recoveryPhoneService.confirmCode(uid, code); + if (!success) { + throw errors.invalidOrExpiredOtpCode(); + } + + // Next make sure the code hasn't expired + const token = await db.totpToken(sessionToken.uid); + const isValidCode = otpUtils.verifyOtpCode(code, token.sharedSecret, { + encoding: 'hex', + step: config.step, + window: config.window, + }); + if (!isValidCode) { + throw errors.invalidOrExpiredOtpCode(); + } + + // Code seems legit. Record metric and proceed with totp deletion + glean.twoStepAuthRemove.success(); + } + await db.deleteTotpToken(uid); // Downgrade the session to email-based verification when TOTP is diff --git a/packages/fxa-auth-server/test/local/metrics/glean.ts b/packages/fxa-auth-server/test/local/metrics/glean.ts index 5f2460c194e..467647cff6f 100644 --- a/packages/fxa-auth-server/test/local/metrics/glean.ts +++ b/packages/fxa-auth-server/test/local/metrics/glean.ts @@ -46,6 +46,7 @@ const recordTwoStepAuthPhoneCodeSentStub = sinon.stub(); const recordTwoStepAuthPhoneCodeSendErrorStub = sinon.stub(); const recordTwoStepAuthPhoneCodeCompleteStub = sinon.stub(); const recordTwoStepAuthPhoneRemoveSuccessStub = sinon.stub(); +const recordTwoStepAuthRemoveSuccessStub = sinon.stub(); const recordPasswordResetTwoFactorSuccessStub = sinon.stub(); const recordPasswordResetRecoveryCodeSuccessStub = sinon.stub(); const recordInactiveAccountDeletionStatusCheckedStub = sinon.stub(); @@ -104,6 +105,7 @@ const gleanProxy = proxyquire('../../../lib/metrics/glean', { recordTwoStepAuthPhoneCodeCompleteStub, recordTwoStepAuthPhoneRemoveSuccess: recordTwoStepAuthPhoneRemoveSuccessStub, + recordTwoStepAuthRemoveSuccess: recordTwoStepAuthRemoveSuccessStub, recordPasswordResetTwoFactorSuccess: recordPasswordResetTwoFactorSuccessStub, recordPasswordResetRecoveryCodeSuccess: diff --git a/packages/fxa-auth-server/test/local/routes/recovery-phone.js b/packages/fxa-auth-server/test/local/routes/recovery-phone.js index ea1f435c29d..7e3fd9b7030 100644 --- a/packages/fxa-auth-server/test/local/routes/recovery-phone.js +++ b/packages/fxa-auth-server/test/local/routes/recovery-phone.js @@ -8,7 +8,10 @@ const chaiAsPromised = require('chai-as-promised'); const sinon = require('sinon'); const assert = { ...sinon.assert, ...chai.assert }; const { recoveryPhoneRoutes } = require('../../../lib/routes/recovery-phone'); -import { RecoveryNumberNotSupportedError } from '@fxa/accounts/recovery-phone'; +import { + RecoveryNumberNotSupportedError, + RecoveryPhoneService, +} from '@fxa/accounts/recovery-phone'; const { getRoute } = require('../../routes_helpers'); const { mockRequest } = require('../../mocks'); @@ -40,7 +43,7 @@ describe('/recovery-phone', () => { let routes = []; before(() => { - Container.set('RecoveryPhoneService', mockRecoveryPhoneService); + Container.set(RecoveryPhoneService, mockRecoveryPhoneService); routes = recoveryPhoneRoutes(mockLog, mockGlean); }); @@ -310,7 +313,6 @@ describe('/recovery-phone', () => { }); assert.equal(mockGlean.twoStepAuthPhoneRemove.success.callCount, 0); }); - }); describe('POST /recovery-phone/available', async () => { diff --git a/packages/fxa-auth-server/test/local/routes/totp.js b/packages/fxa-auth-server/test/local/routes/totp.js index ebf8a74076d..9b1eccb4039 100644 --- a/packages/fxa-auth-server/test/local/routes/totp.js +++ b/packages/fxa-auth-server/test/local/routes/totp.js @@ -12,6 +12,7 @@ const otplib = require('otplib'); const { Container } = require('typedi'); const { AccountEventsManager } = require('../../../lib/account-events'); const authErrors = require('../../../lib/error'); +const { RecoveryPhoneService } = require('@fxa/accounts/recovery-phone'); let log, db, @@ -25,6 +26,9 @@ let log, accountEventsManager; const glean = mocks.mockGlean(); +const mockRecoveryPhoneService = { + confirmCode: sinon.fake(), +}; const TEST_EMAIL = 'test@email.com'; const secret = 'KE3TGQTRNIYFO2KOPE4G6ULBOV2FQQTN'; @@ -52,6 +56,8 @@ describe('totp', () => { accountEventsManager = { recordSecurityEvent: sinon.fake.resolves({}), }; + Container.set(RecoveryPhoneService, mockRecoveryPhoneService); + glean.twoStepAuthRemove.success.reset(); }); after(() => { @@ -194,6 +200,76 @@ describe('totp', () => { ); }); }); + + it('should delete TOTP token in verified session with valid code', async () => { + const authenticator = new otplib.authenticator.Authenticator(); + authenticator.options = Object.assign({}, otplib.authenticator.options, { + secret, + }); + const code = authenticator.generate(secret); + + mockRecoveryPhoneService.confirmCode = sinon.fake.resolves(true); + requestOptions.credentials.tokenVerified = true; + requestOptions.payload = { + code, + }; + + const response = await setup( + { db: { email: TEST_EMAIL }, profile }, + {}, + '/totp/destroy', + requestOptions + ); + assert.ok(response); + + // TODO figure out what to about db.totp + assert.equal(db.totpToken.callCount, 2); + assert.equal(db.totpToken.getCall(0).args[0], 'uid'); + assert.equal(mockRecoveryPhoneService.confirmCode.callCount, 1); + assert.equal(glean.twoStepAuthRemove.success.callCount, 1); + }); + + it('should not delete TOTP token if provided sms code is invalid', async () => { + mockRecoveryPhoneService.confirmCode = sinon.fake.resolves(false); + requestOptions.credentials.tokenVerified = true; + requestOptions.payload = { + code: '000000', + }; + + try { + await setup( + { db: { email: TEST_EMAIL } }, + {}, + '/totp/destroy', + requestOptions + ); + assert.fail(); + } catch (err) { + assert.equal(err.errno, 183); + assert.equal(glean.twoStepAuthRemove.success.callCount, 0); + } + }); + + it('should not delete TOTP token if provided code is invalid totp code', async () => { + mockRecoveryPhoneService.confirmCode = sinon.fake.resolves(true); + requestOptions.credentials.tokenVerified = true; + requestOptions.payload = { + code: '000000', + }; + + try { + await setup( + { db: { email: TEST_EMAIL } }, + {}, + '/totp/destroy', + requestOptions + ); + assert.fail(); + } catch (err) { + assert.deepEqual(err.errno, 183); + assert.equal(glean.twoStepAuthRemove.success.callCount, 0); + } + }); }); describe('/totp/exists', () => { diff --git a/packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml b/packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml index 5b3ae3f6aa9..445e8e66dfa 100644 --- a/packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml +++ b/packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml @@ -1004,6 +1004,25 @@ two_step_auth_phone_code: data_sensitivity: - interaction +two_step_auth_remove: + success: + type: event + description: | + Event that indicates the user successfully remove two-step authentication + lifetime: ping + send_in_pings: + - events + notification_emails: + - vzare@mozilla.com + - fxa-staff@mozilla.com + bugs: + - https://mozilla-hub.atlassian.net/browse/FXA-10360 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1830504 + expires: never + data_sensitivity: + - interaction + inactive_account_deletion: status_checked: type: event