From 4e5910a3d824d46f9c77fddffd87c55399cc71da Mon Sep 17 00:00:00 2001 From: artem-v Date: Mon, 17 Nov 2025 15:00:35 +0200 Subject: [PATCH] Fixed bug(940) with multiple AllowedRole definitions --- .../io/scalecube/services/RequestContext.java | 5 +- .../services/methods/MethodInfo.java | 17 +++--- .../java/io/scalecube/services/AuthTest.java | 58 ++++++++++++++++--- .../services/sut/security/SecuredService.java | 5 ++ .../sut/security/SecuredServiceImpl.java | 12 ++++ 5 files changed, 77 insertions(+), 20 deletions(-) diff --git a/services-api/src/main/java/io/scalecube/services/RequestContext.java b/services-api/src/main/java/io/scalecube/services/RequestContext.java index cdaccd16d..b82bea1aa 100644 --- a/services-api/src/main/java/io/scalecube/services/RequestContext.java +++ b/services-api/src/main/java/io/scalecube/services/RequestContext.java @@ -331,6 +331,7 @@ public static Mono deferSecured() { final var principal = context.principal(); final var methodInfo = context.methodInfo(); + final var role = principal.role(); final var allowedRoles = methodInfo.allowedRoles(); if (allowedRoles != null && !allowedRoles.isEmpty() @@ -338,12 +339,12 @@ public static Mono deferSecured() { LOGGER.warn( "Insufficient permissions -- " + "principal role '{}' is not allowed, request context: {}", - principal.role(), + role, context); throw new ForbiddenException("Insufficient permissions"); } - final var allowedPermissions = methodInfo.allowedPermissions(); + final var allowedPermissions = methodInfo.allowedPermissions(role); if (allowedPermissions != null && !allowedPermissions.isEmpty()) { for (var allowedPermission : allowedPermissions) { if (!principal.hasPermission(allowedPermission)) { diff --git a/services-api/src/main/java/io/scalecube/services/methods/MethodInfo.java b/services-api/src/main/java/io/scalecube/services/methods/MethodInfo.java index 5a599f18b..37cdd0f71 100644 --- a/services-api/src/main/java/io/scalecube/services/methods/MethodInfo.java +++ b/services-api/src/main/java/io/scalecube/services/methods/MethodInfo.java @@ -8,6 +8,7 @@ import io.scalecube.services.auth.Secured; import java.lang.reflect.Type; import java.util.Collection; +import java.util.Map; import java.util.StringJoiner; import java.util.stream.Collectors; import reactor.core.scheduler.Scheduler; @@ -27,8 +28,7 @@ public class MethodInfo { private final Secured secured; private final Scheduler scheduler; private final String restMethod; - private final Collection allowedRoles; - private final Collection allowedPermissions; + private final Map> allowedRoles; /** * Create a new service info. @@ -73,11 +73,9 @@ public MethodInfo( this.scheduler = scheduler; this.restMethod = restMethod; this.allowedRoles = - serviceRoles.stream().map(ServiceRoleDefinition::role).collect(Collectors.toSet()); - this.allowedPermissions = serviceRoles.stream() - .flatMap(definition -> definition.permissions().stream()) - .collect(Collectors.toSet()); + .collect( + Collectors.toMap(ServiceRoleDefinition::role, ServiceRoleDefinition::permissions)); } public String serviceName() { @@ -137,11 +135,11 @@ public String restMethod() { } public Collection allowedRoles() { - return allowedRoles; + return allowedRoles.keySet(); } - public Collection allowedPermissions() { - return allowedPermissions; + public Collection allowedPermissions(String role) { + return allowedRoles.get(role); } @Override @@ -161,7 +159,6 @@ public String toString() { .add("scheduler=" + scheduler) .add("restMethod=" + restMethod) .add("allowedRoles=" + allowedRoles) - .add("allowedPermissions=" + allowedPermissions) .toString(); } } diff --git a/services/src/test/java/io/scalecube/services/AuthTest.java b/services/src/test/java/io/scalecube/services/AuthTest.java index 3e7084974..478102e41 100644 --- a/services/src/test/java/io/scalecube/services/AuthTest.java +++ b/services/src/test/java/io/scalecube/services/AuthTest.java @@ -5,9 +5,9 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.params.provider.Arguments.arguments; -import com.fasterxml.jackson.annotation.JsonAutoDetect; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; +import com.fasterxml.jackson.annotation.JsonInclude.Include; +import com.fasterxml.jackson.annotation.JsonTypeInfo.As; import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DeserializationFeature; @@ -278,6 +278,50 @@ void authenticateSuccessfully() { StepVerifier.create(serviceCall.api(SecuredService.class).writeWithAllowedRoleAnnotation()) .verifyComplete(); } + + @ParameterizedTest + @MethodSource("multipleRolesSuccessfulSource") + void authenticateSuccessfullyByMultipleRoles(String role, TokenCredentials tokenCredentials) { + serviceCall = serviceCall((service, roles) -> credentials(tokenCredentials), role); + StepVerifier.create(serviceCall.api(SecuredService.class).invokeByMultipleRoles()) + .verifyComplete(); + } + + @ParameterizedTest + @MethodSource("multipleRolesFailedSource") + void authenticationFailedByMultipleRoles(String role, TokenCredentials tokenCredentials) { + serviceCall = serviceCall((service, roles) -> credentials(tokenCredentials), role); + StepVerifier.create(serviceCall.api(SecuredService.class).invokeByMultipleRoles()) + .verifyErrorSatisfies( + ex -> { + final var exception = (ForbiddenException) ex; + assertEquals(403, exception.errorCode(), "errorCode"); + assertEquals("Insufficient permissions", exception.getMessage(), "errorMessage"); + }); + } + + private static Stream multipleRolesSuccessfulSource() { + return Stream.of( + Arguments.of( + "gateway", + new TokenCredentials( + VALID_TOKEN, "gateway", List.of("gateway:read", "gateway:write"))), + Arguments.of( + "operations", + new TokenCredentials( + VALID_TOKEN, "operations", List.of("operations:read", "operations:write")))); + } + + private static Stream multipleRolesFailedSource() { + return Stream.of( + Arguments.of( + "gateway", + new TokenCredentials(VALID_TOKEN, "gateway", List.of("gateway:non-exiting-list"))), + Arguments.of( + "operations", + new TokenCredentials( + VALID_TOKEN, "operations", List.of("operations:non-exiting-list")))); + } } private static Mono credentials(TokenCredentials tokenCredentials) { @@ -354,13 +398,11 @@ private static ObjectMapper initMapper() { mapper.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false); mapper.configure(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL, true); mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false); - mapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.ANY); - mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); + mapper.setVisibility(PropertyAccessor.ALL, Visibility.ANY); + mapper.setSerializationInclusion(Include.NON_NULL); mapper.configure(SerializationFeature.WRITE_ENUMS_USING_TO_STRING, true); mapper.activateDefaultTyping( - LaissezFaireSubTypeValidator.instance, - DefaultTyping.JAVA_LANG_OBJECT, - JsonTypeInfo.As.WRAPPER_OBJECT); + LaissezFaireSubTypeValidator.instance, DefaultTyping.JAVA_LANG_OBJECT, As.WRAPPER_OBJECT); mapper.findAndRegisterModules(); return mapper; } diff --git a/services/src/test/java/io/scalecube/services/sut/security/SecuredService.java b/services/src/test/java/io/scalecube/services/sut/security/SecuredService.java index 44c267ada..0c13ebdc4 100644 --- a/services/src/test/java/io/scalecube/services/sut/security/SecuredService.java +++ b/services/src/test/java/io/scalecube/services/sut/security/SecuredService.java @@ -19,4 +19,9 @@ public interface SecuredService { @ServiceMethod Mono writeWithAllowedRoleAnnotation(); + + // Multiple roles + + @ServiceMethod + Mono invokeByMultipleRoles(); } diff --git a/services/src/test/java/io/scalecube/services/sut/security/SecuredServiceImpl.java b/services/src/test/java/io/scalecube/services/sut/security/SecuredServiceImpl.java index 9c293cf83..8428d2e05 100644 --- a/services/src/test/java/io/scalecube/services/sut/security/SecuredServiceImpl.java +++ b/services/src/test/java/io/scalecube/services/sut/security/SecuredServiceImpl.java @@ -57,4 +57,16 @@ public Mono readWithAllowedRoleAnnotation() { public Mono writeWithAllowedRoleAnnotation() { return Mono.empty(); } + + @Secured + @AllowedRole( + name = "gateway", + permissions = {"gateway:read"}) + @AllowedRole( + name = "operations", + permissions = {"operations:read"}) + @Override + public Mono invokeByMultipleRoles() { + return Mono.empty(); + } }