Disallow overriding tenant domain on requests

4 files changed, 27 insertions, 26 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java
index 9c24dc815e0..431020b735b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java
@@ -62,7 +62,7 @@ public class AthenzFacade implements AccessControl {
     public Tenant createTenant(TenantSpec tenantSpec, Credentials credentials, List<Tenant> existing) {
         AthenzTenantSpec spec = (AthenzTenantSpec) tenantSpec;
         AthenzCredentials athenzCredentials = (AthenzCredentials) credentials;
-        AthenzDomain domain = athenzCredentials.domain();
+        AthenzDomain domain = spec.domain();
 
         verifyIsDomainAdmin(athenzCredentials.user().getIdentity(), domain);
 
@@ -94,43 +94,40 @@ public class AthenzFacade implements AccessControl {
     public Tenant updateTenant(TenantSpec tenantSpec, Credentials credentials, List<Tenant> existing, List<Application> applications) {
         AthenzTenantSpec spec = (AthenzTenantSpec) tenantSpec;
         AthenzCredentials athenzCredentials = (AthenzCredentials) credentials;
-        AthenzDomain domain = athenzCredentials.domain();
+        AthenzDomain newDomain = spec.domain();
+        AthenzDomain oldDomain = athenzCredentials.domain();
 
-        verifyIsDomainAdmin(athenzCredentials.user().getIdentity(), domain);
+        verifyIsDomainAdmin(athenzCredentials.user().getIdentity(), newDomain);
 
         Optional<Tenant> existingWithSameDomain = existing.stream()
                                                           .filter(tenant ->    tenant.type() == Tenant.Type.athenz
-                                                                            && domain.equals(((AthenzTenant) tenant).domain()))
+                                                                            && newDomain.equals(((AthenzTenant) tenant).domain()))
                                                           .findAny();
 
         Tenant tenant = AthenzTenant.create(spec.tenant(),
-                                            domain,
+                                            newDomain,
                                             spec.property(),
                                             spec.propertyId());
 
-        int index = existing.indexOf(tenant);
-        if (index == -1) throw new IllegalArgumentException("Cannot update a non-existent tenant.");
-        AthenzTenant oldTenant = (AthenzTenant) existing.get(index);
-
         if (existingWithSameDomain.isPresent()) { // Throw if domain taken by someone else, or do nothing if taken by this tenant.
-            if ( ! existingWithSameDomain.get().equals(oldTenant))
+            if ( ! existingWithSameDomain.get().equals(tenant)) // Equality by name.
                 throw new IllegalArgumentException("Could not create tenant '" + spec.tenant().value() +
                                                    "': The Athens domain '" +
-                                                   domain.getName() + "' is already connected to tenant '" +
+                                                   newDomain.getName() + "' is already connected to tenant '" +
                                                    existingWithSameDomain.get().name().value() + "'");
 
             return tenant; // Short-circuit here if domain is still the same.
         }
         else { // Delete and recreate tenant, and optionally application, resources in Athenz otherwise.
-            log("createTenancy(tenantDomain=%s, service=%s)", domain, service);
-            zmsClient.createTenancy(domain, service, athenzCredentials.token());
+            log("createTenancy(tenantDomain=%s, service=%s)", newDomain, service);
+            zmsClient.createTenancy(newDomain, service, athenzCredentials.token());
             for (Application application : applications)
-                createApplication(domain, application.id().application(), athenzCredentials.token());
+                createApplication(newDomain, application.id().application(), athenzCredentials.token());
 
-            log("deleteTenancy(tenantDomain=%s, service=%s)", oldTenant.domain(), service);
+            log("deleteTenancy(tenantDomain=%s, service=%s)", oldDomain, service);
             for (Application application : applications)
-                deleteApplication(oldTenant.domain(), application.id().application(), athenzCredentials.token());
-            zmsClient.deleteTenancy(oldTenant.domain(), service, athenzCredentials.token());
+                deleteApplication(oldDomain, application.id().application(), athenzCredentials.token());
+            zmsClient.deleteTenancy(oldDomain, service, athenzCredentials.token());
         }
 
         return tenant;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzAccessControlRequests.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzAccessControlRequests.java
index be79f5fb8d4..c85eea2f2e9 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzAccessControlRequests.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzAccessControlRequests.java
@@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 
 import java.security.Principal;
+import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Optional;
 
@@ -34,20 +35,16 @@ public class AthenzAccessControlRequests implements AccessControlRequests {
     @Override
     public TenantSpec specification(TenantName tenant, Inspector requestObject) {
         return new AthenzTenantSpec(tenant,
+                                    new AthenzDomain(required("athensDomain", requestObject)),
                                     new Property(required("property", requestObject)),
                                     optional("propertyId", requestObject).map(PropertyId::new));
     }
 
     @Override
     public Credentials credentials(TenantName tenant, Inspector requestObject, HttpRequest request) {
-        // Get domain from request if present, which it should be for create and update requests.
-        Optional<AthenzDomain> requestDomain = optional("athensDomain", requestObject).map(AthenzDomain::new);
-        // Otherwise the tenant should already exist, and we use the domain stored under the tenant.
-        Optional<AthenzDomain> tenantDomain = tenants.get(tenant).map(AthenzTenant.class::cast).map(AthenzTenant::domain);
-
         return new AthenzCredentials(requireAthenzPrincipal(request),
-                                     requestDomain.orElseGet(() -> tenantDomain
-                                             .orElseThrow(() -> new IllegalArgumentException("Must specify 'athensDomain'."))),
+                                     tenants.get(tenant).map(AthenzTenant.class::cast).map(AthenzTenant::domain)
+                                            .orElseGet(() -> new AthenzDomain(required("athensDomain", requestObject))),
                                      requireOktaAccessToken(request));
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzCredentials.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzCredentials.java
index d8ea9c8feeb..c9e3e249668 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzCredentials.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzCredentials.java
@@ -4,6 +4,8 @@ import com.yahoo.vespa.athenz.api.AthenzDomain;
 import com.yahoo.vespa.athenz.api.AthenzPrincipal;
 import com.yahoo.vespa.athenz.api.OktaAccessToken;
 
+import java.util.Optional;
+
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -27,7 +29,7 @@ public class AthenzCredentials extends Credentials {
     @Override
     public AthenzPrincipal user() { return (AthenzPrincipal) super.user(); }
 
-    /** Returns the Athenz domain on whose behalf this request is made. */
+    /** Returns the Athenz domain of the tenant on whose behalf this request is made. */
     public AthenzDomain domain() { return domain; }
 
     /** Returns the token proving access to the requested action under this domain. */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzTenantSpec.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzTenantSpec.java
index 0cacbf4ccdc..b122199bf05 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzTenantSpec.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AthenzTenantSpec.java
@@ -16,15 +16,20 @@ import static java.util.Objects.requireNonNull;
  */
 public class AthenzTenantSpec extends TenantSpec {
 
+    private final AthenzDomain domain;
     private final Property property;
     private final Optional<PropertyId> propertyId;
 
-    public AthenzTenantSpec(TenantName tenant, Property property, Optional<PropertyId> propertyId) {
+    public AthenzTenantSpec(TenantName tenant, AthenzDomain domain, Property property, Optional<PropertyId> propertyId) {
         super(tenant);
+        this.domain = domain;
         this.property = requireNonNull(property);
         this.propertyId = requireNonNull(propertyId);
     }
 
+    /** The domain to create this tenant under. */
+    public AthenzDomain domain() { return domain; }
+
     /** The property name of the tenant. */
     public Property property() { return property; }