Skip to content

remove startMfaSignIn step for TOTP, since it requires only one step. #7047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 22, 2023
Merged
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
32 changes: 3 additions & 29 deletions packages/auth/src/api/authentication/mfa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,13 @@ export interface StartPhoneMfaSignInRequest {
};
tenantId?: string;
}
export interface StartTotpMfaSignInRequest {
mfaPendingCredential: string;
mfaEnrollmentId: string;
totpSignInInfo: {
verificationCode: string;
};
tenantId?: string;
}

export interface StartPhoneMfaSignInResponse {
phoneResponseInfo: {
sessionInfo: string;
};
}

export interface StartTotpMfaSignInResponse {
totpSignInInfo: {
verificationCode: string;
};
}

export function startSignInPhoneMfa(
auth: Auth,
request: StartPhoneMfaSignInRequest
Expand All @@ -87,27 +73,15 @@ export function startSignInPhoneMfa(
);
}

export function startSignInTotpMfa(
auth: Auth,
request: StartTotpMfaSignInRequest
): Promise<StartTotpMfaSignInResponse> {
return _performApiRequest<
StartTotpMfaSignInRequest,
StartTotpMfaSignInResponse
>(
auth,
HttpMethod.POST,
Endpoint.START_MFA_SIGN_IN,
_addTidIfNecessary(auth, request)
);
}

export interface FinalizePhoneMfaSignInRequest {
mfaPendingCredential: string;
phoneVerificationInfo: SignInWithPhoneNumberRequest;
tenantId?: string;
}

// TOTP MFA Sign in only has a finalize phase. Phone MFA has a start phase to initiate sending an
// SMS and a finalize phase to complete sign in. With TOTP, the user already has the OTP in the
// TOTP/Authenticator app.
export interface FinalizeTotpMfaSignInRequest {
mfaPendingCredential: string;
totpVerificationInfo: { verificationCode: string };
Expand Down
32 changes: 5 additions & 27 deletions packages/auth/src/mfa/assertions/totp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ import * as mockFetch from '../../../test/helpers/mock_fetch';
import { Endpoint } from '../../api';
import { MultiFactorSessionImpl } from '../../mfa/mfa_session';
import { StartTotpMfaEnrollmentResponse } from '../../api/account_management/mfa';
import {
FinalizeMfaResponse,
StartTotpMfaSignInResponse
} from '../../api/authentication/mfa';
import { FinalizeMfaResponse } from '../../api/authentication/mfa';
import {
TotpMultiFactorAssertionImpl,
TotpMultiFactorGenerator,
Expand Down Expand Up @@ -215,7 +212,6 @@ describe('core/mfa/totp/assertions/TotpMultiFactorAssertionImpl', () => {
describe('Testing signin Flow', () => {
let auth: TestAuth;
let assertion: MultiFactorAssertionImpl;
let totpSignInResponse: StartTotpMfaSignInResponse;
let session: MultiFactorSessionImpl;
beforeEach(async () => {
mockFetch.setUp();
Expand All @@ -227,24 +223,18 @@ describe('Testing signin Flow', () => {
afterEach(mockFetch.tearDown);

it('should finalize mfa signin for totp', async () => {
totpSignInResponse = {
verificationCode: '123456',
const mockResponse: FinalizeMfaResponse = {
idToken: 'final-id-token',
refreshToken: 'refresh-token'
} as any;
};
const mock = mockEndpoint(Endpoint.FINALIZE_MFA_SIGN_IN, mockResponse);
assertion = TotpMultiFactorGenerator.assertionForSignIn(
'enrollment-id',
'123456'
) as any;

const mock = mockEndpoint(
Endpoint.FINALIZE_MFA_SIGN_IN,
totpSignInResponse
);

const response = await assertion._process(auth, session);

expect(response).to.eql(totpSignInResponse);
expect(response).to.eql(mockResponse);

expect(mock.calls[0].request).to.eql({
mfaPendingCredential: 'mfa-pending-credential',
Expand All @@ -256,12 +246,6 @@ describe('Testing signin Flow', () => {
});

it('should throw Firebase Error if enrollment-id is undefined', async () => {
let _response: FinalizeMfaResponse;
totpSignInResponse = {
verificationCode: '123456',
idToken: 'final-id-token',
refreshToken: 'refresh-token'
} as any;
assertion = TotpMultiFactorGenerator.assertionForSignIn(
undefined as any,
'123456'
Expand All @@ -273,12 +257,6 @@ describe('Testing signin Flow', () => {
});

it('should throw Firebase Error if otp is undefined', async () => {
let _response: FinalizeMfaResponse;
totpSignInResponse = {
verificationCode: '123456',
idToken: 'final-id-token',
refreshToken: 'refresh-token'
} as any;
assertion = TotpMultiFactorGenerator.assertionForSignIn(
'enrollment-id',
undefined as any
Expand Down
1 change: 0 additions & 1 deletion packages/auth/src/mfa/mfa_info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export abstract class MultiFactorInfoImpl implements MultiFactorInfo {
if ('phoneInfo' in enrollment) {
return PhoneMultiFactorInfoImpl._fromServerResponse(auth, enrollment);
} else if ('totpInfo' in enrollment) {
// TODO(prameshj) ensure that this field is set by the backend once the tracking bug is fixed.
return TotpMultiFactorInfoImpl._fromServerResponse(auth, enrollment);
}
return _fail(auth, AuthErrorCode.INTERNAL_ERROR);
Expand Down