lingwei.kong/hawkbit
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Feature/fix sonar warnings (#1226)

Browse Source 
* Fixed sonar warnings

- "Cognitive Complexity"
- "Do not use replaceAll when not using a regex"
- java:S5869 - Character classes in regular expressions should not contain the same character twice
- Improved bad name
- Typos
- reduced code duplications
- Replaced hand-made wait-utility with Awaitility
- Log messages
- Duplicate code
- Typos
- Removed Thread.sleep, instead relaxed check condition
- Removed use of deprecated API
- Removed use of deprecated API
- Added supress-warnings as I do not see a better way to write the tests
- Removed Thread.sleep / redundant functionality to Awaitility
- Fixed other warnings (use isZero, isEmpty, hasToString)
- Removed/Reduced duplicate code
- Added generics
- Fixed asserts
- removed: field.setAccessible(true) actually should not be needed for public static fields!
- Too long constructor passes arguments in wrong order - how surprisingly...
- Clean-up use of varargs arguments
- Fixed regex
- Fixed typos and other minor stuff
- Making public constructors protected in abstract classes
- Swapped expected and asserted argument
- volatile not enough for syncing threads
- volatile not enough for syncing threads
- out-commented code
- Made regex not-greedy, added tests for verification
- Avoid exposure of thread-local member var

Signed-off-by: Peter Vigier <Peter.Vigier@bosch.io>

* Fixed Sonar warnings

* License header fix

Signed-off-by: Peter Vigier <Peter.Vigier@bosch.io>

* License header fix #2

Signed-off-by: Peter Vigier <Peter.Vigier@bosch.io>

* Fixing review findings

Signed-off-by: Peter Vigier <Peter.Vigier@bosch.io>

* Fixing tests

- Fixed '&' usage in javadoc and typos
- Fixing some warnings

Signed-off-by: Peter Vigier <Peter.Vigier@bosch.io>
This commit is contained in:
Peter Vigier
2022-01-31 21:59:46 +01:00
committed by GitHub
parent 5443b5df9c
commit 44a85f20eb
98 changed files with 2583 additions and 2702 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java
View File

@@ -19,7 +19,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* An pre-authenticated processing filter which extracts the principal from a
* A pre-authenticated processing filter which extracts the principal from a
* request URI and the credential from a request header in a the
* {@link DmfTenantSecurityToken}.
*
@@ -44,9 +44,8 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont
// X-Ssl-Issuer-Dn-1: CN=Bosch-CA-DE,CN=PKI,DC=Bosch,DC=com
// X-Ssl-Issuer-Hash-1: ae:11:f5:6a:0a:e8:74:50:81:0e:0c:37:ec:c5:22:fc
private final String caCommonNameHeader;
// the X-Ssl-Issuer-Hash basic header header which contains the x509
// fingerprint hash, this
// header exists multiple in the request for all trusted chain.
// the X-Ssl-Issuer-Hash basic header: Contains the x509 fingerprint hash, this
// header exists multiple times in the request for all trusted chains.
private final String sslIssuerHashBasicHeader;
/**
@@ -76,18 +75,18 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont
}
@Override
public HeaderAuthentication getPreAuthenticatedPrincipal(final DmfTenantSecurityToken secruityToken) {
public HeaderAuthentication getPreAuthenticatedPrincipal(final DmfTenantSecurityToken securityToken) {
// retrieve the common name header and the authority name header from
// the http request and combine them together
final String commonNameValue = secruityToken.getHeader(caCommonNameHeader);
final String knownSslIssuerConfigurationValue = tenantAware.runAsTenant(secruityToken.getTenant(),
final String commonNameValue = securityToken.getHeader(caCommonNameHeader);
final String knownSslIssuerConfigurationValue = tenantAware.runAsTenant(securityToken.getTenant(),
sslIssuerNameConfigTenantRunner);
final String sslIssuerHashValue = getIssuerHashHeader(secruityToken, knownSslIssuerConfigurationValue);
final String sslIssuerHashValue = getIssuerHashHeader(securityToken, knownSslIssuerConfigurationValue);
if (commonNameValue != null && LOGGER.isTraceEnabled()) {
LOGGER.trace("Found commonNameHeader {}={}, using as credentials", caCommonNameHeader, commonNameValue);
}
if (sslIssuerHashValue != null && LOGGER.isTraceEnabled()) {
LOGGER.trace("Found sslIssuerHash ****, using as credentials for tenant {}", secruityToken.getTenant());
LOGGER.trace("Found sslIssuerHash ****, using as credentials for tenant {}", securityToken.getTenant());
}
if (commonNameValue != null && sslIssuerHashValue != null) {
@@ -97,37 +96,36 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont
}
@Override
public Object getPreAuthenticatedCredentials(final DmfTenantSecurityToken secruityToken) {
final String authorityNameConfigurationValue = tenantAware.runAsTenant(secruityToken.getTenant(),
public Object getPreAuthenticatedCredentials(final DmfTenantSecurityToken securityToken) {
final String authorityNameConfigurationValue = tenantAware.runAsTenant(securityToken.getTenant(),
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
if (controllerId == null || "anonymous".equals(controllerId)) {
controllerId = secruityToken.getHeader(caCommonNameHeader);
}
final String controllerId = //
(securityToken.getControllerId() == null || "anonymous".equals(securityToken.getControllerId()) //
? securityToken.getHeader(caCommonNameHeader)
: securityToken.getControllerId());
final List<String> knownHashes = splitMultiHashBySemicolon(authorityNameConfigurationValue);
final String cntlId = controllerId;
return knownHashes.stream().map(hashItem -> new HeaderAuthentication(cntlId, hashItem))
return knownHashes.stream().map(hashItem -> new HeaderAuthentication(controllerId, hashItem))
.collect(Collectors.toSet());
}
/**
* Iterates over the {@link #sslIssuerHashBasicHeader} basic header
* {@code X-Ssl-Issuer-Hash-%d} and try to finds the same hash as known.
* It's ok if we find the the hash in any the trusted CA chain to accept
* this request for this tenant.
* {@code X-Ssl-Issuer-Hash-%d} and try to find the same hash as known. It's ok
* if we find the hash in any the trusted CA chain to accept this request for
* this tenant.
*/
private String getIssuerHashHeader(final DmfTenantSecurityToken secruityToken, final String knownIssuerHashes) {
private String getIssuerHashHeader(final DmfTenantSecurityToken securityToken, final String knownIssuerHashes) {
// there may be several knownIssuerHashes configured for the tenant
final List<String> knownHashes = splitMultiHashBySemicolon(knownIssuerHashes);
// iterate over the headers until we get a null header.
int iHeader = 1;
String foundHash;
while ((foundHash = secruityToken.getHeader(String.format(sslIssuerHashBasicHeader, iHeader))) != null) {
while ((foundHash = securityToken.getHeader(String.format(sslIssuerHashBasicHeader, iHeader))) != null) {
if (knownHashes.contains(foundHash.toLowerCase())) {
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Found matching ssl issuer hash at position {}", iHeader);
@@ -137,8 +135,8 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont
iHeader++;
}
LOG_SECURITY_AUTH.debug(
"Certifacte request but no matching hash found in headers {} for common name {} in request",
sslIssuerHashBasicHeader, secruityToken.getHeader(caCommonNameHeader));
"Certificate request but no matching hash found in headers {} for common name {} in request",
sslIssuerHashBasicHeader, securityToken.getHeader(caCommonNameHeader));
return null;
}
@@ -156,6 +154,6 @@ public class ControllerPreAuthenticatedSecurityHeaderFilter extends AbstractCont
}
private static List<String> splitMultiHashBySemicolon(final String knownIssuerHashes) {
return Arrays.stream(knownIssuerHashes.split(";|,")).map(String::toLowerCase).collect(Collectors.toList());
return Arrays.stream(knownIssuerHashes.split("[;,]")).map(String::toLowerCase).collect(Collectors.toList());
}
}

17
hawkbit-security-integration/src/test/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilterTest.java
View File

@@ -9,7 +9,6 @@
package org.eclipse.hawkbit.security;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.when;
import java.util.Collection;
@@ -23,11 +22,11 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import io.qameta.allure.Description;
import io.qameta.allure.Feature;
import io.qameta.allure.Story;
import org.mockito.junit.jupiter.MockitoExtension;
@Feature("Unit Tests - Security")
@Story("Issuer hash based authentication")
@@ -57,8 +56,6 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest {
@Mock
private TenantConfigurationManagement tenantConfigurationManagementMock;
@Mock
private DmfTenantSecurityToken tenantSecurityTokenMock;
@Mock
private UserAuthoritiesResolver authoritiesResolver;
@BeforeEach
@@ -74,7 +71,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest {
final DmfTenantSecurityToken securityToken = prepareSecurityToken(SINGLE_HASH);
// use single known hash
when(tenantConfigurationManagementMock.getConfigurationValue(
eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class)))
TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME, String.class))
.thenReturn(CONFIG_VALUE_SINGLE_HASH);
assertThat(underTest.getPreAuthenticatedPrincipal(securityToken)).isNotNull();
}
@@ -84,7 +81,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest {
public void testIssuerHashBasedAuthenticationWithMultipleKnownHashes() {
// use multiple known hashes
when(tenantConfigurationManagementMock.getConfigurationValue(
eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class)))
TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME, String.class))
.thenReturn(CONFIG_VALUE_MULTI_HASH);
assertThat(underTest.getPreAuthenticatedPrincipal(prepareSecurityToken(SINGLE_HASH))).isNotNull();
assertThat(underTest.getPreAuthenticatedPrincipal(prepareSecurityToken(SECOND_HASH))).isNotNull();
@@ -97,7 +94,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest {
final DmfTenantSecurityToken securityToken = prepareSecurityToken(UNKNOWN_HASH);
// use single known hash
when(tenantConfigurationManagementMock.getConfigurationValue(
eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class)))
TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME, String.class))
.thenReturn(CONFIG_VALUE_MULTI_HASH);
assertThat(underTest.getPreAuthenticatedPrincipal(securityToken)).isNull();
}
@@ -112,7 +109,7 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest {
final HeaderAuthentication expected2 = new HeaderAuthentication(CA_COMMON_NAME_VALUE, SECOND_HASH);
when(tenantConfigurationManagementMock.getConfigurationValue(
eq(TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME), eq(String.class)))
TenantConfigurationKey.AUTHENTICATION_MODE_HEADER_AUTHORITY_NAME, String.class))
.thenReturn(CONFIG_VALUE_MULTI_HASH);
final Collection<HeaderAuthentication> credentials1 = (Collection<HeaderAuthentication>) underTest
@@ -123,8 +120,8 @@ public class ControllerPreAuthenticatedSecurityHeaderFilterTest {
final Object principal1 = underTest.getPreAuthenticatedPrincipal(securityToken1);
final Object principal2 = underTest.getPreAuthenticatedPrincipal(securityToken2);
assertThat(credentials1.contains(expected1)).isTrue();
assertThat(credentials2.contains(expected2)).isTrue();
assertThat(credentials1).contains(expected1);
assertThat(credentials2).contains(expected2);
assertThat(expected1).as("hash1 expected in principal!").isEqualTo(principal1);
assertThat(expected2).as("hash2 expected in principal!").isEqualTo(principal2);