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

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

Open
pieterlukasse opened this issue Aug 21, 2024 · 2 comments

Comments

@pieterlukasse
Copy link
Contributor

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

// stop session to make logout of OAuth users possible
Session session = SecurityUtils.getSubject().getSession(false);
if (session != null) {
session.stop();
}

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:

  • Would it be possible to find more details on why the session.stop() was added here?
  • Do we need sessions at all?
@ganisimov
Copy link
Contributor

@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
Copy link
Contributor Author

@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...

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

No branches or pull requests

2 participants