From 8f44d0d8d2caa733ca6f191974d2bd9ac6024407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Mon, 9 Dec 2024 17:53:11 +0100 Subject: [PATCH] fix(security): call @PermissionChecker methods with final augmented identity --- ...entityAugmentorsPermissionCheckerTest.java | 127 ++++++++++++++++++ .../QuarkusIdentityProviderManagerImpl.java | 11 +- ...usPermissionSecurityIdentityAugmentor.java | 6 + .../security/test/utils/IdentityMock.java | 46 ++++--- 4 files changed, 169 insertions(+), 21 deletions(-) create mode 100644 extensions/security/deployment/src/test/java/io/quarkus/security/test/permissionsallowed/checker/SecurityIdentityAugmentorsPermissionCheckerTest.java diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/permissionsallowed/checker/SecurityIdentityAugmentorsPermissionCheckerTest.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/permissionsallowed/checker/SecurityIdentityAugmentorsPermissionCheckerTest.java new file mode 100644 index 0000000000000..4fdba98068504 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/permissionsallowed/checker/SecurityIdentityAugmentorsPermissionCheckerTest.java @@ -0,0 +1,127 @@ +package io.quarkus.security.test.permissionsallowed.checker; + +import static io.quarkus.security.test.utils.IdentityMock.ADMIN; +import static io.quarkus.security.test.utils.IdentityMock.USER; +import static io.quarkus.security.test.utils.SecurityTestUtils.assertFailureFor; +import static io.quarkus.security.test.utils.SecurityTestUtils.assertSuccess; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.logging.Log; +import io.quarkus.security.ForbiddenException; +import io.quarkus.security.PermissionChecker; +import io.quarkus.security.PermissionsAllowed; +import io.quarkus.security.identity.AuthenticationRequestContext; +import io.quarkus.security.identity.SecurityIdentity; +import io.quarkus.security.identity.SecurityIdentityAugmentor; +import io.quarkus.security.runtime.QuarkusSecurityIdentity; +import io.quarkus.security.test.utils.AuthData; +import io.quarkus.security.test.utils.IdentityMock; +import io.quarkus.security.test.utils.SecurityTestUtils; +import io.quarkus.test.QuarkusUnitTest; +import io.smallrye.mutiny.Uni; + +public class SecurityIdentityAugmentorsPermissionCheckerTest { + + private static final AuthData USER_WITH_AUGMENTORS = new AuthData(USER, true); + private static final AuthData ADMIN_WITH_AUGMENTORS = new AuthData(ADMIN, true); + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar.addClasses(IdentityMock.class, AuthData.class, SecurityTestUtils.class)); + + @Inject + SecuredBean bean; + + /** + * Tests that {@link SecurityIdentity} passed to the {@link PermissionChecker} methods is augmented by all the + * augmentors (because that's the last operation we do on the identity, then it's de facto final). + */ + @Test + public void testPermissionCheckerUsesAugmentedIdentity() { + assertSuccess(bean::securedMethod, "secured", ADMIN_WITH_AUGMENTORS); + assertFailureFor(bean::securedMethod, ForbiddenException.class, USER_WITH_AUGMENTORS); + } + + @ApplicationScoped + public static class SecuredBean { + + @PermissionsAllowed("canCallSecuredMethod") + String securedMethod() { + return "secured"; + } + + @PermissionChecker("canCallSecuredMethod") + boolean canCallSecuredMethod(SecurityIdentity identity) { + if (!identity.hasRole("lowest-priority-augmentor")) { + Log.error("Role granted by the augmentor with the smallest priority is missing"); + return false; + } + if (!identity.hasRole("default-priority-augmentor")) { + Log.error("Role granted by the augmentor with a default priority is missing"); + return false; + } + if (!identity.hasRole("highest-priority-augmentor")) { + Log.error("Role granted by the augmentor with the highest priority is missing"); + return false; + } + return "admin".equals(identity.getPrincipal().getName()); + } + } + + @ApplicationScoped + public static class AugmentorWithLowestPriority implements SecurityIdentityAugmentor { + + @Override + public int priority() { + return Integer.MIN_VALUE; + } + + @Override + public Uni augment(SecurityIdentity securityIdentity, + AuthenticationRequestContext authenticationRequestContext) { + return Uni.createFrom().item( + QuarkusSecurityIdentity + .builder(securityIdentity) + .addRole("lowest-priority-augmentor") + .build()); + } + } + + @ApplicationScoped + public static class AugmentorWithDefaultPriority implements SecurityIdentityAugmentor { + + @Override + public Uni augment(SecurityIdentity securityIdentity, + AuthenticationRequestContext authenticationRequestContext) { + return Uni.createFrom().item( + QuarkusSecurityIdentity + .builder(securityIdentity) + .addRole("default-priority-augmentor") + .build()); + } + } + + @ApplicationScoped + public static class AugmentorWithHighestPriority implements SecurityIdentityAugmentor { + + @Override + public int priority() { + return Integer.MAX_VALUE; + } + + @Override + public Uni augment(SecurityIdentity securityIdentity, + AuthenticationRequestContext authenticationRequestContext) { + return Uni.createFrom().item( + QuarkusSecurityIdentity + .builder(securityIdentity) + .addRole("highest-priority-augmentor") + .build()); + } + } +} diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java index 6749d79d9c230..783e09e325fd5 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java @@ -182,6 +182,7 @@ public static class Builder { private final Map, List>> providers = new HashMap<>(); private final List augmentors = new ArrayList<>(); + private QuarkusPermissionSecurityIdentityAugmentor quarkusPermissionAugmentor = null; private BlockingSecurityExecutor blockingExecutor; private boolean built = false; @@ -206,7 +207,11 @@ public Builder addProvider(IdentityProvider pro * @return this builder */ public Builder addSecurityIdentityAugmentor(SecurityIdentityAugmentor augmentor) { - augmentors.add(augmentor); + if (augmentor instanceof QuarkusPermissionSecurityIdentityAugmentor quarkusPermissionAugmentor) { + this.quarkusPermissionAugmentor = quarkusPermissionAugmentor; + } else { + augmentors.add(augmentor); + } return this; } @@ -254,6 +259,10 @@ public int compare(SecurityIdentityAugmentor o1, SecurityIdentityAugmentor o2) { return Integer.compare(o2.priority(), o1.priority()); } }); + if (quarkusPermissionAugmentor != null) { + // @PermissionChecker methods must always run with the final SecurityIdentity + augmentors.add(quarkusPermissionAugmentor); + } return new QuarkusIdentityProviderManagerImpl(this); } } diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusPermissionSecurityIdentityAugmentor.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusPermissionSecurityIdentityAugmentor.java index 7ca713dc4c4bd..6557b84a3b1fc 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusPermissionSecurityIdentityAugmentor.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusPermissionSecurityIdentityAugmentor.java @@ -61,4 +61,10 @@ public Uni apply(Permission requiredpermission) { }) .build()); } + + @Override + public int priority() { + // we do not rely on this value and always add this augmentor as the last one manually + return Integer.MAX_VALUE; + } } diff --git a/extensions/security/test-utils/src/main/java/io/quarkus/security/test/utils/IdentityMock.java b/extensions/security/test-utils/src/main/java/io/quarkus/security/test/utils/IdentityMock.java index 78240d4fd25f8..5e2ee21aaa923 100644 --- a/extensions/security/test-utils/src/main/java/io/quarkus/security/test/utils/IdentityMock.java +++ b/extensions/security/test-utils/src/main/java/io/quarkus/security/test/utils/IdentityMock.java @@ -6,21 +6,19 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.function.Supplier; import jakarta.annotation.Priority; import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.Alternative; -import jakarta.enterprise.inject.Instance; import jakarta.inject.Inject; -import io.quarkus.arc.Arc; import io.quarkus.security.credential.Credential; 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.SecurityIdentityAugmentor; +import io.quarkus.security.identity.request.BaseAuthenticationRequest; import io.quarkus.security.runtime.SecurityIdentityAssociation; -import io.quarkus.security.spi.runtime.BlockingSecurityExecutor; import io.smallrye.mutiny.Uni; /** @@ -107,16 +105,17 @@ public Uni checkPermission(Permission permission) { @ApplicationScoped @Priority(1) public static class IdentityAssociationMock extends SecurityIdentityAssociation { + @Inject IdentityMock identity; @Inject - Instance augmentors; + IdentityProviderManager identityProviderManager; @Override public Uni getDeferredIdentity() { if (applyAugmentors) { - return augmentIdentity(identity); + return identityProviderManager.authenticate(new IdentityMockAuthenticationRequest()); } return Uni.createFrom().item(identity); } @@ -124,25 +123,32 @@ public Uni getDeferredIdentity() { @Override public SecurityIdentity getIdentity() { if (applyAugmentors) { - return augmentIdentity(identity).await().indefinitely(); + return getDeferredIdentity().await().indefinitely(); } return identity; } - private Uni augmentIdentity(SecurityIdentity identity) { - var authReqContexts = new TestAuthenticationRequestContext(); - Uni result = Uni.createFrom().item(identity); - for (SecurityIdentityAugmentor augmentor : augmentors) { - result = result.flatMap(si -> augmentor.augment(si, authReqContexts, Map.of())); - } - return result; + } + + public static final class IdentityMockAuthenticationRequest extends BaseAuthenticationRequest { + + } + + @ApplicationScoped + public static final class IdentityMockProvider implements IdentityProvider { + + @Inject + IdentityMock identity; + + @Override + public Class getRequestType() { + return IdentityMockAuthenticationRequest.class; } - private static final class TestAuthenticationRequestContext implements AuthenticationRequestContext { - @Override - public Uni runBlocking(Supplier function) { - return Arc.container().instance(BlockingSecurityExecutor.class).get().executeBlocking(function); - } + @Override + public Uni authenticate(IdentityMockAuthenticationRequest identityMockAuthenticationRequest, + AuthenticationRequestContext authenticationRequestContext) { + return Uni.createFrom().item(identity); } } }