ORCID / orcid-angular

Angular UI for ORCID
MIT License
21 stars 16 forks source link

Double calls to client's Redirect URI breaks client SSO #1926

Open cslzchen opened 1 year ago

cslzchen commented 1 year ago

Describe the bug

When client attempts to do OAuth, after the authroize request is successful, ORCiD server returns a 200 (1), in which a script runs and triggers the redirection to the client's configured Redirect URI (2). However, a second request to the Redirect URI is made about 4-5 seconds later (3). The second request causes the browser to cancel the first request.

The issue is, the first request has already cleared the client server of the one-time OAuth SSO state when it is canceled. It is in the process of a few final redirections between our authentication server (which is also the OAuth client, from ORCiD server's perspective) and our app server to finish authentication into our app. Thus, when the second request comes in, it fails SSO due to the invalid state.

Below, I attached both screenshots of doc-only network history and a full network history, a screenshot of the error page and a short video clip.

prod-orcid-sso-network-requests-doc-only

prod-orcid-sso-requests-all

prod-osf-orcid-sso-error-page

https://user-images.githubusercontent.com/3750414/230430631-f7c9f7fc-65d6-4f5d-ad34-ea6dd610d306.mov

To Reproduce Steps to reproduce the behavior:

  1. Go to "osf.io": https://osf.io/
  2. Click on "Sign in" which will redirect you our authentication server "accounts.osf.io": https://accounts.osf.io/login?service=https://osf.io/login/?next=https%253A%252F%252Fosf.io%252F
  3. Click "Sign in via ORCiD"
  4. Follow the steps and you will see the error.

Note: If this is your first time login into OSF with ORCiD, you will go through a different process that ask you to enter an email and your full name. You will then receive a confirmation email to confirm account creation with ORCiD. After confirmation, log out of OSF to reproduce the error.

Expected behavior /authorize should return a 302 to client's redirect URI; or if it returns 200, the script on that page should make only one call the the client's Redirect URI; or if it needs to make another request after some timeout, currently 4 seconds is way too short.

Screenshots

See Description

Desktop (please complete the following information):

This is not browser/os/version specific. It happenes on safari, chrome and firefox, before and after update. I personally tested it on macOS 12 and 10 with latest Chrome, Safari and Firefox.

Smartphone (please complete the following information):

Not tested

Additional context

kadet1090 commented 1 year ago

We also experience same issue with our Bridge of Knowledge platform.

image

In the logs of the platform we can see such entries:

OAuth error: "Reused authorization code: <code>"

And in the browser devtools we can see same situation as mentioned in this issue: image

The issue seems to be the same, and we also experience it starting with April 3rd: image

I've created simple PR that should solve this issue by simply introducing flag that would prevent double call of the sendUserToRedirectURL. This should however be temporary solution because as noted in this issue 4s timeout is way to short for ensuring that redirect ends with GTM enabled and is already noticeable by the users.

leomendoza123 commented 1 year ago

thank you @cslzchen and @kadet1090 for all the details provided to resolve this issue. We now have PR to solve this issue.

amontenegro commented 1 year ago

Thank a lot @cslzchen and @kadet1090 for reporting the issue and all your help!

We are going to push this fix to our QA environment to review it on Monday, then, if we don't find any other issues we will deploy the fix to orcid.org early on Tuesday.

Thanks @leomendoza123 for the quick fix.

amontenegro commented 1 year ago

Hi @cslzchen and @kadet1090

Thanks again for reporting this, we just released the fix and I can't reproduce the issue anymore, could you please verify and confirm?

Thanks a lot!

amontenegro commented 1 year ago

Hey @cslzchen,

Sorry, I forgot to answer your question, we use the development branch for the QA process, then, when we are ready to go live we merge the development branch into the main branch, so, main contains the code that is currently live.

Thanks

cslzchen commented 1 year ago

@leomendoza123 and @amontenegro , fix confirmed for our OSF app (5/5 successful SSO), thanks a bunch!

kadet1090 commented 1 year ago

Unfortunately I cannot confirm that this fixed the issue - I still can see "Reused authorization code" entries in our logs, the last entry is from 2023-04-11T19:23:35.073Z. As of me (and in fact other team members), I cannot reproduce the issue, so perhaps this is some caching issue on our users side. Tomorrow I'll try to check if the log number is lower and if there is any issue on our side.

image image

cslzchen commented 1 year ago

@leomendoza123 and @amontenegro

Cc @kadet1090

I didn't verify Safari last time. The same error and behavior still happens on Safari (reported by our users). Chrome, Firefox and Edge work fine (on macOS and on windows).

In addition, Safari, Chrome and Firefox on IOS/iPhones still fail too. (Verified by our employees and by our QA via browser stack). No issue at all with multiple browsers on android phones.

I think it might be the script not working as expected on Safari (or Safari-ish browsers such as IOS version of Chrome and Firefox).

Should I report a new issue?

cslzchen commented 1 year ago

Safari's developer tool is very confusing, and I am not seeing any call to redirected URI there from the network logs. Attaching the screenshots but they are not very helpful.

However, we are confident this is still the 4 seconds time-out issue. When we use a feature in our auth server that intercepts the redirections to land on a 200 page much faster, (such as enabling OSF's two-factor), it almost never fails.

Screen Shot 2023-04-14 at 11 04 24 AM Screen Shot 2023-04-14 at 11 03 42 AM

amontenegro commented 1 year ago

Thanks for reporting this @cslzchen. @leomendoza123 could you please take a look?

leomendoza123 commented 1 year ago

@cslzchen We have resolve this issue on our Sandbox environment, and this should be on production next week. Thanks for reporting this issue, and please keep us updating if you found anything else.

amontenegro commented 1 year ago

Hi @cslzchen,

This should be fixed in orcid.org now, could you please take a look?

Thanks

kadet1090 commented 1 year ago

Now it fails for users who have the GTM blockers enabled, but it hangs on sign in screen not on redirect. After refreshing the sign in page it works just fine. Any login after that (which does not require signing in because we are already authorized) works fine as well. I took screenshot from devtools which may be some help: image

As for the logs I don't see any "Reused authorization code" after the fix went live - so this seems to be fixed.

cslzchen commented 1 year ago

@amontenegro Yes, SSO works as expected. Verified in Safari on all three OS (macOS, iPadOS, iOS). Thanks for the quick fix again.

leomendoza123 commented 1 year ago

@kadet1090 thank you for the report! Firefox (with add blockers) and Brave still had some issues. This should be fix with our last PR on the next sprint.