Misc improvements

Remove duplicate unit tests. Improve symbol names.
Use `Map` to simplify code and reduce cost.
Introduce constant for the number of bytes in token check hash.
Improve code comments.

2 files changed, 21 insertions, 25 deletions

diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java
index b4b51fbb8dc..b2a71d2e1b9 100644
--- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java
+++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java
@@ -23,8 +23,10 @@ import java.security.Principal;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.EnumSet;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
@@ -47,6 +49,7 @@ import static com.yahoo.jdisc.http.server.jetty.AccessLoggingRequestHandler.CONT
 public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase {
 
     private static final Logger log = Logger.getLogger(CloudDataPlaneFilter.class.getName());
+    static final int CHECK_HASH_BYTES = 32;
 
     private final boolean legacyMode;
     private final List<Client> allowedClients;
@@ -100,14 +103,15 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase {
                     throw new IllegalArgumentException(
                             "Client '%s' contains invalid X.509 certificate PEM: %s".formatted(c.id(), e.toString()), e);
                 }
-                clients.add(new Client(c.id(), permissions, certs, List.of()));
+                clients.add(new Client(c.id(), permissions, certs, Map.of()));
                 hasClientRequiringCertificate = true;
             } else {
-                var tokens = new ArrayList<TokenVersion>();
+                var tokens = new HashMap<TokenCheckHash, TokenVersion>();
                 for (var token : c.tokens()) {
                     for (int version = 0; version < token.checkAccessHashes().size(); version++) {
-                        tokens.add(TokenVersion.of(
-                                token.id(), token.fingerprints().get(version), token.checkAccessHashes().get(version)));
+                        var tokenVersion = TokenVersion.of(
+                                token.id(), token.fingerprints().get(version), token.checkAccessHashes().get(version));
+                        tokens.put(tokenVersion.accessHash(), tokenVersion);
                     }
                 }
                 // Add reverse proxy certificate as required certificate for client definition
@@ -147,7 +151,7 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase {
             return Optional.of(new ErrorResponse(Response.Status.FORBIDDEN, "Forbidden"));
         }
         var clientCert = certs.get(0);
-        var requestTokenHash = requestToken(req).orElse(null);
+        var requestTokenHash = requestTokenHash(req).orElse(null);
         var clientIds = new TreeSet<String>();
         var permissions = new TreeSet<Permission>();
         var matchedTokens = new HashSet<TokenVersion>();
@@ -155,8 +159,8 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase {
             if (!c.permissions().contains(permission)) continue;
             if (!c.certificates().contains(clientCert)) continue;
             if (!c.tokens().isEmpty()) {
-                var matchedToken = c.tokens().stream()
-                        .filter(t -> t.accessHash().equals(requestTokenHash)).findAny().orElse(null);
+                if (requestTokenHash == null) continue;
+                var matchedToken  = c.tokens().get(requestTokenHash);
                 if (matchedToken == null) continue;
                 matchedTokens.add(matchedToken);
             }
@@ -180,11 +184,11 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase {
         return Optional.empty();
     }
 
-    private Optional<TokenCheckHash> requestToken(DiscFilterRequest req) {
+    private Optional<TokenCheckHash> requestTokenHash(DiscFilterRequest req) {
         return Optional.ofNullable(req.getHeader("Authorization"))
                 .filter(h -> h.startsWith("Bearer "))
                 .map(t -> t.substring("Bearer ".length()).trim())
-                .map(t -> TokenCheckHash.of(Token.of(tokenDomain, t), 32));
+                .map(t -> TokenCheckHash.of(Token.of(tokenDomain, t), CHECK_HASH_BYTES));
     }
 
     private static void addAccessLogEntry(DiscFilterRequest req, String key, String value) {
@@ -225,7 +229,10 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase {
         }
     }
 
-    private record Client(String id, EnumSet<Permission> permissions, List<X509Certificate> certificates, List<TokenVersion> tokens) {
-        Client { permissions = EnumSet.copyOf(permissions); certificates = List.copyOf(certificates); tokens = List.copyOf(tokens); }
+    private record Client(String id, EnumSet<Permission> permissions, List<X509Certificate> certificates,
+                          Map<TokenCheckHash, TokenVersion> tokens) {
+        Client {
+            permissions = EnumSet.copyOf(permissions); certificates = List.copyOf(certificates); tokens = Map.copyOf(tokens);
+        }
     }
 }
diff --git a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java
index 2dd577c18d6..e81ef45d3af 100644
--- a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java
+++ b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java
@@ -27,6 +27,7 @@ import java.util.Set;
 
 import static com.yahoo.jdisc.Response.Status.FORBIDDEN;
 import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED;
+import static com.yahoo.jdisc.http.filter.security.cloud.CloudDataPlaneFilter.CHECK_HASH_BYTES;
 import static com.yahoo.jdisc.http.filter.security.cloud.CloudDataPlaneFilter.Permission.READ;
 import static com.yahoo.jdisc.http.filter.security.cloud.CloudDataPlaneFilter.Permission.WRITE;
 import static com.yahoo.security.KeyAlgorithm.EC;
@@ -52,9 +53,9 @@ class CloudDataPlaneFilterTest {
     private static final String TOKEN_CONTEXT = "my-token-context";
     private static final String TOKEN_ID = "my-token-id";
     private static final Token VALID_TOKEN =
-            TokenGenerator.generateToken(TokenDomain.of("fp-ctx", TOKEN_CONTEXT), "vespa_token_", 32);
+            TokenGenerator.generateToken(TokenDomain.of("fp-ctx", TOKEN_CONTEXT), "vespa_token_", CHECK_HASH_BYTES);
     private static final Token UNKNOWN_TOKEN =
-            TokenGenerator.generateToken(TokenDomain.of("fp-ctx", TOKEN_CONTEXT), "vespa_token_", 32);
+            TokenGenerator.generateToken(TokenDomain.of("fp-ctx", TOKEN_CONTEXT), "vespa_token_", CHECK_HASH_BYTES);
 
     @Test
     void accepts_any_trusted_client_certificate_in_legacy_mode() {
@@ -190,18 +191,6 @@ class CloudDataPlaneFilterTest {
     }
 
     @Test
-    void fails_for_reverse_proxy_without_configured_token() {
-        var req = FilterTestUtils.newRequestBuilder()
-                .withMethod(Method.GET)
-                .withClientCertificate(REVERSE_PROXY_CERT)
-                .build();
-        var responseHandler = new MockResponseHandler();
-        newFilterWithClientsConfig().filter(req, responseHandler);
-        assertNotNull(responseHandler.getResponse());
-        assertEquals(FORBIDDEN, responseHandler.getResponse().getStatus());
-    }
-
-    @Test
     void fails_for_missing_certificate_with_token() {
         var req = FilterTestUtils.newRequestBuilder()
                 .withMethod(Method.GET)