wso2 / product-is

Welcome to the WSO2 Identity Server source code! For info on working with the WSO2 Identity Server repository and contributing code, click the link below.
http://wso2.github.io/
Apache License 2.0
746 stars 724 forks source link

Double submission issue in TOTP #17530

Open Dinithi-Hasanika opened 11 months ago

Dinithi-Hasanika commented 11 months ago

Describe the issue: The double submission is not handled correctly in the TOTP submission button. After entering the TOTP value, and then submitting the form multiple times leads to authentication errors.

The cause of the issue is jquery is imported in the includes/footer.jsp file and in totp.jsp file the double submission preventing logic is implemented before including the footer.jsp file. Hence it gives following javascript error.

Uncaught ReferenceError: $ is not defined at :9443/totpauthenticationendpoint/totp.jsp?sessionDataKey=<>&authenticators=totp&type=totp&username=Bob%40carbon.super:171:17

How to reproduce: Set up TOTP authenticator as a MFA step. Then submit TOTP value by hitting submit button multiple times.

Expected behavior: User should not observe authentication error and double submission needs to be correctly handled

Environment information (Please complete the following information; remove any unnecessary fields) :


Optional Fields

Related issues:

Suggested labels:

jdchathuranga commented 11 months ago

This can still be reproducible for both SMS OTP and TOTP. There was a previous issue which effects only for the iOS, but the current bug affected both iOS and Android platforms.

RushanNanayakkara commented 11 months ago

Hi @jdchathuranga ,

I've tried to reproduce this locally as well as on accounts.wso2.com(as per our discussion on chat) as well. However I could not reproduce the issue on any of them. I've attached the steps I followed in the following view. Can you please let me know if there should be any changes in the steps. environment : MacOS, Safari Browser and MacOS, Google Chrome browser.

Since the TOTP issue is urgent we've already sent PRs for that issue. If there are any mistakes in the steps we will further investigate and send the fix in a separate update.

https://github.com/wso2/product-is/assets/42939752/01598e8b-45e8-4ba2-8a33-b46c7b9b5736

jdchathuranga commented 11 months ago

Hi @RushanNanayakkara

This is easy to re-produce on the browser on iOS.

  1. Open Safari on iOS or iOS Simulator (Tested on iOS 17 Simulator)
  2. Go to Gmail, initiate the login flow
  3. Provide WSO2 email
  4. Then it will redirect to accounts.wso2.com, IS login page
  5. Provide Credentials (If you are using saved credentials on iOS, there is another bug where it submits the form before checking the "Iam not a robot", that should also be addressed separately)
  6. Select TOTP for the 2FA
  7. Type Correct TOTP Code and double/triple tap on the Authenticate button.

https://github.com/wso2/product-is/assets/3407831/89c2a5f8-22e3-4131-9623-a5f874ff3ccd

RushanNanayakkara commented 11 months ago

Hi @jdchathuranga ,

As I've mentioned in the previous comment we've sent the PRs for the TOTP issue since it was present in the pack. This was reproducible for TOTP flow due to an import issue. The jquery library is loaded[1][2] after the function in which it is used[3]. Therefore, the Uncaught ReferenceError: $ is not defined error is occurred. However, this was not reproducible for the SMS OTP flow. Upon inspection of the SMS OTP codebase[4], the imports seem to be in order and there seem to be no reason for this issue to occur.

Since the SMS OTP issue needs further investigation and does not seem to be related to the TOTP issue, shall we track this through a different issue?

[1] https://github.com/wso2/identity-apps/blob/c7ff1ea0021631cdfbf873f28bce3d8431e5d02b/apps/totp-authentication-portal/src/main/webapp/totp.jsp#L182 [2] https://github.com/wso2/identity-apps/blob/c7ff1ea0021631cdfbf873f28bce3d8431e5d02b/apps/totp-authentication-portal/src/main/webapp/includes/footer.jsp#L19 [3] https://github.com/wso2/identity-apps/blob/c7ff1ea0021631cdfbf873f28bce3d8431e5d02b/apps/totp-authentication-portal/src/main/webapp/totp.jsp#L80 [4] https://github.com/wso2/identity-apps/blob/20f048b19ffefa36f87de2a2717d1e161ad7daaa/apps/sms-otp-authentication-portal/src/main/webapp/smsotp.jsp

jdchathuranga commented 11 months ago

Yes,

Also we need to track following issue as well, that is also impacting login flow on mobile devices.

If you are using saved credentials on iOS, there is a bug where it submits the form before checking the captcha, that should. so the login attempt fails. This is implemented correctly on the Asgardeo provided login page, we can use saved password and then it prompts to fill the captacha.