From 59d329be74b4da0a0521214630fac94939eadae0 Mon Sep 17 00:00:00 2001 From: Brett Willis Date: Tue, 23 Aug 2022 09:55:03 +1200 Subject: [PATCH 1/5] Add customUserClaims to UpdateRequest --- src/auth/auth-api-request.ts | 19 ++++++++++++++++++- src/auth/auth-config.ts | 6 ++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/auth/auth-api-request.ts b/src/auth/auth-api-request.ts index a962a4f719..2bcfe1a7e6 100644 --- a/src/auth/auth-api-request.ts +++ b/src/auth/auth-api-request.ts @@ -1349,7 +1349,7 @@ export abstract class AbstractAuthRequestHandler { if (customUserClaims === null) { customUserClaims = {}; } - // Construct custom user attribute editting request. + // Construct custom user attribute editing request. const request: any = { localId: uid, customAttributes: JSON.stringify(customUserClaims), @@ -1405,6 +1405,13 @@ export abstract class AbstractAuthRequestHandler { 'providersToUnlink of properties argument must be an array of strings.'); } }); + } else if ((typeof properties.customUserClaims !== 'undefined') && !validator.isObject(properties.customUserClaims)) { + return Promise.reject( + new FirebaseAuthError( + AuthClientErrorCode.INVALID_ARGUMENT, + 'customUserClaims of properties argument must be an object, null, or undefined.', + ), + ); } // Build the setAccountInfo request. @@ -1470,6 +1477,16 @@ export abstract class AbstractAuthRequestHandler { request.disableUser = request.disabled; delete request.disabled; } + // Rewrite customClaims to customAttributes + if (typeof request.customUserClaims !== 'undefined') { + if (request.customUserClaims === null) { + // Delete operation. Replace null with an empty object. + request.customAttributes = JSON.stringify({}); + } else { + request.customAttributes = JSON.stringify(request.customUserClaims); + } + delete request.customUserClaims; + } // Construct mfa related user data. if (validator.isNonNullObject(request.multiFactor)) { if (request.multiFactor.enrolledFactors === null) { diff --git a/src/auth/auth-config.ts b/src/auth/auth-config.ts index ce45713f97..03816c18ba 100644 --- a/src/auth/auth-config.ts +++ b/src/auth/auth-config.ts @@ -188,6 +188,12 @@ export interface UpdateRequest { * Unlinks this user from the specified providers. */ providersToUnlink?: string[]; + + /** + * If provided, sets additional developer claims on the user's token, overwriting + * any existing claims. If not provided or `undefined`, then existing claims will remain unchanged. + */ + customUserClaims?: object | null; } /** From a8f394581f5041dada3dfe1856401e0af6b59134 Mon Sep 17 00:00:00 2001 From: Brett Willis Date: Tue, 23 Aug 2022 09:56:00 +1200 Subject: [PATCH 2/5] Test coverage for updating custom claims --- test/unit/auth/auth-api-request.spec.ts | 77 +++++++++++++++++++++++++ test/unit/auth/auth.spec.ts | 3 + 2 files changed, 80 insertions(+) diff --git a/test/unit/auth/auth-api-request.spec.ts b/test/unit/auth/auth-api-request.spec.ts index 574962df53..1e72b98e28 100644 --- a/test/unit/auth/auth-api-request.spec.ts +++ b/test/unit/auth/auth-api-request.spec.ts @@ -2044,6 +2044,9 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { photoURL: 'http://localhost/1234/photo.png', password: 'password', phoneNumber: '+11234567890', + customUserClaims: { + claim1: true, + }, multiFactor: { enrolledFactors: [ { @@ -2076,6 +2079,7 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { photoUrl: 'http://localhost/1234/photo.png', password: 'password', phoneNumber: '+11234567890', + customAttributes: '{"claim1":true}', mfa: { enrollments: [ { @@ -2122,6 +2126,33 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { password: 'password', deleteProvider: ['phone'], }; + // Valid request to delete custom claims. + const validDeleteCustomClaimsData = deepCopy(validData); + validDeleteCustomClaimsData.customUserClaims = null; + delete validDeletePhoneNumberData.multiFactor; + const expectedValidDeleteCustomClaimsData = { + localId: uid, + displayName: 'John Doe', + email: 'user@example.com', + emailVerified: true, + disableUser: false, + photoUrl: 'http://localhost/1234/photo.png', + password: 'password', + customAttributes: '{}', + }; + // Valid request to leave custom claims unchanged. + const validUnchangedCustomClaimsData = deepCopy(validData); + delete validUnchangedCustomClaimsData.customUserClaims; + delete validUnchangedCustomClaimsData.multiFactor; + const expectedValidUnchangedCustomClaimsData = { + localId: uid, + displayName: 'John Doe', + email: 'user@example.com', + emailVerified: true, + disableUser: false, + photoUrl: 'http://localhost/1234/photo.png', + password: 'password', + }; // Valid request to delete all second factors. const expectedValidDeleteMfaData = { localId: uid, @@ -2222,6 +2253,52 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { callParams(path, method, expectedValidDeletePhoneNumberData)); }); }); + + it('should be fulfilled given null custom claims', () => { + // Successful result server response. + const expectedResult = utils.responseFrom({ + kind: 'identitytoolkit#SetAccountInfoResponse', + localId: uid, + }); + + const stub = sinon.stub(HttpClient.prototype, 'send').resolves(expectedResult); + stubs.push(stub); + + const requestHandler = handler.init(mockApp); + // Send update request to delete phone number. + return requestHandler.updateExistingAccount(uid, validDeleteCustomClaimsData) + .then((returnedUid: string) => { + // uid should be returned. + expect(returnedUid).to.be.equal(uid); + // Confirm expected rpc request parameters sent. In this case, phoneNumber + // removed from request and deleteProvider added. + expect(stub).to.have.been.calledOnce.and.calledWith( + callParams(path, method, expectedValidDeleteCustomClaimsData)); + }); + }); + + it('should be fulfilled given undefined custom claims', () => { + // Successful result server response. + const expectedResult = utils.responseFrom({ + kind: 'identitytoolkit#SetAccountInfoResponse', + localId: uid, + }); + + const stub = sinon.stub(HttpClient.prototype, 'send').resolves(expectedResult); + stubs.push(stub); + + const requestHandler = handler.init(mockApp); + // Send update request to delete phone number. + return requestHandler.updateExistingAccount(uid, validUnchangedCustomClaimsData) + .then((returnedUid: string) => { + // uid should be returned. + expect(returnedUid).to.be.equal(uid); + // Confirm expected rpc request parameters sent. In this case, phoneNumber + // removed from request and deleteProvider added. + expect(stub).to.have.been.calledOnce.and.calledWith( + callParams(path, method, expectedValidUnchangedCustomClaimsData)); + }); + }); it('should be fulfilled given null enrolled factors', () => { // Successful result server response. diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index 0995a310d4..d8738b9557 100644 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -1845,6 +1845,9 @@ AUTH_CONFIGS.forEach((testConfig) => { emailVerified: expectedUserRecord.emailVerified, password: 'password', phoneNumber: expectedUserRecord.phoneNumber, + customUserClaims: { + claim1: true, + }, providerToLink: { providerId: 'google.com', uid: 'google_uid', From dc1055dd45e0df4bbecd8211e47a4d83fa7e281a Mon Sep 17 00:00:00 2001 From: Brett Willis Date: Thu, 9 Mar 2023 10:15:35 +1300 Subject: [PATCH 3/5] Fix tests and lint --- etc/firebase-admin.auth.api.md | 1 + src/auth/auth-api-request.ts | 3 ++- src/auth/auth-config.ts | 3 ++- test/unit/auth/auth-api-request.spec.ts | 23 +++++++++++++---------- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/etc/firebase-admin.auth.api.md b/etc/firebase-admin.auth.api.md index c7090af304..3a357a766c 100644 --- a/etc/firebase-admin.auth.api.md +++ b/etc/firebase-admin.auth.api.md @@ -439,6 +439,7 @@ export interface UpdateProjectConfigRequest { // @public export interface UpdateRequest { + customUserClaims?: object | null; disabled?: boolean; displayName?: string | null; email?: string; diff --git a/src/auth/auth-api-request.ts b/src/auth/auth-api-request.ts index 1e02420b65..5bb762bd68 100644 --- a/src/auth/auth-api-request.ts +++ b/src/auth/auth-api-request.ts @@ -1405,7 +1405,8 @@ export abstract class AbstractAuthRequestHandler { 'providersToUnlink of properties argument must be an array of strings.'); } }); - } else if ((typeof properties.customUserClaims !== 'undefined') && !validator.isObject(properties.customUserClaims)) { + } else if ((typeof properties.customUserClaims !== 'undefined') + && !validator.isObject(properties.customUserClaims)) { return Promise.reject( new FirebaseAuthError( AuthClientErrorCode.INVALID_ARGUMENT, diff --git a/src/auth/auth-config.ts b/src/auth/auth-config.ts index 46c3aab4f2..aaf2412d2a 100644 --- a/src/auth/auth-config.ts +++ b/src/auth/auth-config.ts @@ -191,7 +191,8 @@ export interface UpdateRequest { /** * If provided, sets additional developer claims on the user's token, overwriting - * any existing claims. If not provided or `undefined`, then existing claims will remain unchanged. + * any existing claims. Providing `null` will clear any existing custom claims. + * If not provided or `undefined`, then existing claims will remain unchanged. */ customUserClaims?: object | null; } diff --git a/test/unit/auth/auth-api-request.spec.ts b/test/unit/auth/auth-api-request.spec.ts index 1e72b98e28..27bab859ba 100644 --- a/test/unit/auth/auth-api-request.spec.ts +++ b/test/unit/auth/auth-api-request.spec.ts @@ -2045,7 +2045,8 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { password: 'password', phoneNumber: '+11234567890', customUserClaims: { - claim1: true, + admin: true, + groupId: '123', }, multiFactor: { enrolledFactors: [ @@ -2079,7 +2080,7 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { photoUrl: 'http://localhost/1234/photo.png', password: 'password', phoneNumber: '+11234567890', - customAttributes: '{"claim1":true}', + customAttributes: JSON.stringify({ admin: true, groupId: '123' }), mfa: { enrollments: [ { @@ -2110,6 +2111,7 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { disableUser: false, password: 'password', phoneNumber: '+11234567890', + customAttributes: JSON.stringify({ admin: true, groupId: '123' }), deleteAttribute: ['DISPLAY_NAME', 'PHOTO_URL'], }; // Valid request to delete phoneNumber. @@ -2124,12 +2126,13 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { disableUser: false, photoUrl: 'http://localhost/1234/photo.png', password: 'password', + customAttributes: JSON.stringify({ admin: true, groupId: '123' }), deleteProvider: ['phone'], }; // Valid request to delete custom claims. const validDeleteCustomClaimsData = deepCopy(validData); validDeleteCustomClaimsData.customUserClaims = null; - delete validDeletePhoneNumberData.multiFactor; + delete validDeleteCustomClaimsData.multiFactor; const expectedValidDeleteCustomClaimsData = { localId: uid, displayName: 'John Doe', @@ -2138,7 +2141,8 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { disableUser: false, photoUrl: 'http://localhost/1234/photo.png', password: 'password', - customAttributes: '{}', + phoneNumber: '+11234567890', + customAttributes: JSON.stringify({}), }; // Valid request to leave custom claims unchanged. const validUnchangedCustomClaimsData = deepCopy(validData); @@ -2152,6 +2156,7 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { disableUser: false, photoUrl: 'http://localhost/1234/photo.png', password: 'password', + phoneNumber: '+11234567890', }; // Valid request to delete all second factors. const expectedValidDeleteMfaData = { @@ -2265,13 +2270,12 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { stubs.push(stub); const requestHandler = handler.init(mockApp); - // Send update request to delete phone number. + // Send update request to delete custom claims. return requestHandler.updateExistingAccount(uid, validDeleteCustomClaimsData) .then((returnedUid: string) => { // uid should be returned. expect(returnedUid).to.be.equal(uid); - // Confirm expected rpc request parameters sent. In this case, phoneNumber - // removed from request and deleteProvider added. + // Confirm expected rpc request parameters sent. In this case, customAttributes added. expect(stub).to.have.been.calledOnce.and.calledWith( callParams(path, method, expectedValidDeleteCustomClaimsData)); }); @@ -2288,13 +2292,12 @@ AUTH_REQUEST_HANDLER_TESTS.forEach((handler) => { stubs.push(stub); const requestHandler = handler.init(mockApp); - // Send update request to delete phone number. + // Send update request to update account excluding custom claims. return requestHandler.updateExistingAccount(uid, validUnchangedCustomClaimsData) .then((returnedUid: string) => { // uid should be returned. expect(returnedUid).to.be.equal(uid); - // Confirm expected rpc request parameters sent. In this case, phoneNumber - // removed from request and deleteProvider added. + // Confirm expected rpc request parameters sent. In this case, customAttributes removed. expect(stub).to.have.been.calledOnce.and.calledWith( callParams(path, method, expectedValidUnchangedCustomClaimsData)); }); From aaf3a2fcd6a220b3524d12ea5cfd691a654592fe Mon Sep 17 00:00:00 2001 From: Brett Willis Date: Thu, 9 Mar 2023 12:45:56 +1300 Subject: [PATCH 4/5] Add integration tests --- test/integration/auth.spec.ts | 50 +++++++++++++++++++++++++++++++++++ test/unit/auth/auth.spec.ts | 17 +++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index 68e66aaf06..2f74bfde47 100644 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -799,6 +799,56 @@ describe('admin.auth', () => { }); }); + it('sets claims that are accessible via user\'s ID token', () => { + // Set custom claims on the user. + return getAuth().updateUser(updateUser.uid, { customUserClaims: customClaims }) + .then((userRecord) => { + // Confirm custom claims set on the UserRecord. + expect(userRecord.customClaims).to.deep.equal(customClaims); + expect(userRecord.email).to.exist; + return clientAuth().signInWithEmailAndPassword( + userRecord.email!, mockUserData.password); + }) + .then(({ user }) => { + // Get the user's ID token. + expect(user).to.exist; + return user!.getIdToken(); + }) + .then((idToken) => { + // Verify ID token contents. + return getAuth().verifyIdToken(idToken); + }) + .then((decodedIdToken: { [key: string]: any }) => { + // Confirm expected claims set on the user's ID token. + for (const key in customClaims) { + if (Object.prototype.hasOwnProperty.call(customClaims, key)) { + expect(decodedIdToken[key]).to.equal(customClaims[key]); + } + } + // Test clearing of custom claims. + return getAuth().updateUser(newUserUid, { customUserClaims: null }); + }) + .then((userRecord) => { + // Custom claims should be cleared. + expect(userRecord.customClaims).to.deep.equal({}); + // Force token refresh. All claims should be cleared. + expect(clientAuth().currentUser).to.exist; + return clientAuth().currentUser!.getIdToken(true); + }) + .then((idToken) => { + // Verify ID token contents. + return getAuth().verifyIdToken(idToken); + }) + .then((decodedIdToken: { [key: string]: any }) => { + // Confirm all custom claims are cleared. + for (const key in customClaims) { + if (Object.prototype.hasOwnProperty.call(customClaims, key)) { + expect(decodedIdToken[key]).to.be.undefined; + } + } + }); + }); + it('creates, updates, and removes second factors', function () { if (authEmulatorHost) { return this.skip(); // Not yet supported in Auth Emulator. diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index acb412d964..0f0657b09c 100644 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -1846,7 +1846,8 @@ AUTH_CONFIGS.forEach((testConfig) => { password: 'password', phoneNumber: expectedUserRecord.phoneNumber, customUserClaims: { - claim1: true, + admin: true, + groupId: '123', }, providerToLink: { providerId: 'google.com', @@ -1858,10 +1859,12 @@ AUTH_CONFIGS.forEach((testConfig) => { beforeEach(() => { sinon.spy(validator, 'isUid'); sinon.spy(validator, 'isNonNullObject'); + sinon.spy(validator, 'isObject'); }); afterEach(() => { (validator.isUid as any).restore(); (validator.isNonNullObject as any).restore(); + (validator.isObject as any).restore(); _.forEach(stubs, (stub) => stub.restore()); stubs = []; }); @@ -1981,6 +1984,18 @@ AUTH_CONFIGS.forEach((testConfig) => { }); }); + it('should be rejected given invalid custom user claims', () => { + return auth.updateUser(uid, { customUserClaims: 'invalid' as any }) + .then(() => { + throw new Error('Unexpected success'); + }) + .catch((error) => { + expect(error).to.have.property('code', 'auth/argument-error'); + expect(validator.isObject).to.have.been.calledOnce.and.calledWith('invalid'); + }); + }); + + describe('non-federated providers', () => { let invokeRequestHandlerStub: sinon.SinonStub; let getAccountInfoByUidStub: sinon.SinonStub; From 9fa048445bd6e10753abda5fa598abac6c67d79f Mon Sep 17 00:00:00 2001 From: Brett Willis Date: Thu, 8 Feb 2024 09:21:37 +1300 Subject: [PATCH 5/5] Remove unsupported properties from CreateRequest --- src/auth/auth-config.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/auth/auth-config.ts b/src/auth/auth-config.ts index aaf2412d2a..3a0cf3ddd0 100644 --- a/src/auth/auth-config.ts +++ b/src/auth/auth-config.ts @@ -124,10 +124,9 @@ export interface MultiFactorUpdateSettings { } /** - * Interface representing the properties to update on the provided user. + * Interface representing the base properties for `CreateRequest` and `UpdateRequest`. */ -export interface UpdateRequest { - +export interface BaseCreateUpdateUserRequest { /** * Whether or not the user is disabled: `true` for disabled; * `false` for enabled. @@ -163,6 +162,12 @@ export interface UpdateRequest { * The user's photo URL. */ photoURL?: string | null; +} + +/** + * Interface representing the properties to update on the provided user. + */ +export interface UpdateRequest extends BaseCreateUpdateUserRequest { /** * The user's updated multi-factor related properties. @@ -238,7 +243,7 @@ export interface UserProvider { * Interface representing the properties to set on a new user record to be * created. */ -export interface CreateRequest extends UpdateRequest { +export interface CreateRequest extends BaseCreateUpdateUserRequest { /** * The user's `uid`.