Use roles for authorization

6 files changed, 128 insertions, 179 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 309c37af341..3cbfcfc7795 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
@@ -3,8 +3,9 @@ package com.yahoo.vespa.hosted.controller.restapi.filter;
 
 import com.google.inject.Inject;
 import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.jdisc.http.HttpRequest.Method;
+import com.yahoo.jdisc.http.HttpRequest;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
 import com.yahoo.jdisc.http.filter.security.cors.CorsFilterConfig;
 import com.yahoo.jdisc.http.filter.security.cors.CorsRequestFilterBase;
@@ -20,6 +21,10 @@ import com.yahoo.vespa.hosted.controller.TenantController;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
 import com.yahoo.vespa.hosted.controller.athenz.ApplicationAction;
 import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade;
+import com.yahoo.vespa.hosted.controller.role.Action;
+import com.yahoo.vespa.hosted.controller.role.Context;
+import com.yahoo.vespa.hosted.controller.role.Role;
+import com.yahoo.vespa.hosted.controller.role.RoleMembership;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 import com.yahoo.vespa.hosted.controller.tenant.UserTenant;
@@ -28,18 +33,12 @@ import com.yahoo.yolean.chain.Provides;
 
 import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.InternalServerErrorException;
-import javax.ws.rs.NotAuthorizedException;
 import javax.ws.rs.WebApplicationException;
-import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Logger;
 
-import static com.yahoo.jdisc.http.HttpRequest.Method.GET;
-import static com.yahoo.jdisc.http.HttpRequest.Method.HEAD;
-import static com.yahoo.jdisc.http.HttpRequest.Method.OPTIONS;
-import static com.yahoo.jdisc.http.HttpRequest.Method.POST;
-import static com.yahoo.jdisc.http.HttpRequest.Method.PUT;
 import static com.yahoo.vespa.hosted.controller.athenz.HostedAthenzIdentities.SCREWDRIVER_DOMAIN;
 
 /**
@@ -51,50 +50,36 @@ import static com.yahoo.vespa.hosted.controller.athenz.HostedAthenzIdentities.SC
 @Provides("ControllerAuthorizationFilter")
 public class ControllerAuthorizationFilter extends CorsRequestFilterBase {
 
-    private static final List<Method> WHITELISTED_METHODS = List.of(GET, OPTIONS, HEAD);
-
     private static final Logger log = Logger.getLogger(ControllerAuthorizationFilter.class.getName());
 
     private final AthenzFacade athenz;
-    private final TenantController tenantController;
+    private final Controller controller;
 
     @Inject
     public ControllerAuthorizationFilter(AthenzClientFactory clientFactory,
                                          Controller controller,
                                          CorsFilterConfig corsConfig) {
-        super(corsConfig);
-        this.athenz = new AthenzFacade(clientFactory);
-        this.tenantController = controller.tenants();
+        this(clientFactory, controller, Set.copyOf(corsConfig.allowedUrls()));
     }
 
     ControllerAuthorizationFilter(AthenzClientFactory clientFactory,
-                                  TenantController tenantController,
+                                  Controller controller,
                                   Set<String> allowedUrls) {
         super(allowedUrls);
         this.athenz = new AthenzFacade(clientFactory);;
-        this.tenantController = tenantController;
+        this.controller = controller;
     }
 
-    // NOTE: Be aware of the ordering of the path pattern matching. Semantics may change if the patterns are evaluated
-    //       in different order.
     @Override
     public Optional<ErrorResponse> filterRequest(DiscFilterRequest request) {
-        Method method = getMethod(request);
-        if (isWhiteListedMethod(method)) return Optional.empty();
-
         try {
             Path path = new Path(request.getRequestURI());
-            AthenzPrincipal principal = getPrincipalOrThrow(request);
-            if (isWhiteListedOperation(path, method)) {
-                // no authz check
-            } else if (isHostedOperatorOperation(path, method)) {
-                verifyIsHostedOperator(principal);
-            } else if (isTenantAdminOperation(path, method)) {
-                verifyIsTenantAdmin(principal, getTenantName(path));
-            } else if (isTenantPipelineOperation(path, method)) {
-                verifyIsTenantPipelineOperator(principal, getTenantName(path), getApplicationName(path));
-            } else {
-                throw new ForbiddenException("No access control is declared for path: '" + path.asString() + "'");
+            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();
+            if (!roles.allow(action, request.getRequestURI())) {
+                throw new ForbiddenException("Access denied");
             }
             return Optional.empty();
         } catch (WebApplicationException e) {
@@ -105,146 +90,112 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase {
         }
     }
 
-    private static boolean isWhiteListedMethod(Method method) {
-        return WHITELISTED_METHODS.contains(method);
-    }
-
-    private static boolean isWhiteListedOperation(Path path, Method method) {
-        return path.matches("/application/v4/user") && method == PUT || // Create user tenant
-               path.matches("/application/v4/tenant/{tenant}") && method == POST; // Create tenant
-    }
-
-    private static boolean isHostedOperatorOperation(Path path, Method method) {
-        if (isWhiteListedOperation(path, method)) return false;
-        return path.matches("/controller/v1/{*}") ||
-               path.matches("/provision/v2/{*}") ||
-               path.matches("/flags/v1/{*}") ||
-               path.matches("/os/v1/{*}") ||
-               path.matches("/zone/v2/{*}") ||
-               path.matches("/nodes/v2/{*}") ||
-               path.matches("/orchestrator/v1/{*}");
-    }
+    // TODO: Pull class up and resolve roles from roles defined in Athenz
+    private static class AthenzRoleResolver implements RoleMembership.Resolver {
 
-    private static boolean isTenantAdminOperation(Path path, Method method) {
-        if (isHostedOperatorOperation(path, method)) return false;
-        return path.matches("/application/v4/tenant/{tenant}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/{*}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{job}/{*}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/dev/{*}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/perf/{*}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override");
-    }
-
-    private static boolean isTenantPipelineOperation(Path path, Method method) {
-        if (isTenantAdminOperation(path, method)) return false;
-        return path.matches("/application/v4/tenant/{tenant}/application/{application}/jobreport") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/submit") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/promote") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/prod/{*}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/test/{*}") ||
-               path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/staging/{*}");
-    }
+        private final Optional<AthenzPrincipal> principal;
+        private final AthenzFacade athenz;
+        private final TenantController tenants;
+        private final Path path;
+        private final SystemName system;
 
-    private void verifyIsHostedOperator(AthenzPrincipal principal) {
-        if (!isHostedOperator(principal.getIdentity())) {
-            throw new ForbiddenException("Vespa operator role required");
+        public AthenzRoleResolver(Optional<AthenzPrincipal> principal, AthenzFacade athenz, Controller controller, Path path) {
+            this.principal = principal;
+            this.athenz = athenz;
+            this.tenants = controller.tenants();
+            this.path = path;
+            this.system = controller.system();
         }
-    }
-
-    private boolean isHostedOperator(AthenzIdentity identity) {
-        return athenz.hasHostedOperatorAccess(identity);
-    }
-
-    private void verifyIsTenantAdmin(AthenzPrincipal principal, TenantName name) {
-        tenantController.get(name)
-                .ifPresent(tenant -> {
-                    if (!isTenantAdmin(principal.getIdentity(), tenant)) {
-                        throw new ForbiddenException("Tenant admin or Vespa operator role required");
-                    }
-                });
-    }
 
-    private boolean isTenantAdmin(AthenzIdentity identity, Tenant tenant) {
-        if (tenant instanceof AthenzTenant) {
-            return athenz.hasTenantAdminAccess(identity, ((AthenzTenant) tenant).domain());
-        } else if (tenant instanceof UserTenant) {
-            if (!(identity instanceof AthenzUser)) {
-                return false;
+        private boolean isTenantAdmin(AthenzIdentity identity, Tenant tenant) {
+            if (tenant instanceof AthenzTenant) {
+                return athenz.hasTenantAdminAccess(identity, ((AthenzTenant) tenant).domain());
+            } else if (tenant instanceof UserTenant) {
+                if (!(identity instanceof AthenzUser)) {
+                    return false;
+                }
+                AthenzUser user = (AthenzUser) identity;
+                return ((UserTenant) tenant).is(user.getName()) || isHostedOperator(identity);
             }
-            AthenzUser user = (AthenzUser) identity;
-            return ((UserTenant) tenant).is(user.getName()) || isHostedOperator(identity);
+            throw new InternalServerErrorException("Unknown tenant type: " + tenant.getClass().getSimpleName());
         }
-        throw new InternalServerErrorException("Unknown tenant type: " + tenant.getClass().getSimpleName());
-    }
 
-    private void verifyIsTenantPipelineOperator(AthenzPrincipal principal,
-                                                TenantName name,
-                                                ApplicationName application) {
-        tenantController.get(name)
-                .ifPresent(tenant -> verifyIsTenantPipelineOperator(principal.getIdentity(), tenant, application));
-    }
-
-    private void verifyIsTenantPipelineOperator(AthenzIdentity identity, Tenant tenant, ApplicationName application) {
-        if (isHostedOperator(identity)) return;
+        private boolean hasDeployerAccess(AthenzIdentity identity, AthenzDomain tenantDomain, ApplicationName application) {
+            try {
+                return athenz.hasApplicationAccess(identity,
+                                                   ApplicationAction.deploy,
+                                                   tenantDomain,
+                                                   application);
+            } catch (ZmsClientException e) {
+                throw new InternalServerErrorException("Failed to authorize operation:  (" + e.getMessage() + ")", e);
+            }
+        }
 
-        AthenzDomain principalDomain = identity.getDomain();
-        if (!principalDomain.equals(SCREWDRIVER_DOMAIN)) {
-            throw new ForbiddenException(String.format(
-                    "'%s' is not a Screwdriver identity. Only Screwdriver is allowed to deploy to this environment.",
-                    identity.getFullName()));
+        private boolean isHostedOperator(AthenzIdentity identity) {
+            return athenz.hasHostedOperatorAccess(identity);
         }
 
-        // NOTE: no fine-grained deploy authorization for non-Athenz tenants
-        if (tenant instanceof AthenzTenant) {
-            AthenzDomain tenantDomain = ((AthenzTenant) tenant).domain();
-            if (!hasDeployerAccess(identity, tenantDomain, application)) {
-                throw new ForbiddenException(String.format(
-                        "'%1$s' does not have access to '%2$s'. " +
+        @Override
+        public RoleMembership membership() {
+            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())) {
+                return new RoleMembership(Map.of(Role.hostedOperator, contexts));
+            }
+            if (tenant.isPresent() && isTenantAdmin(principal.get().getIdentity(), tenant.get())) {
+                return new RoleMembership(Map.of(Role.tenantAdmin, contexts));
+            }
+            AthenzDomain principalDomain = principal.get().getIdentity().getDomain();
+            if (principalDomain.equals(SCREWDRIVER_DOMAIN)) {
+                // NOTE: Only fine-grained deploy authorization for Athenz tenants
+                Optional<ApplicationName> application = context.application();
+                if (application.isPresent() && tenant.isPresent() && tenant.get() instanceof AthenzTenant) {
+                    AthenzDomain tenantDomain = ((AthenzTenant) tenant.get()).domain();
+                    if (!hasDeployerAccess(principal.get().getIdentity(), tenantDomain, application.get())) {
+                        throw new ForbiddenException(String.format(
+                                "'%1$s' does not have access to '%2$s'. " +
                                 "Either the application has not been created at Vespa dashboard or " +
                                 "'%1$s' is not added to the application's deployer role in Athenz domain '%3$s'.",
-                        identity.getFullName(), application.value(), tenantDomain.getName()));
+                                principal.get().getIdentity().getFullName(), application.get().value(), tenantDomain.getName()));
+                    }
+                }
+                return new RoleMembership(Map.of(Role.tenantPipelineOperator, contexts));
             }
+            return new RoleMembership(Map.of(Role.everyone, contexts));
         }
-    }
 
-    private boolean hasDeployerAccess(AthenzIdentity identity, AthenzDomain tenantDomain, ApplicationName application) {
-        try {
-            return athenz
-                    .hasApplicationAccess(
-                            identity,
-                            ApplicationAction.deploy,
-                            tenantDomain,
-                            application);
-        } catch (ZmsClientException e) {
-            throw new InternalServerErrorException("Failed to authorize operation:  (" + e.getMessage() + ")", e);
+        private Optional<Tenant> tenant(Path path) {
+            if (!path.matches("/application/v4/tenant/{tenant}/{*}")) {
+                return Optional.empty();
+            }
+            return tenants.get(TenantName.from(path.get("tenant")));
         }
-    }
 
-    private static TenantName getTenantName(Path path) {
-        if (!path.matches("/application/v4/tenant/{tenant}/{*}"))
-            throw new InternalServerErrorException("Unable to handle path: " + path.asString());
-        return TenantName.from(path.get("tenant"));
-    }
-
-    private static ApplicationName getApplicationName(Path path) {
-        if (!path.matches("/application/v4/tenant/{tenant}/application/{application}/{*}"))
-            throw new InternalServerErrorException("Unable to handle path: " + path.asString());
-        return ApplicationName.from(path.get("application"));
-    }
-
-    private static Method getMethod(DiscFilterRequest request) {
-        return Method.valueOf(request.getMethod().toUpperCase());
-    }
-
-    private static AthenzPrincipal getPrincipalOrThrow(DiscFilterRequest request) {
-        return getPrincipal(request)
-                .orElseThrow(() -> new NotAuthorizedException("User not authenticated"));
+        // 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()) {
+                return Context.empty(system);
+            }
+            if (!isTenantAdmin(principal.get().getIdentity(), tenant.get())) {
+                return Context.empty(system);
+            }
+            // TODO: Remove this. Current behaviour always allows tenant full access to all its applications, but with
+            //       the new role setup, each role will include a complete context (tenant + app)
+            Optional<ApplicationName> application = Optional.empty();
+            if (path.matches("/application/v4/tenant/{tenant}/application/{application}/{*}")) {
+                application = Optional.of(ApplicationName.from(path.get("application")));
+            }
+            return Context.of(tenant.get().name(), application, system);
+        }
     }
 
-    private static Optional<AthenzPrincipal> getPrincipal(DiscFilterRequest request) {
+    private static Optional<AthenzPrincipal> principalFrom(DiscFilterRequest request) {
         return Optional.ofNullable(request.getUserPrincipal())
-                .map(AthenzPrincipal.class::cast);
+                       .map(AthenzPrincipal.class::cast);
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index a6573f61c29..de7ed754f03 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -382,10 +382,10 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               new File("application1-recursive.json"));
 
         // GET logs
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application//application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214", GET)
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214", GET)
                                         .userIdentity(USER_ID),
                                         new File("logs.json"));
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application//application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214&streaming", GET)
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214&streaming", GET)
                         .userIdentity(USER_ID),
                 "INFO - All good");
 
@@ -674,6 +674,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         // Setup
         tester.computeVersionStatus();
         createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID);
+        addUserToHostedOperatorRole(HostedAthenzIdentities.from(HOSTED_VESPA_OPERATOR));
 
         // Create tenant
         tester.assertResponse(request("/application/v4/tenant/tenant1", POST).userIdentity(USER_ID)
@@ -703,19 +704,19 @@ public class ApplicationApiTest extends ControllerContainerTest {
         HttpEntity noAppEntity = createApplicationDeployData(Optional.empty(), true);
         tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/routing/environment/prod/region/us-central-1/instance/default/deploy", POST)
                                       .data(noAppEntity)
-                                      .userIdentity(USER_ID),
+                                      .userIdentity(HOSTED_VESPA_OPERATOR),
                               "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Deployment of system applications during a system upgrade is not allowed\"}",
                               400);
         tester.upgradeSystem(tester.controller().versionStatus().controllerVersion().get().versionNumber());
         tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/routing/environment/prod/region/us-central-1/instance/default/deploy", POST)
                         .data(noAppEntity)
-                        .userIdentity(USER_ID),
+                        .userIdentity(HOSTED_VESPA_OPERATOR),
                 new File("deploy-result.json"));
 
         // POST (deploy) a system application without an application package
         tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/proxy-host/environment/prod/region/us-central-1/instance/default/deploy", POST)
                         .data(noAppEntity)
-                        .userIdentity(USER_ID),
+                        .userIdentity(HOSTED_VESPA_OPERATOR),
                 new File("deploy-no-deployment.json"), 400);
     }
 
@@ -809,13 +810,13 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.computeVersionStatus();
         createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID);
 
-        // PUT (update) non-existing tenant
+        // PUT (update) non-existing tenant returns 403 as tenant access cannot be determined when the tenant does not exist
         tester.assertResponse(request("/application/v4/tenant/tenant1", PUT)
                                       .userIdentity(USER_ID)
                                       .oktaAccessToken(OKTA_AT)
                                       .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}"),
-                              "{\"error-code\":\"NOT_FOUND\",\"message\":\"Tenant 'tenant1' does not exist\"}",
-                              404);
+                              "{\n  \"code\" : 403,\n  \"message\" : \"Access denied\"\n}",
+                              403);
 
         // GET non-existing tenant
         tester.assertResponse(request("/application/v4/tenant/tenant1", GET)
@@ -936,20 +937,21 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .userIdentity(USER_ID),
                               "{\"error-code\":\"NOT_FOUND\",\"message\":\"Could not delete application 'tenant1.application1': Application not found\"}",
                               404);
+
         // DELETE tenant
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE)
                                       .userIdentity(USER_ID)
                                       .oktaAccessToken(OKTA_AT),
                               new File("tenant-without-applications.json"));
-        // DELETE tenant again - should produce 404
+        // DELETE tenant again returns 403 as tenant access cannot be determined when the tenant does not exist
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE)
                                       .userIdentity(USER_ID),
-                              "{\"error-code\":\"NOT_FOUND\",\"message\":\"Could not delete tenant 'tenant1': Tenant not found\"}",
-                              404);
+                              "{\n  \"code\" : 403,\n  \"message\" : \"Access denied\"\n}",
+                              403);
 
         // Promote application chef env for nonexistent tenant/application
         tester.assertResponse(request("/application/v4/tenant/dontexist/application/dontexist/environment/prod/region/us-west-1/instance/default/promote", POST)
-                                      .userIdentity(USER_ID),
+                                      .screwdriverIdentity(SCREWDRIVER_ID),
                               "{\"error-code\":\"INTERNAL_SERVER_ERROR\",\"message\":\"Unable to promote Chef environments for application\"}",
                               500);
 
@@ -1005,7 +1007,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", POST)
                                       .userIdentity(unauthorizedUser)
                                       .oktaAccessToken(OKTA_AT),
-                              "{\n  \"code\" : 403,\n  \"message\" : \"Tenant admin or Vespa operator role required\"\n}",
+                              "{\n  \"code\" : 403,\n  \"message\" : \"Access denied\"\n}",
                               403);
 
         // (Create it with the right tenant id)
@@ -1020,13 +1022,13 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
-                              "{\n  \"code\" : 403,\n  \"message\" : \"'user.myuser' is not a Screwdriver identity. Only Screwdriver is allowed to deploy to this environment.\"\n}",
+                              "{\n  \"code\" : 403,\n  \"message\" : \"Access denied\"\n}",
                               403);
 
         // Deleting an application for an Athens domain the user is not admin for is disallowed
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", DELETE)
                                       .userIdentity(unauthorizedUser),
-                              "{\n  \"code\" : 403,\n  \"message\" : \"Tenant admin or Vespa operator role required\"\n}",
+                              "{\n  \"code\" : 403,\n  \"message\" : \"Access denied\"\n}",
                               403);
 
         // (Deleting it with the right tenant id)
@@ -1040,7 +1042,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant1", PUT)
                                       .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}")
                                       .userIdentity(unauthorizedUser),
-                              "{\n  \"code\" : 403,\n  \"message\" : \"Tenant admin or Vespa operator role required\"\n}",
+                              "{\n  \"code\" : 403,\n  \"message\" : \"Access denied\"\n}",
                               403);
         
         // Change Athens domain
@@ -1055,7 +1057,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         // Deleting a tenant for an Athens domain the user is not admin for is disallowed
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE)
                                       .userIdentity(unauthorizedUser),
-                              "{\n  \"code\" : 403,\n  \"message\" : \"Tenant admin or Vespa operator role required\"\n}",
+                              "{\n  \"code\" : 403,\n  \"message\" : \"Access denied\"\n}",
                               403);
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java
index 3b47ba7bc71..fc73590e448 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java
@@ -10,6 +10,9 @@ import org.junit.Test;
 
 import java.io.File;
 
+/**
+ * @author jonmv
+ */
 public class AthenzApiTest extends ControllerContainerTest {
 
     private static final String responseFiles = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/";
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java
index 60ba8235b39..6eb7663e4ab 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java
@@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.controller.restapi.deployment;
 import com.yahoo.config.provision.AthenzService;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.Application;
-import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId;
-import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java
index 1dc0e14d447..1fe17379c71 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java
@@ -25,13 +25,13 @@ import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 import static com.yahoo.container.jdisc.RequestHandlerTestDriver.MockResponseHandler;
 import static com.yahoo.jdisc.http.HttpRequest.Method.DELETE;
 import static com.yahoo.jdisc.http.HttpRequest.Method.POST;
 import static com.yahoo.jdisc.http.HttpRequest.Method.PUT;
 import static com.yahoo.jdisc.http.HttpResponse.Status.FORBIDDEN;
-import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -160,8 +160,8 @@ public class ControllerAuthorizationFilterTest {
 
     private static ControllerAuthorizationFilter createFilter(ControllerTester controllerTester) {
         return new ControllerAuthorizationFilter(new AthenzClientFactoryMock(controllerTester.athenzDb()),
-                                                 controllerTester.controller().tenants(),
-                                                 singleton("http://localhost"));
+                                                 controllerTester.controller(),
+                                                 Set.of("http://localhost"));
     }
 
     private static Optional<AuthorizationResponse> invokeFilter(ControllerAuthorizationFilter filter,
diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java
index eafe17153ad..b565ad374ed 100644
--- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java
+++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java
@@ -5,7 +5,6 @@ import com.yahoo.jdisc.Response;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
 import com.yahoo.jdisc.http.filter.security.base.JsonSecurityRequestFilterBase;
 
-import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
 
@@ -21,10 +20,6 @@ public abstract class CorsRequestFilterBase extends JsonSecurityRequestFilterBas
 
     private final Set<String> allowedUrls;
 
-    protected CorsRequestFilterBase(CorsFilterConfig config) {
-        this(new HashSet<>(config.allowedUrls()));
-    }
-
     protected CorsRequestFilterBase(Set<String> allowedUrls) {
         this.allowedUrls = allowedUrls;
     }