From d33bf8a38e06e021afe2c76fa69b4936b347013f Mon Sep 17 00:00:00 2001 From: Kenji Chao Date: Wed, 10 Feb 2021 14:56:13 -0800 Subject: [PATCH] Add a custom header to parse the caller Spiffe Id from Add an optional custom header identifies the "caller" who sent the original request. If present, we attempt to parse the client Spiffe Id from this header instead of the XFCC header. It is to support a scenario where a real client is behind a proxy. The Spiffe Id extracted from the XFCC header may or may not match the one set in this custom header. For example, considering a request flow like this: client -> proxy -> Envoy -> Keywhiz. In this case, the XFCC header set by the envoy instance contains the proxy cert, while the custom Spiffe Id header contains the client information behind the proxy. --- .../auth/mutualssl/SpiffePrincipal.java | 29 +++++ .../service/config/XfccSourceConfig.java | 19 ++- .../service/providers/ClientAuthFactory.java | 35 +++++- .../providers/ClientAuthenticator.java | 84 ++++++++++--- .../providers/ClientAuthFactoryTest.java | 118 +++++++++++++++++- .../providers/ClientAuthenticatorTest.java | 63 +++++++++- 6 files changed, 315 insertions(+), 33 deletions(-) create mode 100644 server/src/main/java/keywhiz/auth/mutualssl/SpiffePrincipal.java diff --git a/server/src/main/java/keywhiz/auth/mutualssl/SpiffePrincipal.java b/server/src/main/java/keywhiz/auth/mutualssl/SpiffePrincipal.java new file mode 100644 index 000000000..70df4e0f7 --- /dev/null +++ b/server/src/main/java/keywhiz/auth/mutualssl/SpiffePrincipal.java @@ -0,0 +1,29 @@ +package keywhiz.auth.mutualssl; + +import java.net.URI; +import java.security.Principal; + +public class SpiffePrincipal implements Principal { + private final URI spiffeId; + + public SpiffePrincipal(URI spiffeId) { + this.spiffeId = spiffeId; + } + + @Override public String getName() { + return spiffeId.toString(); + } + + public URI getSpiffeId() { + return spiffeId; + } + + /** + * Use the workload id of a Spiffe Id as the client name. + */ + public String getClientName() { + String path = spiffeId.getPath(); + // Drop the leading '/' character. + return path.isEmpty() ? path : path.substring(1); + } +} diff --git a/server/src/main/java/keywhiz/service/config/XfccSourceConfig.java b/server/src/main/java/keywhiz/service/config/XfccSourceConfig.java index d3ee8b25f..cab358d16 100644 --- a/server/src/main/java/keywhiz/service/config/XfccSourceConfig.java +++ b/server/src/main/java/keywhiz/service/config/XfccSourceConfig.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.auto.value.AutoValue; import java.util.List; +import javax.annotation.Nullable; /** * Configuration for x-forwarded-client-cert header support, as set by the Envoy proxy @@ -30,8 +31,10 @@ public abstract class XfccSourceConfig { @JsonCreator public static XfccSourceConfig of( @JsonProperty("port") Integer port, @JsonProperty("allowedClientNames") List allowedClientNames, - @JsonProperty("allowedSpiffeIds") List allowedSpiffeIds) { - return new AutoValue_XfccSourceConfig(port, allowedClientNames, allowedSpiffeIds); + @JsonProperty("allowedSpiffeIds") List allowedSpiffeIds, + @JsonProperty("callerSpiffeIdHeader") String callerSpiffeIdHeader) { + return new AutoValue_XfccSourceConfig(port, allowedClientNames, allowedSpiffeIds, + callerSpiffeIdHeader); } /** @@ -49,4 +52,16 @@ public abstract class XfccSourceConfig { public abstract List allowedClientNames(); public abstract List allowedSpiffeIds(); + + /** + * An optional custom header identifies the "caller" who sent the original request. If present, + * we attempt to parse the client Spiffe Id from this header instead of the XFCC header. + * + * It is to support a scenario where a real client is behind a proxy. The Spiffe Id extracted from + * the XFCC header may or may not match the one set in this custom header. For example, + * considering a request flow like this: client -> proxy -> Envoy -> Keywhiz. In this case, the + * XFCC header set by the envoy instance contains the proxy cert, while the custom Spiffe Id + * header contains the client information behind the proxy. + */ + @Nullable public abstract String callerSpiffeIdHeader(); } diff --git a/server/src/main/java/keywhiz/service/providers/ClientAuthFactory.java b/server/src/main/java/keywhiz/service/providers/ClientAuthFactory.java index be1b84c64..15628a7b5 100644 --- a/server/src/main/java/keywhiz/service/providers/ClientAuthFactory.java +++ b/server/src/main/java/keywhiz/service/providers/ClientAuthFactory.java @@ -35,6 +35,7 @@ import keywhiz.KeywhizConfig; import keywhiz.api.model.Client; import keywhiz.auth.mutualssl.CertificatePrincipal; +import keywhiz.auth.mutualssl.SpiffePrincipal; import keywhiz.service.config.ClientAuthConfig; import keywhiz.service.config.XfccSourceConfig; import keywhiz.service.daos.ClientDAO; @@ -107,10 +108,10 @@ public Client provide(ContainerRequest containerRequest, // on the security context of this request if (possibleXfccConfig.isEmpty()) { // The XFCC header is not used; use the security context of this request to identify the client - return authenticateClientFromCertificate(requestPrincipal); + return authenticateClientFromPrincipal(requestPrincipal); } else { return authorizeClientFromXfccHeader(possibleXfccConfig.get(), xfccHeaderValues, - requestPrincipal); + requestPrincipal, containerRequest); } } @@ -134,12 +135,34 @@ private Optional getXfccConfigForPort(int port) { } private Client authorizeClientFromXfccHeader(XfccSourceConfig xfccConfig, - List xfccHeaderValues, Principal requestPrincipal) { + List xfccHeaderValues, Principal requestPrincipal, + ContainerRequest containerRequest) { // Do not allow the XFCC header to be set by all incoming traffic. This throws a // NotAuthorizedException when the traffic is not coming from a source allowed to set the // header. validateXfccHeaderAllowed(xfccConfig, requestPrincipal); + Optional callerSpiffeIdHeader = Optional.ofNullable(xfccConfig.callerSpiffeIdHeader()); + List callerSpiffeIdList = callerSpiffeIdHeader.map( + header -> Optional.ofNullable(containerRequest.getRequestHeader(header)) + .orElse(List.of())) + .orElse(List.of()); + int size = callerSpiffeIdList.size(); + + Optional callerSpiffeId = callerSpiffeIdHeader.flatMap( + header -> ClientAuthenticator.getSpiffeIdFromHeader(containerRequest, header)); + + if (size > 1 || size == 1 && callerSpiffeId.isEmpty()) { + throw new NotAuthorizedException(format( + "Invalid caller Spiffe Id header. It should contain only one URI and follow Spiffe Id format. size: %d, header: %s", + size, callerSpiffeIdList)); + } + + if (callerSpiffeId.isPresent()) { + SpiffePrincipal spiffePrincipal = new SpiffePrincipal(callerSpiffeId.get()); + return authenticateClientFromPrincipal(spiffePrincipal); + } + // Extract client information from the XFCC header X509Certificate clientCert = getClientCertFromXfccHeaderEnvoyFormatted(xfccHeaderValues).orElseThrow(() -> @@ -149,9 +172,9 @@ private Client authorizeClientFromXfccHeader(XfccSourceConfig xfccConfig, CertificatePrincipal certificatePrincipal = new CertificatePrincipal(clientCert.getSubjectDN().toString(), - new X509Certificate[] {clientCert}); + new X509Certificate[] { clientCert }); - return authenticateClientFromCertificate(certificatePrincipal); + return authenticateClientFromPrincipal(certificatePrincipal); } private void validateXfccHeaderAllowed(XfccSourceConfig xfccConfig, Principal requestPrincipal) { @@ -296,7 +319,7 @@ protected boolean createMissingClient() { return clientAuthConfig.createMissingClients(); } - private Client authenticateClientFromCertificate(Principal clientPrincipal) { + private Client authenticateClientFromPrincipal(Principal clientPrincipal) { Optional possibleClient = authenticator.authenticate(clientPrincipal, createMissingClient()); return possibleClient.orElseThrow(() -> new NotAuthorizedException( diff --git a/server/src/main/java/keywhiz/service/providers/ClientAuthenticator.java b/server/src/main/java/keywhiz/service/providers/ClientAuthenticator.java index 10b5305b2..60595be20 100644 --- a/server/src/main/java/keywhiz/service/providers/ClientAuthenticator.java +++ b/server/src/main/java/keywhiz/service/providers/ClientAuthenticator.java @@ -12,12 +12,14 @@ import javax.ws.rs.NotAuthorizedException; import keywhiz.api.model.Client; import keywhiz.auth.mutualssl.CertificatePrincipal; +import keywhiz.auth.mutualssl.SpiffePrincipal; import keywhiz.service.config.ClientAuthConfig; import keywhiz.service.daos.ClientDAO; import org.bouncycastle.asn1.x500.RDN; import org.bouncycastle.asn1.x500.X500Name; import org.bouncycastle.asn1.x500.style.BCStyle; import org.bouncycastle.asn1.x500.style.IETFUtils; +import org.glassfish.jersey.server.ContainerRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -120,6 +122,10 @@ private Optional handleMissingClient(boolean createMissingClient, String } static Optional getClientName(Principal principal) { + if (principal instanceof SpiffePrincipal) { + return Optional.of(((SpiffePrincipal) principal).getClientName()); + } + X500Name name = new X500Name(principal.getName()); RDN[] rdns = name.getRDNs(BCStyle.CN); if (rdns.length == 0) { @@ -129,20 +135,23 @@ static Optional getClientName(Principal principal) { } static Optional getSpiffeId(Principal principal) { - if (!(principal instanceof CertificatePrincipal)) { - // A SPIFFE ID can only be parsed from a principal with a certificate - return Optional.empty(); + if (principal instanceof CertificatePrincipal) { + // This chain is either from the XFCC header's "Cert" field, which includes only the + // client certificate rather than the chain, or from the CertificateSecurityContext + // configured by Keywhiz' ClientCertificateFilter, which sets it based on + // X509Certificate[] chain = + // (X509Certificate[]) context.getProperty("javax.servlet.request.X509Certificate"); + // which appears to place the leaf as the zero-index entry in the chain. + X509Certificate cert = ((CertificatePrincipal) principal).getCertificateChain() + .get(0); + return getSpiffeIdFromCertificate(cert); + } + + if (principal instanceof SpiffePrincipal) { + return Optional.of(((SpiffePrincipal) principal).getSpiffeId()); } - // This chain is either from the XFCC header's "Cert" field, which includes only the - // client certificate rather than the chain, or from the CertificateSecurityContext - // configured by Keywhiz' ClientCertificateFilter, which sets it based on - // X509Certificate[] chain = - // (X509Certificate[]) context.getProperty("javax.servlet.request.X509Certificate"); - // which appears to place the leaf as the zero-index entry in the chain. - X509Certificate cert = ((CertificatePrincipal) principal).getCertificateChain() - .get(0); - return getSpiffeIdFromCertificate(cert); + return Optional.empty(); } static Optional getSpiffeIdFromCertificate(X509Certificate cert) { @@ -165,14 +174,7 @@ static Optional getSpiffeIdFromCertificate(X509Certificate cert) { .map(sanPair -> (String) sanPair.get(1)) .collect(Collectors.toUnmodifiableList()); - // https://spiffe.io/spiffe/concepts/#spiffe-verifiable-identity-document-svid - // > An SVID contains a single SPIFFE ID, which represents the identity of the service presenting it - // - // https://github.com/spiffe/spiffe/blob/master/standards/X509-SVID.md#2-spiffe-id - // > An X.509 SVID MUST contain exactly one URI SAN. - List spiffeUriNames = providedUris.stream() - .filter(uri -> uri.startsWith(SPIFFE_SCHEME)) - .collect(Collectors.toUnmodifiableList()); + List spiffeUriNames = spiffeUriNames(providedUris); if (spiffeUriNames.size() > 1) { logger.warn("Got multiple SPIFFE URIs from certificate: {}", spiffeUriNames); @@ -197,4 +199,46 @@ static Optional getSpiffeIdFromCertificate(X509Certificate cert) { } }); } + + static Optional getSpiffeIdFromHeader(ContainerRequest containerRequest, + String spiffeIdHeader) { + List spiffeIdHeaderValues = + Optional.ofNullable(containerRequest.getRequestHeader(spiffeIdHeader)).orElse(List.of()); + List spiffeUriNames = spiffeUriNames(spiffeIdHeaderValues); + + if (spiffeUriNames.isEmpty()) { + logger.warn("No SPIFFE URI found from header {}", spiffeIdHeader); + return Optional.empty(); + } else if (spiffeUriNames.size() > 1) { + logger.warn("Got multiple SPIFFE URIs from header {}: {}", spiffeIdHeader, spiffeUriNames); + return Optional.empty(); + } else if (spiffeIdHeaderValues.size() > 1) { + logger.warn( + "Multiple URIs are not allowed in the header {} that includes a SPIFFE URI (URIs: {})", + spiffeIdHeader, spiffeIdHeaderValues); + return Optional.empty(); + } + + String uri = spiffeUriNames.get(0); + try { + return Optional.of(new URI(uri)); + } catch (URISyntaxException e) { + logger.warn( + format("Error parsing SPIFFE URI (%s) from the header %s as a URI", uri, + spiffeIdHeader), + e); + return Optional.empty(); + } + } + + private static List spiffeUriNames(List uris) { + // https://spiffe.io/spiffe/concepts/#spiffe-verifiable-identity-document-svid + // > An SVID contains a single SPIFFE ID, which represents the identity of the service presenting it + // + // https://github.com/spiffe/spiffe/blob/master/standards/X509-SVID.md#2-spiffe-id + // > An X.509 SVID MUST contain exactly one URI SAN. + return uris.stream() + .filter(uri -> uri.startsWith(SPIFFE_SCHEME)) + .collect(Collectors.toUnmodifiableList()); + } } diff --git a/server/src/test/java/keywhiz/service/providers/ClientAuthFactoryTest.java b/server/src/test/java/keywhiz/service/providers/ClientAuthFactoryTest.java index 94944689f..9bcacd35c 100644 --- a/server/src/test/java/keywhiz/service/providers/ClientAuthFactoryTest.java +++ b/server/src/test/java/keywhiz/service/providers/ClientAuthFactoryTest.java @@ -18,6 +18,7 @@ import java.io.ByteArrayInputStream; import java.net.URI; +import java.net.URISyntaxException; import java.security.Principal; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; @@ -44,6 +45,8 @@ import static java.lang.String.format; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.when; public class ClientAuthFactoryTest { @@ -56,6 +59,17 @@ public class ClientAuthFactoryTest { new Client(0, clientName, null, clientSpiffe, null, null, null, null, null, null, true, false); + private static final String newClientName = "new-principal"; + private static final String newClientSpiffe = "spiffe://example.org/new-principal"; + private static final Client newClient = + new Client(123L, newClientName, null, newClientSpiffe, null, null, null, null, null, null, + true, + false); + + private static final String callerSpiffeHeaderName = "x-caller-spiffe-id"; + private static final String nonExistentClientSpiffe = + "spiffe://example.org/non-existent-principal"; + private static Principal xfccPrincipal; private static final String xfccName = "principal-allowed-for-xfcc"; private static final String xfccSpiffe = "spiffe://example.org/principal-allowed-for-xfcc"; @@ -144,12 +158,12 @@ public class ClientAuthFactoryTest { X509Certificate clientCert = (X509Certificate) cf.generateCertificate( new ByteArrayInputStream(clientPem.getBytes(UTF_8))); clientPrincipal = new CertificatePrincipal(clientCert.getSubjectDN().toString(), - new X509Certificate[] {clientCert}); + new X509Certificate[] { clientCert }); X509Certificate xfccCert = (X509Certificate) cf.generateCertificate( new ByteArrayInputStream(xfccPem.getBytes(UTF_8))); xfccPrincipal = new CertificatePrincipal(xfccCert.getSubjectDN().toString(), - new X509Certificate[] {xfccCert}); + new X509Certificate[] { xfccCert }); factory = new ClientAuthFactory(clientDAO, clientAuthConfig); @@ -193,6 +207,52 @@ public void clientWhenPrincipalAbsentThrows() { assertThat(factory.provide(request, httpServletRequest)).isEqualTo(client); } + @Test public void returnsClientWhenClientPresent_fromXfccHeader_customSpiffeIdHeader() { + when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); + when(request.getRequestHeader(ClientAuthFactory.XFCC_HEADER_NAME)).thenReturn( + List.of(xfccHeader)); + when(securityContext.getUserPrincipal()).thenReturn(xfccPrincipal); + + when(xfccSourceConfig.callerSpiffeIdHeader()).thenReturn(callerSpiffeHeaderName); + when(request.getRequestHeader(callerSpiffeHeaderName)).thenReturn(List.of(clientSpiffe)); + + assertThat(factory.provide(request, httpServletRequest)).isEqualTo(client); + } + + @Test + public void returnsClientWhenClientPresent_fromXfccHeader_customSpiffeIdHeader_fallbackToXfccWhenNoValue() { + when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); + when(request.getRequestHeader(ClientAuthFactory.XFCC_HEADER_NAME)).thenReturn( + List.of(xfccHeader)); + when(securityContext.getUserPrincipal()).thenReturn(xfccPrincipal); + + when(xfccSourceConfig.callerSpiffeIdHeader()).thenReturn(callerSpiffeHeaderName); + + assertThat(factory.provide(request, httpServletRequest)).isEqualTo(client); + } + + @Test public void returnsNewClientWhenClientNotPresent_fromXfccHeader_customSpiffeIdHeader() + throws URISyntaxException { + when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); + when(request.getRequestHeader(ClientAuthFactory.XFCC_HEADER_NAME)).thenReturn( + List.of(xfccHeader)); + when(securityContext.getUserPrincipal()).thenReturn(xfccPrincipal); + + when(xfccSourceConfig.callerSpiffeIdHeader()).thenReturn(callerSpiffeHeaderName); + when(request.getRequestHeader(callerSpiffeHeaderName)).thenReturn(List.of(newClientSpiffe)); + + // Create missing clients + when(clientAuthConfig.createMissingClients()).thenReturn(true); + // lookup doesn't find client + when(clientDAO.getClientByName(newClientName)).thenReturn(Optional.empty()); + // a new DB record is created + when(clientDAO.createClient(eq(newClientName), eq("automatic"), any(), + eq(new URI(newClientSpiffe)))).thenReturn(123L); + when(clientDAO.getClientById(123L)).thenReturn(Optional.of(newClient)); + + assertThat(factory.provide(request, httpServletRequest)).isEqualTo(newClient); + } + @Test(expected = NotAuthorizedException.class) public void rejectsXfcc_requesterAuthMissing() { when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); @@ -229,6 +289,19 @@ public void rejectsXfcc_requesterSpiffeNotAllowed() { factory.provide(request, httpServletRequest); } + @Test(expected = NotAuthorizedException.class) + public void rejectsXfcc_requesterNameNotAllowed_requesterSpiffeNotAllowed() { + when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); + when(request.getRequestHeader(ClientAuthFactory.XFCC_HEADER_NAME)).thenReturn( + List.of(xfccHeader)); + when(securityContext.getUserPrincipal()).thenReturn(xfccPrincipal); + + when(xfccSourceConfig.allowedClientNames()).thenReturn(List.of()); + when(xfccSourceConfig.allowedSpiffeIds()).thenReturn(List.of()); + + factory.provide(request, httpServletRequest); + } + @Test(expected = NotAuthorizedException.class) public void rejectsXfcc_portConfigurationInvalid() { when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); @@ -319,4 +392,45 @@ public void rejectsXfcc_missingClientCert() { factory.provide(request, httpServletRequest); } + + @Test(expected = NotAuthorizedException.class) + public void rejectsXfcc_customSpiffeIdHeader_invalidSpiffe() { + when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); + when(request.getRequestHeader(ClientAuthFactory.XFCC_HEADER_NAME)).thenReturn( + List.of(xfccHeader)); + when(securityContext.getUserPrincipal()).thenReturn(xfccPrincipal); + + when(xfccSourceConfig.callerSpiffeIdHeader()).thenReturn(callerSpiffeHeaderName); + when(request.getRequestHeader(callerSpiffeHeaderName)).thenReturn(List.of("not-spiffe-id")); + + factory.provide(request, httpServletRequest); + } + + @Test(expected = NotAuthorizedException.class) + public void rejectsXfcc_customSpiffeIdHeader_multipleSpiffe() { + when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); + when(request.getRequestHeader(ClientAuthFactory.XFCC_HEADER_NAME)).thenReturn( + List.of(xfccHeader)); + when(securityContext.getUserPrincipal()).thenReturn(xfccPrincipal); + + when(xfccSourceConfig.callerSpiffeIdHeader()).thenReturn(callerSpiffeHeaderName); + when(request.getRequestHeader(callerSpiffeHeaderName)).thenReturn( + List.of(clientSpiffe, newClientSpiffe)); + + factory.provide(request, httpServletRequest); + } + + @Test(expected = NotAuthorizedException.class) + public void rejectsXfcc_customSpiffeIdHeader_clientNotPresent() { + when(httpServletRequest.getLocalPort()).thenReturn(xfccAllowedPort); + when(request.getRequestHeader(ClientAuthFactory.XFCC_HEADER_NAME)).thenReturn( + List.of(xfccHeader)); + when(securityContext.getUserPrincipal()).thenReturn(xfccPrincipal); + + when(xfccSourceConfig.callerSpiffeIdHeader()).thenReturn(callerSpiffeHeaderName); + when(request.getRequestHeader(callerSpiffeHeaderName)).thenReturn( + List.of(nonExistentClientSpiffe)); + + factory.provide(request, httpServletRequest); + } } diff --git a/server/src/test/java/keywhiz/service/providers/ClientAuthenticatorTest.java b/server/src/test/java/keywhiz/service/providers/ClientAuthenticatorTest.java index f61473ca4..06381955a 100644 --- a/server/src/test/java/keywhiz/service/providers/ClientAuthenticatorTest.java +++ b/server/src/test/java/keywhiz/service/providers/ClientAuthenticatorTest.java @@ -18,6 +18,7 @@ import java.io.ByteArrayInputStream; import java.net.URI; +import java.net.URISyntaxException; import java.security.Principal; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; @@ -27,6 +28,7 @@ import keywhiz.api.model.Client; import keywhiz.auth.mutualssl.CertificatePrincipal; import keywhiz.auth.mutualssl.SimplePrincipal; +import keywhiz.auth.mutualssl.SpiffePrincipal; import keywhiz.service.config.ClientAuthConfig; import keywhiz.service.config.ClientAuthTypeConfig; import keywhiz.service.daos.ClientDAO; @@ -63,6 +65,7 @@ public class ClientAuthenticatorTest { null, null, true, false); private static Principal certPrincipal; + private static Principal spiffePrincipal; // certstrap init --common-name "KeywhizAuth" // certstrap request-cert --common-name principal --ou organizational-unit @@ -174,7 +177,9 @@ public class ClientAuthenticatorTest { X509Certificate clientCert = (X509Certificate) cf.generateCertificate( new ByteArrayInputStream(clientPem.getBytes(UTF_8))); certPrincipal = new CertificatePrincipal(clientCert.getSubjectDN().toString(), - new X509Certificate[] {clientCert}); + new X509Certificate[] { clientCert }); + + spiffePrincipal = new SpiffePrincipal(new URI(clientSpiffeStr)); authenticator = new ClientAuthenticator(clientDAO, clientDAO, clientAuthConfig); @@ -195,6 +200,10 @@ public class ClientAuthenticatorTest { assertThat(authenticator.authenticate(certPrincipal, false)).isEqualTo(Optional.of(client)); } + @Test public void retrievesClientIfPresent_spiffePrincipal() { + assertThat(authenticator.authenticate(spiffePrincipal, false)).isEqualTo(Optional.of(client)); + } + @Test public void rejectsDisabledClients() { Client disabledClient = new Client(1, "disabled", null, null, null, null, null, null, null, null, @@ -223,6 +232,26 @@ public class ClientAuthenticatorTest { Optional.of(newClient)); } + @Test public void createsDbRecordForNewClient_whenConfigured_spiffePrincipal() + throws URISyntaxException { + ApiDate now = ApiDate.now(); + Client newClient = + new Client(2345L, "new-client", "desc", null, now, "automatic", now, "automatic", + null, null, true, false + ); + + // lookup doesn't find client + when(clientDAO.getClientByName("new-client")).thenReturn(Optional.empty()); + + // a new DB record is created + when(clientDAO.createClient(eq("new-client"), eq("automatic"), any(), any())).thenReturn(2345L); + when(clientDAO.getClientById(2345L)).thenReturn(Optional.of(newClient)); + + assertThat( + authenticator.authenticate(new SpiffePrincipal(new URI("spiffe://example.org/new-client")), + true)).isEqualTo(Optional.of(newClient)); + } + @Test public void doesNotCreateDbRecordForNewClient_whenNotConfigured() { ApiDate now = ApiDate.now(); Client newClient = @@ -243,11 +272,39 @@ public class ClientAuthenticatorTest { verify(clientDAO, never()).createClient(anyString(), anyString(), anyString(), any()); } + @Test public void doesNotCreateDbRecordForNewClient_whenNotConfigured_spiffePrincipal() + throws URISyntaxException { + ApiDate now = ApiDate.now(); + Client newClient = + new Client(2345L, "new-client", "desc", null, now, "automatic", now, "automatic", + null, null, true, false + ); + + // lookup doesn't find client + when(clientDAO.getClientByName("new-client")).thenReturn(Optional.empty()); + + // a new DB record should not be created, but mock the DAO to create a client if called + when(clientDAO.createClient(eq("new-client"), eq("automatic"), any(), any())).thenReturn(2345L); + when(clientDAO.getClientById(2345L)).thenReturn(Optional.of(newClient)); + + assertThat( + authenticator.authenticate(new SpiffePrincipal(new URI("spiffe://example.org/new-client")), + false)).isEmpty(); + + // the authenticator should not have tried to create the new client + verify(clientDAO, never()).createClient(anyString(), anyString(), anyString(), any()); + } + @Test public void updatesClientLastSeen() { assertThat(authenticator.authenticate(simplePrincipal, true)).isPresent(); verify(clientDAO, times(1)).sawClient(any(), eq(simplePrincipal)); } + @Test public void updatesClientLastSeen_spiffePrincipal() { + assertThat(authenticator.authenticate(spiffePrincipal, true)).isPresent(); + verify(clientDAO, times(1)).sawClient(any(), eq(spiffePrincipal)); + } + @Test(expected = NotAuthorizedException.class) public void rejectsCertMatchingMultipleClients() { ApiDate now = ApiDate.now(); @@ -294,7 +351,7 @@ public void respectsClientAuthConfig() { new ByteArrayInputStream(multipleSpiffePem.getBytes(UTF_8))); Principal multipleSpiffePrincipal = new CertificatePrincipal(multipleSpiffeClientCert.getSubjectDN().toString(), - new X509Certificate[] {multipleSpiffeClientCert}); + new X509Certificate[] { multipleSpiffeClientCert }); // Use only the (malformatted) SPIFFE IDs to retrieve a client (which should fail) when(clientAuthTypeConfig.useCommonName()).thenReturn(false); @@ -310,7 +367,7 @@ public void respectsClientAuthConfig() { new ByteArrayInputStream(multipleUriPem.getBytes(UTF_8))); Principal multipleUriPrincipal = new CertificatePrincipal(multipleUriClientCert.getSubjectDN().toString(), - new X509Certificate[] {multipleUriClientCert}); + new X509Certificate[] { multipleUriClientCert }); // Use only the (malformatted) URIs to retrieve a client (which should fail) when(clientAuthTypeConfig.useCommonName()).thenReturn(false);