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);