quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

OIDC redirect-path property #17563

Closed fouad-j closed 3 years ago

fouad-j commented 3 years ago

Description

We faced an issue with quarkus.oidc.tenantName.authentication.redirect-path=http://mydomain.com/tenantName

Current behavior of redirect-path is to append value set in application properties to request URL from context

For example for quarkus.oidc.tenantName.authentication.redirect-path=http://mydomain.com/tenantName

The redirect url would be http://requestContextUrl.com/http://mydomain.com/tenantName

Implementation ideas

I have a proposal to fix this issue https://github.com/quarkusio/quarkus/pull/17562

If the redirect-path start with http or https we take it as it is

quarkus-bot[bot] commented 3 years ago

/cc @pedroigor, @sberyozkin

sberyozkin commented 3 years ago

Hi @fouad-j Can you explain please why do you need to use an absolute redirect path ? If a user needs to access a given Quarkus endpoint then why would the user be redirected, after the authentication, to the endpoint in some other domain ? I appreciate it may be necessary but I'd like to understand why. Do you also set a cookie-domain property for the redirect to the other domain endpoint to complete successfully (for the state cookie be available, etc) ?

AJ1062910 commented 3 years ago

Hi @sberyozkin , thank you for your answer, I work with @fouad-j,

Our case is that after login to the provider (let's say : tenant-a) quarkus redirect (the app to the redirect-path) as we configured in application.properties with : quarkus.oidc.tenantName.authentication.redirect-path=/token. (let's say /token endpoint as redirect path) And the /token endpoint is a quarkus backend endpoint (here we can inject Userinfo, JsonWebToken ...)

Problem is, in our case, quarkus redirect to our: KUBERNETES_INGRESS_HOST/token, which is not our "backend" Url domain, but the DNS of our front end part. So we wanted to just change the "KUBERNETES_INGRESS_HOST" part to our Backend URL (let's say : https://project.com) and the redirect path registered in the tenant-a provider is : https://project.com/token.

So we wanted just to change the redirect-path property : quarkus.oidc.tenantName.authentication.redirect-path=https://project.com/token, but it doesn't work like that because quarkus take the Context requested URL : it will give : http://requestedURLhttps://project.com/token, as redirect path.

That is why we want to have the choice of an absolute path for the redirect-path , we would like that the configuration give : https://project.com/token and not KUBERNETES_INGRESS_HOST/token

So the problem is not that we want to redirect in some other domain but more because quarkus redirect to the bad redirect path, (in our case at least: redirect context is KUBERNETES_INGRESS_HOST instead of https://project.com which is our url for our backend services) I do not know if I was understandable. thanks

sberyozkin commented 3 years ago

@AJ1032480 Hi, thanks, I believe we've had similar problems reported before. It is a technical problem which for example in case of Keycloak should be resolvable as documented here: https://github.com/quarkusio/quarkus/blob/main/docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc#external-and-internal-access-to-openid-connect-provider

Can that help ?

AJ1062910 commented 3 years ago

It is interresting to see that keykloak have a KEYCLOAK_FRONTEND_URL mode, in our case, we do not use keycloak, I do not think the providers we used have a similar property to set, Without that, how can we set the full url of the redirect path ? (on quarkus)

fouad-j commented 3 years ago

Hello @sberyozkin,

Thanks for your reactivity.

I drew a diagram to explain the flow

image

  1. User call https://company.com/authentication?tenantId=tenant1 to get authenticated

  2. Kubernetes Ingress forwards the request to the right service in our cluster

  3. When the user is authenticated, the service redirect to http://identityprovider.com?state=111&scopes=ss&**redirect_uri=http://auth-svc.cluster.local/callback**

http://auth-svc.cluster.local is accessible only in the cluster.

sberyozkin commented 3 years ago

@fouad-j thanks, have you tried your patch in your deployment ? What happens when, after the redirect, Quarkus attempts to exchange the authorization code for the tokens ?

sberyozkin commented 3 years ago

@AJ1032480 @fouad-j Here is another link for you:

https://quarkus.io/guides/vertx#reverse-proxy

Can you try:

quarkus.http.proxy.enable-forwarded-prefix=true
quarkus.http.proxy.allow-forwarded=true

Can you please check what X-Forwarded heades the Kubernetes Ingress proxy sets before forwarding to the internal endpoint if the above does not work ?

I'm quite sure we've had such an issue before, can't find the Zulip thread.

We'll make sure it works for you - but if possible I'd like to avoid tweaking the code to workaround various networking setup issues

AJ1039593 commented 3 years ago

Hello @sberyozkin,

Thanks a lot for last message, it was very helpful.

Based on X-Forwarded and X-Original headers, we did a bad workaround

Workaround

application.properties

quarkus.http.proxy.proxy-address-forwarding=true
quarkus.http.proxy.allow-forwarded=false
quarkus.http.proxy.enable-forwarded-host=true

quarkus.http.proxy.allow-forwarded=false should be false otherwise X-Forwaded are not taken ForwardedParser.java#L135

CustomResolver

@ApplicationScoped
public class CustomTenantResolver implements TenantResolver {

    @Context
    HttpServerRequest request;

    @Override
    public String resolve(RoutingContext context) {
        String originalHost = context.request().headers().get("X-ORIGINAL-HOST");
        context.request().headers().set("X-Forwarded-Host", url);
        context.request().headers().set("X-Forwarded-For", url);

        ....
    }
}

We override X-Forwarded headers by X-Original values because ForwardedParser.java doesn't manage X-Original headers.

Here are headers we receive from ingress

X-Forwarded-Host: kubedns.cluster.local:80
X-Forwarded-Port: 443
X-Forwarded-Proto: https
X-Scheme: https
X-Original-Forwarded-For: 99.999.999.99:47111
X-Original-URL: /authentication?tenantId=tenant1
X-ORIGINAL-HOST: company.com

It is interresting to see that keykloak have a KEYCLOAK_FRONTEND_URL mode, in our case, we do not use keycloak, I do not think the providers we used have a similar property to set, Without that, how can we set the full url of the redirect path ? (on quarkus)

It works because the URL we defined refers to the public DNS of our authentication service

Questions

For workaround:

For Oidc extension

sberyozkin commented 3 years ago

@AJ1039593 Thanks for making it work, it is a good enough workaround IMHO :-)

Should we create a ticket to ask if it's possible to manage X-Original in ForwardedParser.java ?

Please do - I think we need to make it very easy for cases where the server is running behind Kubernetes Ingress and i can imagine it would not only be needed for quarkus-oidc.

Is our PR good for you ? or should we do it differently maybe we have to create a new properties instead of quarkus.oidc.tenantName.authentication.redirect-path

The redirect-path alone is enough - but can you confirm your PR is enough and all works when this PR is applied ?

sberyozkin commented 3 years ago

@AJ1039593 can you try adding:

quarkus.http.proxy.forwarded-host-header=X-ORIGINAL-HOST

See https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ForwardedParser.java#L146

That should really do it without any workaround

sberyozkin commented 3 years ago

I'll update the OIDC docs once it works - it should really be supported well at the Quarkus Vert.x level - for the users not having to be concerned about such details as the external URLs a given Quarkus endpoint can be accessed at (OIDC logout would be another redirect which would be affected, etc)

fouad-j commented 3 years ago

Hello @sberyozkin,

Sorry for delay, we managed to install OIDC MULTI-TENANCY in our microservices architecture and behind Kubenetes.

We used two identity providers

The configuration to make OIDC works behind proxy/reverse proxy like ingress

Solution 1 (easy)

Merge my PR or extends OidcAuthenticationMechanism and CodeAuthenticationMechanismto apply the fix if PR is reject.

then set you redirect_uri in application.properties

quarkus.oidc.tenantName.authentication.redirect-path=http://mydomain.com/tenantName

http://mydomain.com/tenantName refer to the public DNS of our authentication service in the cluster

Solution 2 (hard)

This solution is a little difficult because you need to know how (kubernetes, ingress, proxy/reverse proxy, headers...) work

Here is the configuration that you may adjust to your case (more details)

#mandatory config when quarkus is running behind a reverse proxy
quarkus.http.proxy.proxy-address-forwarding=true
quarkus.http.proxy.allow-forwarded=false
quarkus.http.proxy.enable-forwarded-host=true
quarkus.http.proxy.forwarded-host-header=X-ORIGINAL-HOST

@sberyozkin thanks a lot for your help, please let me know if it's not clear or if you need more details

sberyozkin commented 3 years ago

Hi @fouad-j thanks for the update, and glad to hear it works with configuring the forwarded header.

The hardest part here is finding out the name of X-ORIGINAL-HOST - I will document it and it will become easier - and it is likely documented somewhere in Kunerners networking guides.

Setting http://mydomain.com/tenantName does look easier - but it basically requires the Quarkus endpoint to be configured to point to its own absolute address - which is a workaround with its own side-effects - the configuration is tied to the external environment properties while we have users depending on supporting the forwarded headers - which appears to be a common approach to adapt between the various external and internal HTTP properties

sberyozkin commented 3 years ago

@pedroigor What is your opinion ? The problem is well descried above: when the OIDC provider (it is not Keycloak, so no KEYCLOAK_FRONTEND_URL) is redirecting back to the endpoint running behing Kubernetes Ingress the endpoint is not found - because when quarkus-oidc calculates the redirect_uri it uses a relative redirect-path which is added to the current request URI - which is an internal URI.

So one solution which already works is that the Quarkus forwarding filter is configured to recognize the external address (similar approach is already used for SSL terminating proxies, forwarded prefixes, etc) @fouad-j has also proposed to support absolute redirect-path values - but this effectively requires the Quarkus endpoint configuration to include the absolute address of this endpoint - which may appear easier but does not feel right at the same time - especially if we add a requirement to support for ex the forwarding prefixes - where the forwarding filter would have to be used anyway.

Hence I'm thinking of resolving this issue with a PR updating the docs with the info how to configure the forwarding filter in cases like this one.

sberyozkin commented 3 years ago

@AJ1039593 @AJ1032480 I've updated the docs in #17921 and it will close this issue, and I will close your PR. The time you've spent looking into this issue is appreciated and it helped to improve the docs which will help other users. We can revisit your PR if checking the X-Forwarded-* proves problematic in some cases thanks