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.

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

[SDK-2970] Remove captcha for enterprise SSO connections #2071

Merged
merged 5 commits into from
Dec 21, 2021

Conversation

stevehobbsdev
Copy link
Contributor

@stevehobbsdev stevehobbsdev commented Dec 8, 2021

Changes

This PR hides the Captcha for enterprise (non-ADFS) SSO connections. As the user is redirected to another vendor to input their password, having a Captcha here makes no sense and is not validated by Auth0 server anyway. However, leaving it in place is an awkward experience, as it looks like the Captcha can be bypassed (you can enter a random value and it still works).

This PR also rolls in #2065, which has now been closed.

References

SDK-2970 (internal support ticket)

Testing

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of the platform/language

Checklist

l.captcha(lock) && l.captcha(lock).get('required') ? (
l.captcha(lock) &&
l.captcha(lock).get('required') &&
(isHRDDomain(lock, databaseUsernameValue(lock)) || !sso) ? (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the real crux of the change - showing captcha if required, but also for ADFS SSO connections only (other SSO connections should hide the Captcha).

@stevehobbsdev stevehobbsdev marked this pull request as ready for review December 8, 2021 15:38
@stevehobbsdev stevehobbsdev requested a review from a team as a code owner December 8, 2021 15:38
expectShallowComponent(<Component {...defaultProps} />).toMatchSnapshot();
});

it('shows the Captcha pane for SSO (ADFS) connections', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Another one that goes thru the same endpoint and is enterprise is the "ad" strategy... Are we also handling that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic here relies on the result of isHRDDomain, which looks like it checks both the ad and auth0-adldap strategies:

return isEnterpriseDomain(m, email, ['ad', 'auth0-adldap']);

Is this sufficient?

Copy link
Member

Choose a reason for hiding this comment

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

mak sense!

@stevehobbsdev stevehobbsdev merged commit 6e5b27d into master Dec 21, 2021
@stevehobbsdev stevehobbsdev deleted the sdk-2970/remove-captcha branch December 21, 2021 10:18
@stevehobbsdev stevehobbsdev mentioned this pull request Jan 7, 2022
@sergioaguirreok
Copy link

sergioaguirreok commented Jan 25, 2022

Hi team, Mercado Libre is evaluating Bot Detection, their use case is B2E, and they have one AD connection to validate users. Does Bot Detection support for non Auth0 DB connections? They are using lock.js, and we tried with the default lock.js template, and it didn’t work with an AD connection. Is that related to this change?
image (1)
image
Thanks!

@thameera
Copy link

thameera commented Jan 26, 2022

Looks like we don't show the captcha when only a DB connection is enabled. Opened ESD-17783.

@stevehobbsdev
Copy link
Contributor Author

Thanks @sergioaguirreok, @thameera, I have raised #2096 to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:medium Medium review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants