Merge pull request #12558 from vespa-engine/jonmv/headless-keys-give-headless-access

Change headless keys to give only headless access

2 files changed, 20 insertions, 41 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilter.java
index a41103453eb..4916b7b24a1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilter.java
@@ -29,8 +29,9 @@ import java.util.logging.Logger;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 /**
- * Assigns the {@link Role#buildService(TenantName, ApplicationName)} role to requests with a
- * Authorization header signature matching the public key of the indicated application.
+ * Assigns the {@link Role#headless(TenantName, ApplicationName)} role or
+ * {@link Role#developer(TenantName)} to requests with a X-Authorization header signature
+ * matching the public key of the indicated application.
  * Requests which already have a set of roles assigned to them are not modified.
  *
  * @author jonmv
@@ -63,14 +64,6 @@ public class SignatureFilter extends JsonSecurityRequestFilterBase {
         return Optional.empty();
     }
 
-    // TODO jonmv: Remove after October 2019.
-    private boolean anyDeployKeyMatches(TenantAndApplicationId id, DiscFilterRequest request) {
-        return controller.applications().getApplication(id).stream()
-                         .map(Application::deployKeys)
-                         .flatMap(Set::stream)
-                         .anyMatch(key -> keyVerifies(key, request));
-    }
-
     private boolean keyVerifies(PublicKey key, DiscFilterRequest request) {
         return new RequestVerifier(key, controller.clock()).verify(Method.valueOf(request.getMethod()),
                                                                    request.getUri(),
@@ -80,30 +73,23 @@ public class SignatureFilter extends JsonSecurityRequestFilterBase {
     }
 
     private Optional<SecurityContext> getSecurityContext(DiscFilterRequest request) {
-        ApplicationId id = ApplicationId.fromSerializedForm(request.getHeader("X-Key-Id"));
-        if (request.getHeader("X-Key") != null) { // TODO jonmv: Remove check and else branch after Oct 2019.
-            PublicKey key = KeyUtils.fromPemEncodedPublicKey(new String(Base64.getDecoder().decode(request.getHeader("X-Key")), UTF_8));
-            if (keyVerifies(key, request)) {
-                Optional<CloudTenant> tenant = controller.tenants().get(id.tenant())
-                                                         .filter(CloudTenant.class::isInstance)
-                                                         .map(CloudTenant.class::cast);
-                if (tenant.isPresent() && tenant.get().developerKeys().containsKey(key))
-                    return Optional.of(new SecurityContext(tenant.get().developerKeys().get(key),
-                                                           Set.of(Role.reader(id.tenant()),
-                                                                  Role.developer(id.tenant()))));
+        PublicKey key = KeyUtils.fromPemEncodedPublicKey(new String(Base64.getDecoder().decode(request.getHeader("X-Key")), UTF_8));
+        if (keyVerifies(key, request)) {
+            ApplicationId id = ApplicationId.fromSerializedForm(request.getHeader("X-Key-Id"));
+            Optional<CloudTenant> tenant = controller.tenants().get(id.tenant())
+                                                     .filter(CloudTenant.class::isInstance)
+                                                     .map(CloudTenant.class::cast);
+            if (tenant.isPresent() && tenant.get().developerKeys().containsKey(key))
+                return Optional.of(new SecurityContext(tenant.get().developerKeys().get(key),
+                                                       Set.of(Role.reader(id.tenant()),
+                                                              Role.developer(id.tenant()))));
 
-                Optional <Application> application = controller.applications().getApplication(TenantAndApplicationId.from(id));
-                if (application.isPresent() && application.get().deployKeys().contains(key))
-                    return Optional.of(new SecurityContext(new SimplePrincipal("headless@" + id.tenant() + "." + id.application()),
-                                                           Set.of(Role.reader(id.tenant()),
-                                                                  Role.developer(id.tenant())))); // TODO jonmv: Change to headless after Oct 10 2019.
-            }
+            Optional <Application> application = controller.applications().getApplication(TenantAndApplicationId.from(id));
+            if (application.isPresent() && application.get().deployKeys().contains(key))
+                return Optional.of(new SecurityContext(new SimplePrincipal("headless@" + id.tenant() + "." + id.application()),
+                                                       Set.of(Role.reader(id.tenant()),
+                                                              Role.headless(id.tenant(), id.application()))));
         }
-        else if (anyDeployKeyMatches(TenantAndApplicationId.from(id), request))
-            return Optional.of(new SecurityContext(new SimplePrincipal("headless@" + id.tenant() + "." + id.application()),
-                                                   Set.of(Role.reader(id.tenant()),
-                                                          Role.developer(id.tenant()))));
-
         return Optional.empty();
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java
index ef3a31693b5..6e1480d45e7 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java
@@ -94,21 +94,14 @@ public class SignatureFilterTest {
         verifySecurityContext(requestOf(signer.signed(request.copy(), Method.GET, InputStream::nullInputStream), emptyBody),
                               new SecurityContext(new SimplePrincipal("headless@my-tenant.my-app"),
                                                   Set.of(Role.reader(id.tenant()),
-                                                         Role.developer(id.tenant())))); // TODO jonmv: Change to headless.
-
-        // TODO jonmv: remove after Oct 2019.
-        // Signed request gets a build service role when a matching key is stored for the application and no X-Key header is provided.
-        verifySecurityContext(requestOf(signer.legacySigned(request.copy(), Method.GET, InputStream::nullInputStream), emptyBody),
-                              new SecurityContext(new SimplePrincipal("headless@my-tenant.my-app"),
-                                                  Set.of(Role.reader(id.tenant()),
-                                                         Role.developer(id.tenant()))));
+                                                         Role.headless(id.tenant(), id.application()))));
 
         // Signed POST request with X-Key header gets a headless role.
         byte[] hiBytes = new byte[]{0x48, 0x69};
         verifySecurityContext(requestOf(signer.signed(request.copy(), Method.POST, () -> new ByteArrayInputStream(hiBytes)), hiBytes),
                               new SecurityContext(new SimplePrincipal("headless@my-tenant.my-app"),
                                                   Set.of(Role.reader(id.tenant()),
-                                                         Role.developer(id.tenant())))); // TODO jonmv: Change to headless.
+                                                         Role.headless(id.tenant(), id.application()))));
 
         // Signed request gets a developer role when a matching developer key is stored for the tenant.
         tester.curator().writeTenant(new CloudTenant(appId.tenant(),