Exception mapper breaks OIDC authentication flow

[kucharzyk]

Describe the bug

I have application that is using OIDC only for authentication, for rest of the time I am using JWT token.
I would like to create ExceptionMapper that redirect unauthenticated users to login page.

@Provider
@Priority(Priorities.AUTHENTICATION)
public class UnauthorizedExceptionMapper implements ExceptionMapper<UnauthorizedException> {

    @Context
    UriInfo uriInfo;

    @Override
    public Response toResponse(UnauthorizedException e) {
        Log.warn("Unauthorized request: " + uriInfo.getRequestUri());
        return Response.seeOther(URI.create("/auth/github")).build();
    }
}

My flow for unuthenticated user looks like this:

/secured/page is only for authenticated -> throws UnauthorizedException -> exception mapper redirects to /auth/github

For /auth/github there is configured OIDC tenant and this endpoint has to be annotated as @authenticated to initiate OIDC flow. It should redirect me to github login page but it doesn't because its also throwing UnauthorizedException which is caught by the same exception mapper.

Expected behavior

Exception mapper catching UnauthorizedException shouldn't break OIDC flow.

At least there should be a way to fix it using different priority.

Actual behavior

Exception mapper catching UnauthorizedException breaks OIDC flow.

How to Reproduce?

No response

Output of uname -a or ver

Linux tardis 6.1.8-200.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Jan 24 20:32:16 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "17.0.6" 2023-01-17 OpenJDK Runtime Environment (Red_Hat-17.0.6.0.10-1.fc37) (build 17.0.6+10) OpenJDK 64-Bit Server VM (Red_Hat-17.0.6.0.10-1.fc37) (build 17.0.6+10, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.0.0.Alpha3

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)

Additional information

No response

[quarkus-bot]

/cc @pedroigor (oidc), @sberyozkin (oidc)

[sberyozkin]

@kucharzyk Do you have proactive authentication disabled ?

[kucharzyk]

@sberyozkin No. Proactive authentication is enabled in my case.

[sberyozkin]

@kucharzyk OIDC will throw AuthenticationFailedException, looks like a case of the double, same exception mapping.
Can you throw CustomUnauthorizedException and see if it makes a difference?

[kucharzyk]

I can’t throw different exception.

Exceptions are thrown by Quarkus when I am calling endpoint annotated as @authenticated. The problem is that handling this exception in application breaks OIDC flow.

[sberyozkin]

@kucharzyk In the description you say that the exception is thrown first when a secure page is accessed but you also said that @Authenticated is on the other path which is meant to initiate an OIDC code flow.
Perhaps, it would be simpler if you could create a reproducer

[kucharzyk]

@sberyozkin I might not be clear enough. It will be easier to explain with reproducer.

https://github.com/kucharzyk/reproducer-30893

So just to summarize:

1. I am using both OIDC and JWT security but OIDC is enabled only for /auth/github/** endpoints

https://github.com/kucharzyk/reproducer-30893/blob/main/src/main/resources/application.properties
https://github.com/kucharzyk/reproducer-30893/blob/main/src/main/java/io/quarkus/reporoducer30893/auth/oidc/OidcTenantResolver.java

2. /auth/github is starting OIDC flow, authenticating and creating JWT token

https://github.com/kucharzyk/reproducer-30893/blob/main/src/main/java/io/quarkus/reporoducer30893/auth/oidc/GithubAuthResource.java

3. There is also secured page which is returning 401 for authenticated users (if JWT token is not present)

https://github.com/kucharzyk/reproducer-30893/blob/main/src/main/java/io/quarkus/reporoducer30893/pages/Secure.java

4. I would like to handle this ugly 401 page so I've created exception mapper

https://github.com/kucharzyk/reproducer-30893/blob/main/src/main/java/io/quarkus/reporoducer30893/exception/handlers/UnauthorizedExceptionMapper.java

5. The problem is if you uncomment exception handler OIDC won't be working anymore.
   Both /secure and /auth/github endpoints are throwing UnauthorizedException under the hood.

So creating such exception handler prevent OIDC from redirecting me to github to authenticate.

I hope it will be clear now.

[sberyozkin]

Ok, makes sense now. Right, IMHO it is not a real Quarkus issue, your exception mapper is interfering it is a side effect of us allowing the security exceptions be caught by exception mappers.
Both of these endpoints require protection so your mapper is just causing recursion.
IMHO a different approach should be taken.

I think what can work better is to have pro active auth disabled, and then check SecIdentity manually as suggested in the proactive auth docs and throw the custom exception there if no jwt aware identity is available, that will break the loop.
Another approach is may be to have a pre match jaxrs filter which will reset the req uri if the current request path is secured and no jwt is present

[kucharzyk]

Thanks for you answer. Actually I was thoping it could be fixed using different priorities for exception mappers. Something that could allow OIDC to take precedence.

Another solution could be initiating OIDC flow manually straight from exception mapper.

I could add condition in exception mapper - if oidc tenant is set for this path start OIDC flow (redirect to oidc provider).
Do we have API that could easily start OIDC flow from exception mapper?

[kucharzyk]

@Provider
@Priority(Priorities.AUTHENTICATION)
public class UnauthorizedExceptionMapper implements ExceptionMapper<UnauthorizedException> {

    @Context
    UriInfo uriInfo;

    @Inject
    RoutingContext routingContext;

    @Inject
    TenantResolver tenantResolver;

    @Override
    public Response toResponse(UnauthorizedException e) {
        String requestUri = uriInfo.getRequestUri().toString();
        var tenant = tenantResolver.resolve(routingContext);
        if (tenant == null) {
            Log.warn("Unauthorized request: " + requestUri);
            return Response.seeOther(URI.create("/auth/github")).build();
        } else {
            return buildRedirectToOidcProvider()
        }
    }
}

Is there any easy way to implement buildRedirectToOidcProvider()?

[sberyozkin]

Not really, if the custom code can start affecting the way the sec system flows, i.e it has to follow from the http authentication challenge., then it would become a problem.
So check the custom http auth mechanism option , see the sec customization docs, where you have both jwt and oidc mechanisms injected and delegate accordingly.
Yet another option is to configure path based auth for jwt only and have it configured to redirect to login page from its challenge method, but PR is still open to allow for it, will try to get it merged

[geoand]

@sberyozkin I don't know much about OIDC, but this does sound like something useful. Would it not be possible for us to provide some kind of API to make the use case easier?

[sberyozkin]

We'll have the options, the security system should be flexible enough to support such cases. To correctly deal with the oidc challenge in the custom code one would need to inject routing context, tenant specific config etc. It might be feasible but I hope we can sort it out by making the sec system support it, without the custom code affecting the way the sec system functions

[sberyozkin]

Was mostly on pto today

[sberyozkin]

Well the easy way to create a redirect uri here would be to have OidcAuthenticationMechanism injected and call its get challenge method and get the redirect uri from the returned challenge data. That should do it.
But the PR I have in mind should make having the exception mapper unnecessary, I hope

[kucharzyk]

@sberyozkin @geoand

Actually I've found quite easy solution to handle my usecase:

package io.quarkus.reporoducer30893.exception.handlers;

import io.quarkus.logging.Log;
import io.quarkus.oidc.TenantResolver;
import io.quarkus.security.UnauthorizedException;
import io.quarkus.vertx.http.runtime.security.ChallengeData;
import io.quarkus.vertx.http.runtime.security.HttpAuthenticator;
import io.smallrye.mutiny.Uni;
import io.vertx.ext.web.RoutingContext;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import jakarta.ws.rs.core.Response;
import org.jboss.resteasy.reactive.server.ServerExceptionMapper;

import java.net.URI;

import static org.jboss.resteasy.reactive.server.exceptionmappers.AsyncExceptionMappingUtil.DEFAULT_UNAUTHORIZED_RESPONSE;

@ApplicationScoped
public class UnauthorizedExceptionMapper {

    @Inject
    TenantResolver tenantResolver;

    @ServerExceptionMapper(value = UnauthorizedException.class)
    public Uni<Response> handle(RoutingContext routingContext) {
        var tenant = tenantResolver.resolve(routingContext);
        if (tenant == null) {
            Log.warn("Unauthorized request: " + routingContext.normalizedPath());
            return Uni.createFrom().item(Response.seeOther(URI.create("/auth/github")).build());
        } else {
            return handleWithAuthenticator(routingContext);
        }
    }

    private Uni<Response> handleWithAuthenticator(RoutingContext routingContext) {
        HttpAuthenticator authenticator = routingContext.get(HttpAuthenticator.class.getName());
        if (authenticator != null) {
            Uni<ChallengeData> challenge = authenticator.getChallenge(routingContext);
            return challenge.map(challengeData -> {
                if (challengeData == null) {
                    return DEFAULT_UNAUTHORIZED_RESPONSE;
                }
                Response.ResponseBuilder status = Response.status(challengeData.status);
                if (challengeData.headerName != null) {
                    status.header(challengeData.headerName.toString(), challengeData.headerContent);
                }
                return status.build();
            }).onFailure().recoverWithItem(DEFAULT_UNAUTHORIZED_RESPONSE);
        } else {
            return Uni.createFrom().item(DEFAULT_UNAUTHORIZED_RESPONSE);
        }
    }

}

Basically I need to handle UnauthorizedException how I want but if I am on OIDC url (when tenant is set in my case) I just need to call SecurityExceptionMapperUtil.handleWithAuthenticator(routingContext); to start authentication.

It would be easier if this method wasn't package private so everybody can use it.

@sberyozkin What do you think about mentioning this in guide and exposing this method publicly?

[sberyozkin]

Hey @kucharzyk Good stuff, so this is exactly what I proposed with my last comment as well, but instead of using this internal method, you'd have

@Inject
OidcAuthenticationMechanism oidc;

and then call its getChallenge, can you try please ? It would be better as making those internal utility code doc-ed can become restrictive for Quarkus (in case it will have to be changed)

FYI, I'm going to rebase #28505 and ask you to check it as well, that should help as well, the idea you'd not have to type any code at all to direct to the required authenticator

[sberyozkin]

@kucharzyk Just an off-topic comment, you don't need to do

quarkus.oidc.tenant-enabled=false
quarkus.oidc.github.provider=github
...

unless you have more than one non-default tenant, so simply having

quarkus.oidc.provider=github
...

will do, also, with this provider there's no need to set application-type=web-app and user-info-required=true

[kucharzyk]

@sberyozkin

It's working as well with OidcAuthenticationMechanism:

package com.teaminternational.exception.handlers;

import io.quarkus.logging.Log;
import io.quarkus.oidc.TenantResolver;
import io.quarkus.oidc.runtime.OidcAuthenticationMechanism;
import io.quarkus.security.UnauthorizedException;
import io.quarkus.vertx.http.runtime.security.ChallengeData;
import io.quarkus.vertx.http.runtime.security.HttpAuthenticator;
import io.smallrye.mutiny.Uni;
import io.vertx.ext.web.RoutingContext;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import jakarta.ws.rs.core.Response;
import org.jboss.resteasy.reactive.server.ServerExceptionMapper;

import java.net.URI;

import static org.jboss.resteasy.reactive.server.exceptionmappers.AsyncExceptionMappingUtil.DEFAULT_UNAUTHORIZED_RESPONSE;

@ApplicationScoped
public class UnauthorizedExceptionMapper {

    @Inject
    OidcAuthenticationMechanism oidcAuthenticationMechanism;

    @Inject
    TenantResolver tenantResolver;

    @ServerExceptionMapper(value = UnauthorizedException.class)
    public Uni<Response> handle(RoutingContext routingContext) {
        var tenant = tenantResolver.resolve(routingContext);
        if (tenant == null) {
            Log.warn("Unauthorized request: " + routingContext.normalizedPath());
            return Uni.createFrom().item(Response.seeOther(URI.create("/auth/github")).build());
        } else {
            return handleWithAuthenticator(routingContext);
        }
    }

    private Uni<Response> handleWithAuthenticator(RoutingContext routingContext) {
        Uni<ChallengeData> challenge = oidcAuthenticationMechanism.getChallenge(routingContext);
        return challenge.map(challengeData -> {
            if (challengeData == null) {
                return DEFAULT_UNAUTHORIZED_RESPONSE;
            }
            Response.ResponseBuilder status = Response.status(challengeData.status);
            if (challengeData.headerName != null) {
                status.header(challengeData.headerName.toString(), challengeData.headerContent);
            }
            return status.build();
        }).onFailure().recoverWithItem(DEFAULT_UNAUTHORIZED_RESPONSE);
    }

}

Regarding quarkus.oidc.tenant-enabled I am using this for purpose.
I am using tenant resolver to have OIDC enabled only for selected paths.
This way rest of the application have OIDC disabled and I can use JWT only.

[sberyozkin]

Thanks @kucharzyk, so looks like the exception mapper is working as expected now. I'll keep this issue open though, as I believe registering this mapper will not be necessary once #28505 is merged, please keep the mapper if you prefer but I'd appreciate if you could check if #28505 can help, if Quarkus security system could handle such cases without requiring the custom orchestration then it would be good

[sberyozkin]

Hi @kucharzyk I've been looking today at this issue again, I think in your case it will work without the exception mapper and #28505.
integration-tests/smallrye-jwt-oidc-webapp has a nearly identical setup.
The reason it should work without the mapper is that OIDC is higher priority than JWT and if the user is not authenticated OIDC will send a challenge, there is no need to do a specific redirect from the mapper, OIDC provider will redirect to it as you have the github/success page configured.
However the mapper can be useful if you need OIDC to take over if the existing JWT token verification fails, in this case the mechanism which failed to verify will report a challenge.
I'm going to close this issue anyway as you have a solution, thanks