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

Fix AuthenticationRedirectException handling with disabled proactive security #29239

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,7 @@ public void handle(RoutingContext event) {
@Override
protected void proceed(Throwable throwable) {

if (event.failed()) {
//auth failure handler should never get called from route failure handlers
//but if we get to this point bad things have happened,
//so it is better to send a response than to hang
event.end();
sberyozkin marked this conversation as resolved.
Show resolved Hide resolved
} else {
if (!event.failed()) {
event.fail(throwable);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT;

import java.util.Optional;
import java.util.function.Consumer;

import io.quarkus.builder.item.SimpleBuildItem;
import io.quarkus.deployment.Capabilities;
Expand All @@ -30,7 +29,6 @@
import io.quarkus.vertx.http.deployment.RouteBuildItem;
import io.quarkus.vertx.http.runtime.VertxHttpRecorder;
import io.vertx.core.Handler;
import io.vertx.ext.web.Route;
import io.vertx.ext.web.RoutingContext;

public class ResteasyStandaloneBuildStep {
Expand Down Expand Up @@ -75,6 +73,7 @@ public void boot(ShutdownContextBuildItem shutdown,
BuildProducer<FeatureBuildItem> feature,
BuildProducer<DefaultRouteBuildItem> defaultRoutes,
BuildProducer<RouteBuildItem> routes,
BuildProducer<FilterBuildItem> filterBuildItemBuildProducer,
CoreVertxBuildItem vertx,
ResteasyStandaloneBuildItem standalone,
Optional<RequireVirtualHttpBuildItem> requireVirtual,
Expand All @@ -92,15 +91,16 @@ public void boot(ShutdownContextBuildItem shutdown,
executorBuildItem.getExecutorProxy(), resteasyVertxConfig);

// failure handler for auth failures that occurred before the handler defined right above started processing the request
final Consumer<Route> addFailureHandler = recorder.addVertxFailureHandler(vertx.getVertx(),
final Handler<RoutingContext> failureHandler = recorder.vertxFailureHandler(vertx.getVertx(),
executorBuildItem.getExecutorProxy(), resteasyVertxConfig);
filterBuildItemBuildProducer.produce(new FilterBuildItem(failureHandler,
VertxHttpRecorder.AFTER_DEFAULT_ROUTE_ORDER_MARK + REST_ROUTE_ORDER_OFFSET, true));

// Exact match for resources matched to the root path
routes.produce(
RouteBuildItem.builder()
.orderedRoute(standalone.deploymentRootPath,
VertxHttpRecorder.AFTER_DEFAULT_ROUTE_ORDER_MARK + REST_ROUTE_ORDER_OFFSET,
addFailureHandler)
VertxHttpRecorder.AFTER_DEFAULT_ROUTE_ORDER_MARK + REST_ROUTE_ORDER_OFFSET)
.handler(handler).build());
String matchPath = standalone.deploymentRootPath;
if (matchPath.endsWith("/")) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package io.quarkus.resteasy.runtime.standalone;

import static io.quarkus.vertx.http.runtime.security.HttpSecurityRecorder.DefaultAuthFailureHandler.extractRootCause;

import java.util.concurrent.Executor;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Supplier;

import org.jboss.resteasy.specimpl.ResteasyUriInfo;
Expand All @@ -23,7 +24,6 @@
import io.quarkus.vertx.http.runtime.security.QuarkusHttpUser;
import io.vertx.core.Handler;
import io.vertx.core.Vertx;
import io.vertx.ext.web.Route;
import io.vertx.ext.web.RoutingContext;

/**
Expand Down Expand Up @@ -77,35 +77,30 @@ public Handler<RoutingContext> vertxRequestHandler(Supplier<Vertx> vertx, Execut
return null;
}

public Consumer<Route> addVertxFailureHandler(Supplier<Vertx> vertx, Executor executor, ResteasyVertxConfig config) {
public Handler<RoutingContext> vertxFailureHandler(Supplier<Vertx> vertx, Executor executor, ResteasyVertxConfig config) {
if (deployment == null) {
return null;
} else {
return new Consumer<Route>() {
@Override
public void accept(Route route) {
// allow customization of auth failures with exception mappers; this failure handler is only
// used when auth failed before RESTEasy Classic began processing the request
route.failureHandler(new VertxRequestHandler(vertx.get(), deployment, contextPath,
new ResteasyVertxAllocator(config.responseBufferSize), executor,
readTimeout.getValue().readTimeout.toMillis()) {
// allow customization of auth failures with exception mappers; this failure handler is only
// used when auth failed before RESTEasy Classic began processing the request
return new VertxRequestHandler(vertx.get(), deployment, contextPath,
new ResteasyVertxAllocator(config.responseBufferSize), executor,
readTimeout.getValue().readTimeout.toMillis()) {

@Override
public void handle(RoutingContext request) {
if (request.failure() instanceof AuthenticationFailedException
|| request.failure() instanceof AuthenticationCompletionException
|| request.failure() instanceof AuthenticationRedirectException) {
super.handle(request);
} else {
request.next();
}
}
@Override
public void handle(RoutingContext request) {
if (request.failure() instanceof AuthenticationFailedException
|| request.failure() instanceof AuthenticationCompletionException
|| request.failure() instanceof AuthenticationRedirectException) {
super.handle(request);
} else {
request.next();
}
}

@Override
protected void setCurrentIdentityAssociation(RoutingContext routingContext) {
// security identity is not available as authentication failed
}
});
@Override
protected void setCurrentIdentityAssociation(RoutingContext routingContext) {
// security identity is not available as authentication failed
}
};
}
Expand All @@ -115,7 +110,8 @@ public Handler<RoutingContext> defaultAuthFailureHandler() {
return new Handler<RoutingContext>() {
@Override
public void handle(RoutingContext event) {
if (event.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER) instanceof DefaultAuthFailureHandler) {
if (deployment != null
&& event.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER) instanceof DefaultAuthFailureHandler) {

// only replace default auth failure handler if we can extract URI info
// as org.jboss.resteasy.plugins.server.BaseHttpRequest requires it;
Expand All @@ -135,13 +131,8 @@ public void handle(RoutingContext event) {

@Override
public void accept(RoutingContext event, Throwable throwable) {
if (event.failed()) {
//auth failure handler should never get called from route failure handlers
//but if we get to this point bad things have happened,
//so it is better to send a response than to hang
event.end();
} else {
event.fail(throwable);
if (!event.failed()) {
event.fail(extractRootCause(throwable));
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@
import io.vertx.core.Handler;
import io.vertx.core.http.HttpServerRequest;
import io.vertx.core.http.HttpServerResponse;
import io.vertx.ext.web.Route;
import io.vertx.ext.web.RoutingContext;

public class ResteasyReactiveProcessor {
Expand Down Expand Up @@ -1055,6 +1054,7 @@ public void setupDeployment(BeanContainerBuildItem beanContainerBuildItem,
BuildProducer<ResteasyReactiveDeploymentBuildItem> quarkusRestDeploymentBuildItemBuildProducer,
BuildProducer<ReflectiveClassBuildItem> reflectiveClass,
BuildProducer<RouteBuildItem> routes,
BuildProducer<FilterBuildItem> filterBuildItemBuildProducer,
ApplicationResultBuildItem applicationResultBuildItem,
ResourceInterceptorsBuildItem resourceInterceptorsBuildItem,
ExceptionMappersBuildItem exceptionMappersBuildItem,
Expand Down Expand Up @@ -1187,11 +1187,12 @@ public void setupDeployment(BeanContainerBuildItem beanContainerBuildItem,
if (!requestContextFactoryBuildItem.isPresent()) {
RuntimeValue<RestInitialHandler> restInitialHandler = recorder.restInitialHandler(deployment);
Handler<RoutingContext> handler = recorder.handler(restInitialHandler);
Consumer<Route> addFailureHandler = recorder.addFailureHandler(restInitialHandler);
Handler<RoutingContext> failureHandler = recorder.failureHandler(restInitialHandler);
filterBuildItemBuildProducer.produce(new FilterBuildItem(failureHandler, order, true));

// Exact match for resources matched to the root path
routes.produce(RouteBuildItem.builder()
.orderedRoute(deploymentPath, order, addFailureHandler).handler(handler).build());
.orderedRoute(deploymentPath, order).handler(handler).build());
String matchPath = deploymentPath;
if (matchPath.endsWith("/")) {
matchPath += "*";
Expand All @@ -1200,7 +1201,7 @@ public void setupDeployment(BeanContainerBuildItem beanContainerBuildItem,
}
// Match paths that begin with the deployment path
routes.produce(
RouteBuildItem.builder().orderedRoute(matchPath, order, addFailureHandler)
RouteBuildItem.builder().orderedRoute(matchPath, order)
.handler(handler).build());
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package io.quarkus.resteasy.reactive.server.test.security;

import static org.jboss.resteasy.reactive.RestResponse.StatusCode.FOUND;

import java.util.Set;

import javax.enterprise.context.ApplicationScoped;
import javax.ws.rs.core.Response;

import org.jboss.resteasy.reactive.server.ServerExceptionMapper;
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.smallrye.mutiny.Uni;
import io.vertx.ext.web.RoutingContext;

public class AuthenticationRedirectExceptionMapperTest {

private static final int EXPECTED_STATUS = 409;
private static final String APP_PROPS = "" +
"quarkus.http.auth.proactive=false\n" +
"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 testAuthenticationRedirectExceptionMapper() {
RestAssured
.given()
.redirects()
.follow(false)
.when()
.get("/secured-route")
.then()
.statusCode(EXPECTED_STATUS);
}

public static final class AuthenticationRedirectExceptionMapper {

@ServerExceptionMapper(AuthenticationRedirectException.class)
public Response authenticationRedirectException() {
return Response.status(EXPECTED_STATUS).build();
}
}

@ApplicationScoped
public static class RedirectingAuthenticator implements HttpAuthenticationMechanism {

@Override
public Uni<SecurityIdentity> authenticate(RoutingContext context, IdentityProviderManager identityProviderManager) {
throw new AuthenticationRedirectException(FOUND, "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(FOUND, "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
@@ -1,6 +1,7 @@
package io.quarkus.resteasy.reactive.server.runtime;

import static io.quarkus.resteasy.reactive.server.runtime.NotFoundExceptionMapper.classMappers;
import static io.quarkus.vertx.http.runtime.security.HttpSecurityRecorder.DefaultAuthFailureHandler.extractRootCause;

import java.io.Closeable;
import java.lang.reflect.Constructor;
Expand Down Expand Up @@ -59,7 +60,6 @@
import io.quarkus.vertx.http.runtime.security.HttpSecurityRecorder.DefaultAuthFailureHandler;
import io.quarkus.vertx.http.runtime.security.QuarkusHttpUser;
import io.vertx.core.Handler;
import io.vertx.ext.web.Route;
import io.vertx.ext.web.RoutingContext;

@Recorder
Expand Down Expand Up @@ -211,32 +211,27 @@ public void accept(RoutingContext routingContext) {
return new ResteasyReactiveVertxHandler(eventCustomizer, initialHandler);
}

public Consumer<Route> addFailureHandler(RuntimeValue<RestInitialHandler> restInitialHandlerRuntimeValue) {
public Handler<RoutingContext> failureHandler(RuntimeValue<RestInitialHandler> restInitialHandlerRuntimeValue) {
final RestInitialHandler restInitialHandler = restInitialHandlerRuntimeValue.getValue();
return new Consumer<Route>() {
// process auth failures with abort handlers
return new Handler<RoutingContext>() {
@Override
public void accept(Route route) {
// process auth failures with abort handlers
route.failureHandler(new Handler<RoutingContext>() {
@Override
public void handle(RoutingContext event) {

// this condition prevent exception mappers from handling auth failure exceptions when proactive
// security is enabled as for now, community decided that's expected behavior and only way for
// users to handle the exceptions is to define their own failure handler as in Reactive Routes
// more info here: https://github.com/quarkusio/quarkus/pull/28648#issuecomment-1287203946
final boolean eventFailedByRESTEasyReactive = event
.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER) instanceof FailingDefaultAuthFailureHandler;

if (eventFailedByRESTEasyReactive && (event.failure() instanceof AuthenticationFailedException
|| event.failure() instanceof AuthenticationCompletionException
|| event.failure() instanceof AuthenticationRedirectException)) {
restInitialHandler.beginProcessing(event, event.failure());
} else {
event.next();
}
}
});
public void handle(RoutingContext event) {

// this condition prevent exception mappers from handling auth failure exceptions when proactive
// security is enabled as for now, community decided that's expected behavior and only way for
// users to handle the exceptions is to define their own failure handler as in Reactive Routes
// more info here: https://github.com/quarkusio/quarkus/pull/28648#issuecomment-1287203946
final boolean eventFailedByRESTEasyReactive = event
.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER) instanceof FailingDefaultAuthFailureHandler;

if (eventFailedByRESTEasyReactive && (event.failure() instanceof AuthenticationFailedException
|| event.failure() instanceof AuthenticationCompletionException
|| event.failure() instanceof AuthenticationRedirectException)) {
restInitialHandler.beginProcessing(event, event.failure());
} else {
event.next();
}
}
};
}
Expand Down Expand Up @@ -342,13 +337,8 @@ private static final class FailingDefaultAuthFailureHandler implements BiConsume

@Override
public void accept(RoutingContext event, Throwable throwable) {
if (event.failed()) {
//auth failure handler should never get called from route failure handlers
//but if we get to this point bad things have happened,
//so it is better to send a response than to hang
event.end();
} else {
event.fail(throwable);
if (!event.failed()) {
event.fail(extractRootCause(throwable));
}
}
}
Expand Down