-
Notifications
You must be signed in to change notification settings - Fork 904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update the linkWithCredential API to call the signUp endpoint #7692
Conversation
🦋 Changeset detectedLatest commit: f075b7c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1Affected Products
Test Logs |
890a61d
to
a0e4b95
Compare
@@ -166,7 +169,7 @@ export class EmailAuthCredential extends AuthCredential { | |||
): Promise<IdTokenResponse> { | |||
switch (this.signInMethod) { | |||
case SignInMethod.EMAIL_PASSWORD: | |||
return updateEmailPassword(auth, { | |||
return linkEmailPassword(auth, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once #7666 merges, we should also wrap this with reCAPTCHA Enterprise logic.
29790a2
to
fba2873
Compare
c2859a9
to
84ad6aa
Compare
mockEndpointWithParams only works for URL params/GET and not for POST body. Removed this usage with a TODO.
84ad6aa
to
f075b7c
Compare
// First call without recaptcha token should fail with MISSING_RECAPTCHA_TOKEN error | ||
// Though the internal implementation retries with a reCAPTCHA Enterprise token, the second call will fail in this test. | ||
// This is because our endpoint mock does not support returning different responses based on different request body params. | ||
// TODO(renkelvin) - refactor this once we expose a mockEndpointWithBodyParams or similar method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this method do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean what SHOULD mockEndpointWithBodyParams do? We need a method that will send a different response based on a different request body.
mockEndpointWithParams - is an existing method which sends the specified response, for the request with the given URL Params. But our sign up/sign in are POST methods, so they never match the url param or the mock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I think we may just create a single helper method like mockEndpoint(url, params, body)
.
* Update the linkWithCredential API to call the signUp endpoint * Use the SignUpRequest and Response. * changeset * Add recaptcha enterprise support to the link API (which calls signUp) * Refactored existing recaptcha enterprise tests, added new for link API. mockEndpointWithParams only works for URL params/GET and not for POST body. Removed this usage with a TODO.
The /signUp API supports linking when Email Enumeration Protection is enabled as well as disabled. /setAccountInfo does not work when Email Enumeration Protection is enabled.
Verified with the test app that anonymous user upgrade works.