intershop / intershop-pwa

The Intershop PWA is an Angular based progressive web app storefront for the Intershop Commerce Platform.
https://www.intershop.com/progressive-web-app
MIT License
159 stars 85 forks source link

Captcha is incompatible with SSO #1334

Open tbouliere-datasolution opened 1 year ago

tbouliere-datasolution commented 1 year ago

When using the Google Recaptcha V3 service, the captcha verification information is transmitted to the icm using the Authorization header.

https://github.com/intershop/intershop-pwa/blob/9964ebd929b61c81d763a082a90e721beba0dae4/src/app/core/services/api/api.service.ts#L82

This conflicts with the standard SSO operation which uses this header to pass a bearer token

https://github.com/intershop/intershop-pwa/blob/9964ebd929b61c81d763a082a90e721beba0dae4/src/app/core/identity-provider/auth0.identity-provider.ts#L251

Expected Behavior

Use a custom header. For instance X-Captcha-Token. And remember to modify the corresponding class on the icm side: com.intershop.sellside.rest.common.internal.auth.CaptchaAuthenticationProvider

Steps to Reproduce the Bug

Steps to reproduce the behavior:

  1. Login with any sso provider
  2. Post a form with a captcha that points to an endpoint with required authentication.
  3. Eat popcorn while enjoying the beautiful side iffect

Environment Details

Additional Context, like Screenshots, Log File Snippets etc.

AB#81572

shauke commented 1 year ago

@tbouliere-datasolution Can you please provide more details for the problematic use case? What form needs an additional Captcha authorization once the user is logged in? Usually only form submissions for unregistered users need Captcha protection so you should never have a combination once you are logged in via SSO.

tbouliere-datasolution commented 1 year ago

Captchas are mainly used to prevent spamming. If your website contains a public registration feature, any robot can create an account and then use a form that triggers an email to be sent.

It is a very strong assumption to limit captchas to non-registered users.

tbouliere-datasolution commented 1 year ago

For example, you can take the function "send the product to a friend". It is better to add a captcha at this stage, even for registered user.

shauke commented 1 year ago

Captchas are mainly used to prevent spamming. If your website contains a public registration feature, any robot can create an account and then use a form that triggers an email to be sent.

It is a very strong assumption to limit captchas to non-registered users.

Well the registration is exactly an example for a non-registered user interaction that is protected by a captcha in the PWA if it is not done with SSO. And if you use an SSO provider for the registration then the verification of real users is handled by the SSO provider. Once the user comes back to the PWA the user is already authenticated.

shauke commented 1 year ago

For example, you can take the function "send the product to a friend". It is better to add a captcha at this stage, even for registered user.

The current implementation for "Email a friend" uses the users mail program exactly to prevent spam being send from the PWA/ICM. In the standard there is nothing that we could protect with an additional Captcha here.

But I will talk the according REST API team to check whether they see any use case that requires a combination of both authentication methods for one request.

tbouliere-datasolution commented 1 year ago

Ok I see your point @shauke. The general idea is to say that if the user is logged in, then it is a human and therefore the captcha is unnecessary in some way.

However I still think that a catcha can be necessary, on pages that need to be accessible for both logged in and not logged in users.

For example for the feature, contact-us (https://intershoppwa.azurewebsites.net/contact) This page introduces a hard-coded captcha :

https://github.com/intershop/intershop-pwa/blob/7412e2525dcbc038e4f77073e01050b7eb4dc98f/src/app/extensions/contact-us/pages/contact/contact-form/contact-form.component.ts#L117

and a backend coded check :

if (this.authenticationProvider != null
        && this.authenticationProvider instanceof CaptchaAuthenticationProvider) {
    final CaptchaAuthenticationProvider cap = (CaptchaAuthenticationProvider) this.authenticationProvider;
    if (cap.isCaptchaRequired("intershop.CaptchaPreferences.ContactUs")) {
        cap.authenticate(this.currentRequest, this.currentResponse);
    }
}

And you got a probleme there. Either you activate the captcha feature and you protected non login pages, but it crash when you have an SSO BearerToken, or you desactivate the captcha for everyone.

This is why I think that captcha should not be treat as an authentification state.

shauke commented 1 year ago

After discussion with the responsible ICM REST API team it was decided to implement the suggestion to expect the Captcha token at an alternative header key (with fallback to the ApiService.AUTHORIZATION_HEADER_KEY for backwards compatibility). Once an ICM release with this change is available the PWA implementation will be adapted accordingly.

tbouliere-datasolution commented 1 year ago

Nice !

shauke commented 1 year ago

Adapted headers available with ICM 7.10.40 and ICM 11.0.12.