Revert "Revert "Validate applied permissions in config model""

This reverts commit 1c97bdea2713238c87e44440cb03c913911090d2.

5 files changed, 61 insertions, 15 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/Client.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Client.java
index 29222817d17..e4abef4eb33 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/Client.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Client.java
@@ -4,28 +4,36 @@ package com.yahoo.vespa.model.container.http;
 import com.yahoo.config.provision.DataplaneToken;
 
 import java.security.cert.X509Certificate;
+import java.util.Collection;
 import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static com.yahoo.vespa.model.container.http.Client.Permission.READ;
+import static com.yahoo.vespa.model.container.http.Client.Permission.WRITE;
 
 /**
  * Represents a client. The client is identified by one of the provided certificates and have a set of permissions.
  *
  * @author mortent
+ * @author bjorncs
  */
 public class Client {
     private final String id;
-    private final List<String> permissions;
+    private final Set<Permission> permissions;
     private final List<X509Certificate> certificates;
     private final List<DataplaneToken> tokens;
     private final boolean internal;
 
-    public Client(String id, List<String> permissions, List<X509Certificate> certificates, List<DataplaneToken> tokens) {
+    public Client(String id, Collection<Permission> permissions, List<X509Certificate> certificates, List<DataplaneToken> tokens) {
         this(id, permissions, certificates, tokens, false);
     }
 
-    private Client(String id, List<String> permissions, List<X509Certificate> certificates, List<DataplaneToken> tokens,
+    private Client(String id, Collection<Permission> permissions, List<X509Certificate> certificates, List<DataplaneToken> tokens,
                    boolean internal) {
         this.id = id;
-        this.permissions = List.copyOf(permissions);
+        this.permissions = Set.copyOf(permissions);
         this.certificates = List.copyOf(certificates);
         this.tokens = List.copyOf(tokens);
         this.internal = internal;
@@ -35,7 +43,7 @@ public class Client {
         return id;
     }
 
-    public List<String> permissions() {
+    public Set<Permission> permissions() {
         return permissions;
     }
 
@@ -50,6 +58,29 @@ public class Client {
     }
 
     public static Client internalClient(List<X509Certificate> certificates) {
-        return new Client("_internal", List.of("read","write"), certificates, List.of(), true);
+        return new Client("_internal", Set.of(READ, WRITE), certificates, List.of(), true);
+    }
+
+    public enum Permission {
+        READ, WRITE;
+
+        public String asString() {
+            return switch (this) {
+                case READ -> "read";
+                case WRITE -> "write";
+            };
+        }
+
+        public static Permission fromString(String v) {
+            return switch (v) {
+                case "read" -> READ;
+                case "write" -> WRITE;
+                default -> throw new IllegalArgumentException("Invalid permission '%s'. Valid values are 'read' and 'write'.".formatted(v));
+            };
+        }
+
+        public static Set<Permission> fromCommaSeparatedString(String str) {
+            return Stream.of(str.split(",")).map(v -> Permission.fromString(v.strip())).collect(Collectors.toSet());
+        }
     }
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudDataPlaneFilter.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudDataPlaneFilter.java
index a1b569fa110..53609757d90 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudDataPlaneFilter.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudDataPlaneFilter.java
@@ -46,7 +46,7 @@ class CloudDataPlaneFilter extends Filter implements CloudDataPlaneFilterConfig.
                     .map(x -> new CloudDataPlaneFilterConfig.Clients.Builder()
                             .id(x.id())
                             .certificates(x.certificates().stream().map(X509CertificateUtils::toPem).toList())
-                            .permissions(x.permissions()))
+                            .permissions(x.permissions().stream().map(Client.Permission::asString).toList()))
                     .toList();
             builder.clients(clientsCfg).legacyMode(false);
         }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilter.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilter.java
index bb24f96784e..cba8577860b 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilter.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilter.java
@@ -44,7 +44,7 @@ class CloudTokenDataPlaneFilter extends Filter implements CloudTokenDataPlaneFil
                 .map(x -> new CloudTokenDataPlaneFilterConfig.Clients.Builder()
                         .id(x.id())
                         .tokens(tokensConfig(x.tokens()))
-                        .permissions(x.permissions()))
+                        .permissions(x.permissions().stream().map(Client.Permission::asString).toList()))
                 .toList();
         builder.clients(clientsCfg).tokenContext(tokenContext);
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index e4038a5bca6..8eca29215d4 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -518,10 +518,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         String clientId = XML.attribute("id", clientElement).orElseThrow();
         if (clientId.startsWith("_"))
             throw new IllegalArgumentException("Invalid client id '%s', id cannot start with '_'".formatted(clientId));
-        List<String> permissions = XML.attribute("permissions", clientElement)
-                .map(p -> p.split(",")).stream()
-                .flatMap(Arrays::stream)
-                .toList();
+        var permissions = XML.attribute("permissions", clientElement)
+                .map(Client.Permission::fromCommaSeparatedString).orElse(Set.of());
 
         var certificates = XML.getChildren(clientElement, "certificate").stream()
                 .flatMap(certElem -> {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilterTest.java
index c89ea421b39..1c5eb16be80 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/CloudTokenDataPlaneFilterTest.java
@@ -16,7 +16,6 @@ import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.jdisc.http.ConnectorConfig;
 import com.yahoo.jdisc.http.filter.security.cloud.config.CloudTokenDataPlaneFilterConfig;
-import com.yahoo.processing.response.Data;
 import com.yahoo.vespa.model.container.ApplicationContainer;
 import com.yahoo.vespa.model.container.ContainerModel;
 import com.yahoo.vespa.model.container.http.ConnectorFactory;
@@ -41,14 +40,14 @@ import static com.yahoo.vespa.model.container.xml.CloudDataPlaneFilterTest.creat
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class CloudTokenDataPlaneFilterTest extends ContainerModelBuilderTestBase {
 
     private static final String servicesXmlTemplate = """
             <container version='1.0'>
               <clients>
-                <client id="foo" permissions="read,write">
+                <client id="foo" permissions="read, write">
                     <certificate file="%s"/>
                 </client>
                 <client id="bar" permissions="read">
@@ -145,6 +144,24 @@ public class CloudTokenDataPlaneFilterTest extends ContainerModelBuilderTestBase
 
     }
 
+    @Test
+    void fails_on_unknown_permission() throws IOException {
+        var certFile = securityFolder.resolve("foo.pem");
+        var servicesXml = """
+                <container version='1.0'>
+                  <clients>
+                    <client id="foo" permissions="read,unknown-permission">
+                        <certificate file="%s"/>
+                    </client>
+                  </clients>
+                </container>
+                """.formatted(applicationFolder.toPath().relativize(certFile).toString());
+        var clusterElem = DomBuilderTest.parse(servicesXml);
+        createCertificate(certFile);
+        var exception = assertThrows(IllegalArgumentException.class, () -> buildModel(Set.of(mtlsEndpoint), defaultTokens, clusterElem));
+        assertEquals("Invalid permission 'unknown-permission'. Valid values are 'read' and 'write'.", exception.getMessage());
+    }
+
     private static CloudTokenDataPlaneFilterConfig.Clients.Tokens tokenConfig(
             String id, Collection<String> fingerprints, Collection<String> accessCheckHashes, Collection<String> expirations) {
         return new CloudTokenDataPlaneFilterConfig.Clients.Tokens.Builder()