From 4d09b827fac4cb929ae46bf89c5995e3c25e6fca Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 24 May 2021 16:52:46 -0700 Subject: [PATCH 1/5] fix(auth): Better type heirarchies for Auth API --- src/auth/auth-api-request.ts | 13 ++++-- src/auth/index.ts | 52 +++++++++++++++------- test/integration/auth.spec.ts | 4 +- test/unit/auth/auth-api-request.spec.ts | 12 ++--- test/unit/auth/user-import-builder.spec.ts | 4 +- 5 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/auth/auth-api-request.ts b/src/auth/auth-api-request.ts index bd4b2a5ce8..e7f429f811 100644 --- a/src/auth/auth-api-request.ts +++ b/src/auth/auth-api-request.ts @@ -1480,7 +1480,12 @@ export abstract class AbstractAuthRequestHandler { } // Build the signupNewUser request. - const request: any = deepCopy(properties); + type SignUpNewUserRequest = CreateRequest & { + photoUrl?: string | null; + localId?: string; + mfaInfo?: AuthFactorInfo[]; + }; + const request: SignUpNewUserRequest = deepCopy(properties); // Rewrite photoURL to photoUrl. if (typeof request.photoURL !== 'undefined') { request.photoUrl = request.photoURL; @@ -1496,14 +1501,14 @@ export abstract class AbstractAuthRequestHandler { if (validator.isNonEmptyArray(request.multiFactor.enrolledFactors)) { const mfaInfo: AuthFactorInfo[] = []; try { - request.multiFactor.enrolledFactors.forEach((multiFactorInfo: any) => { + request.multiFactor.enrolledFactors.forEach((multiFactorInfo) => { // Enrollment time and uid are not allowed for signupNewUser endpoint. // They will automatically be provisioned server side. - if (multiFactorInfo.enrollmentTime) { + if ('enrollmentTime' in multiFactorInfo) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_ARGUMENT, '"enrollmentTime" is not supported when adding second factors via "createUser()"'); - } else if (multiFactorInfo.uid) { + } else if ('uid' in multiFactorInfo) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_ARGUMENT, '"uid" is not supported when adding second factors via "createUser()"'); diff --git a/src/auth/index.ts b/src/auth/index.ts index b1bc47f725..917505159a 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -339,7 +339,7 @@ export namespace auth { * Interface representing common properties of a user enrolled second factor * for an `UpdateRequest`. */ - export interface UpdateMultiFactorInfoRequest { + export interface BaseUpdateMultiFactorInfoRequest { /** * The ID of the enrolled second factor. This ID is unique to the user. When not provided, @@ -356,18 +356,18 @@ export namespace auth { * The optional date the second factor was enrolled, formatted as a UTC string. */ enrollmentTime?: string; - - /** - * The type identifier of the second factor. For SMS second factors, this is `phone`. - */ - factorId: string; } /** * Interface representing a phone specific user enrolled second factor * for an `UpdateRequest`. */ - export interface UpdatePhoneMultiFactorInfoRequest extends UpdateMultiFactorInfoRequest { + export interface UpdatePhoneMultiFactorInfoRequest extends BaseUpdateMultiFactorInfoRequest { + + /** + * The type identifier of the second factor. For SMS second factors, this is `phone`. + */ + factorId: 'phone'; /** * The phone number associated with a phone second factor. @@ -375,6 +375,12 @@ export namespace auth { phoneNumber: string; } + /** + * Type representing the properties of a user enrolled second factor + * for an `UpdateRequest`. + */ + export type UpdateMultiFactorInfoRequest = | UpdatePhoneMultiFactorInfoRequest; + /** * Interface representing the properties to update on the provided user. */ @@ -446,24 +452,24 @@ export namespace auth { * Interface representing base properties of a user enrolled second factor for a * `CreateRequest`. */ - export interface CreateMultiFactorInfoRequest { + export interface BaseCreateMultiFactorInfoRequest { /** * The optional display name for an enrolled second factor. */ displayName?: string; - - /** - * The type identifier of the second factor. For SMS second factors, this is `phone`. - */ - factorId: string; } /** * Interface representing a phone specific user enrolled second factor for a * `CreateRequest`. */ - export interface CreatePhoneMultiFactorInfoRequest extends CreateMultiFactorInfoRequest { + export interface CreatePhoneMultiFactorInfoRequest extends BaseCreateMultiFactorInfoRequest { + + /** + * The type identifier of the second factor. For SMS second factors, this is `phone`. + */ + factorId: 'phone'; /** * The phone number associated with a phone second factor. @@ -471,6 +477,12 @@ export namespace auth { phoneNumber: string; } + /** + * Type representing the properties of a user enrolled second factor + * for an `CreateRequest`. + */ + export type CreateMultiFactorInfoRequest = | CreatePhoneMultiFactorInfoRequest; + /** * Interface representing the properties to set on a new user record to be * created. @@ -1221,7 +1233,7 @@ export namespace auth { /** * The base Auth provider configuration interface. */ - export interface AuthProviderConfig { + export interface BaseAuthProviderConfig { /** * The provider ID defined by the developer. @@ -1249,7 +1261,7 @@ export namespace auth { * Auth provider configuration interface. A SAML provider can be created via * {@link auth.Auth.createProviderConfig `createProviderConfig()`}. */ - export interface SAMLAuthProviderConfig extends AuthProviderConfig { + export interface SAMLAuthProviderConfig extends BaseAuthProviderConfig { /** * The SAML IdP entity identifier. @@ -1294,7 +1306,7 @@ export namespace auth { * provider configuration interface. An OIDC provider can be created via * {@link auth.Auth.createProviderConfig `createProviderConfig()`}. */ - export interface OIDCAuthProviderConfig extends AuthProviderConfig { + export interface OIDCAuthProviderConfig extends BaseAuthProviderConfig { /** * This is the required client ID used to confirm the audience of an OIDC @@ -1323,6 +1335,12 @@ export namespace auth { issuer: string; } + /** + * The Auth provider configuration type. + * {@link auth.Auth.createProviderConfig `createProviderConfig()`}. + */ + export type AuthProviderConfig = SAMLAuthProviderConfig | OIDCAuthProviderConfig; + /** * The request interface for updating a SAML Auth provider. This is used * when updating a SAML provider's configuration via diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index 29060b4cdd..b066d0fb06 100644 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -638,14 +638,14 @@ describe('admin.auth', () => { uid: 'mfaUid1', phoneNumber: '+16505550001', displayName: 'Work phone number', - factorId: 'phone', + factorId: 'phone' as const, enrollmentTime: now, }, { uid: 'mfaUid2', phoneNumber: '+16505550002', displayName: 'Personal phone number', - factorId: 'phone', + factorId: 'phone' as const, enrollmentTime: now, }, ]; diff --git a/test/unit/auth/auth-api-request.spec.ts b/test/unit/auth/auth-api-request.spec.ts index 0ca9bd5a85..3745f30778 100644 --- a/test/unit/auth/auth-api-request.spec.ts +++ b/test/unit/auth/auth-api-request.spec.ts @@ -1422,15 +1422,9 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { uid: 'mfaUid1', phoneNumber: '+16505550001', displayName: 'Corp phone number', - factorId: 'phone', + factorId: 'phone' as const, enrollmentTime: new Date().toUTCString(), }, - { - uid: 'mfaUid2', - phoneNumber: '+16505550002', - displayName: 'Personal phone number', - factorId: 'phone', - }, ], }, customClaims: { admin: true }, @@ -2659,11 +2653,11 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { { phoneNumber: '+16505557348', displayName: 'Spouse\'s phone number', - factorId: 'phone', + factorId: 'phone' as const, }, { phoneNumber: '+16505551000', - factorId: 'phone', + factorId: 'phone' as const, }, ], }, diff --git a/test/unit/auth/user-import-builder.spec.ts b/test/unit/auth/user-import-builder.spec.ts index 14deeaeb64..b4a6c5ce94 100644 --- a/test/unit/auth/user-import-builder.spec.ts +++ b/test/unit/auth/user-import-builder.spec.ts @@ -104,13 +104,13 @@ describe('UserImportBuilder', () => { uid: 'enrolledSecondFactor1', phoneNumber: '+16505557348', displayName: 'Spouse\'s phone number', - factorId: 'phone', + factorId: 'phone' as const, enrollmentTime: now.toUTCString(), }, { uid: 'enrolledSecondFactor2', phoneNumber: '+16505551000', - factorId: 'phone', + factorId: 'phone' as const, }, ], }, From 19b2c0eb74e8fda29c46dc7630187a18092bffab Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Tue, 25 May 2021 16:51:09 -0700 Subject: [PATCH 2/5] fix: Moved factorId back to the base types --- etc/firebase-admin.api.md | 35 ++++++++++++---------- src/auth/index.ts | 20 ++++++------- test/integration/auth.spec.ts | 4 +-- test/unit/auth/auth-api-request.spec.ts | 6 ++-- test/unit/auth/user-import-builder.spec.ts | 4 +-- 5 files changed, 36 insertions(+), 33 deletions(-) diff --git a/etc/firebase-admin.api.md b/etc/firebase-admin.api.md index 39c413b7ec..461a365559 100644 --- a/etc/firebase-admin.api.md +++ b/etc/firebase-admin.api.md @@ -113,11 +113,7 @@ export namespace auth { tenantManager(): TenantManager; } export type AuthFactorType = 'phone'; - export interface AuthProviderConfig { - displayName?: string; - enabled: boolean; - providerId: string; - } + export type AuthProviderConfig = SAMLAuthProviderConfig | OIDCAuthProviderConfig; export interface AuthProviderConfigFilter { maxResults?: number; pageToken?: string; @@ -151,11 +147,23 @@ export namespace auth { verifyIdToken(idToken: string, checkRevoked?: boolean): Promise; verifySessionCookie(sessionCookie: string, checkForRevocation?: boolean): Promise; } - export interface CreateMultiFactorInfoRequest { + export interface BaseAuthProviderConfig { + displayName?: string; + enabled: boolean; + providerId: string; + } + export interface BaseCreateMultiFactorInfoRequest { + displayName?: string; + factorId: string; + } + export interface BaseUpdateMultiFactorInfoRequest { displayName?: string; + enrollmentTime?: string; factorId: string; + uid?: string; } - export interface CreatePhoneMultiFactorInfoRequest extends CreateMultiFactorInfoRequest { + export type CreateMultiFactorInfoRequest = CreatePhoneMultiFactorInfoRequest; + export interface CreatePhoneMultiFactorInfoRequest extends BaseCreateMultiFactorInfoRequest { phoneNumber: string; } export interface CreateRequest extends UpdateRequest { @@ -241,7 +249,7 @@ export namespace auth { export interface MultiFactorUpdateSettings { enrolledFactors: UpdateMultiFactorInfoRequest[] | null; } - export interface OIDCAuthProviderConfig extends AuthProviderConfig { + export interface OIDCAuthProviderConfig extends BaseAuthProviderConfig { clientId: string; issuer: string; } @@ -264,7 +272,7 @@ export namespace auth { // (undocumented) providerUid: string; } - export interface SAMLAuthProviderConfig extends AuthProviderConfig { + export interface SAMLAuthProviderConfig extends BaseAuthProviderConfig { callbackURL?: string; idpEntityId: string; rpEntityId: string; @@ -315,13 +323,8 @@ export namespace auth { } // (undocumented) export type UpdateAuthProviderRequest = SAMLUpdateAuthProviderRequest | OIDCUpdateAuthProviderRequest; - export interface UpdateMultiFactorInfoRequest { - displayName?: string; - enrollmentTime?: string; - factorId: string; - uid?: string; - } - export interface UpdatePhoneMultiFactorInfoRequest extends UpdateMultiFactorInfoRequest { + export type UpdateMultiFactorInfoRequest = UpdatePhoneMultiFactorInfoRequest; + export interface UpdatePhoneMultiFactorInfoRequest extends BaseUpdateMultiFactorInfoRequest { phoneNumber: string; } export interface UpdateRequest { diff --git a/src/auth/index.ts b/src/auth/index.ts index 917505159a..35524f1d09 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -356,6 +356,11 @@ export namespace auth { * The optional date the second factor was enrolled, formatted as a UTC string. */ enrollmentTime?: string; + + /** + * The type identifier of the second factor. For SMS second factors, this is `phone`. + */ + factorId: string; } /** @@ -364,11 +369,6 @@ export namespace auth { */ export interface UpdatePhoneMultiFactorInfoRequest extends BaseUpdateMultiFactorInfoRequest { - /** - * The type identifier of the second factor. For SMS second factors, this is `phone`. - */ - factorId: 'phone'; - /** * The phone number associated with a phone second factor. */ @@ -458,6 +458,11 @@ export namespace auth { * The optional display name for an enrolled second factor. */ displayName?: string; + + /** + * The type identifier of the second factor. For SMS second factors, this is `phone`. + */ + factorId: string; } /** @@ -466,11 +471,6 @@ export namespace auth { */ export interface CreatePhoneMultiFactorInfoRequest extends BaseCreateMultiFactorInfoRequest { - /** - * The type identifier of the second factor. For SMS second factors, this is `phone`. - */ - factorId: 'phone'; - /** * The phone number associated with a phone second factor. */ diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index b066d0fb06..29060b4cdd 100644 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -638,14 +638,14 @@ describe('admin.auth', () => { uid: 'mfaUid1', phoneNumber: '+16505550001', displayName: 'Work phone number', - factorId: 'phone' as const, + factorId: 'phone', enrollmentTime: now, }, { uid: 'mfaUid2', phoneNumber: '+16505550002', displayName: 'Personal phone number', - factorId: 'phone' as const, + factorId: 'phone', enrollmentTime: now, }, ]; diff --git a/test/unit/auth/auth-api-request.spec.ts b/test/unit/auth/auth-api-request.spec.ts index 3745f30778..4b7f342ee2 100644 --- a/test/unit/auth/auth-api-request.spec.ts +++ b/test/unit/auth/auth-api-request.spec.ts @@ -1422,7 +1422,7 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { uid: 'mfaUid1', phoneNumber: '+16505550001', displayName: 'Corp phone number', - factorId: 'phone' as const, + factorId: 'phone', enrollmentTime: new Date().toUTCString(), }, ], @@ -2653,11 +2653,11 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { { phoneNumber: '+16505557348', displayName: 'Spouse\'s phone number', - factorId: 'phone' as const, + factorId: 'phone', }, { phoneNumber: '+16505551000', - factorId: 'phone' as const, + factorId: 'phone', }, ], }, diff --git a/test/unit/auth/user-import-builder.spec.ts b/test/unit/auth/user-import-builder.spec.ts index b4a6c5ce94..14deeaeb64 100644 --- a/test/unit/auth/user-import-builder.spec.ts +++ b/test/unit/auth/user-import-builder.spec.ts @@ -104,13 +104,13 @@ describe('UserImportBuilder', () => { uid: 'enrolledSecondFactor1', phoneNumber: '+16505557348', displayName: 'Spouse\'s phone number', - factorId: 'phone' as const, + factorId: 'phone', enrollmentTime: now.toUTCString(), }, { uid: 'enrolledSecondFactor2', phoneNumber: '+16505551000', - factorId: 'phone' as const, + factorId: 'phone', }, ], }, From 76b1e3ea7b96f536d1bf71612c494db3f74f5c22 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Tue, 25 May 2021 17:07:26 -0700 Subject: [PATCH 3/5] fix: Updated API report --- etc/firebase-admin.api.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/etc/firebase-admin.api.md b/etc/firebase-admin.api.md index 461a365559..b16b7127cb 100644 --- a/etc/firebase-admin.api.md +++ b/etc/firebase-admin.api.md @@ -249,15 +249,23 @@ export namespace auth { export interface MultiFactorUpdateSettings { enrolledFactors: UpdateMultiFactorInfoRequest[] | null; } + export interface OAuthResponseType { + code?: boolean; + idToken?: boolean; + } export interface OIDCAuthProviderConfig extends BaseAuthProviderConfig { clientId: string; + clientSecret?: string; issuer: string; + responseType?: OAuthResponseType; } export interface OIDCUpdateAuthProviderRequest { clientId?: string; + clientSecret?: string; displayName?: string; enabled?: boolean; issuer?: string; + responseType?: OAuthResponseType; } export interface PhoneIdentifier { // (undocumented) From 03307914edf5de9b63777719ce8e47a43e890c92 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 26 May 2021 15:14:31 -0700 Subject: [PATCH 4/5] fix: Fixed a grammar error in comment --- src/auth/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/auth/index.ts b/src/auth/index.ts index bc5d5c0274..27b783d081 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -479,7 +479,7 @@ export namespace auth { /** * Type representing the properties of a user enrolled second factor - * for an `CreateRequest`. + * for a `CreateRequest`. */ export type CreateMultiFactorInfoRequest = | CreatePhoneMultiFactorInfoRequest; @@ -1311,7 +1311,7 @@ export namespace auth { export interface OAuthResponseType { /** * Whether ID token is returned from IdP's authorization endpoint. - */ + */ idToken?: boolean; /** @@ -1357,7 +1357,7 @@ export namespace auth { * The OIDC provider's client secret to enable OIDC code flow. */ clientSecret?: string; - + /** * The OIDC provider's response object for OAuth authorization flow. */ @@ -1456,7 +1456,7 @@ export namespace auth { * If not provided, the existing configuration's value is not modified. */ clientSecret?: string; - + /** * The OIDC provider's response object for OAuth authorization flow. */ From 2f1d21ce0cd044268d09a3c7c3beddc9ca62c1b7 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Thu, 27 May 2021 14:24:28 -0700 Subject: [PATCH 5/5] fix: Update to comment text --- src/auth/index.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/auth/index.ts b/src/auth/index.ts index 27b783d081..57fb0a96de 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -112,7 +112,7 @@ export namespace auth { } /** - * Interface representing the common properties of a user enrolled second factor. + * Interface representing the common properties of a user-enrolled second factor. */ export interface MultiFactorInfo { @@ -143,7 +143,7 @@ export namespace auth { } /** - * Interface representing a phone specific user enrolled second factor. + * Interface representing a phone specific user-enrolled second factor. */ export interface PhoneMultiFactorInfo extends MultiFactorInfo { @@ -336,7 +336,7 @@ export namespace auth { } /** - * Interface representing common properties of a user enrolled second factor + * Interface representing common properties of a user-enrolled second factor * for an `UpdateRequest`. */ export interface BaseUpdateMultiFactorInfoRequest { @@ -364,7 +364,7 @@ export namespace auth { } /** - * Interface representing a phone specific user enrolled second factor + * Interface representing a phone specific user-enrolled second factor * for an `UpdateRequest`. */ export interface UpdatePhoneMultiFactorInfoRequest extends BaseUpdateMultiFactorInfoRequest { @@ -376,7 +376,7 @@ export namespace auth { } /** - * Type representing the properties of a user enrolled second factor + * Type representing the properties of a user-enrolled second factor * for an `UpdateRequest`. */ export type UpdateMultiFactorInfoRequest = | UpdatePhoneMultiFactorInfoRequest; @@ -449,7 +449,7 @@ export namespace auth { } /** - * Interface representing base properties of a user enrolled second factor for a + * Interface representing base properties of a user-enrolled second factor for a * `CreateRequest`. */ export interface BaseCreateMultiFactorInfoRequest { @@ -466,7 +466,7 @@ export namespace auth { } /** - * Interface representing a phone specific user enrolled second factor for a + * Interface representing a phone specific user-enrolled second factor for a * `CreateRequest`. */ export interface CreatePhoneMultiFactorInfoRequest extends BaseCreateMultiFactorInfoRequest { @@ -478,7 +478,7 @@ export namespace auth { } /** - * Type representing the properties of a user enrolled second factor + * Type representing the properties of a user-enrolled second factor * for a `CreateRequest`. */ export type CreateMultiFactorInfoRequest = | CreatePhoneMultiFactorInfoRequest;