Skip to content

Add reauthenticateWithCredential, reauthenticateWithPhoneNumber #3225

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 7 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
16 changes: 9 additions & 7 deletions packages-exp/auth-exp/src/core/credentials/anonymous.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,18 @@
* limitations under the License.
*/

import { ProviderId, SignInMethod } from '@firebase/auth-types-exp';
import * as mockFetch from '../../../test/mock_fetch';
import { expect, use } from 'chai';
import * as chaiAsPromised from 'chai-as-promised';
import { testAuth } from '../../../test/mock_auth';
import { Auth } from '../../model/auth';
import { AnonymousCredential } from './anonymous';

import { ProviderId, SignInMethod } from '@firebase/auth-types-exp';

import { mockEndpoint } from '../../../test/api/helper';
import { testAuth } from '../../../test/mock_auth';
import * as mockFetch from '../../../test/mock_fetch';
import { Endpoint } from '../../api';
import { APIUserInfo } from '../../api/account_management/account';
import { Auth } from '../../model/auth';
import { AnonymousCredential } from './anonymous';

use(chaiAsPromised);

Expand Down Expand Up @@ -84,9 +86,9 @@ describe('core/credentials/anonymous', () => {
});
});

describe('#_matchIdTokenWithUid', () => {
describe('#_getReauthenticationResolver', () => {
it('throws', () => {
expect(() => credential._matchIdTokenWithUid(auth, 'other-uid')).to.throw(
expect(() => credential._getReauthenticationResolver(auth)).to.throw(
Error
);
});
Expand Down
5 changes: 3 additions & 2 deletions packages-exp/auth-exp/src/core/credentials/anonymous.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
*/

import { ProviderId, SignInMethod } from '@firebase/auth-types-exp';

import { signUp } from '../../api/authentication/sign_up';
import { Auth } from '../../model/auth';
import { IdTokenResponse } from '../../model/id_token';
import { debugFail } from '../util/assert';
import { AuthCredential } from '.';
import { AuthCredential } from './';

export class AnonymousCredential implements AuthCredential {
providerId = ProviderId.ANONYMOUS;
Expand All @@ -44,7 +45,7 @@ export class AnonymousCredential implements AuthCredential {
debugFail("Can't link to an anonymous credential");
}

_matchIdTokenWithUid(_auth: Auth, _uid: string): Promise<never> {
_getReauthenticationResolver(_auth: Auth): Promise<never> {
debugFail('Method not implemented.');
}
}
26 changes: 14 additions & 12 deletions packages-exp/auth-exp/src/core/credentials/email.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,19 @@
* limitations under the License.
*/

import { ProviderId, SignInMethod } from '@firebase/auth-types-exp';
import { expect, use } from 'chai';
import * as chaiAsPromised from 'chai-as-promised';

import { ProviderId, SignInMethod } from '@firebase/auth-types-exp';

import { mockEndpoint } from '../../../test/api/helper';
import { testAuth } from '../../../test/mock_auth';
import { Auth } from '../../model/auth';
import { EmailAuthProvider } from '../providers/email';
import { EmailAuthCredential } from './email';
import * as mockFetch from '../../../test/mock_fetch';
import { mockEndpoint } from '../../../test/api/helper';
import { Endpoint } from '../../api';
import { APIUserInfo } from '../../api/account_management/account';
import { Auth } from '../../model/auth';
import { EmailAuthProvider } from '../providers/email';
import { EmailAuthCredential } from './email';

use(chaiAsPromised);

Expand Down Expand Up @@ -95,11 +97,11 @@ describe('core/credentials/email', () => {
});
});

describe('#_matchIdTokenWithUid', () => {
describe('#_getReauthenticationResolver', () => {
it('throws', () => {
expect(() =>
credential._matchIdTokenWithUid(auth, 'other-uid')
).to.throw(Error);
expect(() => credential._getReauthenticationResolver(auth)).to.throw(
Error
);
});
});
});
Expand Down Expand Up @@ -160,9 +162,9 @@ describe('core/credentials/email', () => {

describe('#_matchIdTokenWithUid', () => {
it('throws', () => {
expect(() =>
credential._matchIdTokenWithUid(auth, 'other-uid')
).to.throw(Error);
expect(() => credential._getReauthenticationResolver(auth)).to.throw(
Error
);
});
});
});
Expand Down
7 changes: 4 additions & 3 deletions packages-exp/auth-exp/src/core/credentials/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@
*/

import * as externs from '@firebase/auth-types-exp';

import { signInWithPassword } from '../../api/authentication/email_and_password';
import { signInWithEmailLink } from '../../api/authentication/email_link';
import { Auth } from '../../model/auth';
import { IdTokenResponse } from '../../model/id_token';
import { AuthErrorCode, AUTH_ERROR_FACTORY } from '../errors';
import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
import { EmailAuthProvider } from '../providers/email';
import { debugFail } from '../util/assert';
import { AuthCredential } from '.';
import { AuthCredential } from './';

export class EmailAuthCredential implements AuthCredential {
readonly providerId = EmailAuthProvider.PROVIDER_ID;
Expand Down Expand Up @@ -66,7 +67,7 @@ export class EmailAuthCredential implements AuthCredential {
debugFail('Method not implemented.');
}

_matchIdTokenWithUid(_auth: Auth, _uid: string): Promise<never> {
_getReauthenticationResolver(_auth: Auth): Promise<never> {
debugFail('Method not implemented.');
}
}
5 changes: 3 additions & 2 deletions packages-exp/auth-exp/src/core/credentials/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@
*/

import * as externs from '@firebase/auth-types-exp';

import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa';
import { IdTokenResponse } from '../../model/id_token';
import { Auth } from '../../model/auth';
import { IdTokenResponse } from '../../model/id_token';

export abstract class AuthCredential extends externs.AuthCredential {
static fromJSON(json: object | string): AuthCredential | null;

_getIdTokenResponse(auth: Auth): Promise<PhoneOrOauthTokenResponse>;
_linkToIdToken(auth: Auth, idToken: string): Promise<IdTokenResponse>;
_matchIdTokenWithUid(auth: Auth, uid: string): Promise<IdTokenResponse>;
_getReauthenticationResolver(auth: Auth): Promise<IdTokenResponse>;
}
12 changes: 5 additions & 7 deletions packages-exp/auth-exp/src/core/credentials/phone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa';
import {
linkWithPhoneNumber,
signInWithPhoneNumber,
SignInWithPhoneNumberRequest
SignInWithPhoneNumberRequest,
verifyPhoneNumberForExisting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be _verifyPhoneNumberForExisting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these api methods should probably be prefixed with _, but I think it would be worth doing that in a dedicated PR.

} from '../../api/authentication/sms';
import { Auth } from '../../model/auth';
import { IdTokenResponse } from '../../model/id_token';
import { debugFail } from '../util/assert';
import { AuthCredential } from '.';
import { AuthCredential } from './';

export interface PhoneAuthCredentialParameters {
verificationId?: string;
Expand All @@ -53,10 +53,8 @@ export class PhoneAuthCredential
});
}

_matchIdTokenWithUid(auth: Auth, uid: string): Promise<IdTokenResponse> {
void auth;
void uid;
return debugFail('not implemented');
_getReauthenticationResolver(auth: Auth): Promise<IdTokenResponse> {
return verifyPhoneNumberForExisting(auth, this.makeVerificationRequest());
}

private makeVerificationRequest(): SignInWithPhoneNumberRequest {
Expand Down
34 changes: 34 additions & 0 deletions packages-exp/auth-exp/src/core/strategies/credential.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
import { FirebaseError } from '@firebase/util';

import { mockEndpoint } from '../../../test/api/helper';
import { makeJWT } from '../../../test/jwt';
import { testAuth, testUser } from '../../../test/mock_auth';
import { MockAuthCredential } from '../../../test/mock_auth_credential';
import * as mockFetch from '../../../test/mock_fetch';
Expand All @@ -37,6 +38,7 @@ import { User } from '../../model/user';
import {
_assertLinkedStatus,
linkWithCredential,
reauthenticateWithCredential,
signInWithCredential
} from './credential';

Expand Down Expand Up @@ -105,6 +107,38 @@ describe('core/strategies/credential', () => {
});
});

describe('reauthenticateWithCredential', () => {
it('should throw an error if the uid is mismatched', async () => {
authCredential._setIdTokenResponse({
...idTokenResponse,
idToken: makeJWT({ sub: 'not-my-uid' })
});

await expect(
reauthenticateWithCredential(user, authCredential)
).to.be.rejectedWith(
FirebaseError,
'Firebase: The supplied credentials do not correspond to the previously signed in user. (auth/user-mismatch).'
);
});

it('sould return the expected user credential', async () => {
authCredential._setIdTokenResponse({
...idTokenResponse,
idToken: makeJWT({ sub: 'uid' })
});

const {
credential,
user: newUser,
operationType
} = await reauthenticateWithCredential(user, authCredential);
expect(operationType).to.eq(OperationType.REAUTHENTICATE);
expect(newUser).to.eq(user);
expect(credential).to.be.null;
});
});

describe('linkWithCredential', () => {
it('should throw an error if the provider is already linked', async () => {
getAccountInfoEndpoint.response = {
Expand Down
48 changes: 47 additions & 1 deletion packages-exp/auth-exp/src/core/strategies/credential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ import { OperationType, UserCredential } from '@firebase/auth-types-exp';
import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa';
import { SignInWithPhoneNumberResponse } from '../../api/authentication/sms';
import { Auth } from '../../model/auth';
import { IdTokenResponse } from '../../model/id_token';
import { User } from '../../model/user';
import { AuthCredential } from '../credentials';
import { PhoneAuthCredential } from '../credentials/phone';
import { AuthErrorCode } from '../errors';
import { _parseToken } from '../user/id_token_result';
import { _reloadWithoutSaving } from '../user/reload';
import { UserCredentialImpl } from '../user/user_credential_impl';
import { assert } from '../util/assert';
import { assert, fail } from '../util/assert';
import { providerDataAsNames } from '../util/providers';

export async function signInWithCredential(
Expand Down Expand Up @@ -66,6 +68,25 @@ export async function linkWithCredential(
return new UserCredentialImpl(user, newCred, OperationType.LINK);
}

export async function reauthenticateWithCredential(
userExtern: externs.User,
credentialExtern: externs.AuthCredential
): Promise<externs.UserCredential> {
const credential = credentialExtern as AuthCredential;
const user = userExtern as User;

const { auth, uid } = user;
const response = await verifyTokenResponseUid(
credential._getReauthenticationResolver(auth),
uid,
auth.name
);
const newCred = _authCredentialFromTokenResponse(response);

await user._updateTokensIfNecessary(response, /* reload */ true);
return new UserCredentialImpl(user, newCred, OperationType.REAUTHENTICATE);
}

export function _authCredentialFromTokenResponse(
response: PhoneOrOauthTokenResponse
): AuthCredential | null {
Expand Down Expand Up @@ -95,3 +116,28 @@ export async function _assertLinkedStatus(
: AuthErrorCode.NO_SUCH_PROVIDER;
assert(providerIds.has(provider) === expected, user.auth.name, code);
}

async function verifyTokenResponseUid(
idTokenResolver: Promise<IdTokenResponse>,
uid: string,
appName: string
): Promise<IdTokenResponse> {
const code = AuthErrorCode.USER_MISMATCH;
try {
const response = await idTokenResolver;
assert(response.idToken, appName, code);
const parsed = _parseToken(response.idToken);
assert(parsed, appName, AuthErrorCode.INTERNAL_ERROR);

const { sub: localId } = parsed;
assert(uid === localId, appName, code);

return response;
} catch (e) {
// Convert user deleted error into user mismatch
if (e?.code === `auth/${AuthErrorCode.USER_DELETED}`) {
fail(appName, code);
}
throw e;
}
}
Loading