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

Preauthentication via headers filters - Clear security context if preauth headers changed across queries #34

Merged

Conversation

pmauduit
Copy link
Member

@pmauduit pmauduit commented Dec 14, 2023

This is a change needed specifically when using the georchestra gateway. See georchestra/georchestra-gateway#14 for the context.

I suspect that we don't encounter the same issue when running behind the security-proxy because in the case of the SP, the JSESSIONID are kept server-side and managed by the SP, and the security context geoserver-side is reloaded at each request depending on its value. When behind the gateway, the cookies are sent to the browser, so once we obtain a admin session, we keep it as long as we have the cookie.

In any way, clearing the context in both following cases:

  • no header are sent anymore (the user disconnected from the gw)
  • a preauthentication header containing the username which does not correspond to the Principal being stored in the security context
    should address the issue, but I am not a security expert.

Thanks @groldan for the hints ; a similar mechanism already exists on geoserver-cloud here and was mainly the inspiration for this one:
https://github.com/geoserver/geoserver-cloud/blob/main/src/starters/security/src/main/java/org/geoserver/cloud/security/gateway/GatewayPreAuthenticationFilter.java#L42

Tests:

  • UT added to test these particular cases
  • Also fixed another unrelated test (see 2nd commit)

@fvanderbiest
Copy link
Member

Thanks Pierre.
Is it relevant to contribute this patch upstream ?

@pmauduit
Copy link
Member Author

discussed it yesterday with @groldan, yes

…ing logout

.. or if no preauth headers are provided anymore.

See https://osgeo-org.atlassian.net/browse/GEOS-11235 for more details.

Without this change, we can end up with a persisting session across
logout.

Tests: adding utests specific to this change. Runtime tested into
a docker composition.
@pmauduit pmauduit force-pushed the clear-security-context-if-preauth-headers-changed branch from dad8628 to 6d4f7f3 Compare January 8, 2024 10:45
@pmauduit
Copy link
Member Author

pmauduit commented Jan 8, 2024

force-pushed to get the same commit as the one which has been merged upstream

@pmauduit pmauduit merged commit 96e1eae into 2.22.2-georchestra Jan 11, 2024
2 of 6 checks passed
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 this pull request may close these issues.

2 participants