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) <Marcel.Mager@bosch-si.com>
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<HeaderAuthentication> multiHashes = new HashSet<>();
|
||||
final String cntlId = controllerId;
|
||||
Arrays.asList(knownHashes)
|
||||
.forEach(hashItem -> multiHashes.add(new HeaderAuthentication(cntlId, hashItem)));
|
||||
return multiHashes;
|
||||
List<String> knownHashes = splitMultiHash(authorityNameConfigurationValue);
|
||||
|
||||
} else {
|
||||
return new HeaderAuthentication(controllerId, authorityNameConfigurationValue);
|
||||
}
|
||||
Set<HeaderAuthentication> 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<String> 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<String> splitMultiHash(String knownIssuerHashes) {
|
||||
return Arrays.asList(knownIssuerHashes.split(";"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<HeaderAuthentication> 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)) {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<HeaderAuthentication> credentials = (Collection<HeaderAuthentication>) 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<HeaderAuthentication>) 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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user