OHDSI / WebAPI

OHDSI WebAPI contains all OHDSI services that can be called from OHDSI applications
Apache License 2.0
130 stars 169 forks source link

Call to session.stop() with side-effect in UpdateAccessTokenFilter.java #2385

Open pieterlukasse opened 3 months ago

pieterlukasse commented 3 months ago

While reviewing the code in UpdateAccessTokenFilter.java I stumbled upon the following lines

https://github.com/OHDSI/WebAPI/blob/093e1f197222fdd69aa56247b4f7c225257a69ed/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java#L118-L122

which struck me as odd. In fact, session.stop() is called only twice in the whole WebAPI code base, both times in this UpdateAccessTokenFilter class. Given UpdateAccessTokenFilter's place in the grand scheme of filters configured, the above lines seem to always be called upon login, and result in an immediate end of the session that actually just started... The rest of the user interactions seem to continue based on token authentication alone.

Code blame shows that the code itself and the surrounding parts are many years old.

Questions:

ganisimov commented 2 months ago

@pieterlukasse , looks like the code you're referencing was added by me, but quite long ago and I'm far from this context now.

However, one thing to keep in mind is that there are number of auth methods available. My guess is that session is needed to handle OAuth and we don't need it with token.

Tagging @chrisknoll for awareness.

pieterlukasse commented 2 months ago

@ganisimov thanks. So are you saying that session is essential for the OAuth part, but then, after that has succeeded and a token has been issued, all other interactions do not rely on or use the session in any way? It does seem to be what is happening now...