Skip to content

Commit

Permalink
Merge pull request #30026 from michalvavrik/feature/fix-issue-30011
Browse files Browse the repository at this point in the history
Prevent repeated Quarkus Security exception handling that lead to duplicate headers during OIDC redirect
  • Loading branch information
sberyozkin authored Dec 22, 2022
2 parents 47111d2 + 101796c commit 0b33456
Show file tree
Hide file tree
Showing 9 changed files with 570 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@

import java.util.Optional;

import javax.ws.rs.ext.ExceptionMapper;

import org.jboss.jandex.DotName;
import org.jboss.jandex.IndexView;
import org.jboss.jandex.Type;

import io.quarkus.builder.item.SimpleBuildItem;
import io.quarkus.deployment.Capabilities;
import io.quarkus.deployment.Capability;
Expand All @@ -14,26 +20,35 @@
import io.quarkus.deployment.annotations.ExecutionTime;
import io.quarkus.deployment.annotations.Record;
import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem;
import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
import io.quarkus.deployment.builditem.ExecutorBuildItem;
import io.quarkus.deployment.builditem.FeatureBuildItem;
import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
import io.quarkus.resteasy.common.deployment.ResteasyInjectionReadyBuildItem;
import io.quarkus.resteasy.runtime.AuthenticationCompletionExceptionMapper;
import io.quarkus.resteasy.runtime.AuthenticationFailedExceptionMapper;
import io.quarkus.resteasy.runtime.AuthenticationRedirectExceptionMapper;
import io.quarkus.resteasy.runtime.ResteasyVertxConfig;
import io.quarkus.resteasy.runtime.standalone.ResteasyStandaloneRecorder;
import io.quarkus.resteasy.server.common.deployment.ResteasyDeploymentBuildItem;
import io.quarkus.security.AuthenticationCompletionException;
import io.quarkus.security.AuthenticationFailedException;
import io.quarkus.security.AuthenticationRedirectException;
import io.quarkus.vertx.core.deployment.CoreVertxBuildItem;
import io.quarkus.vertx.http.deployment.DefaultRouteBuildItem;
import io.quarkus.vertx.http.deployment.FilterBuildItem;
import io.quarkus.vertx.http.deployment.HttpRootPathBuildItem;
import io.quarkus.vertx.http.deployment.RequireVirtualHttpBuildItem;
import io.quarkus.vertx.http.deployment.RouteBuildItem;
import io.quarkus.vertx.http.runtime.HttpBuildTimeConfig;
import io.quarkus.vertx.http.runtime.VertxHttpRecorder;
import io.vertx.core.Handler;
import io.vertx.ext.web.RoutingContext;

public class ResteasyStandaloneBuildStep {

private static final int REST_ROUTE_ORDER_OFFSET = 500;
private static final DotName EXCEPTION_MAPPER = DotName.createSimple(ExceptionMapper.class.getName());

public static final class ResteasyStandaloneBuildItem extends SimpleBuildItem {

Expand Down Expand Up @@ -75,6 +90,8 @@ public void boot(ShutdownContextBuildItem shutdown,
BuildProducer<RouteBuildItem> routes,
BuildProducer<FilterBuildItem> filterBuildItemBuildProducer,
CoreVertxBuildItem vertx,
CombinedIndexBuildItem combinedIndexBuildItem,
HttpBuildTimeConfig vertxConfig,
ResteasyStandaloneBuildItem standalone,
Optional<RequireVirtualHttpBuildItem> requireVirtual,
ExecutorBuildItem executorBuildItem,
Expand All @@ -90,11 +107,28 @@ public void boot(ShutdownContextBuildItem shutdown,
Handler<RoutingContext> handler = recorder.vertxRequestHandler(vertx.getVertx(),
executorBuildItem.getExecutorProxy(), resteasyVertxConfig);

final boolean noCustomAuthCompletionExMapper;
final boolean noCustomAuthFailureExMapper;
final boolean noCustomAuthRedirectExMapper;
if (vertxConfig.auth.proactive) {
noCustomAuthCompletionExMapper = notFoundCustomExMapper(AuthenticationCompletionException.class.getName(),
AuthenticationCompletionExceptionMapper.class.getName(), combinedIndexBuildItem.getIndex());
noCustomAuthFailureExMapper = notFoundCustomExMapper(AuthenticationFailedException.class.getName(),
AuthenticationFailedExceptionMapper.class.getName(), combinedIndexBuildItem.getIndex());
noCustomAuthRedirectExMapper = notFoundCustomExMapper(AuthenticationRedirectException.class.getName(),
AuthenticationRedirectExceptionMapper.class.getName(), combinedIndexBuildItem.getIndex());
} else {
// with disabled proactive auth we need to handle exceptions anyway as default auth failure handler did not
noCustomAuthCompletionExMapper = false;
noCustomAuthFailureExMapper = false;
noCustomAuthRedirectExMapper = false;
}
// failure handler for auth failures that occurred before the handler defined right above started processing the request
// we add the failure handler right before QuarkusErrorHandler
// so that user can define failure handlers that precede exception mappers
final Handler<RoutingContext> failureHandler = recorder.vertxFailureHandler(vertx.getVertx(),
executorBuildItem.getExecutorProxy(), resteasyVertxConfig);
executorBuildItem.getExecutorProxy(), resteasyVertxConfig, noCustomAuthCompletionExMapper,
noCustomAuthFailureExMapper, noCustomAuthRedirectExMapper, vertxConfig.auth.proactive);
filterBuildItemBuildProducer.produce(FilterBuildItem.ofAuthenticationFailureHandler(failureHandler));

// Exact match for resources matched to the root path
Expand All @@ -117,6 +151,24 @@ public void boot(ShutdownContextBuildItem shutdown,
recorder.start(shutdown, requireVirtual.isPresent());
}

private static boolean notFoundCustomExMapper(String exSignatureStr, String exMapperSignatureStr, IndexView index) {
for (var implementor : index.getAllKnownImplementors(EXCEPTION_MAPPER)) {
if (exMapperSignatureStr.equals(implementor.name().toString())) {
continue;
}
for (Type interfaceType : implementor.interfaceTypes()) {
if (EXCEPTION_MAPPER.equals(interfaceType.name())) {
final String mapperExSignature = interfaceType.asParameterizedType().arguments().get(0).name().toString();
if (exSignatureStr.equals(mapperExSignature)) {
return false;
}
break;
}
}
}
return true;
}

@BuildStep
@Record(value = ExecutionTime.STATIC_INIT)
public FilterBuildItem addDefaultAuthFailureHandler(ResteasyStandaloneRecorder recorder) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package io.quarkus.resteasy.test.security;

import static io.vertx.core.http.HttpHeaders.LOCATION;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.Set;

import javax.enterprise.context.ApplicationScoped;
import javax.ws.rs.GET;
import javax.ws.rs.Path;

import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.AuthenticationFailedException;
import io.quarkus.security.identity.AuthenticationRequestContext;
import io.quarkus.security.identity.IdentityProvider;
import io.quarkus.security.identity.IdentityProviderManager;
import io.quarkus.security.identity.SecurityIdentity;
import io.quarkus.security.identity.request.AuthenticationRequest;
import io.quarkus.security.identity.request.BaseAuthenticationRequest;
import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.vertx.http.runtime.security.ChallengeData;
import io.quarkus.vertx.http.runtime.security.HttpAuthenticationMechanism;
import io.restassured.RestAssured;
import io.restassured.http.Header;
import io.smallrye.mutiny.Uni;
import io.vertx.ext.web.RoutingContext;

public class AuthenticationFailedExceptionHeaderTest {

private static final String APP_PROPS = "" +
"quarkus.http.auth.permission.default.paths=/*\n" +
"quarkus.http.auth.permission.default.policy=authenticated";

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addAsResource(new StringAsset(APP_PROPS), "application.properties"));

@Test
public void testHeaders() {
// case-insensitive test that there is only one location header
// there has been duplicate location when both default auth failure handler and auth ex mapper send challenge
var response = RestAssured
.given()
.redirects()
.follow(false)
.when()
.get("/secured-route");
response.then().statusCode(302);
assertEquals(1, response.headers().asList().stream().map(Header::getName).map(String::toLowerCase)
.filter(LOCATION.toString()::equals).count());
}

@Path("/hello")
public static class HelloResource {
@GET
public String hello() {
return "hello";
}
}

@ApplicationScoped
public static class FailingAuthenticator implements HttpAuthenticationMechanism {

@Override
public Uni<SecurityIdentity> authenticate(RoutingContext context, IdentityProviderManager identityProviderManager) {
return Uni.createFrom().failure(new AuthenticationFailedException());
}

@Override
public Set<Class<? extends AuthenticationRequest>> getCredentialTypes() {
return Set.of(BaseAuthenticationRequest.class);
}

@Override
public Uni<ChallengeData> getChallenge(RoutingContext context) {
return Uni.createFrom().item(new ChallengeData(302, LOCATION, "http://localhost:8080/"));
}

}

@ApplicationScoped
public static class BasicIdentityProvider implements IdentityProvider<BaseAuthenticationRequest> {

@Override
public Class<BaseAuthenticationRequest> getRequestType() {
return BaseAuthenticationRequest.class;
}

@Override
public Uni<SecurityIdentity> authenticate(
BaseAuthenticationRequest simpleAuthenticationRequest,
AuthenticationRequestContext authenticationRequestContext) {
return Uni.createFrom().nothing();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package io.quarkus.resteasy.test.security;

import static io.vertx.core.http.HttpHeaders.CACHE_CONTROL;
import static io.vertx.core.http.HttpHeaders.LOCATION;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.Set;

import javax.enterprise.context.ApplicationScoped;
import javax.ws.rs.GET;
import javax.ws.rs.Path;

import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.AuthenticationRedirectException;
import io.quarkus.security.identity.AuthenticationRequestContext;
import io.quarkus.security.identity.IdentityProvider;
import io.quarkus.security.identity.IdentityProviderManager;
import io.quarkus.security.identity.SecurityIdentity;
import io.quarkus.security.identity.request.AuthenticationRequest;
import io.quarkus.security.identity.request.BaseAuthenticationRequest;
import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.vertx.http.runtime.security.ChallengeData;
import io.quarkus.vertx.http.runtime.security.HttpAuthenticationMechanism;
import io.restassured.RestAssured;
import io.restassured.http.Header;
import io.restassured.response.Response;
import io.smallrye.mutiny.Uni;
import io.vertx.ext.web.RoutingContext;

public class AuthenticationRedirectExceptionHeaderTest {

private static final String APP_PROPS = "" +
"quarkus.http.auth.permission.default.paths=/*\n" +
"quarkus.http.auth.permission.default.policy=authenticated";

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addAsResource(new StringAsset(APP_PROPS), "application.properties"));

@Test
public void testHeaders() {
// case-insensitive test that Pragma, cache-control and location headers are only present once
// there were duplicate headers when both default auth failure handler and auth ex mapper set headers
var response = RestAssured
.given()
.redirects()
.follow(false)
.when()
.get("/secured-route");
response.then().statusCode(302);
assertEquals(1, getHeaderCount(response, LOCATION.toString()));
assertEquals(1, getHeaderCount(response, CACHE_CONTROL.toString()));
assertEquals(1, getHeaderCount(response, "Pragma"));
}

private static int getHeaderCount(Response response, String headerName) {
headerName = headerName.toLowerCase();
return (int) response.headers().asList().stream().map(Header::getName).map(String::toLowerCase)
.filter(headerName::equals).count();
}

@Path("/hello")
public static class HelloResource {
@GET
public String hello() {
return "hello";
}
}

@ApplicationScoped
public static class RedirectingAuthenticator implements HttpAuthenticationMechanism {

@Override
public Uni<SecurityIdentity> authenticate(RoutingContext context, IdentityProviderManager identityProviderManager) {
return Uni.createFrom().failure(new AuthenticationRedirectException(302, "https://quarkus.io/"));
}

@Override
public Set<Class<? extends AuthenticationRequest>> getCredentialTypes() {
return Set.of(BaseAuthenticationRequest.class);
}

@Override
public Uni<ChallengeData> getChallenge(RoutingContext context) {
return Uni.createFrom().item(new ChallengeData(302, "header-name", "header-value"));
}

}

@ApplicationScoped
public static class BasicIdentityProvider implements IdentityProvider<BaseAuthenticationRequest> {

@Override
public Class<BaseAuthenticationRequest> getRequestType() {
return BaseAuthenticationRequest.class;
}

@Override
public Uni<SecurityIdentity> authenticate(
BaseAuthenticationRequest simpleAuthenticationRequest,
AuthenticationRequestContext authenticationRequestContext) {
return Uni.createFrom().nothing();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ public Handler<RoutingContext> vertxRequestHandler(Supplier<Vertx> vertx, Execut
return null;
}

public Handler<RoutingContext> vertxFailureHandler(Supplier<Vertx> vertx, Executor executor, ResteasyVertxConfig config) {
public Handler<RoutingContext> vertxFailureHandler(Supplier<Vertx> vertx, Executor executor, ResteasyVertxConfig config,
boolean noCustomAuthCompletionExMapper, boolean noCustomAuthFailureExMapper, boolean noCustomAuthRedirectExMapper,
boolean proactive) {
if (deployment == null) {
return null;
} else {
Expand All @@ -90,6 +92,40 @@ public Handler<RoutingContext> vertxFailureHandler(Supplier<Vertx> vertx, Execut

@Override
public void handle(RoutingContext request) {

// special handling when proactive auth is enabled as then we know default auth failure handler already run
if (proactive && request.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER) instanceof DefaultAuthFailureHandler) {
// we want to prevent repeated handling of exceptions if user don't want to handle exception himself
// we do not pass exception to abort handlers if proactive auth is enabled and user did not
// provide custom ex. mapper; we replace default auth failure handler as soon as we can, so that
// we can handle Quarkus Security Exceptions ourselves
if (request.failure() instanceof AuthenticationFailedException) {
if (noCustomAuthFailureExMapper) {
request.next();
} else {
// allow response customization
super.handle(request);
}
return;
} else if (request.failure() instanceof AuthenticationCompletionException) {
if (noCustomAuthCompletionExMapper) {
request.next();
} else {
// allow response customization
super.handle(request);
}
return;
} else if (request.failure() instanceof AuthenticationRedirectException) {
if (noCustomAuthRedirectExMapper) {
request.next();
} else {
// allow response customization
super.handle(request);
}
return;
}
}

if (request.failure() instanceof AuthenticationFailedException
|| request.failure() instanceof AuthenticationCompletionException
|| request.failure() instanceof AuthenticationRedirectException
Expand Down
Loading

0 comments on commit 0b33456

Please sign in to comment.