Principal is always present and an Athenz principal

4 files changed, 18 insertions, 36 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index c2c7b942fab..7eb1a76fa6a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -10,7 +10,6 @@ import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
@@ -19,6 +18,11 @@ import com.yahoo.log.LogLevel;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
+import com.yahoo.vespa.athenz.api.AthenzDomain;
+import com.yahoo.vespa.athenz.api.AthenzIdentity;
+import com.yahoo.vespa.athenz.api.AthenzPrincipal;
+import com.yahoo.vespa.athenz.api.AthenzUser;
+import com.yahoo.vespa.athenz.api.NToken;
 import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.AlreadyExistsException;
 import com.yahoo.vespa.hosted.controller.Application;
@@ -36,7 +40,6 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.ScrewdriverBui
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RefeedAction;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RestartAction;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ServiceInfo;
-import com.yahoo.vespa.athenz.api.AthenzDomain;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.GitBranch;
 import com.yahoo.vespa.hosted.controller.api.identifiers.GitCommit;
@@ -48,10 +51,13 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.User;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus;
+import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.application.Change;
@@ -62,12 +68,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentCost;
 import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.application.SourceRevision;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
-import com.yahoo.vespa.athenz.api.AthenzIdentity;
-import com.yahoo.vespa.athenz.api.AthenzPrincipal;
-import com.yahoo.vespa.athenz.api.AthenzUser;
-import com.yahoo.vespa.athenz.api.NToken;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsException;
 import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse;
 import com.yahoo.vespa.hosted.controller.restapi.MessageResponse;
 import com.yahoo.vespa.hosted.controller.restapi.Path;
@@ -85,7 +85,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.security.Principal;
 import java.time.Duration;
 import java.util.Collections;
 import java.util.List;
@@ -780,7 +779,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                                                                   .map(ApplicationPackage::new);
         ApplicationInstanceAuthorizer applicationInstanceAuthorizer = new ApplicationInstanceAuthorizer(controller.zoneRegistry(), athenzClientFactory);
         Tenant tenant = controller.tenants().tenant(new TenantId(tenantName)).orElseThrow(() -> new NotExistsException(new TenantId(tenantName)));
-        Principal principal = authorizer.getPrincipal(request);
+        AthenzPrincipal principal = authorizer.getPrincipal(request);
         applicationInstanceAuthorizer.throwIfUnauthorizedForDeploy(principal, Environment.from(environment), tenant, applicationId, applicationPackage);
 
         // TODO: get rid of the json object
@@ -867,7 +866,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
 
     private Optional<UserId> userFrom(HttpRequest request) {
-        return authorizer.getPrincipalIfAny(request)
+        return Optional.of(authorizer.getPrincipal(request))
                 .map(AthenzPrincipal::getIdentity)
                 .filter(AthenzUser.class::isInstance)
                 .map(AthenzUser.class::cast)
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java
index 8a9ac88d3e5..e9a6afd0da8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java
@@ -16,7 +16,6 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 
 import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.NotAuthorizedException;
-import java.security.Principal;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Logger;
@@ -42,7 +41,7 @@ public class ApplicationInstanceAuthorizer {
         this.athenzClientFactory = athenzClientFactory;
     }
 
-    public void throwIfUnauthorizedForDeploy(Principal principal,
+    public void throwIfUnauthorizedForDeploy(AthenzPrincipal principal,
                                              Environment environment,
                                              Tenant tenant,
                                              ApplicationId applicationId,
@@ -65,18 +64,7 @@ public class ApplicationInstanceAuthorizer {
             return;
         }
 
-        if (principal == null) {
-            throw loggedUnauthorizedException("Principal not authenticated!");
-        }
-
-        if (!(principal instanceof AthenzPrincipal)) {
-            throw loggedUnauthorizedException(
-                    "Principal '%s' of type '%s' is not an Athenz principal, which is required for production deployments.",
-                    principal.getName(), principal.getClass().getSimpleName());
-        }
-
-        AthenzPrincipal athenzPrincipal = (AthenzPrincipal) principal;
-        AthenzDomain principalDomain = athenzPrincipal.getDomain();
+        AthenzDomain principalDomain = principal.getDomain();
 
         if (!principalDomain.equals(SCREWDRIVER_DOMAIN)) {
             throw loggedForbiddenException(
@@ -91,12 +79,12 @@ public class ApplicationInstanceAuthorizer {
         // NOTE: no fine-grained deploy authorization for non-Athenz tenants
         if (tenant.isAthensTenant()) {
             AthenzDomain tenantDomain = tenant.getAthensDomain().get();
-            if (!hasDeployAccessToAthenzApplication(athenzPrincipal, tenantDomain, applicationId)) {
+            if (!hasDeployAccessToAthenzApplication(principal, tenantDomain, applicationId)) {
                 throw loggedForbiddenException(
                         "Screwdriver principal '%1$s' does not have deploy access to '%2$s'. " +
                         "Either the application has not been created at " + zoneRegistry.getDashboardUri() + " or " +
                         "'%1$s' is not added to the application's deployer role in Athenz domain '%3$s'.",
-                        athenzPrincipal.getIdentity().getFullName(), applicationId, tenantDomain.getName());
+                        principal.getIdentity().getFullName(), applicationId, tenantDomain.getName());
             }
         }
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
index 06d078e8a36..f7bbde60086 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
@@ -66,18 +66,14 @@ public class Authorizer {
 
     /** Returns the principal or throws forbidden */ // TODO: Avoid REST exceptions
     public AthenzPrincipal getPrincipal(HttpRequest request) {
-        return getPrincipalIfAny(request).orElseThrow(() -> Authorizer.loggedForbiddenException("User is not authenticated"));
-    }
-
-    /** Returns the principal if there is any */
-    public Optional<AthenzPrincipal> getPrincipalIfAny(HttpRequest request) {
         return securityContextOf(request)
                 .map(SecurityContext::getUserPrincipal)
-                .map(AthenzPrincipal.class::cast);
+                .map(AthenzPrincipal.class::cast)
+                .orElseThrow(() -> loggedForbiddenException("User is not authenticated"));
     }
 
     public Optional<NToken> getNToken(HttpRequest request) {
-        return getPrincipalIfAny(request).flatMap(AthenzPrincipal::getNToken);
+        return getPrincipal(request).getNToken();
     }
 
     public boolean isSuperUser(HttpRequest request) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/MockAuthorizer.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/MockAuthorizer.java
index d0f5f4dbdb9..1e284c11c93 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/MockAuthorizer.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/MockAuthorizer.java
@@ -30,8 +30,7 @@ public class MockAuthorizer extends Authorizer {
     }
 
     /** Returns a principal given by the request parameters 'domain' and 'user' */
-    @Override
-    public Optional<AthenzPrincipal> getPrincipalIfAny(HttpRequest request) {
+    private static Optional<AthenzPrincipal> getPrincipalIfAny(HttpRequest request) {
         String domain = request.getHeader("Athenz-Identity-Domain");
         String name = request.getHeader("Athenz-Identity-Name");
         if (domain == null || name == null) return Optional.empty();