Cleanup demo code & get it working with MFA flows #3380
Conversation
avolkovi
commented
Jul 9, 2020
avolkovi
commented
- lint the demo
- fix some error handling code
- fix verifyPhoneNumber when called with MFA context
- add to/fromJSON to EmailAuthCredential
💥 No ChangesetLatest commit: 2c36f93 Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂 If these changes should be published to npm, you need to add a changeset. This PR includes no changesetsWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Click here to learn what changesets are, and how to add one. Click here if you're a maintainer who wants to add a changeset to this PR |
Binary Size ReportAffected SDKsNo changes between base commit (49a7240) and head commit (5d508e2). Test Logs
|
avolkovi
force-pushed
the
avolkovi/auth-next-demo
branch
from
8741896 to
47034e9
Compare
| @@ -54,6 +55,7 @@ export const enum AuthErrorCode { | |||
| INVALID_EMAIL = 'invalid-email', | |||
| INVALID_IDP_RESPONSE = 'invalid-credential', | |||
| INVALID_MESSAGE_PAYLOAD = 'invalid-message-payload', | |||
| INVALID_MFA_SESSION = 'invalid-multi-factor-session', | |||
There was a problem hiding this comment.
verified against public docs: https://cloud.google.com/identity-platform/docs/error-codes
| @@ -125,7 +125,8 @@ export async function _performFetchWithErrorHandling<V>( | |||
| return response.json(); | |||
| } else { | |||
| const json: JsonError = await response.json(); | |||
| const authError = errorMap[json.error.message]; | |||
| const serverErrorCode = json.error.message.split(' : ')[0] as ServerError; | |||
There was a problem hiding this comment.
consider using a regex to make this potentially-less-brittle
/\s*:/ should do it
There was a problem hiding this comment.
looks like the existing SDK does it a bit different... https://github.com/firebase/firebase-js-sdk/blob/master/packages/auth/src/rpchandler.js#L2916 they use everything to the right of the colon as the message, overriding whatever is in the error map
| @@ -2175,6 +2175,24 @@ | |||
| dependencies: | |||
| slash "^3.0.0" | |||
|
|
|||
| "@rollup/plugin-strip@^1.3.2": | |||
There was a problem hiding this comment.
Are the changes to yarn.lock necessary?
There was a problem hiding this comment.
they're specific to our branch, i reverted the yarn.lock when I rebased against master yesterday
- lint the demo - fix some error handling code - fix verifyPhoneNumber when called with MFA context - add to/fromJSON to EmailAuthCredential
avolkovi
force-pushed
the
avolkovi/auth-next-demo
branch
from
47034e9 to
2c36f93
Compare
- lint the demo - fix some error handling code - fix verifyPhoneNumber when called with MFA context - add to/fromJSON to EmailAuthCredential
