Merge pull request #23690 from vespa-engine/jonmv/compare-same-principal-types

Compare apples to apples MERGEOK

3 files changed, 13 insertions, 1 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java
index ac7c6319c1b..a340982bec0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
 import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact;
 import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore;
+import com.yahoo.vespa.hosted.controller.api.role.SimplePrincipal;
 import com.yahoo.vespa.hosted.controller.tenant.ArchiveAccess;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
@@ -154,8 +155,11 @@ public abstract class LockedTenant {
 
         public Cloud withDeveloperKey(PublicKey key, Principal principal) {
             BiMap<PublicKey, Principal> keys = HashBiMap.create(developerKeys);
+            principal = new SimplePrincipal(principal.getName());
             if (keys.containsKey(key))
                 throw new IllegalArgumentException("Key " + KeyUtils.toPem(key) + " is already owned by " + keys.get(key));
+            if (keys.inverse().containsKey(principal))
+                throw new IllegalArgumentException(principal + " is already associated with key " + KeyUtils.toPem(keys.inverse().get(principal)));
             keys.put(key, principal);
             return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore);
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java
index 324c9706df9..a927439de1c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java
@@ -80,7 +80,7 @@ public class ControllerContainerCloudTest extends ControllerContainerTest {
         }
         public RequestBuilder data(byte[] data) { this.data = data; return this; }
         public RequestBuilder data(String data) { this.data = data.getBytes(StandardCharsets.UTF_8); return this; }
-        public RequestBuilder principal(String principal) { this.principal = new SimplePrincipal(principal); return this; }
+        public RequestBuilder principal(String principal) { this.principal = new SimplePrincipal(principal){ }; return this; }
         public RequestBuilder user(User user) { this.user = user; return this; }
         public RequestBuilder roles(Set<Role> roles) { this.roles = roles; return this; }
         public RequestBuilder roles(Role... roles) { return roles(Set.of(roles)); }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
index 1344b106bbe..f34dd3fe629 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
@@ -136,6 +136,14 @@ public class UserApiTest extends ControllerContainerCloudTest {
                 "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Key " +  quotedPemPublicKey + " is already owned by joe@dev\"}",
                 400);
 
+        // POST a different developer key for an existing user is forbidden
+        tester.assertResponse(request("/application/v4/tenant/my-tenant/key", POST)
+                                      .principal("joe@dev")
+                                      .roles(Set.of(Role.developer(id.tenant())))
+                                      .data("{\"key\":\"" + otherPemPublicKey + "\"}"),
+                              "{\"error-code\":\"BAD_REQUEST\",\"message\":\"joe@dev is already associated with key " + quotedPemPublicKey + "\"}",
+                              400);
+
         // POST in a different pem developer key
         tester.assertResponse(request("/application/v4/tenant/my-tenant/key", POST)
                         .principal("developer@tenant")