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

Atlas 2.14.0 - Characterization - /user/refresh with 404 when user authentication not enabled #2899

Closed
harry-anton opened this issue Dec 13, 2023 · 4 comments · Fixed by #2900

Comments

@harry-anton
Copy link

Expected behavior

In 2.14.0 version of Atlas, can navigate to Characterizations section and browse any of existing characterization from the list displayed. Shall display selected characterization information.

(Likewise, creating New Characterization is problematic in the same way.)

Actual behavior

Pop-up dialog with message Error! Please see server logs for details. and blank page appears.

The culprit seems to be access attempt to /user/refresh endpoint when user authentication for the whole setup is not enabled.

Firefox console:

user-refresh-404-firefox-console

Tomcat access logs, IP redacted:

...
192.168.XX.YYY - - [13/Dec/2023:12:30:56 +0000] "GET /WebAPI/cohort-characterization/52/design HTTP/1.1" 200 1146
192.168.XX.YYY - - [13/Dec/2023:12:30:56 +0000] "GET /WebAPI/user/refresh HTTP/1.1" 404 5
192.168.XX.YYY - - [13/Dec/2023:12:31:18 +0000] "GET /WebAPI/notifications?hide_statuses= HTTP/1.1" 200 625
...

Steps to reproduce behavior

In 2.14.0 version of Atlas, navigate to Characterizations section and try to browse any of existing characterization from the list displayed. Problematic behaviour discovered by Marek Oja.

Possible fix

We used to fix a line in js/services/AuthAPI.js where instead

        if (!isPromisePending(refreshTokenPromise)) {

we use

        if (!isPromisePending(refreshTokenPromise) && (config.userAuthenticationEnabled)) {

Works fine for us, for now.

Related

user-refresh-refreshToken-in-loadCharacterizationDesign

@chrisknoll
Copy link
Collaborator

Yes, that works, we can post a fix. I think, tho, that we should just a top-level check for auth enabled, and return a resolved promise instead of putting it into the same if as the promise pending check.

@chrisknoll
Copy link
Collaborator

Could you please try the change suggested in this PR: #2900

I've added the enable check in refreshToken() since it makes sense to do it there, and I have removed some redundant checks elsewhere.

@harry-anton
Copy link
Author

Tested suggested changes, those worked fine:

192.168.XX.YYY - - [14/Dec/2023:06:58:34 +0000] "GET /WebAPI/cohort-characterization?size=10000 HTTP/1.1" 200 1599
192.168.XX.YYY - - [14/Dec/2023:06:58:50 +0000] "GET /WebAPI/cohort-characterization/56/design HTTP/1.1" 200 369
192.168.XX.YYY - - [14/Dec/2023:06:58:50 +0000] "POST /WebAPI/cohort-characterization/check HTTP/1.1" 200 35
192.168.XX.YYY - - [14/Dec/2023:06:59:10 +0000] "OPTIONS /WebAPI/cohort-characterization/56 HTTP/1.1" 200 5
192.168.XX.YYY - - [14/Dec/2023:06:59:11 +0000] "DELETE /WebAPI/cohort-characterization/56 HTTP/1.1" 204 -
192.168.XX.YYY - - [14/Dec/2023:06:59:11 +0000] "GET /WebAPI/cohort-characterization?size=10000 HTTP/1.1" 200 1567
192.168.XX.YYY - - [14/Dec/2023:06:59:20 +0000] "GET /WebAPI/notifications?hide_statuses= HTTP/1.1" 200 628
192.168.XX.YYY - - [14/Dec/2023:06:59:22 +0000] "POST /WebAPI/cohort-characterization/check HTTP/1.1" 200 129
192.168.XX.YYY - - [14/Dec/2023:06:59:24 +0000] "GET /WebAPI/cohortdefinition/ HTTP/1.1" 200 24949
192.168.XX.YYY - - [14/Dec/2023:06:59:29 +0000] "POST /WebAPI/cohort-characterization/check HTTP/1.1" 200 116
192.168.XX.YYY - - [14/Dec/2023:06:59:30 +0000] "GET /WebAPI/feature-analysis?size=100000 HTTP/1.1" 200 2486
192.168.XX.YYY - - [14/Dec/2023:06:59:35 +0000] "POST /WebAPI/cohort-characterization/check HTTP/1.1" 200 35
192.168.XX.YYY - - [14/Dec/2023:06:59:48 +0000] "POST /WebAPI/cohort-characterization/check HTTP/1.1" 200 35
192.168.XX.YYY - - [14/Dec/2023:06:59:51 +0000] "OPTIONS /WebAPI/cohort-characterization/0/exists?name=harry-testib-4 HTTP/1.1" 200 5
192.168.XX.YYY - - [14/Dec/2023:06:59:51 +0000] "GET /WebAPI/cohort-characterization/0/exists?name=harry-testib-4 HTTP/1.1" 200 21
192.168.XX.YYY - - [14/Dec/2023:06:59:51 +0000] "POST /WebAPI/cohort-characterization HTTP/1.1" 200 371
192.168.XX.YYY - - [14/Dec/2023:06:59:51 +0000] "OPTIONS /WebAPI/cohort-characterization/57/design HTTP/1.1" 200 5
192.168.XX.YYY - - [14/Dec/2023:06:59:51 +0000] "POST /WebAPI/cohort-characterization/check HTTP/1.1" 200 35
192.168.XX.YYY - - [14/Dec/2023:06:59:51 +0000] "GET /WebAPI/cohort-characterization/57/design HTTP/1.1" 200 371
192.168.XX.YYY - - [14/Dec/2023:06:59:51 +0000] "POST /WebAPI/cohort-characterization/check HTTP/1.1" 200 35
192.168.XX.YYY - - [14/Dec/2023:07:00:22 +0000] "GET /WebAPI/notifications?hide_statuses= HTTP/1.1" 200 628

Also, noticed typo in comments: // no-op if userAthenticationEnabmed == false.

@chrisknoll
Copy link
Collaborator

Thanks, I'll correct and merge it into a hotfix.

Thanks very much for taking the time to report and test.

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

Successfully merging a pull request may close this issue.

2 participants