Skip to content

Commit

Permalink
Optimize jwt format by grouping namespaces in rolebindings
Browse files Browse the repository at this point in the history
  • Loading branch information
ThomasCAI-mlv committed Jan 7, 2025
1 parent 0822de2 commit bf53d85
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 28 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.michelin.ns4kafka.security;

import static com.michelin.ns4kafka.security.auth.JwtCustomClaimNames.ROLE_BINDINGS;
import static io.micronaut.core.util.StringUtils.EMPTY_STRING;

import com.michelin.ns4kafka.model.RoleBinding;
import com.michelin.ns4kafka.property.SecurityProperties;
Expand Down Expand Up @@ -110,7 +111,9 @@ public SecurityRuleResult checkSecurity(HttpRequest<?> request, @Nullable Authen
// No role binding for the target namespace. User is targeting a namespace that he is not allowed to access
List<AuthenticationRoleBinding> namespaceRoleBindings = authenticationInfo.getRoleBindings()
.stream()
.filter(roleBinding -> roleBinding.getNamespace().equals(namespace))
.filter(roleBinding -> roleBinding.getNamespaces()
.stream()
.anyMatch(ns -> ns.equals(namespace)))
.toList();

if (namespaceRoleBindings.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
@NoArgsConstructor
@AllArgsConstructor
public class AuthenticationRoleBinding {
private String namespace;
private List<String> namespaces;
private List<RoleBinding.Verb> verbs;
private List<String> resourceTypes;

record VerbResourceTypes(List<RoleBinding.Verb> verbs, List<String> resourceTypes) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;

/**
Expand Down Expand Up @@ -50,10 +51,21 @@ public AuthenticationResponse buildAuthJwtGroups(String username, List<String> g
return AuthenticationResponse.success(username, resourceBasedSecurityRule.computeRolesFromGroups(groups),
Map.of(ROLE_BINDINGS, roleBindings
.stream()
.map(roleBinding -> AuthenticationRoleBinding.builder()
.namespace(roleBinding.getMetadata().getNamespace())
.verbs(new ArrayList<>(roleBinding.getSpec().getRole().getVerbs()))
.resourceTypes(new ArrayList<>(roleBinding.getSpec().getRole().getResourceTypes()))
// group the namespaces by verbs + resourceTypes in a mapping
.collect(Collectors.groupingBy(roleBinding ->
new AuthenticationRoleBinding.VerbResourceTypes(
new ArrayList<>(roleBinding.getSpec().getRole().getVerbs()),
new ArrayList<>(roleBinding.getSpec().getRole().getResourceTypes())
),
Collectors.mapping(rb -> rb.getMetadata().getNamespace(), Collectors.toList())
))
// build JWT with a list of namespaces for each combination of verbs + resourceTypes
.entrySet()
.stream()
.map(entry -> AuthenticationRoleBinding.builder()
.namespaces(entry.getValue())
.verbs(entry.getKey().verbs())
.resourceTypes(entry.getKey().resourceTypes())
.build())
.toList()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

@ExtendWith(MockitoExtension.class)
class ResourceBasedSecurityRuleTest {
private static final String NAMESPACE = "namespace";
private static final String NAMESPACES = "namespaces";
private static final String VERBS = "verbs";
private static final String RESOURCE_TYPES = "resourceTypes";

Expand Down Expand Up @@ -133,7 +133,7 @@ void checkReturnsAllowedNamespaceAsAdmin() {
"topics,/api/namespaces/test/topics/topic.with.dots"})
void shouldReturnAllowedWhenHyphenAndDotResourcesAndHandleRoleBindingsType(String resourceType, String path) {
List<Map<String, ?>> jwtRoleBindings = List.of(
Map.of(NAMESPACE, "test",
Map.of(NAMESPACES, List.of("test"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of(resourceType)));

Expand All @@ -148,7 +148,7 @@ void shouldReturnAllowedWhenHyphenAndDotResourcesAndHandleRoleBindingsType(Strin

List<AuthenticationRoleBinding> basicAuthRoleBindings = List.of(
AuthenticationRoleBinding.builder()
.namespace("test")
.namespaces(List.of("test"))
.verbs(List.of(GET))
.resourceTypes(List.of(resourceType))
.build());
Expand All @@ -167,7 +167,7 @@ void shouldReturnAllowedWhenHyphenAndDotResourcesAndHandleRoleBindingsType(Strin
@Test
void shouldReturnAllowedWhenSubResource() {
List<Map<String, ?>> jwtRoleBindings = List.of(
Map.of(NAMESPACE, "test",
Map.of(NAMESPACES, List.of("test"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("connectors/restart", "topics/delete-records")));

Expand All @@ -192,7 +192,7 @@ void shouldReturnAllowedWhenSubResource() {
@CsvSource({"namespace", "name-space", "name.space", "_name_space_", "namespace123"})
void shouldReturnAllowedWhenSpecialNamespaceName(String namespace) {
List<Map<String, ?>> roleBindings = List.of(
Map.of(NAMESPACE, namespace,
Map.of(NAMESPACES, List.of(namespace),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("topics")));

Expand All @@ -207,6 +207,45 @@ void shouldReturnAllowedWhenSpecialNamespaceName(String namespace) {
assertEquals(SecurityRuleResult.ALLOWED, actual);
}

@Test
void shouldReturnAllowedWhenMultipleNamespaces() {
List<Map<String, ?>> roleBindings = List.of(
Map.of(NAMESPACES, List.of("ns1", "ns2", "ns3"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("topics")));

Map<String, Object> claims = Map.of(SUBJECT, "user", ROLES, List.of(), ROLE_BINDINGS, roleBindings);
Authentication auth = Authentication.build("user", claims);

when(namespaceRepository.findByName("ns3"))
.thenReturn(Optional.of(Namespace.builder().build()));

SecurityRuleResult actual =
resourceBasedSecurityRule.checkSecurity(HttpRequest.GET("/api/namespaces/ns3/topics"), auth);
assertEquals(SecurityRuleResult.ALLOWED, actual);
}

@Test
void shouldReturnAllowedWhenMultipleVerbsResourceTypesCombinations() {
List<Map<String, ?>> roleBindings = List.of(
Map.of(NAMESPACES, List.of("ns1"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("topics")),
Map.of(NAMESPACES, List.of("ns2"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("connectors")));

Map<String, Object> claims = Map.of(SUBJECT, "user", ROLES, List.of(), ROLE_BINDINGS, roleBindings);
Authentication auth = Authentication.build("user", claims);

when(namespaceRepository.findByName("ns2"))
.thenReturn(Optional.of(Namespace.builder().build()));

SecurityRuleResult actual =
resourceBasedSecurityRule.checkSecurity(HttpRequest.GET("/api/namespaces/ns2/connectors"), auth);
assertEquals(SecurityRuleResult.ALLOWED, actual);
}

@Test
void shouldReturnForbiddenNamespaceWhenNoRoleBinding() {
Map<String, Object> claims = Map.of(SUBJECT, "user", ROLES, List.of(), ROLE_BINDINGS, List.of());
Expand All @@ -226,7 +265,7 @@ void shouldReturnForbiddenNamespaceWhenNoRoleBinding() {
@Test
void shouldReturnForbiddenNamespaceWhenNoRoleBindingMatchingRequestedNamespace() {
List<Map<String, ?>> roleBindings = List.of(
Map.of(NAMESPACE, "namespace",
Map.of(NAMESPACES, List.of("namespace"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("connectors")));

Expand All @@ -247,7 +286,7 @@ void shouldReturnForbiddenNamespaceWhenNoRoleBindingMatchingRequestedNamespace()
@Test
void checkReturnsUnknownSubResource() {
List<Map<String, ?>> roleBindings = List.of(
Map.of(NAMESPACE, "test",
Map.of(NAMESPACES, List.of("test"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("connectors")));

Expand All @@ -266,7 +305,7 @@ void checkReturnsUnknownSubResource() {
@Test
void checkReturnsUnknownSubResourceWithDot() {
List<Map<String, ?>> roleBindings = List.of(
Map.of(NAMESPACE, "test",
Map.of(NAMESPACES, List.of("test"),
VERBS, List.of(GET),
RESOURCE_TYPES, List.of("connectors")));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AuthenticationInfoTest {
@Test
void shouldConvertFromMapRoleBindingsType() {
Map<String, Object> attributes = new HashMap<>();
attributes.put(ROLE_BINDINGS, List.of(Map.of("namespace", "namespace",
attributes.put(ROLE_BINDINGS, List.of(Map.of("namespaces", List.of("namespace"),
"verbs", List.of("GET"),
"resourceTypes", List.of("topics"))));
Authentication authentication = Authentication.build("name", List.of("role"), attributes);
Expand All @@ -28,22 +28,22 @@ void shouldConvertFromMapRoleBindingsType() {

assertEquals("name", authenticationInfo.getName());
assertEquals(List.of("role"), authenticationInfo.getRoles().stream().toList());
assertIterableEquals(List.of(new AuthenticationRoleBinding("namespace", List.of(GET), List.of("topics"))),
authenticationInfo.getRoleBindings().stream().toList());
assertIterableEquals(List.of(new AuthenticationRoleBinding(List.of("namespace"), List.of(GET),
List.of("topics"))), authenticationInfo.getRoleBindings().stream().toList());
}

@Test
void shouldConvertFromAuthenticationRoleBindingsType() {
Map<String, Object> attributes = new HashMap<>();
attributes.put(ROLE_BINDINGS, List.of(new AuthenticationRoleBinding("namespace", List.of(GET),
attributes.put(ROLE_BINDINGS, List.of(new AuthenticationRoleBinding(List.of("namespace"), List.of(GET),
List.of("topics"))));
Authentication authentication = Authentication.build("name", List.of("role"), attributes);

AuthenticationInfo authenticationInfo = AuthenticationInfo.of(authentication);

assertEquals("name", authenticationInfo.getName());
assertEquals(List.of("role"), authenticationInfo.getRoles().stream().toList());
assertIterableEquals(List.of(new AuthenticationRoleBinding("namespace", List.of(GET), List.of("topics"))),
authenticationInfo.getRoleBindings().stream().toList());
assertIterableEquals(List.of(new AuthenticationRoleBinding(List.of("namespace"), List.of(GET),
List.of("topics"))), authenticationInfo.getRoleBindings().stream().toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ void shouldReturnAuthenticationSuccessWhenAdminWithGroups() {
assertTrue(response.getAuthentication().get().getRoles().contains(ResourceBasedSecurityRule.IS_ADMIN));
assertTrue(response.getAuthentication().get().getAttributes()
.containsKey("roleBindings"));
assertEquals("ns1",
assertEquals(List.of("ns1"),
((List<AuthenticationRoleBinding>) response.getAuthentication().get().getAttributes()
.get("roleBindings")).getFirst()
.getNamespace());
.getNamespaces());
assertTrue(
((List<AuthenticationRoleBinding>) response.getAuthentication().get().getAttributes()
.get("roleBindings")).getFirst()
Expand Down Expand Up @@ -156,10 +156,10 @@ void shouldReturnAuthenticationSuccessWhenUserWithGroups() {
assertTrue(response.getAuthentication().get().getRoles().isEmpty());
assertTrue(response.getAuthentication().get().getAttributes()
.containsKey("roleBindings"));
assertEquals("ns1",
assertEquals(List.of("ns1"),
((List<AuthenticationRoleBinding>) response.getAuthentication().get().getAttributes()
.get("roleBindings")).getFirst()
.getNamespace());
.getNamespaces());
assertTrue(
((List<AuthenticationRoleBinding>) response.getAuthentication().get().getAttributes()
.get("roleBindings")).getFirst()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void authenticationSuccess() {
.thenReturn(Flux.fromIterable(groups));

AuthenticationRoleBinding authenticationRoleBinding = AuthenticationRoleBinding.builder()
.namespace("namespace")
.namespaces(List.of("namespace"))
.verbs(List.of(RoleBinding.Verb.GET))
.resourceTypes(List.of("topics"))
.build();
Expand Down Expand Up @@ -94,7 +94,7 @@ void authenticationSuccessAdmin() {
.thenReturn(Flux.fromIterable(groups));

AuthenticationRoleBinding authenticationRoleBinding = AuthenticationRoleBinding.builder()
.namespace("namespace")
.namespaces(List.of("namespace"))
.verbs(List.of(RoleBinding.Verb.GET))
.resourceTypes(List.of("topics"))
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class LdapAuthenticationMapperTest {
@SuppressWarnings("unchecked")
void shouldMapAttributesToAuthenticationResponse() {
AuthenticationRoleBinding authenticationRoleBinding = AuthenticationRoleBinding.builder()
.namespace("namespace")
.namespaces(List.of("namespace"))
.verbs(List.of(RoleBinding.Verb.GET))
.resourceTypes(List.of("topics"))
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void authenticateMatchUserMatchPassword() {
.build()));

AuthenticationRoleBinding authenticationRoleBinding = AuthenticationRoleBinding.builder()
.namespace("namespace")
.namespaces(List.of("namespace"))
.verbs(List.of(RoleBinding.Verb.GET))
.resourceTypes(List.of("topics"))
.build();
Expand Down

0 comments on commit bf53d85

Please sign in to comment.