Merge pull request #4267 from vespa-engine/mortent/validate-identity

Validate athenz-domain in deployment.xml equal to tenant domain

5 files changed, 101 insertions, 8 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 d7324450d4c..82c607b89fc 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
@@ -21,7 +21,6 @@ import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
 import com.yahoo.vespa.config.SlimeUtils;
-import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.AlreadyExistsException;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -53,7 +52,6 @@ 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.configserver.ConfigServerException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
-import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId;
 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.application.ApplicationPackage;
@@ -783,17 +781,17 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
         Inspector deployOptions = SlimeUtils.jsonToSlime(dataParts.get("deployOptions")).get();
 
+        ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get("applicationZip"));
         DeployAuthorizer deployAuthorizer = new DeployAuthorizer(controller.zoneRegistry(), athenzClientFactory);
         Tenant tenant = controller.tenants().tenant(new TenantId(tenantName)).orElseThrow(() -> new NotExistsException(new TenantId(tenantName)));
         Principal principal = authorizer.getPrincipal(request);
-        deployAuthorizer.throwIfUnauthorizedForDeploy(principal, Environment.from(environment), tenant, applicationId);
+        deployAuthorizer.throwIfUnauthorizedForDeploy(principal, Environment.from(environment), tenant, applicationId, applicationPackage);
 
         // TODO: get rid of the json object
         DeployOptions deployOptionsJsonClass = new DeployOptions(screwdriverBuildJobFromSlime(deployOptions.field("screwdriverBuildJob")),
                                                                  optional("vespaVersion", deployOptions).map(Version::new),
                                                                  deployOptions.field("ignoreValidationErrors").asBool(),
                                                                  deployOptions.field("deployCurrentVersion").asBool());
-        ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get("applicationZip"));
         controller.applications().validate(applicationPackage.deploymentSpec());
         ActivateResult result = controller.applications().deployApplication(applicationId,
                                                                             zone,
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java
index 71126259417..2e627676766 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java
@@ -6,6 +6,7 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.api.Tenant;
 import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
+import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.athenz.ApplicationAction;
 import com.yahoo.vespa.hosted.controller.athenz.AthenzClientFactory;
 import com.yahoo.vespa.hosted.controller.athenz.AthenzPrincipal;
@@ -15,6 +16,7 @@ import com.yahoo.vespa.hosted.controller.athenz.ZmsException;
 import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.NotAuthorizedException;
 import java.security.Principal;
+import java.util.Objects;
 import java.util.logging.Logger;
 
 import static com.yahoo.vespa.hosted.controller.restapi.application.Authorizer.environmentRequiresAuthorization;
@@ -38,7 +40,21 @@ public class DeployAuthorizer {
     public void throwIfUnauthorizedForDeploy(Principal principal,
                                              Environment environment,
                                              Tenant tenant,
-                                             ApplicationId applicationId) {
+                                             ApplicationId applicationId,
+                                             ApplicationPackage applicationPackage) {
+        // Validate that domain in identity configuration (deployment.xml) is same as tenant domain
+        applicationPackage.deploymentSpec().athenzDomain().ifPresent(identityDomain -> {
+            AthenzDomain tenantDomain = tenant.getAthensDomain().orElseThrow(() -> new IllegalArgumentException("Identity provider only available to Athenz onboarded tenants"));
+            if (! Objects.equals(tenantDomain.id(), identityDomain.value())) {
+                throw new ForbiddenException(
+                        String.format(
+                                "Athenz domain in deployment.xml: [%s] must match tenant domain: [%s]",
+                                identityDomain.value(),
+                                tenantDomain.id()
+                        ));
+            }
+        });
+
         if (!environmentRequiresAuthorization(environment)) {
             return;
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
index 72bfa238094..a2b24864d1e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
@@ -2,6 +2,8 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.yahoo.config.application.api.ValidationId;
+import com.yahoo.config.provision.AthenzDomain;
+import com.yahoo.config.provision.AthenzService;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 
@@ -29,6 +31,7 @@ public class ApplicationPackageBuilder {
     private final StringBuilder environmentBody = new StringBuilder();
     private final StringBuilder validationOverridesBody = new StringBuilder();
     private final StringBuilder blockChange = new StringBuilder();
+    private String athenzIdentityAttributes = null;
     private String searchDefinition = "search test { }";
 
     public ApplicationPackageBuilder upgradePolicy(String upgradePolicy) {
@@ -83,6 +86,11 @@ public class ApplicationPackageBuilder {
         return this;
     }
 
+    public ApplicationPackageBuilder athenzIdentity(AthenzDomain domain, AthenzService service) {
+        this.athenzIdentityAttributes = String.format("athenz-domain='%s' athenz-service='%s'", domain.value(), service.value());
+        return this;
+    }
+
     /** Sets the content of the search definition test.sd */
     public ApplicationPackageBuilder searchDefinition(String testSearchDefinition) {
         this.searchDefinition = testSearchDefinition;
@@ -90,7 +98,12 @@ public class ApplicationPackageBuilder {
     }
     
     private byte[] deploymentSpec() {
-        StringBuilder xml = new StringBuilder("<deployment version='1.0'>\n");
+        StringBuilder xml = new StringBuilder();
+        xml.append("<deployment version='1.0' ");
+        if(athenzIdentityAttributes != null) {
+            xml.append(athenzIdentityAttributes);
+        }
+        xml.append(">\n");
         if (upgradePolicy != null) {
             xml.append("<upgrade policy='");
             xml.append(upgradePolicy);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
index 6c5120df515..71ad6560126 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
@@ -23,9 +23,11 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
+import com.yahoo.vespa.hosted.controller.athenz.ApplicationAction;
 import com.yahoo.vespa.hosted.controller.athenz.AthenzPrincipal;
-import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock;
+import com.yahoo.vespa.hosted.controller.athenz.AthenzUtils;
 import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock;
+import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock;
 import com.yahoo.vespa.hosted.controller.maintenance.JobControl;
 import com.yahoo.vespa.hosted.controller.maintenance.Upgrader;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
@@ -121,4 +123,16 @@ public class ContainerControllerTester {
         containerTester.assertResponse(request, expectedResponse, expectedStatusCode);
     }
 
+    /*
+     * Authorize action on tenantDomain/application for a given screwdriverId
+     */
+    public void authorize(AthenzDomain tenantDomain, ScrewdriverId screwdriverId, ApplicationAction action, Application application) {
+        AthenzClientFactoryMock mock = (AthenzClientFactoryMock) containerTester.container().components()
+                .getComponent(AthenzClientFactoryMock.class.getName());
+
+        mock.getSetup()
+                .domains.get(tenantDomain)
+                .applications.get(new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(application.id().application().value()))
+                .addRoleMember(action, AthenzUtils.createPrincipal(screwdriverId));
+    }
 }
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 bf4586f9fd0..7902e49288c 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
@@ -3,12 +3,14 @@ package com.yahoo.vespa.hosted.controller.restapi.application;
 
 import com.yahoo.application.container.handler.Request;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.AthenzService;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ConfigServerClientMock;
 import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain;
 import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
+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.MetricsService.ApplicationMetrics;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
@@ -21,10 +23,11 @@ import com.yahoo.vespa.hosted.controller.application.ClusterUtilization;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
+import com.yahoo.vespa.hosted.controller.athenz.ApplicationAction;
 import com.yahoo.vespa.hosted.controller.athenz.AthenzPrincipal;
 import com.yahoo.vespa.hosted.controller.athenz.AthenzUtils;
-import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock;
 import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock;
+import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester;
 import com.yahoo.vespa.hosted.controller.restapi.ContainerTester;
@@ -598,6 +601,55 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               403);
     }
 
+    @Test
+    public void deployment_fails_on_illegal_domain_in_deployment_spec() throws IOException {
+        ContainerControllerTester controllerTester = new ContainerControllerTester(container, responseFiles);
+        ContainerTester tester = controllerTester.containerTester();
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .upgradePolicy("default")
+                .athenzIdentity(com.yahoo.config.provision.AthenzDomain.from("invalid.domain"), AthenzService.from("service"))
+                .environment(Environment.prod)
+                .region("us-west-1")
+                .build();
+        long screwdriverProjectId = 123;
+        AthenzDomain domain = addTenantAthenzDomain(athenzUserDomain, "mytenant");
+
+        Application application = controllerTester.createApplication(athenzUserDomain, "tenant1", "application1");
+        controllerTester.authorize(domain, new ScrewdriverId(Long.toString(screwdriverProjectId)), ApplicationAction.deploy, application);
+
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default/", POST)
+                                      .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)))
+                                      .domain(athenzScrewdriverDomain).user("sd" + screwdriverProjectId),
+                              "{\"error-code\":\"FORBIDDEN\",\"message\":\"Athenz domain in deployment.xml: [invalid.domain] must match tenant domain: [domain1]\"}",
+                              403);
+
+    }
+
+    @Test
+    public void deployment_succeeds_when_correct_domain_is_used() throws IOException {
+        ContainerControllerTester controllerTester = new ContainerControllerTester(container, responseFiles);
+        ContainerTester tester = controllerTester.containerTester();
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .upgradePolicy("default")
+                .athenzIdentity(com.yahoo.config.provision.AthenzDomain.from("domain1"), AthenzService.from("service"))
+                .environment(Environment.prod)
+                .region("us-west-1")
+                .build();
+        long screwdriverProjectId = 123;
+        AthenzDomain domain = addTenantAthenzDomain(athenzUserDomain, "mytenant");
+
+        Application application = controllerTester.createApplication(athenzUserDomain, "tenant1", "application1");
+        controllerTester.authorize(domain, new ScrewdriverId(Long.toString(screwdriverProjectId)), ApplicationAction.deploy, application);
+
+        // Allow systemtest to succeed by notifying completion of system test
+        controllerTester.notifyJobCompletion(application.id(), screwdriverProjectId, true, DeploymentJobs.JobType.component);
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default/", POST)
+                                      .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)))
+                                      .domain(athenzScrewdriverDomain).user("sd" + screwdriverProjectId),
+                              new File("deploy-result.json"));
+
+    }
+
     private HttpEntity createApplicationDeployData(ApplicationPackage applicationPackage, Optional<Long> screwdriverJobId) {
         MultipartEntityBuilder builder = MultipartEntityBuilder.create();
         builder.addTextBody("deployOptions", deployOptions(screwdriverJobId), ContentType.APPLICATION_JSON);