Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Create handleRecaptchaFlow helper method #7666

Merged
merged 5 commits into from
Oct 13, 2023
Merged

Create handleRecaptchaFlow helper method #7666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b934c9
Create handleRecaptchaFlow helper method
Sep 28, 2023
53c51dc
Revert "Create handleRecaptchaFlow helper method"
Oct 11, 2023
22f1369
Add handleRecaptchaFlow
Oct 11, 2023
15ec2b5
Revert tests
Oct 11, 2023
edc0f50
Integration
Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-dragons-juggle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@firebase/auth': patch
---

Create handleRecaptchaFlow helper method
34 changes: 7 additions & 27 deletions packages/auth/src/core/credentials/email.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { IdTokenResponse } from '../../model/id_token';
import { AuthErrorCode } from '../errors';
import { _fail } from '../util/assert';
import { AuthCredential } from './auth_credential';
import { injectRecaptchaFields } from '../../platform_browser/recaptcha/recaptcha_enterprise_verifier';
import { handleRecaptchaFlow } from '../../platform_browser/recaptcha/recaptcha_enterprise_verifier';
import { RecaptchaActionName, RecaptchaClientType } from '../../api';
/**
* Interface that represents the credentials returned by {@link EmailAuthProvider} for
Expand Down Expand Up @@ -123,32 +123,12 @@ export class EmailAuthCredential extends AuthCredential {
password: this._password,
clientType: RecaptchaClientType.WEB
};
if (auth._getRecaptchaConfig()?.emailPasswordEnabled) {
const requestWithRecaptcha = await injectRecaptchaFields(
auth,
request,
RecaptchaActionName.SIGN_IN_WITH_PASSWORD
);
return signInWithPassword(auth, requestWithRecaptcha);
} else {
return signInWithPassword(auth, request).catch(async error => {
if (
error.code === `auth/${AuthErrorCode.MISSING_RECAPTCHA_TOKEN}`
) {
console.log(
'Sign-in with email address and password is protected by reCAPTCHA for this project. Automatically triggering the reCAPTCHA flow and restarting the sign-in flow.'
);
const requestWithRecaptcha = await injectRecaptchaFields(
auth,
request,
RecaptchaActionName.SIGN_IN_WITH_PASSWORD
);
return signInWithPassword(auth, requestWithRecaptcha);
} else {
return Promise.reject(error);
}
});
}
return handleRecaptchaFlow(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of calling signInWithCredential in packages/auth/src/core/strategies/email_and_password.ts, can we instead duplicate a bit of code and do something like:

export function signInWithEmailAndPassword(
  auth: Auth,
  email: string,
  password: string
): Promise<UserCredential> {

        const request: SignInWithPasswordRequest = {
          returnSecureToken: true,
          email: this._email,
          password: this._password,
          clientType: RecaptchaClientType.WEB
        };
 const response = handleRecaptchaFlow(auth, request, action, signInWithPassword)
 .catch(error => {
    if (error.code === `auth/${AuthErrorCode.MFA_REQUIRED}`) {
      throw MultiFactorError._fromErrorAndOperation(
        auth,
        error,
        operationType,
        user
      );
    } else if (
      error.code === `auth/${AuthErrorCode.PASSWORD_DOES_NOT_MEET_REQUIREMENTS}`
    ) {
      void recachePasswordPolicy(auth);
    } else {
      throw error;
    }
  })
;

  const userCredential = await UserCredentialImpl._fromIdTokenResponse(
    auth,
    OperationType.SIGN_IN,
    response
  );

    await auth._updateCurrentUser(userCredential.user);

This will make the email login flow more consistent with the other flows in strategies/email_and_password.ts and we don't have to import any recaptcha enterprise methods here (enabling tree shaking out the JS script)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the comment here, but it is for the implementation in packages/auth/src/core/strategies/email_and_password.ts

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But doing this will not perform recaptcha enterprise wrapping if the developer is calling reauthenticateWithCredential or signInWithCredential directly... hmm.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I actually tried similar things when implementing the recapthca flow originally. I agree this part needs some refactors.

auth,
request,
RecaptchaActionName.SIGN_IN_WITH_PASSWORD,
signInWithPassword
);
case SignInMethod.EMAIL_LINK:
return signInWithEmailLink(auth, {
email: this._email,
Expand Down
96 changes: 15 additions & 81 deletions packages/auth/src/core/strategies/email_and_password.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { _castAuth } from '../auth/auth_impl';
import { AuthErrorCode } from '../errors';
import { getModularInstance } from '@firebase/util';
import { OperationType } from '../../model/enums';
import { injectRecaptchaFields } from '../../platform_browser/recaptcha/recaptcha_enterprise_verifier';
import { handleRecaptchaFlow } from '../../platform_browser/recaptcha/recaptcha_enterprise_verifier';
import { IdTokenResponse } from '../../model/id_token';
import { RecaptchaActionName, RecaptchaClientType } from '../../api';

Expand Down Expand Up @@ -103,61 +103,15 @@ export async function sendPasswordResetEmail(
email,
clientType: RecaptchaClientType.WEB
};
if (authInternal._getRecaptchaConfig()?.emailPasswordEnabled) {
const requestWithRecaptcha = await injectRecaptchaFields(
authInternal,
request,
RecaptchaActionName.GET_OOB_CODE,
true
);
if (actionCodeSettings) {
_setActionCodeSettingsOnRequest(
authInternal,
requestWithRecaptcha,
actionCodeSettings
);
}
await authentication.sendPasswordResetEmail(
authInternal,
requestWithRecaptcha
);
} else {
if (actionCodeSettings) {
_setActionCodeSettingsOnRequest(
authInternal,
request,
actionCodeSettings
);
}
await authentication
.sendPasswordResetEmail(authInternal, request)
.catch(async error => {
if (error.code === `auth/${AuthErrorCode.MISSING_RECAPTCHA_TOKEN}`) {
console.log(
'Password resets are protected by reCAPTCHA for this project. Automatically triggering the reCAPTCHA flow and restarting the password reset flow.'
);
const requestWithRecaptcha = await injectRecaptchaFields(
authInternal,
request,
RecaptchaActionName.GET_OOB_CODE,
true
);
if (actionCodeSettings) {
_setActionCodeSettingsOnRequest(
authInternal,
requestWithRecaptcha,
actionCodeSettings
);
}
await authentication.sendPasswordResetEmail(
authInternal,
requestWithRecaptcha
);
} else {
return Promise.reject(error);
}
});
if (actionCodeSettings) {
_setActionCodeSettingsOnRequest(authInternal, request, actionCodeSettings);
}
await handleRecaptchaFlow(
authInternal,
request,
RecaptchaActionName.GET_OOB_CODE,
authentication.sendPasswordResetEmail
);
}

/**
Expand Down Expand Up @@ -318,32 +272,12 @@ export async function createUserWithEmailAndPassword(
password,
clientType: RecaptchaClientType.WEB
};
let signUpResponse: Promise<IdTokenResponse>;
if (authInternal._getRecaptchaConfig()?.emailPasswordEnabled) {
const requestWithRecaptcha = await injectRecaptchaFields(
authInternal,
request,
RecaptchaActionName.SIGN_UP_PASSWORD
);
signUpResponse = signUp(authInternal, requestWithRecaptcha);
} else {
signUpResponse = signUp(authInternal, request).catch(async error => {
if (error.code === `auth/${AuthErrorCode.MISSING_RECAPTCHA_TOKEN}`) {
console.log(
'Sign-up is protected by reCAPTCHA for this project. Automatically triggering the reCAPTCHA flow and restarting the sign-up flow.'
);
const requestWithRecaptcha = await injectRecaptchaFields(
authInternal,
request,
RecaptchaActionName.SIGN_UP_PASSWORD
);
return signUp(authInternal, requestWithRecaptcha);
}

throw error;
});
}

const signUpResponse: Promise<IdTokenResponse> = handleRecaptchaFlow(
authInternal,
request,
RecaptchaActionName.SIGN_UP_PASSWORD,
signUp
);
const response = await signUpResponse.catch(error => {
if (
error.code === `auth/${AuthErrorCode.PASSWORD_DOES_NOT_MEET_REQUIREMENTS}`
Expand Down
40 changes: 8 additions & 32 deletions packages/auth/src/core/strategies/email_link.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { AuthErrorCode } from '../errors';
import { _assert } from '../util/assert';
import { getModularInstance } from '@firebase/util';
import { _castAuth } from '../auth/auth_impl';
import { injectRecaptchaFields } from '../../platform_browser/recaptcha/recaptcha_enterprise_verifier';
import { handleRecaptchaFlow } from '../../platform_browser/recaptcha/recaptcha_enterprise_verifier';
import { RecaptchaActionName, RecaptchaClientType } from '../../api';

/**
Expand Down Expand Up @@ -101,37 +101,13 @@ export async function sendSignInLinkToEmail(
);
}
}
if (authInternal._getRecaptchaConfig()?.emailPasswordEnabled) {
const requestWithRecaptcha = await injectRecaptchaFields(
authInternal,
request,
RecaptchaActionName.GET_OOB_CODE,
true
);
setActionCodeSettings(requestWithRecaptcha, actionCodeSettings);
await api.sendSignInLinkToEmail(authInternal, requestWithRecaptcha);
} else {
setActionCodeSettings(request, actionCodeSettings);
await api
.sendSignInLinkToEmail(authInternal, request)
.catch(async error => {
if (error.code === `auth/${AuthErrorCode.MISSING_RECAPTCHA_TOKEN}`) {
console.log(
'Email link sign-in is protected by reCAPTCHA for this project. Automatically triggering the reCAPTCHA flow and restarting the sign-in flow.'
);
const requestWithRecaptcha = await injectRecaptchaFields(
authInternal,
request,
RecaptchaActionName.GET_OOB_CODE,
true
);
setActionCodeSettings(requestWithRecaptcha, actionCodeSettings);
await api.sendSignInLinkToEmail(authInternal, requestWithRecaptcha);
} else {
return Promise.reject(error);
}
});
}
setActionCodeSettings(request, actionCodeSettings);
await handleRecaptchaFlow(
authInternal,
request,
RecaptchaActionName.GET_OOB_CODE,
api.sendSignInLinkToEmail
);
}

/**
Expand Down
40 changes: 40 additions & 0 deletions packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { Auth } from '../../model/public_types';
import { AuthInternal } from '../../model/auth';
import { _castAuth } from '../../core/auth/auth_impl';
import * as jsHelpers from '../load_js';
import { AuthErrorCode } from '../../core/errors';

const RECAPTCHA_ENTERPRISE_URL =
'https://www.google.com/recaptcha/enterprise.js?render=';
Expand Down Expand Up @@ -175,6 +176,45 @@ export async function injectRecaptchaFields<T>(
return newRequest;
}

type ActionMethod<TRequest, TResponse> = (
auth: Auth,
request: TRequest
) => Promise<TResponse>;

export async function handleRecaptchaFlow<TRequest, TResponse>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some tests for this method? I understand that usages of this methods in the various flows are being tested.. would be good to test this too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@renkelvin can you update the description that all rCE protected flows were manually verified with the demo app?

I am ok with moving the new unit tests to a different PR to debug them.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, done.

authInstance: AuthInternal,
request: TRequest,
actionName: RecaptchaActionName,
actionMethod: ActionMethod<TRequest, TResponse>
): Promise<TResponse> {
if (authInstance._getRecaptchaConfig()?.emailPasswordEnabled) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also include "provider" name or enum as a parameter? Based on that, we will know whether to check emailPasswordEnabled or a different provider.

Ok to handle this the next time we add a provider too. Please add a TODO in that case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as an internal method, I prefer add the parameter only when needed so the context is more clear.

const requestWithRecaptcha = await injectRecaptchaFields(
authInstance,
request,
actionName,
actionName === RecaptchaActionName.GET_OOB_CODE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that for oobCodeFlows, we do not call the request field name as "captchaResp: token"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, this is due to backend legacy issues.

);
return actionMethod(authInstance, requestWithRecaptcha);
} else {
return actionMethod(authInstance, request).catch(async error => {
if (error.code === `auth/${AuthErrorCode.MISSING_RECAPTCHA_TOKEN}`) {
console.log(
`${actionName} is protected by reCAPTCHA Enterprise for this project. Automatically triggering the reCAPTCHA flow and restarting the flow.`
);
const requestWithRecaptcha = await injectRecaptchaFields(
authInstance,
request,
actionName,
actionName === RecaptchaActionName.GET_OOB_CODE
);
return actionMethod(authInstance, requestWithRecaptcha);
} else {
return Promise.reject(error);
}
});
}
}

export async function _initializeRecaptchaConfig(auth: Auth): Promise<void> {
const authInternal = _castAuth(auth);

Expand Down
Loading