-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Wrong session auth handler order #34174
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
Comments
Thank you for pointing out. #18452 is from an org's account. Due to GitHub's limit, it can't be edited by maintainers, so no maintainer could help to update or improve. Would you like to re-target that PR from a individual's account?
Could you elaborate or show an example? A related case in my mind is:
Does it make sense? |
Feel free to copy/fork this mod to your own PR and adjust to your needs.
Browser must not allow user B to reuse "gitea session x" created by user A. Session data should be accessible only to its owner (user A in case of "gitea session x") and if user B can access it - it's security hole and should be fixed there not in auth handler order. Session should be created after successful user auth with other methods (i.e. basic auth, reverse proxy auth, 2fa, etc.) and allow to avoid such costly user auth on every other request in this session so probably should be placed on top of auth handler list. Session should be automatically purged when not active for X hours to force user reauthentication (new session). |
TBH I don't need it, and if there are no enough interests and you don't need it, I might not spend time on it.
That's the problem. We can't control browser or SSO. In this step:
User A only logout from SSO site, but the browser still store "gitea session x" in its cookie. Unless the SSO provider could help to revoke the "gitea session x", the following visits to Gitea instance will still use the old gitea session. Yes, it is a security problem, need many methods to ensure correct. |
It's not web application job to teach user to use secure client software (SSO, OS, browser) and gitea should not waste user resources for quirks like this IHMO (LDAP auth flooding in this case).
Session cookie should be accessible only from users A account (regardless of if it's SSO or any other technology). If such cookie is accessible for other users also - consider posting an issue to such buggy SSO/OS/browser provider (to fix it and warn their users) and don't try to pollute web application code with workarounds. |
My understanding of the problem is: although the current approach (session handler last) is not efficient, at least the result is right. Because "reverse auth proxy" can't clear Gitea's session. Gitea's auth handler should be able to handle the account switch. But if you'd like to change the session handler's order, then it would become the wrong result I described above. If we'd really like to improve it, at the moment the only feasible approach in my mind is: refactor the whole auth system, and make each auth handler could detect whether there is a account switch (session revoked), before the "verify" step. |
Why? Do you know any professional environment that allows one user to access session cookie of other user and does not see point in fixing it ASAP? |
BTW: consider using session cookies by default to destroy session cookie when user closes browser (must reauthenticate on next application use). Persistent cookies should be optin IMHO. |
Do you know any professional reverseproxy auth provider could clear Gitea's session cookie?
The problem is that some users just switch account without closing the current browser (that's what I meant "We can't control browser or SSO") |
That's interesting. Any example of real environment that allows changing reverse auth login without closing browser? |
But there is no standard/RFC/spec saying that "switching account must close browser". Actually end users don't care closing or not, they only follow their intuition: we don't close browser when switching GitHub account, gmail account, facebook account, AWS account, and even back account, etc. So "requiring end users to close browser to re-login Gitea" is quite counter-intuitive and seems more likely a bug. |
My understanding is that reverse proxy auth is used in professional environments (i.e with kerberos or client certs) when users do not leave their devices unattended (i.e. unlocked, logged in).
If you see risk of stealing session cookie such improvement makes sense. What about removing
from |
Agree, I think that's more or less similar to my proposal (#34174 (comment)):
|
Uh oh!
There was an error while loading. Please reload this page.
Description
According to #18452 (comment) session auth handler should probably be called before all other auth handlers (i.e. basic auth, reverse proxy, etc.) to avoid costly auth on every web request (i.e. LDAP flooding when used together with reverse proxy auth).
Seems root of the #27821 problem is not auth handler order; session should be simply destroyed on user logout (or page close in case of SSO scenarios) to disallow reusing same session by another user.
Gitea Version
1.23+
Can you reproduce the bug on the Gitea demo site?
No
Log Gist
No response
Screenshots
No response
Git Version
No response
Operating System
No response
How are you running Gitea?
compiled from source
Database
None
The text was updated successfully, but these errors were encountered: