Extends createUser to support multi-factor user creation. #718
Conversation
Note that only phoneNumber and displayName are allowed to be passed in this operation. Adds relevant unit and integration tests. This capability is only available in staging.
src/auth/auth-api-request.ts
Outdated
| } catch (e) { | ||
| return Promise.reject(e); | ||
| } | ||
| if (request.mfaInfo.length === 0) { |
There was a problem hiding this comment.
Nit: To keep the mutations to request to a minimum, you can do this:
const mfaInfo = [];
// Populate mfaInfo
if (request.mfaInfo.length > 0) {
// You can drop the if condition, if you checked for isNonEmptyArray earlier.
request.mfaInfo = mfaInfo;
}
src/auth/auth-api-request.ts
Outdated
| @@ -1145,6 +1162,34 @@ export abstract class AbstractAuthRequestHandler { | |||
| request.localId = request.uid; | |||
| delete request.uid; | |||
| } | |||
| // Construct mfa related user data. | |||
| if (validator.isNonNullObject(request.multiFactor)) { | |||
| if (validator.isArray(request.multiFactor.enrolledFactors)) { | |||
There was a problem hiding this comment.
isNonEmptyArray makes more sense here.
test/integration/auth.spec.ts
Outdated
| // Confirm second factors added to user. | ||
| expect(userRecord.multiFactor.enrolledFactors.length).to.equal(2); | ||
| // Confirm first enrolled second factor. | ||
| expect(userRecord.multiFactor.enrolledFactors[0].uid).not.to.be.undefined; |
There was a problem hiding this comment.
const firstFactor = enrolledFactors[0];
There was a problem hiding this comment.
I set firstMultiFactor to userRecord.multiFactor.enrolledFactors[0]. That improves readability.
test/integration/auth.spec.ts
Outdated
| expect(userRecord.multiFactor.enrolledFactors[0].factorId) | ||
| .to.equal(enrolledFactors[0].factorId); | ||
| // Confirm second enrolled second factor. | ||
| expect(userRecord.multiFactor.enrolledFactors[1].uid).not.to.be.undefined; |
There was a problem hiding this comment.
const secondFactor = enrolledFactors[1];
There was a problem hiding this comment.
I set secondMultiFactor to userRecord.multiFactor.enrolledFactors[1]. It is easier to read this way.
| }); | ||
| }); | ||
|
|
||
| it('should be fulfilled given empty enrolled factors array', () => { |
There was a problem hiding this comment.
This seems identical to the previous test case. Can we consolidate with an array?
const noEnrolledFactors = [ [], null ];
noEnrolledFactors.forEach((arg) => {
...
});
| const unsupportedSecondFactor = { | ||
| secret: 'SECRET', | ||
| displayName: 'Google Authenticator on personal phone', | ||
| factorId: 'totp', |
There was a problem hiding this comment.
Add a comment here indicating totp is not yet supported.
|
Thanks for the quick review! |
Extends createUser to support multi-factor user creation.
Note that only phoneNumber and displayName are allowed to be passed in this operation.
Adds relevant unit and integration tests.
This capability is only available in staging.