Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new error code, PASSWORD_DOES_NOT_MEET_REQUIREMENTS, to handle password policy violations, along with its client-side mapping and a corresponding unit test. The feedback suggests reorganizing the placement of the new error code and its configuration in src/auth/error.ts to avoid breaking the contiguous block of INVALID_ error codes, and adding an assertion for the error message in the unit test to ensure consistency.
| INVALID_PASSWORD: 'invalid-password', | ||
| INVALID_PASSWORD_HASH: 'invalid-password-hash', | ||
| INVALID_PASSWORD_SALT: 'invalid-password-salt', | ||
| PASSWORD_DOES_NOT_MEET_REQUIREMENTS: 'password-does-not-meet-requirements', |
There was a problem hiding this comment.
The PASSWORD_DOES_NOT_MEET_REQUIREMENTS key is currently inserted in the middle of a contiguous block of INVALID_ error codes (lines 39 to 81). To maintain the grouping and readability of the error codes, it should be moved out of the INVALID_ block and placed in alphabetical order among the other error codes (for example, after OPERATION_NOT_ALLOWED and before PHONE_NUMBER_ALREADY_EXISTS).
| PASSWORD_DOES_NOT_MEET_REQUIREMENTS: { | ||
| code: AuthErrorCode.PASSWORD_DOES_NOT_MEET_REQUIREMENTS, | ||
| message: 'The password does not meet the requirements.', | ||
| }, |
| it('should map password policy server errors to the public password policy code', () => { | ||
| const error = FirebaseAuthError.fromServerError('PASSWORD_DOES_NOT_MEET_REQUIREMENTS'); | ||
| expect(error.code).to.be.equal('auth/password-does-not-meet-requirements'); | ||
| }); |
There was a problem hiding this comment.
To ensure consistency with other test cases in this file, we should also assert that the mapped error message is correct.
| it('should map password policy server errors to the public password policy code', () => { | |
| const error = FirebaseAuthError.fromServerError('PASSWORD_DOES_NOT_MEET_REQUIREMENTS'); | |
| expect(error.code).to.be.equal('auth/password-does-not-meet-requirements'); | |
| }); | |
| it('should map password policy server errors to the public password policy code', () => { | |
| const error = FirebaseAuthError.fromServerError('PASSWORD_DOES_NOT_MEET_REQUIREMENTS'); | |
| expect(error.code).to.be.equal('auth/password-does-not-meet-requirements'); | |
| expect(error.message).to.be.equal('The password does not meet the requirements.'); | |
| }); |
Summary
PASSWORD_DOES_NOT_MEET_REQUIREMENTSto the publicauth/password-does-not-meet-requirementserror code.Evidence
FirebaseAuthError.fromServerError('PASSWORD_DOES_NOT_MEET_REQUIREMENTS')returnedauth/internal-error; the regression failed with that result.auth/password-does-not-meet-requirementsand its default message.INVALID_*block and asserting the default message. Both suggestions were applied in commit26cff32.test/unit/utils/error.spec.ts: 26 passing.npm run build: passed.git diff --check: passed.npm run test:unit: 6,147 passing, 8 failing. Failures are HTTP/2 session error assertions intest/unit/utils/api-request.spec.tsandtest/unit/messaging/messaging.spec.ts.Merge Danger
Door: two-way
Blast Radius: limited to the Auth server error mapping for this response code.
Fixes #3265