Merge pull request #9139 from vespa-engine/mortent/allow-multiple-roles

Allow principal to have both tenantadmin and deployer roles

2 files changed, 19 insertions, 4 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java
index 15cdf034ca0..fa43b56b937 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java
@@ -17,15 +17,16 @@ import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.TenantController;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
 import com.yahoo.vespa.hosted.controller.api.role.Role;
+import com.yahoo.vespa.hosted.controller.api.role.SecurityContext;
 import com.yahoo.vespa.hosted.controller.athenz.ApplicationAction;
 import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade;
-import com.yahoo.vespa.hosted.controller.api.role.SecurityContext;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 import com.yahoo.vespa.hosted.controller.tenant.UserTenant;
 import com.yahoo.yolean.Exceptions;
 
 import java.net.URI;
+import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Logger;
@@ -79,16 +80,20 @@ public class AthenzRoleFilter extends CorsRequestFilterBase { // TODO: No need f
         if (athenz.hasHostedOperatorAccess(identity))
             return Set.of(Role.hostedOperator());
 
+        // A principal can be both tenant admin and tenantPipeline
+        Set<Role> roleMemberships = new HashSet<>();
         if (tenant.isPresent() && isTenantAdmin(identity, tenant.get()))
-            return Set.of(Role.athenzTenantAdmin(tenant.get().name()));
+            roleMemberships.add(Role.athenzTenantAdmin(tenant.get().name()));
 
         if (identity.getDomain().equals(SCREWDRIVER_DOMAIN) && application.isPresent() && tenant.isPresent())
             // NOTE: Only fine-grained deploy authorization for Athenz tenants
             if (   tenant.get().type() != Tenant.Type.athenz
                 || hasDeployerAccess(identity, ((AthenzTenant) tenant.get()).domain(), application.get()))
-                    return Set.of(Role.tenantPipeline(tenant.get().name(), application.get()));
+                    roleMemberships.add(Role.tenantPipeline(tenant.get().name(), application.get()));
 
-        return Set.of(Role.everyone());
+        return roleMemberships.isEmpty()
+                ? Set.of(Role.everyone())
+                : Set.copyOf(roleMemberships);
     }
 
     private boolean isTenantAdmin(AthenzIdentity identity, Tenant tenant) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java
index e36a02f387c..1efc3e856e3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java
@@ -34,6 +34,7 @@ public class AthenzRoleFilterTest {
     private static final AthenzDomain TENANT_DOMAIN2 = new AthenzDomain("tenantdomain2");
     private static final AthenzPrincipal TENANT_ADMIN = new AthenzPrincipal(new AthenzService(TENANT_DOMAIN, "adminservice"));
     private static final AthenzPrincipal TENANT_PIPELINE = new AthenzPrincipal(HostedAthenzIdentities.from(new ScrewdriverId("12345")));
+    private static final AthenzPrincipal TENANT_ADMIN_AND_PIPELINE = new AthenzPrincipal(HostedAthenzIdentities.from(new ScrewdriverId("56789")));
     private static final TenantName TENANT = TenantName.from("mytenant");
     private static final TenantName TENANT2 = TenantName.from("othertenant");
     private static final ApplicationName APPLICATION = ApplicationName.from("myapp");
@@ -58,7 +59,9 @@ public class AthenzRoleFilterTest {
         tester.createApplication(TENANT, APPLICATION.value(), "default", 12345);
         AthenzDbMock.Domain tenantDomain = tester.athenzDb().domains.get(TENANT_DOMAIN);
         tenantDomain.admins.add(TENANT_ADMIN.getIdentity());
+        tenantDomain.admins.add(TENANT_ADMIN_AND_PIPELINE.getIdentity());
         tenantDomain.applications.get(new ApplicationId(APPLICATION.value())).addRoleMember(ApplicationAction.deploy, TENANT_PIPELINE.getIdentity());
+        tenantDomain.applications.get(new ApplicationId(APPLICATION.value())).addRoleMember(ApplicationAction.deploy, TENANT_ADMIN_AND_PIPELINE.getIdentity());
         tester.createTenant(TENANT2.value(), TENANT_DOMAIN2.getName(), null);
         tester.createApplication(TENANT2, APPLICATION.value(), "default", 42);
     }
@@ -105,6 +108,13 @@ public class AthenzRoleFilterTest {
         assertEquals(Set.of(Role.everyone()),
                      filter.roles(TENANT_PIPELINE, APPLICATION2_CONTEXT_PATH));
 
+        // Principals member of both tenantPipeline and tenantAdmin roles get correct roles
+        assertEquals(Set.of(Role.athenzTenantAdmin(TENANT)),
+                     filter.roles(TENANT_ADMIN_AND_PIPELINE, TENANT_CONTEXT_PATH));
+
+        assertEquals(Set.of(Role.athenzTenantAdmin(TENANT), Role.tenantPipeline(TENANT, APPLICATION)),
+                     filter.roles(TENANT_ADMIN_AND_PIPELINE, APPLICATION_CONTEXT_PATH));
+
         // Unprivileged users are just members of the everyone role.
         assertEquals(Set.of(Role.everyone()),
                      filter.roles(USER, NO_CONTEXT_PATH));