search

OHDSI / Atlas

ATLAS is an open source software tool for researchers to conduct scientific analyses on standardized observational data
http://atlas-demo.ohdsi.org/
Apache License 2.0
266 stars 136 forks source link

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

Closed harry-anton closed 9 months ago

harry-anton commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

Could you please try the change suggested in this PR: https://github.com/OHDSI/Atlas/pull/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 commented 9 months ago

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 commented 9 months ago

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

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