Skip to content

Commit

Permalink
Merge pull request quarkusio#40840 from michalvavrik/feature/fix-cont…
Browse files Browse the repository at this point in the history
…ext-activation-for-sec-interceptors

Fix priority of interceptors preventing repeated security checks so that interceptor activating CDI request context runs first
  • Loading branch information
sberyozkin authored May 25, 2024
2 parents 232c608 + 94ed0d3 commit edf4d5f
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public Object intercept(InvocationContext ic) throws Exception {
*/
@Interceptor
@RolesAllowed("")
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class RolesAllowedInterceptor extends StandardSecurityCheckInterceptor {

}
Expand All @@ -68,7 +68,7 @@ public static final class RolesAllowedInterceptor extends StandardSecurityCheckI
*/
@Interceptor
@PermissionsAllowed("")
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class PermissionsAllowedInterceptor extends StandardSecurityCheckInterceptor {

}
Expand All @@ -78,7 +78,7 @@ public static final class PermissionsAllowedInterceptor extends StandardSecurity
*/
@Interceptor
@PermitAll
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class PermitAllInterceptor extends StandardSecurityCheckInterceptor {

}
Expand All @@ -88,7 +88,7 @@ public static final class PermitAllInterceptor extends StandardSecurityCheckInte
*/
@Interceptor
@Authenticated
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class AuthenticatedInterceptor extends StandardSecurityCheckInterceptor {

}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.resteasy.reactive.server.test.security;

import static io.quarkus.resteasy.reactive.server.test.security.RolesAllowedService.EVENT_BUS_MESSAGES;
import static org.hamcrest.Matchers.is;

import java.io.IOException;
Expand All @@ -14,6 +15,7 @@
import jakarta.ws.rs.ext.MessageBodyReader;
import jakarta.ws.rs.ext.Provider;

import org.awaitility.Awaitility;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -90,6 +92,20 @@ public void testSecurityRunsBeforeValidation() {
Assertions.assertFalse(read);
}

@Test
public void testSecurityInterceptorsAfterHttpRequestCompleted() {
RestAssured
.given()
.auth().preemptive().basic("user", "user")
.body("message one")
.post("/roles-service/secured-event-bus")
.then()
.statusCode(204);
Awaitility.await().until(() -> !EVENT_BUS_MESSAGES.isEmpty());
Assertions.assertEquals(1, EVENT_BUS_MESSAGES.size(), EVENT_BUS_MESSAGES.toString());
Assertions.assertEquals("permit all message one", EVENT_BUS_MESSAGES.get(0));
}

static volatile boolean read = false;

@Provider
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
package io.quarkus.resteasy.reactive.server.test.security;

import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;

import jakarta.annotation.security.PermitAll;
import jakarta.annotation.security.RolesAllowed;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.context.control.ActivateRequestContext;

@ApplicationScoped
public class RolesAllowedService {

public static final String SERVICE_HELLO = "Hello from Service!";
public static final String SERVICE_BYE = "Bye from Service!";
public static final List<String> EVENT_BUS_MESSAGES = new CopyOnWriteArrayList<>();

@RolesAllowed("admin")
public String hello() {
Expand All @@ -20,4 +25,15 @@ public String bye() {
return SERVICE_BYE;
}

@PermitAll
@ActivateRequestContext
void receivePermitAllMessage(String m) {
EVENT_BUS_MESSAGES.add("permit all " + m);
}

@RolesAllowed("admin")
@ActivateRequestContext
void receiveRolesAllowedMessage(String m) {
EVENT_BUS_MESSAGES.add("roles allowed " + m);
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
package io.quarkus.resteasy.reactive.server.test.security;

import jakarta.annotation.security.RolesAllowed;
import jakarta.enterprise.event.Observes;
import jakarta.inject.Inject;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;

import io.quarkus.runtime.ShutdownEvent;
import io.quarkus.runtime.StartupEvent;
import io.vertx.core.Vertx;
import io.vertx.mutiny.core.eventbus.EventBus;
import io.vertx.mutiny.core.eventbus.MessageConsumer;

@Path("/roles-service")
public class RolesAllowedServiceResource {

private MessageConsumer<String> permitAllConsumer;
private MessageConsumer<String> rolesAllowedConsumer;

@Inject
RolesAllowedService rolesAllowedService;

@Inject
EventBus bus;

@Path("/hello")
@RolesAllowed({ "user", "admin" })
@GET
Expand All @@ -23,4 +37,35 @@ public String getServiceHello() {
public String getServiceBye() {
return rolesAllowedService.bye();
}

@Path("/secured-event-bus")
@POST
public void sendMessage(String message) {
bus.send("roles-allowed-message", message);
bus.send("permit-all-message", message);
}

void observeStartup(@Observes StartupEvent startupEvent, EventBus eventBus, Vertx vertx) {
permitAllConsumer = eventBus
.<String> consumer("permit-all-message")
.handler(msg -> rolesAllowedService.receivePermitAllMessage(msg.body()));

// this must always fail because the authorization is happening in a blank CDI request context
rolesAllowedConsumer = eventBus
.<String> consumer("roles-allowed-message")
.handler(msg -> vertx.executeBlocking(() -> {
// make sure authentication is attempted on a worker thread to prevent blocking event loop
rolesAllowedService.receiveRolesAllowedMessage(msg.body());
return null;
}));
}

void observerShutdown(@Observes ShutdownEvent shutdownEvent) {
if (permitAllConsumer != null) {
permitAllConsumer.unregister().await().indefinitely();
}
if (rolesAllowedConsumer != null) {
rolesAllowedConsumer.unregister().await().indefinitely();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private boolean alreadyDoneByEagerSecurityHandler(Object methodWithFinishedCheck
*/
@Interceptor
@RolesAllowed("")
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class RolesAllowedInterceptor extends StandardSecurityCheckInterceptor {

}
Expand All @@ -64,7 +64,7 @@ public static final class RolesAllowedInterceptor extends StandardSecurityCheckI
*/
@Interceptor
@PermissionsAllowed("")
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class PermissionsAllowedInterceptor extends StandardSecurityCheckInterceptor {

}
Expand All @@ -74,7 +74,7 @@ public static final class PermissionsAllowedInterceptor extends StandardSecurity
*/
@Interceptor
@PermitAll
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class PermitAllInterceptor extends StandardSecurityCheckInterceptor {

}
Expand All @@ -84,7 +84,7 @@ public static final class PermitAllInterceptor extends StandardSecurityCheckInte
*/
@Interceptor
@Authenticated
@Priority(Interceptor.Priority.PLATFORM_BEFORE)
@Priority(Interceptor.Priority.LIBRARY_BEFORE - 100)
public static final class AuthenticatedInterceptor extends StandardSecurityCheckInterceptor {

}
Expand Down

0 comments on commit edf4d5f

Please sign in to comment.