OIDC CodeAuthenticationMechanism does not work if the proactive authentication is disabled

[sberyozkin]

Describe the bug

Adding quarkus.http.auth.proactive=false breaks the OIDC adapter code flow (ex, adding this property to integration-tests/oidc-code-flow/..../application.properties breaks the tests)
Reported originally at https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/OIDC.20BearerAuthenticationMechanism.20for.20unproteced.20resources

[sberyozkin]

Hi Stuart, @stuartwdouglas, I will start investigating tomorrow, if you have some ideas what might be going wrong then let me know please

[sberyozkin]

I think it can be a simple fix, hopefully, which needs to be added somewhere around the code which deals with the disabled proactive authentication:

2020-09-14 18:15:22,066 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-1) HTTP Request to /web-app/callback-after-redirect?state=13d4ff40-f6b1-4f83-abad-e696113135dd&session_state=d4aa68b2-6a89-4073-8060-b2eedd3887c7&code=c3ff900f-f716-46d5-a7b3-e2d155ba7b1f.d4aa68b2-6a89-4073-8060-b2eedd3887c7.3d383035-f469-4344-9402-5fbda2b1f033 failed, error id: cf969c9e-cae8-4e92-ab17-944c0bde31a1-1: org.jboss.resteasy.spi.UnhandledException: io.quarkus.vertx.http.runtime.security.AuthenticationRedirectException

	at org.jboss.resteasy.core.ExceptionHandler.handleApplicationException(ExceptionHandler.java:106)
	at org.jboss.resteasy.core.ExceptionHandler.handleException(ExceptionHandler.java:372)
	at org.jboss.resteasy.core.SynchronousDispatcher.writeException(SynchronousDispatcher.java:216)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:515)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:259)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:160)
	at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364)
	at org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:163)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:245)
	at io.quarkus.resteasy.runtime.standalone.RequestDispatcher.service(RequestDispatcher.java:73)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch(VertxRequestHandler.java:131)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.access$000(VertxRequestHandler.java:37)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler$1.run(VertxRequestHandler.java:94)
	at org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35)
	at org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:2046)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1578)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1452)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
	at java.lang.Thread.run(Thread.java:748)
	at org.jboss.threads.JBossThread.run(JBossThread.java:479)
Caused by: io.quarkus.vertx.http.runtime.security.AuthenticationRedirectException
	at io.quarkus.oidc.runtime.CodeAuthenticationMechanism$4$1.accept(CodeAuthenticationMechanism.java:326)
...

It is handled in the proactive auth case here:
https://github.com/quarkusio/quarkus/blob/master/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java#L146

The same does not seem to work at
https://github.com/quarkusio/quarkus/blob/master/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java#L178

[stuartwdouglas]

You likely need to add a new RESTEasy exception mapper to handle this.

[haraldatbmw]

This issue occurs when using Quarkus OIDC this way:

1. I have one multi-tenant OIDC quarkus application
2. Tenant 1 uses OIDC type web-app to provide the OIDC Authorization Code Flow (tokens are stored in cookie)
3. Tenant 2 uses OIDC type service to protect some endpoints of the REST API (checks token in authorization http-header)
4. Some other endpoints of the REST API are not protected and should not fail if called with expired token
5. A custom Vertx RouteFilter is in place to translate the token inside the cookie (provided by tenant 1) to the authorization http-header to work for tenant 2

The flag quarkus.http.auth.proactive=false is needed for 4.

[sberyozkin]

@stuartwdouglas You are right, I'll need to create a PR to add AuthenticationRedirectException and AuthenticationCompletionException to the quarkus security (currently located in the vertx-http extension), see quarkusio/quarkus-security#10

[sberyozkin]

@haraldatbmw I have reproduced a 404 error. It only happens when a redirect_uri like /api/web/callback has no any matching JAX-RS resource method - with the proactive authentication being off it is the RestEasy runtime which takes an early control and this is why 404 is reported.

Ideally, you'd have a matching JAX-RS resource path, but if you'd like to have the original request URI be restored then adding the following will do:

quarkus.http.auth.permission.callback.paths=/api/web/callback
quarkus.http.auth.permission.callback.policy=authenticated

Note we still need to resolve this issue before it will work :-)

[sberyozkin]

@stuartwdouglas hi Stuart, I'm still seeing one remaining issue related to the RP-initiated logout, CodeFlowTest#testRPInitiatedLogout. So there we allow the users who are already authenticated issue a request like http://localhost:8081/tenant-logout/logout and if it matches quarkus.oidc.tenant-logout.logout.path=/tenant-logout/logout then CodeAuthenticationMechanism will initiate RP-initiated logout by redirecting the user to the OIDC logout endpoint with AuthenticationRedirectException, here. Note no JAX-RS endpoint with this path exists.

Because the user is already authenticated, I'm adding:

quarkus.http.auth.permission.post-logout.paths=/tenant-logout/logout
quarkus.http.auth.permission.post-logout.policy=permit

which gives control to CodeAuthenticationMechanism, but my AuthenticationRedirectException exception mapper is not invoked due to:

org.jboss.resteasy.spi.UnhandledException: io.smallrye.mutiny.CompositeException: Multiple exceptions caught:
	[Exception 0] io.quarkus.vertx.http.runtime.security.AuthenticationRedirectException
	[Exception 1] io.quarkus.vertx.http.runtime.security.AuthenticationRedirectException
....
Suppressed: io.quarkus.vertx.http.runtime.security.AuthenticationRedirectException
		at io.quarkus.oidc.runtime.CodeAuthenticationMechanism.redirectToLogoutEndpoint(CodeAuthenticationMechanism.java:535)
		at io.quarkus.oidc.runtime.CodeAuthenticationMechanism.access$300(CodeAuthenticationMechanism.java:51)
		at io.quarkus.oidc.runtime.CodeAuthenticationMechanism$3.apply(CodeAuthenticationMechanism.java:103)
		at io.quarkus.oidc.runtime.CodeAuthenticationMechanism$3.apply(CodeAuthenticationMechanism.java:99)
		at io.smallrye.mutiny.operators.UniOnItemTransform$1.onItem(UniOnItemTransform.java:31)
		... 117 more
	[CIRCULAR REFERENCE:io.quarkus.vertx.http.runtime.security.AuthenticationRedirectException]

Looks like it is circling, do you have some ideas why the exception mapper is not reached in this case ?
I see, it starts circling a few lines below :-), here.

Update: never mind about this circular ref, the same happens in the proactive case, but what is different with the proactive being off is that at this point,
So at https://github.com/quarkusio/quarkus/blob/master/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java#L178 we have a Uni CompositeException coming in, this throwable handler is always null (looks like that block can be removed ?) and RestEasy deals with the unwrapped CompositeException, so may be another mapper has to be created

[sberyozkin]

Cool, the generic CompositeException mapper works well

[stuartwdouglas]

Is there some easy way I can reproduce the ciruclar ref issue?

[sberyozkin]

@stuartwdouglas sure, probably the simplest would be to get this branch and disable all but integration-tests/oidc-code-flow/CodeFlowTest#testRPInitiatedLogout and set a breakpoint here which will eventually lead the recovery block a few lines below here.
And also temp remove this mapper if you'd like the exact exception be reproduced.

Or you can do it on the master branch as well, add

quarkus.http.auth.permission.post-logout.paths=/tenant-logout/logout
quarkus.http.auth.permission.post-logout.policy=permit

to the test application.properties, plus disable the proactive-auth, and copy 2 exception mappers from here, I'll move them to quarkus-resteasy once you release quarkus-security-1.1.3.Final

[sberyozkin]

@stuartwdouglas by the way, I removed a permit policy for the logout /tenant-logout/logout path, not sure now why I added it, requiring a permit would be a problem as only the authenticated users can invoke on the logout URI.
So yeah, if all otherwise is looking good to you in the branch then I'll just update it once you get quarkus-security-1.1.3.Final out and then we can merge this PR

[haraldatbmw]

@sberyozkin I just tried the fix with Quarkus-1.9.0.Final. It works for the flow and the REST endpoints but breaks my unit-tests which are using the @TestSecurity(user = "harald") annotation. SecurityContext.getUserPrincipal() returns null.

My current workaround is %test.quarkus.http.auth.proactive=true but I don't like it.

[haraldatbmw]

Created a new issue #12882