Move Principal to RoleResolver.membership signature

3 files changed, 34 insertions, 22 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java
index fcd5ec9a64e..982442c028e 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java
@@ -34,6 +34,7 @@ import com.yahoo.yolean.chain.Provides;
 import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.InternalServerErrorException;
 import javax.ws.rs.WebApplicationException;
+import java.security.Principal;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -74,10 +75,10 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase {
     public Optional<ErrorResponse> filterRequest(DiscFilterRequest request) {
         try {
             Path path = new Path(request.getRequestURI());
-            Optional<AthenzPrincipal> principal = principalFrom(request);
             Action action = Action.from(HttpRequest.Method.valueOf(request.getMethod()));
-            AthenzRoleResolver resolver = new AthenzRoleResolver(principal, athenz, controller, path);
-            RoleMembership roles = resolver.membership();
+            RoleMembership roles = Optional.ofNullable(request.getUserPrincipal())
+                                           .map(new AthenzRoleResolver(athenz, controller, path)::membership)
+                                           .orElse(RoleMembership.everyone());
             if (!roles.allows(action, request.getRequestURI())) {
                 throw new ForbiddenException("Access denied");
             }
@@ -93,14 +94,12 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase {
     // TODO: Pull class up and resolve roles from roles defined in Athenz
     private static class AthenzRoleResolver implements RoleMembership.Resolver {
 
-        private final Optional<AthenzPrincipal> principal;
         private final AthenzFacade athenz;
         private final TenantController tenants;
         private final Path path;
         private final SystemName system;
 
-        public AthenzRoleResolver(Optional<AthenzPrincipal> principal, AthenzFacade athenz, Controller controller, Path path) {
-            this.principal = principal;
+        public AthenzRoleResolver(AthenzFacade athenz, Controller controller, Path path) {
             this.athenz = athenz;
             this.tenants = controller.tenants();
             this.path = path;
@@ -136,25 +135,26 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase {
         }
 
         @Override
-        public RoleMembership membership() {
+        public RoleMembership membership(Principal principal) {
+            if ( ! (principal instanceof AthenzPrincipal))
+                throw new IllegalStateException("Expected an AthenzPrincipal to be set on the request.");
+
+            AthenzIdentity identity = ((AthenzPrincipal) principal).getIdentity();
             Optional<Tenant> tenant = tenant(path);
             Context context = context(tenant);
             Set<Context> contexts = Set.of(context);
-            if (principal.isEmpty()) {
-                return new RoleMembership(Map.of(Role.everyone, contexts));
-            }
-            if (isHostedOperator(principal.get().getIdentity())) {
+            if (isHostedOperator(identity)) {
                 return new RoleMembership(Map.of(Role.hostedOperator, contexts));
             }
-            if (tenant.isPresent() && isTenantAdmin(principal.get().getIdentity(), tenant.get())) {
+            if (tenant.isPresent() && isTenantAdmin(identity, tenant.get())) {
                 return new RoleMembership(Map.of(Role.tenantAdmin, contexts));
             }
-            AthenzDomain principalDomain = principal.get().getIdentity().getDomain();
+            AthenzDomain principalDomain = identity.getDomain();
             if (principalDomain.equals(SCREWDRIVER_DOMAIN)) {
                 // NOTE: Only fine-grained deploy authorization for Athenz tenants
                 if (context.application().isPresent() && tenant.isPresent() && tenant.get() instanceof AthenzTenant) {
                     AthenzDomain tenantDomain = ((AthenzTenant) tenant.get()).domain();
-                    if (hasDeployerAccess(principal.get().getIdentity(), tenantDomain, context.application().get())) {
+                    if (hasDeployerAccess(identity, tenantDomain, context.application().get())) {
                         return new RoleMembership(Map.of(Role.tenantPipelineOperator, contexts));
                     }
                 } else {
@@ -173,7 +173,7 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase {
 
         // TODO: Currently there's only one context for each role, but this will change
         private Context context(Optional<Tenant> tenant) {
-            if (principal.isEmpty() || tenant.isEmpty()) {
+            if (tenant.isEmpty()) {
                 return Context.unlimitedIn(system);
             }
             // TODO: Remove this. Current behaviour always allows tenant full access to all its applications, but with
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java
index 588aac4e3ad..d3940ac631f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java
@@ -1,8 +1,13 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.role;
 
+import com.yahoo.config.provision.SystemName;
+
+import java.security.Principal;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * A list of roles and their associated contexts. This defines the role membership of a tenant, and in which contexts
@@ -12,12 +17,19 @@ import java.util.Set;
  */
 public class RoleMembership {
 
+    private static final RoleMembership everyone = new RoleMembership(Map.of(Role.everyone,
+                                                                             Stream.of(SystemName.values())
+                                                                                   .map(Context::unlimitedIn)
+                                                                                   .collect(Collectors.toUnmodifiableSet())));
+
     private final Map<Role, Set<Context>> roles;
 
     public RoleMembership(Map<Role, Set<Context>> roles) {
         this.roles = Map.copyOf(roles);
     }
 
+    public static RoleMembership everyone() { return everyone; }
+
     /** Returns whether any role in this allows action to take place in path */
     public boolean allows(Action action, String path) {
         return roles.entrySet().stream().anyMatch(kv -> {
@@ -37,7 +49,7 @@ public class RoleMembership {
      * membership to a {@link RoleMembership}.
      */
     public interface Resolver {
-        RoleMembership membership();
+        RoleMembership membership(Principal user);
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java
index a4d685dfb1e..b7c751638c8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java
@@ -5,6 +5,8 @@ import org.junit.Test;
 import java.util.LinkedHashSet;
 import java.util.Set;
 
+import static org.junit.Assert.fail;
+
 /**
  * @author jonmv
  * @author mpolden
@@ -19,8 +21,7 @@ public class PathGroupTest {
                 Set<String> overlapping = new LinkedHashSet<>(pg.pathSpecs);
                 overlapping.retainAll(pg2.pathSpecs);
                 if (!overlapping.isEmpty()) {
-                    throw new AssertionError("The following path specs overlap in " + pg + " and " + pg2 +
-                                             ": " + overlapping);
+                    fail("The following path specs overlap in " + pg + " and " + pg2 + ": " + overlapping);
                 }
             }
         }
@@ -29,7 +30,7 @@ public class PathGroupTest {
     @Test
     public void uniqueMatches() {
         // Ensure that each path group contains at most one match for any given path, to avoid undefined context extraction.
-        for (PathGroup group : PathGroup.values()) {
+        for (PathGroup group : PathGroup.values())
             for (String path1 : group.pathSpecs)
                 for (String path2 : group.pathSpecs) {
                     if (path1 == path2) continue;
@@ -43,13 +44,12 @@ public class PathGroupTest {
 
                     int i;
                     for (i = 0; i < end; i++)
-                        if (   ! parts1[i].equals(parts2[i])
+                        if (   !  parts1[i].equals(parts2[i])
                             && ! (parts1[i].startsWith("{") && parts1[i].endsWith("}"))
                             && ! (parts2[i].startsWith("{") && parts2[i].endsWith("}"))) break;
 
-                    if (i == end) throw new AssertionError("Paths '" + path1 + "' and '" + path2 +"' overlap.");
+                    if (i == end) fail("Paths '" + path1 + "' and '" + path2 + "' overlap.");
                 }
-        }
     }
 
 }