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

Use a GatewayFilter to redirect to the login page when given a login query parameter #133

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

groldan
Copy link
Member

@groldan groldan commented Jul 13, 2024

The geOrchestra Gateway has a requirement to redirect to /login when a request has a login query parameter (e.g. /geonetwork/?login -> /login), that comes from the old security proxy.

So far it was implemented in AccessRulesCustomizer by requiring such requests to be authenticated, and delegating to spring security to perform the redirection.

When the request is not authenticated already, and the request headers do not contain Accept: text/html, spring security would challenge with a basic auth popup instead.

This patch introduces a LoginParamRedirectGatewayFilter, set up as one of the default filters in application.yml, that performs a redirect in either case, delegating to a
org.springframework.cloud.gateway.filter.factory.RedirectToGatewayFilterFactory.

Check out docs/custom_filters.adoc for configuration information.


Note for the docker composition, the following datadir PR is required:
georchestra/datadir#415
The rationale is in the PR description


Fixes #132

…query parameter

The geOrchestra Gateway has a requirement to redirect to /login when
a request has a `login` query parameter (e.g. `/geonetwork/?login` ->
`/login`), that comes from the old security proxy.

So far it was implemented in `AccessRulesCustomizer` by requiring such
requests to be authenticated, and delegating to spring security to
perform the redirection.

When the request is not authenticated already, and the request headers
do not contain `Accept: text/html`, spring security would challenge with a
basic auth popup instead.

This patch introduces a `LoginParamRedirectGatewayFilter`, set up as one
of the default filters in `application.yml`, that performs a redirect in
either case, delegating to a
`org.springframework.cloud.gateway.filter.factory.RedirectToGatewayFilterFactory`.

Check out `docs/custom_filters.adoc` for configuration information.
groldan added a commit to georchestra/datadir that referenced this pull request Jul 13, 2024
…query parameter

Configuration required for this gateway change:
georchestra/georchestra-gateway#133

This patch adds the `LoginParamRedirect` default filter to the gateway.

Note the filter is added by default in the embedded `application.yml`,
but since `gateway/application.yaml` is overriding the
`spring.cloud.gateway.default-filters` list, it must be added here too.
@groldan groldan requested a review from edevosc2c July 13, 2024 23:41
Copy link
Member

@edevosc2c edevosc2c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @groldan.
I just deployed this PR in dev at https://geodata.dev.inrae.fr and this works great!
I'm now using the all default-filters and the ?login redirect behavior is now the same whatever the "accept" header.
For me it's good to be merged. I didn't review the code, just validated that it fixes the bug.

@groldan groldan merged commit 209861d into georchestra:main Jul 26, 2024
3 checks passed
@groldan groldan deleted the bug/login_redirects branch July 26, 2024 12:22
github-actions bot pushed a commit to georchestra/datadir that referenced this pull request Jul 26, 2024
…query parameter

Configuration required for this gateway change:
georchestra/georchestra-gateway#133

This patch adds the `LoginParamRedirect` default filter to the gateway.

Note the filter is added by default in the embedded `application.yml`,
but since `gateway/application.yaml` is overriding the
`spring.cloud.gateway.default-filters` list, it must be added here too.

(cherry picked from commit 5ba6510)
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.

Redirect behavior differences on ?login when header-authentication (sec-*) is enabled and is not enabled
2 participants