From 0ccd458585f43fbb324a02cfbe5c0d60b8386a36 Mon Sep 17 00:00:00 2001 From: "Marcel Mager (INST-IOT/ESB)" Date: Thu, 25 Aug 2016 09:35:18 +0200 Subject: [PATCH 1/7] Add support for multiple hashes in Issuer hash based authentication Signed-off-by: Marcel Mager (INST-IOT/ESB) --- ...bstractControllerAuthenticationFilter.java | 4 +- ...rPreAuthenticatedSecurityHeaderFilter.java | 27 ++++- ...okenSourceTrustAuthenticationProvider.java | 16 ++- .../security/PreAuthentificationFilter.java | 10 +- ...AuthenticatedSecurityHeaderFilterTest.java | 113 ++++++++++++++++++ ...ficateAuthenticationConfigurationItem.java | 8 +- 6 files changed, 160 insertions(+), 18 deletions(-) create mode 100644 hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java index edf2ef034..c45cfb5e4 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java @@ -18,7 +18,7 @@ import org.slf4j.LoggerFactory; /** * An abstraction for all controller based security. Check if the tenant * configuration is enabled. - * + * * * */ @@ -50,7 +50,7 @@ public abstract class AbstractControllerAuthenticationFilter implements PreAuthe public abstract HeaderAuthentication getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); @Override - public abstract HeaderAuthentication getPreAuthenticatedCredentials(TenantSecurityToken secruityToken); + public abstract Object getPreAuthenticatedCredentials(TenantSecurityToken secruityToken); private final class SecurityConfigurationKeyTenantRunner implements TenantAware.TenantRunner { @Override diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java index 6836b8a31..6fd13a1c6 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java @@ -8,6 +8,10 @@ */ package org.eclipse.hawkbit.security; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + import org.eclipse.hawkbit.dmf.json.model.TenantSecurityToken; import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.tenancy.TenantAware; @@ -97,7 +101,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont } @Override - public HeaderAuthentication getPreAuthenticatedCredentials(final TenantSecurityToken secruityToken) { + public Object getPreAuthenticatedCredentials(final TenantSecurityToken secruityToken) { final String authorityNameConfigurationValue = tenantAware.runAsTenant(secruityToken.getTenant(), sslIssuerNameConfigTenantRunner); String controllerId = secruityToken.getControllerId(); @@ -108,7 +112,17 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont controllerId = secruityToken.getHeader(caCommonNameHeader); } - return new HeaderAuthentication(controllerId, authorityNameConfigurationValue); + String[] knownHashes = splitMultiHash(authorityNameConfigurationValue); + if (knownHashes.length > 1) { + Set multiHashes = new HashSet<>(); + final String cntlId = controllerId; + Arrays.asList(knownHashes) + .forEach(hashItem -> multiHashes.add(new HeaderAuthentication(cntlId, hashItem))); + return multiHashes; + + } else { + return new HeaderAuthentication(controllerId, authorityNameConfigurationValue); + } } /** @@ -117,12 +131,13 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont * It's ok if we find the the hash in any the trusted CA chain to accept * this request for this tenant. */ - private String getIssuerHashHeader(final TenantSecurityToken secruityToken, final String knownIssuerHash) { + private String getIssuerHashHeader(final TenantSecurityToken secruityToken, final String knownIssuerHashes) { // iterate over the headers until we get a null header. + String[] knownHashes = splitMultiHash(knownIssuerHashes); int iHeader = 1; String foundHash; while ((foundHash = secruityToken.getHeader(String.format(sslIssuerHashBasicHeader, iHeader))) != null) { - if (foundHash.equals(knownIssuerHash)) { + if (Arrays.asList(knownHashes).contains(foundHash)) { if (LOGGER.isTraceEnabled()) { LOGGER.trace("Found matching ssl issuer hash at position {}", iHeader); } @@ -148,4 +163,8 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME, String.class).getValue()); } } + + private static String[] splitMultiHash(String knownIssuerHashes) { + return knownIssuerHashes.split(";"); + } } diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java index b4960737d..c653108a0 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java @@ -27,17 +27,17 @@ import org.springframework.security.web.authentication.preauth.PreAuthenticatedA * An spring authentication provider which supports authentication tokens of * type {@link PreAuthenticatedAuthenticationToken} created by the * {@link ControllerPreAuthenticatedSecurityHeaderFilter}. - * + * * Additionally to the authentication token providing the principal and the * credentials which must be match, this authentication provider can also check * the remote IP address of the request. - * + * * E.g. The request path is /controller/v1/{controllerId} then the controllerId * in the path is the principal. The credentials are the extracted information * from e.g. a certificate provided by an reverse proxy. Due this request is * only allowed from a specific source address this authentication manager can * also check the remote IP address of the request. - * + * * * */ @@ -58,7 +58,7 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica * Creates a new PreAuthTokenSourceTrustAuthenticationProvider with given * source IP addresses which are trusted and should be checked against the * request remote IP address. - * + * * @param authorizedSourceIps * a list of IP addresses. */ @@ -70,7 +70,7 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica * Creates a new PreAuthTokenSourceTrustAuthenticationProvider with given * source IP addresses which are trusted and should be checked against the * request remote IP address. - * + * * @param authorizedSourceIps * a list of IP addresses. */ @@ -104,6 +104,12 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica // proxy which extracted the CN from the certificate if (principal.equals(credentials)) { successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); + + } else if (Collection.class.isAssignableFrom(credentials.getClass())) { + final Collection multiValueCredentials = (Collection) credentials; + if (multiValueCredentials.contains(principal)) { + successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); + } } if (successAuthentication) { diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java index 90f2acd50..f0801ff4d 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java @@ -22,7 +22,7 @@ public interface PreAuthentificationFilter { /** * Check if the filter is enabled. - * + * * @param secruityToken * the secruity info * @return is enabled diabled @@ -31,7 +31,7 @@ public interface PreAuthentificationFilter { /** * Extract the principal information from the current secruityToken. - * + * * @param secruityToken * the secruityToken * @return the extracted tenant and controller id @@ -40,17 +40,17 @@ public interface PreAuthentificationFilter { /** * Extract the principal credentials from the current secruityToken. - * + * * @param secruityToken * the secruityToken * @return the extracted tenant and controller id */ - HeaderAuthentication getPreAuthenticatedCredentials(TenantSecurityToken secruityToken); + Object getPreAuthenticatedCredentials(TenantSecurityToken secruityToken); /** * Allows to add additional authorities to the successful authenticated * token. - * + * * @return the authorities granted to the principal, or an empty collection * if the token has not been authenticated. Never null. * @see Authentication#getAuthorities() diff --git a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java new file mode 100644 index 000000000..eb34d5253 --- /dev/null +++ b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java @@ -0,0 +1,113 @@ +/** + * Copyright (c) 2015 Bosch Software Innovations GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.security; + +import static org.junit.Assert.*; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.when; + +import org.eclipse.hawkbit.dmf.json.model.TenantSecurityToken; +import org.eclipse.hawkbit.dmf.json.model.TenantSecurityToken.FileResource; +import org.eclipse.hawkbit.repository.TenantConfigurationManagement; +import org.eclipse.hawkbit.repository.model.TenantConfigurationValue; +import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationKey; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import ru.yandex.qatools.allure.annotations.Description; +import ru.yandex.qatools.allure.annotations.Features; +import ru.yandex.qatools.allure.annotations.Stories; + +@Features("Unit Tests - Security") +@Stories("Issuer hash based authentication") +@RunWith(MockitoJUnitRunner.class) +public class ControllerPreAuthenticatedSecurityHeaderFilterTest { + + private ControllerPreAuthenticatedSecurityHeaderFilter underTest; + + @Mock + private TenantConfigurationManagement tenantConfigurationManagementMock; + + @Mock + private TenantSecurityToken tenantSecurityTokenMock; + + private SecurityContextTenantAware tenantAware = new SecurityContextTenantAware(); + + private static final String CA_COMMON_NAME = "ca-cn"; + + private static final String X_SSL_ISSUER_HASH_1 = "X-Ssl-Issuer-Hash-1"; + + private static final String SINGLE_HASH = "hash1"; + + private static final String MULTI_HASH = "hash1;hash2;hash3"; + + private static final TenantConfigurationValue CONFIG_VALUE_SINGLE_HASH = TenantConfigurationValue + .builder().value(SINGLE_HASH).build(); + + private static final TenantConfigurationValue CONFIG_VALUE_MULTI_HASH = TenantConfigurationValue + .builder().value(MULTI_HASH).build(); + + @Before + public void before() { + underTest = new ControllerPreAuthenticatedSecurityHeaderFilter(CA_COMMON_NAME, "X-Ssl-Issuer-Hash-%d", + tenantConfigurationManagementMock, + tenantAware, new SystemSecurityContext(tenantAware)); + } + + @Test + @Description("Tests the filter for issuer hash based authentication with a single known hash") + public void testIssuerHashBasedAuthenticationWithSingleKnownHash() { + // prepare security token + final TenantSecurityToken securityToken = prepareSecurityToken(); + securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, SINGLE_HASH); + // use single known hash + when(tenantConfigurationManagementMock.getConfigurationValue( + eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) + .thenReturn(CONFIG_VALUE_SINGLE_HASH); + assertNotNull(underTest.getPreAuthenticatedPrincipal(securityToken)); + } + + @Test + @Description("Tests the filter for issuer hash based authentication with multiple known hashes") + public void testIssuerHashBasedAuthenticationWithMultipleKnownHashes() { + // prepare security token + final TenantSecurityToken securityToken = prepareSecurityToken(); + securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, SINGLE_HASH); + // use multiple known hashes + when(tenantConfigurationManagementMock.getConfigurationValue( + eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) + .thenReturn(CONFIG_VALUE_MULTI_HASH); + assertNotNull(underTest.getPreAuthenticatedPrincipal(securityToken)); + } + + @Test + @Description("Tests the filter for issuer hash based authentication with unknown hash") + public void testIssuerHashBasedAuthenticationWithUnknownHash() { + // prepare security token + final TenantSecurityToken securityToken = prepareSecurityToken(); + securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "unknown"); + // use single known hash + when(tenantConfigurationManagementMock.getConfigurationValue( + eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) + .thenReturn(CONFIG_VALUE_MULTI_HASH); + assertNull(underTest.getPreAuthenticatedPrincipal(securityToken)); + } + + private static TenantSecurityToken prepareSecurityToken() { + final TenantSecurityToken securityToken = new TenantSecurityToken("default", "1234", + FileResource.createFileResourceBySha1("12345")); + securityToken.getHeaders().put(CA_COMMON_NAME, "any"); + + return securityToken; + } + +} diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/tenantconfiguration/authentication/CertificateAuthenticationConfigurationItem.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/tenantconfiguration/authentication/CertificateAuthenticationConfigurationItem.java index c50b9f8fb..e041f5564 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/tenantconfiguration/authentication/CertificateAuthenticationConfigurationItem.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/tenantconfiguration/authentication/CertificateAuthenticationConfigurationItem.java @@ -63,13 +63,17 @@ public class CertificateAuthenticationConfigurationItem extends AbstractAuthenti final Label caRootAuthorityLabel = new LabelBuilder().name("SSL Issuer Hash:").buildLabel(); caRootAuthorityLabel.setDescription( "The SSL Issuer iRules.X509 hash, to validate against the controller request certifcate."); + caRootAuthorityLabel.setWidthUndefined(); - caRootAuthorityTextField = new TextFieldBuilder().immediate(true).maxLengthAllowed(128).buildTextComponent(); - caRootAuthorityTextField.setWidth("500px"); + caRootAuthorityTextField = new TextFieldBuilder().immediate(true).maxLengthAllowed(160).buildTextComponent(); + caRootAuthorityTextField.setWidth("100%"); caRootAuthorityTextField.addTextChangeListener(event -> caRootAuthorityChanged()); caRootAuthorityLayout.addComponent(caRootAuthorityLabel); + caRootAuthorityLayout.setExpandRatio(caRootAuthorityLabel, 0); caRootAuthorityLayout.addComponent(caRootAuthorityTextField); + caRootAuthorityLayout.setExpandRatio(caRootAuthorityTextField, 1); + caRootAuthorityLayout.setWidth("100%"); detailLayout.addComponent(caRootAuthorityLayout); From 5215580cd75557ad1cc95401ee7a4c992cdba3ae Mon Sep 17 00:00:00 2001 From: "Marcel Mager (INST-IOT/ESB)" Date: Thu, 15 Sep 2016 09:45:28 +0200 Subject: [PATCH 2/7] Fix code review findings: * adapt return type of #getPreAuthenticatedPrincipal to the #getPreAuthenticatedCredentials return type (both Object) * #splitMultiHash returns list instead of array Signed-off-by: Marcel Mager (INST-IOT/ESB) --- ...SourceTrustAuthenticationProviderTest.java | 32 +++++++++------- ...bstractControllerAuthenticationFilter.java | 2 +- ...rPreAuthenticatedSecurityHeaderFilter.java | 35 ++++++++---------- ...okenSourceTrustAuthenticationProvider.java | 14 ++++--- .../security/PreAuthentificationFilter.java | 2 +- ...AuthenticatedSecurityHeaderFilterTest.java | 37 ++++++++++++++++++- 6 files changed, 79 insertions(+), 43 deletions(-) diff --git a/hawkbit-http-security/src/test/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProviderTest.java b/hawkbit-http-security/src/test/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProviderTest.java index eb4cacf33..28e9708ac 100644 --- a/hawkbit-http-security/src/test/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProviderTest.java +++ b/hawkbit-http-security/src/test/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProviderTest.java @@ -21,6 +21,8 @@ import org.springframework.security.authentication.InsufficientAuthenticationExc import org.springframework.security.core.Authentication; import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; +import com.google.common.collect.Lists; + import ru.yandex.qatools.allure.annotations.Description; import ru.yandex.qatools.allure.annotations.Features; import ru.yandex.qatools.allure.annotations.Stories; @@ -45,8 +47,8 @@ public class PreAuthTokenSourceTrustAuthenticationProviderTest { public void principalAndCredentialsNotTheSameThrowsAuthenticationException() { final String principal = "controllerIdURL"; final String credentials = "controllerIdHeader"; - final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken(principal, - credentials); + final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken( + principal, Lists.newArrayList(credentials)); token.setDetails(webAuthenticationDetailsMock); // test, should throw authentication exception @@ -64,11 +66,12 @@ public class PreAuthTokenSourceTrustAuthenticationProviderTest { public void principalAndCredentialsAreTheSameWithNoSourceIpCheckIsSuccessful() { final String principal = "controllerId"; final String credentials = "controllerId"; - final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken(principal, - credentials); + final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken( + principal, Lists.newArrayList(credentials)); token.setDetails(webAuthenticationDetailsMock); - final Authentication authenticate = underTestWithoutSourceIpCheck.authenticate(token); + final Authentication authenticate = underTestWithoutSourceIpCheck + .authenticate(token); assertThat(authenticate.isAuthenticated()).isTrue(); } @@ -78,8 +81,8 @@ public class PreAuthTokenSourceTrustAuthenticationProviderTest { final String remoteAddress = "192.168.1.1"; final String principal = "controllerId"; final String credentials = "controllerId"; - final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken(principal, - credentials); + final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken( + principal, Lists.newArrayList(credentials)); token.setDetails(webAuthenticationDetailsMock); when(webAuthenticationDetailsMock.getRemoteAddress()).thenReturn(remoteAddress); @@ -99,14 +102,15 @@ public class PreAuthTokenSourceTrustAuthenticationProviderTest { public void priniciapAndCredentialsAreTheSameAndSourceIpIsTrusted() { final String principal = "controllerId"; final String credentials = "controllerId"; - final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken(principal, - credentials); + final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken( + principal, Lists.newArrayList(credentials)); token.setDetails(webAuthenticationDetailsMock); when(webAuthenticationDetailsMock.getRemoteAddress()).thenReturn(REQUEST_SOURCE_IP); // test, should throw authentication exception - final Authentication authenticate = underTestWithSourceIpCheck.authenticate(token); + final Authentication authenticate = underTestWithSourceIpCheck + .authenticate(token); assertThat(authenticate.isAuthenticated()).isTrue(); } @@ -116,8 +120,8 @@ public class PreAuthTokenSourceTrustAuthenticationProviderTest { "192.168.1.3" }; final String principal = "controllerId"; final String credentials = "controllerId"; - final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken(principal, - credentials); + final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken( + principal, Lists.newArrayList(credentials)); token.setDetails(webAuthenticationDetailsMock); when(webAuthenticationDetailsMock.getRemoteAddress()).thenReturn(REQUEST_SOURCE_IP); @@ -135,8 +139,8 @@ public class PreAuthTokenSourceTrustAuthenticationProviderTest { final String[] trustedIPAddresses = new String[] { "192.168.1.1", "192.168.1.2", "192.168.1.3" }; final String principal = "controllerId"; final String credentials = "controllerId"; - final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken(principal, - credentials); + final PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken( + principal, Lists.newArrayList(credentials)); token.setDetails(webAuthenticationDetailsMock); when(webAuthenticationDetailsMock.getRemoteAddress()).thenReturn(REQUEST_SOURCE_IP); diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java index c45cfb5e4..cf64e6c59 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java @@ -47,7 +47,7 @@ public abstract class AbstractControllerAuthenticationFilter implements PreAuthe } @Override - public abstract HeaderAuthentication getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); + public abstract Object getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); @Override public abstract Object getPreAuthenticatedCredentials(TenantSecurityToken secruityToken); diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java index 6fd13a1c6..2f0ecaf6a 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java @@ -10,6 +10,7 @@ package org.eclipse.hawkbit.security; import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.eclipse.hawkbit.dmf.json.model.TenantSecurityToken; @@ -79,10 +80,9 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont } @Override - public HeaderAuthentication getPreAuthenticatedPrincipal(final TenantSecurityToken secruityToken) { + public Object getPreAuthenticatedPrincipal(final TenantSecurityToken secruityToken) { // retrieve the common name header and the authority name header from - // the http request and - // combine them together + // the http request and combine them together final String commonNameValue = secruityToken.getHeader(caCommonNameHeader); final String knownSslIssuerConfigurationValue = tenantAware.runAsTenant(secruityToken.getTenant(), sslIssuerNameConfigTenantRunner); @@ -106,23 +106,17 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont sslIssuerNameConfigTenantRunner); String controllerId = secruityToken.getControllerId(); // in case of legacy download artifact, the controller ID is not in the - // URL path, so then - // we just use the common name header + // URL path, so then we just use the common name header if (controllerId == null || "anonymous".equals(controllerId)) { controllerId = secruityToken.getHeader(caCommonNameHeader); } - String[] knownHashes = splitMultiHash(authorityNameConfigurationValue); - if (knownHashes.length > 1) { - Set multiHashes = new HashSet<>(); - final String cntlId = controllerId; - Arrays.asList(knownHashes) - .forEach(hashItem -> multiHashes.add(new HeaderAuthentication(cntlId, hashItem))); - return multiHashes; + List knownHashes = splitMultiHash(authorityNameConfigurationValue); - } else { - return new HeaderAuthentication(controllerId, authorityNameConfigurationValue); - } + Set multiHashes = new HashSet<>(); + final String cntlId = controllerId; + knownHashes.forEach(hashItem -> multiHashes.add(new HeaderAuthentication(cntlId, hashItem))); + return multiHashes; } /** @@ -132,12 +126,15 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont * this request for this tenant. */ private String getIssuerHashHeader(final TenantSecurityToken secruityToken, final String knownIssuerHashes) { + // there may be several knownIssuerHashes configured for the tenant + // separated by a semicolon + List knownHashes = splitMultiHash(knownIssuerHashes); + // iterate over the headers until we get a null header. - String[] knownHashes = splitMultiHash(knownIssuerHashes); int iHeader = 1; String foundHash; while ((foundHash = secruityToken.getHeader(String.format(sslIssuerHashBasicHeader, iHeader))) != null) { - if (Arrays.asList(knownHashes).contains(foundHash)) { + if (knownHashes.contains(foundHash)) { if (LOGGER.isTraceEnabled()) { LOGGER.trace("Found matching ssl issuer hash at position {}", iHeader); } @@ -164,7 +161,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont } } - private static String[] splitMultiHash(String knownIssuerHashes) { - return knownIssuerHashes.split(";"); + private static List splitMultiHash(String knownIssuerHashes) { + return Arrays.asList(knownIssuerHashes.split(";")); } } diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java index c653108a0..7f69180be 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java @@ -97,14 +97,16 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica throw new BadCredentialsException("The provided principal and credentials are not match"); } - // check if principal equals credentials because we want to check if - // e.g. controllerId - // containing in the URL equals the controllerId in the special header - // set by the reverse - // proxy which extracted the CN from the certificate + // The credentials may either be of type HeaderAuthentication or of type + // Collection depending on the authentication mode + // in use (the latter is used in case of trusted reverse-proxy). + // It is checked whether principal equals credentials (respectively if + // credentials contains principal in case of collection) because we want + // to check if e.g. controllerId containing in the URL equals the + // controllerId in the special header set by the reverse-proxy which + // extracted the CN from the certificate. if (principal.equals(credentials)) { successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); - } else if (Collection.class.isAssignableFrom(credentials.getClass())) { final Collection multiValueCredentials = (Collection) credentials; if (multiValueCredentials.contains(principal)) { diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java index f0801ff4d..72f362fa0 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java @@ -36,7 +36,7 @@ public interface PreAuthentificationFilter { * the secruityToken * @return the extracted tenant and controller id */ - HeaderAuthentication getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); + Object getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); /** * Extract the principal credentials from the current secruityToken. diff --git a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java index eb34d5253..dc99df7e7 100644 --- a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java +++ b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java @@ -12,6 +12,8 @@ import static org.junit.Assert.*; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.when; +import java.util.Collection; + import org.eclipse.hawkbit.dmf.json.model.TenantSecurityToken; import org.eclipse.hawkbit.dmf.json.model.TenantSecurityToken.FileResource; import org.eclipse.hawkbit.repository.TenantConfigurationManagement; @@ -102,10 +104,41 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { assertNull(underTest.getPreAuthenticatedPrincipal(securityToken)); } + @Test + @Description("Tests different values for issuer hash header and inspects the credentials") + public void useDifferentValuesForIssuerHashHeader() { + + // prepare security token + TenantSecurityToken securityToken = prepareSecurityToken(); + securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "hash1"); + + when(tenantConfigurationManagementMock.getConfigurationValue( + eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) + .thenReturn(CONFIG_VALUE_MULTI_HASH); + + HeaderAuthentication expected = new HeaderAuthentication("box1", "hash1"); + Collection credentials = (Collection) underTest + .getPreAuthenticatedCredentials(securityToken); + assertTrue(credentials.contains(expected)); + + Object principal = underTest.getPreAuthenticatedPrincipal(securityToken); + assertEquals(expected, principal); + + securityToken = prepareSecurityToken(); + securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "hash2"); + expected = new HeaderAuthentication("box1", "hash2"); + credentials = (Collection) underTest.getPreAuthenticatedCredentials(securityToken); + assertTrue(credentials.contains(expected)); + + principal = underTest.getPreAuthenticatedPrincipal(securityToken); + assertEquals(expected, principal); + + } + private static TenantSecurityToken prepareSecurityToken() { - final TenantSecurityToken securityToken = new TenantSecurityToken("default", "1234", + final TenantSecurityToken securityToken = new TenantSecurityToken("DEFAULT", "box1", FileResource.createFileResourceBySha1("12345")); - securityToken.getHeaders().put(CA_COMMON_NAME, "any"); + securityToken.getHeaders().put(CA_COMMON_NAME, "box1"); return securityToken; } From a0c5915ce651603d4614a3e55c9aa3aeec2eeec7 Mon Sep 17 00:00:00 2001 From: "Marcel Mager (INST-IOT/ESB)" Date: Wed, 21 Sep 2016 16:40:32 +0200 Subject: [PATCH 3/7] Reduce cyclomatic complexity by extracting method Signed-off-by: Marcel Mager (INST-IOT/ESB) --- ...okenSourceTrustAuthenticationProvider.java | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java index 7f69180be..78cfdfd47 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java @@ -87,7 +87,6 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica return null; } - boolean successAuthentication = false; final PreAuthenticatedAuthenticationToken token = (PreAuthenticatedAuthenticationToken) authentication; final Object credentials = token.getCredentials(); final Object principal = token.getPrincipal(); @@ -97,22 +96,7 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica throw new BadCredentialsException("The provided principal and credentials are not match"); } - // The credentials may either be of type HeaderAuthentication or of type - // Collection depending on the authentication mode - // in use (the latter is used in case of trusted reverse-proxy). - // It is checked whether principal equals credentials (respectively if - // credentials contains principal in case of collection) because we want - // to check if e.g. controllerId containing in the URL equals the - // controllerId in the special header set by the reverse-proxy which - // extracted the CN from the certificate. - if (principal.equals(credentials)) { - successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); - } else if (Collection.class.isAssignableFrom(credentials.getClass())) { - final Collection multiValueCredentials = (Collection) credentials; - if (multiValueCredentials.contains(principal)) { - successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); - } - } + boolean successAuthentication = calculateAuthenticationSuccess(principal, credentials, tokenDetails); if (successAuthentication) { final Collection controllerAuthorities = new ArrayList<>(); @@ -126,6 +110,36 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica throw new BadCredentialsException("The provided principal and credentials are not match"); } + + /** + * + * The credentials may either be of type HeaderAuthentication or of type + * Collection depending on the authentication mode in + * use (the latter is used in case of trusted reverse-proxy). It is checked + * whether principal equals credentials (respectively if credentials + * contains principal in case of collection) because we want to check if + * e.g. controllerId containing in the URL equals the controllerId in the + * special header set by the reverse-proxy which extracted the CN from the + * certificate. + * + * @param principal + * @param credentials + * @param tokenDetails + * @return + */ + private boolean calculateAuthenticationSuccess(Object principal, Object credentials, Object tokenDetails) { + boolean successAuthentication = false; + if (principal.equals(credentials)) { + successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); + } else if (Collection.class.isAssignableFrom(credentials.getClass())) { + final Collection multiValueCredentials = (Collection) credentials; + if (multiValueCredentials.contains(principal)) { + successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); + } + } + + return successAuthentication; + } private boolean checkSourceIPAddressIfNeccessary(final Object tokenDetails) { boolean success = authorizedSourceIps == null; From 54c4c8c481fabc2c3a27461fcad2a8609906f92a Mon Sep 17 00:00:00 2001 From: "Marcel Mager (INST-IOT/ESB)" Date: Thu, 22 Sep 2016 16:02:27 +0200 Subject: [PATCH 4/7] Review fixes of code quality. Signed-off-by: Marcel Mager (INST-IOT/ESB) --- ...rPreAuthenticatedSecurityHeaderFilter.java | 4 +- ...okenSourceTrustAuthenticationProvider.java | 41 ++++++++++--------- ...AuthenticatedSecurityHeaderFilterTest.java | 16 +++++--- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java index 2f0ecaf6a..c3d6bd3bd 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java @@ -20,6 +20,8 @@ import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationKey; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.collect.Sets; + /** * An pre-authenticated processing filter which extracts the principal from a * request URI and the credential from a request header in a the @@ -113,7 +115,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont List knownHashes = splitMultiHash(authorityNameConfigurationValue); - Set multiHashes = new HashSet<>(); + Set multiHashes = Sets.newHashSetWithExpectedSize(knownHashes.size()); final String cntlId = controllerId; knownHashes.forEach(hashItem -> multiHashes.add(new HeaderAuthentication(cntlId, hashItem))); return multiHashes; diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java index 78cfdfd47..4abf0deb0 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java @@ -110,34 +110,35 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica throw new BadCredentialsException("The provided principal and credentials are not match"); } - + /** - * - * The credentials may either be of type HeaderAuthentication or of type - * Collection depending on the authentication mode in - * use (the latter is used in case of trusted reverse-proxy). It is checked - * whether principal equals credentials (respectively if credentials - * contains principal in case of collection) because we want to check if - * e.g. controllerId containing in the URL equals the controllerId in the - * special header set by the reverse-proxy which extracted the CN from the - * certificate. - * - * @param principal - * @param credentials - * @param tokenDetails - * @return - */ + * + * The credentials may either be of type HeaderAuthentication or of type + * Collection depending on the authentication mode in + * use (the latter is used in case of trusted reverse-proxy). It is checked + * whether principal equals credentials (respectively if credentials + * contains principal in case of collection) because we want to check if + * e.g. controllerId containing in the URL equals the controllerId in the + * special header set by the reverse-proxy which extracted the CN from the + * certificate. + * + * @param principal + * @param credentials + * @param tokenDetails + * @return true if authentication succeeded, otherwise + * false + */ private boolean calculateAuthenticationSuccess(Object principal, Object credentials, Object tokenDetails) { boolean successAuthentication = false; - if (principal.equals(credentials)) { - successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); - } else if (Collection.class.isAssignableFrom(credentials.getClass())) { + if (Collection.class.isAssignableFrom(credentials.getClass())) { final Collection multiValueCredentials = (Collection) credentials; if (multiValueCredentials.contains(principal)) { successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); } + } else if (principal.equals(credentials)) { + successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); } - + return successAuthentication; } diff --git a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java index dc99df7e7..59605b76d 100644 --- a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java +++ b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java @@ -8,7 +8,10 @@ */ package org.eclipse.hawkbit.security; -import static org.junit.Assert.*; + +//import static org.junit.Assert.*; +import static org.fest.assertions.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.when; @@ -75,7 +78,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { when(tenantConfigurationManagementMock.getConfigurationValue( eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) .thenReturn(CONFIG_VALUE_SINGLE_HASH); - assertNotNull(underTest.getPreAuthenticatedPrincipal(securityToken)); + assertThat(underTest.getPreAuthenticatedPrincipal(securityToken)).isNotNull(); } @Test @@ -88,7 +91,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { when(tenantConfigurationManagementMock.getConfigurationValue( eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) .thenReturn(CONFIG_VALUE_MULTI_HASH); - assertNotNull(underTest.getPreAuthenticatedPrincipal(securityToken)); + assertThat(underTest.getPreAuthenticatedPrincipal(securityToken)).isNotNull(); } @Test @@ -101,7 +104,8 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { when(tenantConfigurationManagementMock.getConfigurationValue( eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) .thenReturn(CONFIG_VALUE_MULTI_HASH); - assertNull(underTest.getPreAuthenticatedPrincipal(securityToken)); + assertThat(underTest.getPreAuthenticatedPrincipal(securityToken)).isNull(); + ; } @Test @@ -119,7 +123,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { HeaderAuthentication expected = new HeaderAuthentication("box1", "hash1"); Collection credentials = (Collection) underTest .getPreAuthenticatedCredentials(securityToken); - assertTrue(credentials.contains(expected)); + assertThat(credentials.contains(expected)).isTrue(); Object principal = underTest.getPreAuthenticatedPrincipal(securityToken); assertEquals(expected, principal); @@ -128,7 +132,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "hash2"); expected = new HeaderAuthentication("box1", "hash2"); credentials = (Collection) underTest.getPreAuthenticatedCredentials(securityToken); - assertTrue(credentials.contains(expected)); + assertThat(credentials.contains(expected)).isTrue(); principal = underTest.getPreAuthenticatedPrincipal(securityToken); assertEquals(expected, principal); From 7176f93ca4bf25089a0a0c3a2a51b7b0be99ee11 Mon Sep 17 00:00:00 2001 From: "Marcel Mager (INST-IOT/ESB)" Date: Fri, 23 Sep 2016 10:11:36 +0200 Subject: [PATCH 5/7] Fix code smells. Signed-off-by: Marcel Mager (INST-IOT/ESB) --- .../ControllerPreAuthenticatedSecurityHeaderFilter.java | 1 - .../PreAuthTokenSourceTrustAuthenticationProvider.java | 6 +++--- .../ControllerPreAuthenticatedSecurityHeaderFilterTest.java | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java index c3d6bd3bd..6965afe3f 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java @@ -9,7 +9,6 @@ package org.eclipse.hawkbit.security; import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Set; diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java index 4abf0deb0..0809f6691 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java @@ -111,7 +111,7 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica throw new BadCredentialsException("The provided principal and credentials are not match"); } - /** + /** * * The credentials may either be of type HeaderAuthentication or of type * Collection depending on the authentication mode in @@ -129,7 +129,7 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica * false */ private boolean calculateAuthenticationSuccess(Object principal, Object credentials, Object tokenDetails) { - boolean successAuthentication = false; + boolean successAuthentication = false; if (Collection.class.isAssignableFrom(credentials.getClass())) { final Collection multiValueCredentials = (Collection) credentials; if (multiValueCredentials.contains(principal)) { @@ -139,7 +139,7 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); } - return successAuthentication; + return successAuthentication; } private boolean checkSourceIPAddressIfNeccessary(final Object tokenDetails) { diff --git a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java index 59605b76d..1275e44b0 100644 --- a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java +++ b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java @@ -126,7 +126,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { assertThat(credentials.contains(expected)).isTrue(); Object principal = underTest.getPreAuthenticatedPrincipal(securityToken); - assertEquals(expected, principal); + assertEquals("hash1 expected in principal!", expected, principal); securityToken = prepareSecurityToken(); securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "hash2"); @@ -135,7 +135,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { assertThat(credentials.contains(expected)).isTrue(); principal = underTest.getPreAuthenticatedPrincipal(securityToken); - assertEquals(expected, principal); + assertEquals("hash2 expected in principal!", expected, principal); } From 97ab881d6e9bebd2b4b4ade32d6b0b23c44d14e3 Mon Sep 17 00:00:00 2001 From: Dominik Herbst Date: Wed, 5 Oct 2016 12:46:21 +0200 Subject: [PATCH 6/7] Code improvements Signed-off-by: Dominik Herbst --- ...lerPreAuthenticatedSecurityHeaderFilter.java | 17 +++++------------ ...hTokenSourceTrustAuthenticationProvider.java | 6 +++++- ...reAuthenticatedSecurityHeaderFilterTest.java | 3 --- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java index 6965afe3f..a536b319d 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java @@ -10,7 +10,7 @@ package org.eclipse.hawkbit.security; import java.util.Arrays; import java.util.List; -import java.util.Set; +import java.util.stream.Collectors; import org.eclipse.hawkbit.dmf.json.model.TenantSecurityToken; import org.eclipse.hawkbit.repository.TenantConfigurationManagement; @@ -19,15 +19,11 @@ import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationKey; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.collect.Sets; - /** * An pre-authenticated processing filter which extracts the principal from a * request URI and the credential from a request header in a the * {@link TenantSecurityToken}. * - * - * */ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractControllerAuthenticationFilter { @@ -112,12 +108,10 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont controllerId = secruityToken.getHeader(caCommonNameHeader); } - List knownHashes = splitMultiHash(authorityNameConfigurationValue); + List knownHashes = splitMultiHashBySemicolon(authorityNameConfigurationValue); - Set multiHashes = Sets.newHashSetWithExpectedSize(knownHashes.size()); final String cntlId = controllerId; - knownHashes.forEach(hashItem -> multiHashes.add(new HeaderAuthentication(cntlId, hashItem))); - return multiHashes; + return knownHashes.stream().map(hashItem -> new HeaderAuthentication(cntlId, hashItem)).collect(Collectors.toSet()); } /** @@ -128,8 +122,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont */ private String getIssuerHashHeader(final TenantSecurityToken secruityToken, final String knownIssuerHashes) { // there may be several knownIssuerHashes configured for the tenant - // separated by a semicolon - List knownHashes = splitMultiHash(knownIssuerHashes); + List knownHashes = splitMultiHashBySemicolon(knownIssuerHashes); // iterate over the headers until we get a null header. int iHeader = 1; @@ -162,7 +155,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont } } - private static List splitMultiHash(String knownIssuerHashes) { + private static List splitMultiHashBySemicolon(String knownIssuerHashes) { return Arrays.asList(knownIssuerHashes.split(";")); } } diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java index 0809f6691..ffe1b7377 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthTokenSourceTrustAuthenticationProvider.java @@ -123,14 +123,18 @@ public class PreAuthTokenSourceTrustAuthenticationProvider implements Authentica * certificate. * * @param principal + * the {@link HeaderAuthentication} from the header * @param credentials + * a single {@link HeaderAuthentication} or a Collection of + * HeaderAuthentication * @param tokenDetails + * authentication details * @return true if authentication succeeded, otherwise * false */ private boolean calculateAuthenticationSuccess(Object principal, Object credentials, Object tokenDetails) { boolean successAuthentication = false; - if (Collection.class.isAssignableFrom(credentials.getClass())) { + if (credentials instanceof Collection) { final Collection multiValueCredentials = (Collection) credentials; if (multiValueCredentials.contains(principal)) { successAuthentication = checkSourceIPAddressIfNeccessary(tokenDetails); diff --git a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java index 1275e44b0..15e9c2055 100644 --- a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java +++ b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java @@ -8,8 +8,6 @@ */ package org.eclipse.hawkbit.security; - -//import static org.junit.Assert.*; import static org.fest.assertions.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.eq; @@ -105,7 +103,6 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) .thenReturn(CONFIG_VALUE_MULTI_HASH); assertThat(underTest.getPreAuthenticatedPrincipal(securityToken)).isNull(); - ; } @Test From 05cebdba54aacc40096f7874f2a7d79678057e1a Mon Sep 17 00:00:00 2001 From: Dominik Herbst Date: Wed, 12 Oct 2016 15:43:20 +0200 Subject: [PATCH 7/7] Used HeaderAuthentication for getPreAuthenticatedPrincipal. Improved test quality. Signed-off-by: Dominik Herbst --- ...bstractControllerAuthenticationFilter.java | 6 --- ...rPreAuthenticatedSecurityHeaderFilter.java | 2 +- .../security/PreAuthentificationFilter.java | 2 +- ...AuthenticatedSecurityHeaderFilterTest.java | 53 +++++++++---------- 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java index cf64e6c59..56baf3bfc 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/AbstractControllerAuthenticationFilter.java @@ -46,12 +46,6 @@ public abstract class AbstractControllerAuthenticationFilter implements PreAuthe return tenantAware.runAsTenant(secruityToken.getTenant(), configurationKeyTenantRunner); } - @Override - public abstract Object getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); - - @Override - public abstract Object getPreAuthenticatedCredentials(TenantSecurityToken secruityToken); - private final class SecurityConfigurationKeyTenantRunner implements TenantAware.TenantRunner { @Override public Boolean run() { diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java index a536b319d..fae0b53ad 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java @@ -77,7 +77,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont } @Override - public Object getPreAuthenticatedPrincipal(final TenantSecurityToken secruityToken) { + public HeaderAuthentication getPreAuthenticatedPrincipal(final TenantSecurityToken secruityToken) { // retrieve the common name header and the authority name header from // the http request and combine them together final String commonNameValue = secruityToken.getHeader(caCommonNameHeader); diff --git a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java index 72f362fa0..f0801ff4d 100644 --- a/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java +++ b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/PreAuthentificationFilter.java @@ -36,7 +36,7 @@ public interface PreAuthentificationFilter { * the secruityToken * @return the extracted tenant and controller id */ - Object getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); + HeaderAuthentication getPreAuthenticatedPrincipal(TenantSecurityToken secruityToken); /** * Extract the principal credentials from the current secruityToken. diff --git a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java index 15e9c2055..80979795f 100644 --- a/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java +++ b/hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java @@ -46,10 +46,13 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { private SecurityContextTenantAware tenantAware = new SecurityContextTenantAware(); private static final String CA_COMMON_NAME = "ca-cn"; + private static final String CA_COMMON_NAME_VALUE = "box1"; private static final String X_SSL_ISSUER_HASH_1 = "X-Ssl-Issuer-Hash-1"; private static final String SINGLE_HASH = "hash1"; + private static final String SECOND_HASH = "hash2"; + private static final String UNKNOWN_HASH = "unknown"; private static final String MULTI_HASH = "hash1;hash2;hash3"; @@ -69,9 +72,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { @Test @Description("Tests the filter for issuer hash based authentication with a single known hash") public void testIssuerHashBasedAuthenticationWithSingleKnownHash() { - // prepare security token - final TenantSecurityToken securityToken = prepareSecurityToken(); - securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, SINGLE_HASH); + final TenantSecurityToken securityToken = prepareSecurityToken(SINGLE_HASH); // use single known hash when(tenantConfigurationManagementMock.getConfigurationValue( eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) @@ -82,9 +83,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { @Test @Description("Tests the filter for issuer hash based authentication with multiple known hashes") public void testIssuerHashBasedAuthenticationWithMultipleKnownHashes() { - // prepare security token - final TenantSecurityToken securityToken = prepareSecurityToken(); - securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, SINGLE_HASH); + final TenantSecurityToken securityToken = prepareSecurityToken(SINGLE_HASH); // use multiple known hashes when(tenantConfigurationManagementMock.getConfigurationValue( eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) @@ -95,9 +94,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { @Test @Description("Tests the filter for issuer hash based authentication with unknown hash") public void testIssuerHashBasedAuthenticationWithUnknownHash() { - // prepare security token - final TenantSecurityToken securityToken = prepareSecurityToken(); - securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "unknown"); + final TenantSecurityToken securityToken = prepareSecurityToken(UNKNOWN_HASH); // use single known hash when(tenantConfigurationManagementMock.getConfigurationValue( eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) @@ -108,39 +105,37 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest { @Test @Description("Tests different values for issuer hash header and inspects the credentials") public void useDifferentValuesForIssuerHashHeader() { + final TenantSecurityToken securityToken1 = prepareSecurityToken(SINGLE_HASH); + final TenantSecurityToken securityToken2 = prepareSecurityToken(SECOND_HASH); - // prepare security token - TenantSecurityToken securityToken = prepareSecurityToken(); - securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "hash1"); + final HeaderAuthentication expected1 = new HeaderAuthentication(CA_COMMON_NAME_VALUE, SINGLE_HASH); + final HeaderAuthentication expected2 = new HeaderAuthentication(CA_COMMON_NAME_VALUE, SECOND_HASH); when(tenantConfigurationManagementMock.getConfigurationValue( eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class))) .thenReturn(CONFIG_VALUE_MULTI_HASH); - HeaderAuthentication expected = new HeaderAuthentication("box1", "hash1"); - Collection credentials = (Collection) underTest - .getPreAuthenticatedCredentials(securityToken); - assertThat(credentials.contains(expected)).isTrue(); + final Collection credentials1 = (Collection) underTest + .getPreAuthenticatedCredentials(securityToken1); + final Collection credentials2 = (Collection) underTest + .getPreAuthenticatedCredentials(securityToken2); - Object principal = underTest.getPreAuthenticatedPrincipal(securityToken); - assertEquals("hash1 expected in principal!", expected, principal); + Object principal1 = underTest.getPreAuthenticatedPrincipal(securityToken1); + Object principal2 = underTest.getPreAuthenticatedPrincipal(securityToken2); - securityToken = prepareSecurityToken(); - securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, "hash2"); - expected = new HeaderAuthentication("box1", "hash2"); - credentials = (Collection) underTest.getPreAuthenticatedCredentials(securityToken); - assertThat(credentials.contains(expected)).isTrue(); + assertThat(credentials1.contains(expected1)).isTrue(); + assertThat(credentials2.contains(expected2)).isTrue(); - principal = underTest.getPreAuthenticatedPrincipal(securityToken); - assertEquals("hash2 expected in principal!", expected, principal); + assertEquals("hash1 expected in principal!", expected1, principal1); + assertEquals("hash2 expected in principal!", expected2, principal2); } - private static TenantSecurityToken prepareSecurityToken() { - final TenantSecurityToken securityToken = new TenantSecurityToken("DEFAULT", "box1", + private static TenantSecurityToken prepareSecurityToken(String issuerHashHeaderValue) { + final TenantSecurityToken securityToken = new TenantSecurityToken("DEFAULT", CA_COMMON_NAME_VALUE, FileResource.createFileResourceBySha1("12345")); - securityToken.getHeaders().put(CA_COMMON_NAME, "box1"); - + securityToken.getHeaders().put(CA_COMMON_NAME, CA_COMMON_NAME_VALUE); + securityToken.getHeaders().put(X_SSL_ISSUER_HASH_1, issuerHashHeaderValue); return securityToken; }