Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sonar lint fixes #682

Merged
merged 15 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ public void handleGetAllRequest_whenUserLacksPermission_returnsEmptyList() {
.thenReturn(emptySet());

CertificateCredentialsView certificateCredentialsView = subjectWithAcls.handleGetAllRequest();
assertEquals(certificateCredentialsView.getCertificates().size(), 0);
assertEquals(0, certificateCredentialsView.getCertificates().size());

}

Expand Down Expand Up @@ -723,10 +723,10 @@ public void handleRegenerate_passesOnTransitionalFlagWhenRegeneratingCertificate
when(certificate.getName()).thenReturn("test");
when(permissionCheckingService.hasPermission(USER, "test", PermissionOperation.WRITE))
.thenReturn(true);
when(certificateService.findByCredentialUuid(eq(UUID_STRING))).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(eq(certificate)))
when(certificateService.findByCredentialUuid(UUID_STRING)).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(certificate))
.thenReturn(generateRequest);
when(universalCredentialGenerator.generate(eq(generateRequest))).thenReturn(newValue);
when(universalCredentialGenerator.generate(generateRequest)).thenReturn(newValue);
when(certificateService.save(eq(certificate), any(), any()))
.thenReturn(mock(CertificateCredentialVersion.class));

Expand Down Expand Up @@ -764,10 +764,10 @@ public void handleRegenerate_whenAclsDisabled_doesNotCheckPermissions_andPassesO

when(certificate.getName()).thenReturn("test");

when(certificateService.findByCredentialUuid(eq(UUID_STRING))).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(eq(certificate)))
when(certificateService.findByCredentialUuid(UUID_STRING)).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(certificate))
.thenReturn(generateRequest);
when(universalCredentialGenerator.generate(eq(generateRequest))).thenReturn(newValue);
when(universalCredentialGenerator.generate(generateRequest)).thenReturn(newValue);
when(certificateService.save(eq(certificate), any(), any()))
.thenReturn(mock(CertificateCredentialVersion.class));

Expand All @@ -790,10 +790,10 @@ public void handleRegenerate_whenConcatenateCasisDisabled_doesNotConcatenateCas(
when(credentialVersion.getCa()).thenReturn(TEST_CA);
when(credentialVersion.getTrustedCa()).thenReturn(TEST_TRUSTED_CA);

when(certificateService.findByCredentialUuid(eq(UUID_STRING))).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(eq(certificate)))
when(certificateService.findByCredentialUuid(UUID_STRING)).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(certificate))
.thenReturn(generateRequest);
when(universalCredentialGenerator.generate(eq(generateRequest))).thenReturn(newValue);
when(universalCredentialGenerator.generate(generateRequest)).thenReturn(newValue);
when(certificateService.save(eq(certificate), any(), any()))
.thenReturn(credentialVersion);

Expand All @@ -816,10 +816,10 @@ public void handleRegenerate_whenConcatenateCasisEnabled_ConcatenateCas() {
when(credentialVersion.getCa()).thenReturn(TEST_CA);
when(credentialVersion.getTrustedCa()).thenReturn(TEST_TRUSTED_CA);

when(certificateService.findByCredentialUuid(eq(UUID_STRING))).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(eq(certificate)))
when(certificateService.findByCredentialUuid(UUID_STRING)).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(certificate))
.thenReturn(generateRequest);
when(universalCredentialGenerator.generate(eq(generateRequest))).thenReturn(newValue);
when(universalCredentialGenerator.generate(generateRequest)).thenReturn(newValue);
when(certificateService.save(eq(certificate), any(), any()))
.thenReturn(credentialVersion);

Expand All @@ -845,10 +845,10 @@ public void handleRegenerate_whenAllowTransitionalParentToSignIsTrue_passesValue
when(credentialVersion.getCa()).thenReturn(TEST_CA);
when(credentialVersion.getTrustedCa()).thenReturn(TEST_TRUSTED_CA);

when(certificateService.findByCredentialUuid(eq(UUID_STRING))).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(eq(certificate)))
when(certificateService.findByCredentialUuid(UUID_STRING)).thenReturn(certificate);
when(generationRequestGenerator.createGenerateRequest(certificate))
.thenReturn(generateRequest);
when(universalCredentialGenerator.generate(eq(generateRequest))).thenReturn(newValue);
when(universalCredentialGenerator.generate(generateRequest)).thenReturn(newValue);
when(certificateService.save(eq(certificate), any(), any()))
.then(
invocation -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,20 +166,20 @@ public void beforeEach() {

@Test
public void deleteCredential_whenTheDeletionSucceeds_deletesTheCredential() {
when(credentialService.delete(eq(CREDENTIAL_NAME))).thenReturn(true);
when(credentialService.delete(CREDENTIAL_NAME)).thenReturn(true);
when(permissionCheckingService.hasPermission(USER, CREDENTIAL_NAME, PermissionOperation.DELETE))
.thenReturn(true);

subjectWithAcls.deleteCredential(CREDENTIAL_NAME);

verify(credentialService, times(1)).delete(eq(CREDENTIAL_NAME));
verify(credentialService, times(1)).delete(CREDENTIAL_NAME);
}

@Test
public void deleteCredential_whenTheCredentialIsNotDeleted_throwsAnException() {
when(permissionCheckingService.hasPermission(USER, CREDENTIAL_NAME, PermissionOperation.DELETE))
.thenReturn(true);
when(credentialService.delete(eq(CREDENTIAL_NAME))).thenReturn(false);
when(credentialService.delete(CREDENTIAL_NAME)).thenReturn(false);

try {
subjectWithAcls.deleteCredential(CREDENTIAL_NAME);
Expand Down Expand Up @@ -207,18 +207,18 @@ public void deleteCredential_whenTheUserLacksPermission_throwsException() {

@Test
public void deleteCredential_whenAclsDisabled_doesNotCheckPermission_andDeletesTheCredential() {
when(credentialService.delete(eq(CREDENTIAL_NAME))).thenReturn(true);
when(credentialService.delete(CREDENTIAL_NAME)).thenReturn(true);

subjectWithoutAcls.deleteCredential(CREDENTIAL_NAME);

verify(credentialService, times(1)).delete(eq(CREDENTIAL_NAME));
verify(credentialService, times(1)).delete(CREDENTIAL_NAME);
verify(permissionCheckingService, times(0)).hasPermission(any(), anyString(), any());
}

@Test
public void getAllCredentialVersions_whenTheCredentialExists_returnsADataResponse() {
final List<CredentialVersion> credentials = newArrayList(version1, version2);
when(credentialService.findAllByName(eq(CREDENTIAL_NAME)))
when(credentialService.findAllByName(CREDENTIAL_NAME))
.thenReturn(credentials);
when(permissionCheckingService.hasPermission(USER, CREDENTIAL_NAME, PermissionOperation.READ))
.thenReturn(true);
Expand All @@ -235,7 +235,7 @@ public void getAllCredentialVersions_whenTheCredentialExists_returnsADataRespons

@Test
public void getAllCredentialVersions_whenTheCredentialDoesNotExist_throwsException() {
when(credentialService.findAllByName(eq(CREDENTIAL_NAME)))
when(credentialService.findAllByName(CREDENTIAL_NAME))
.thenReturn(emptyList());
when(permissionCheckingService.hasPermission(USER, CREDENTIAL_NAME, PermissionOperation.READ))
.thenReturn(true);
Expand Down Expand Up @@ -266,7 +266,7 @@ public void getAllCredentialVersion_whenTheUserLacksPermission_throwsException()
@Test
public void getAllCredentialVersions_whenAclsDisabled_doesNotCheckPermission_andReturnsADataResponse() {
final List<CredentialVersion> credentials = newArrayList(version1, version2);
when(credentialService.findAllByName(eq(CREDENTIAL_NAME)))
when(credentialService.findAllByName(CREDENTIAL_NAME))
.thenReturn(credentials);

final DataResponse credentialVersions = subjectWithoutAcls.getAllCredentialVersions(CREDENTIAL_NAME);
Expand All @@ -282,7 +282,7 @@ public void getAllCredentialVersions_whenAclsDisabled_doesNotCheckPermission_and

@Test
public void getCurrentCredentialVersion_whenTheCredentialExists_returnsDataResponse() {
when(credentialService.findActiveByName(eq(CREDENTIAL_NAME)))
when(credentialService.findActiveByName(CREDENTIAL_NAME))
.thenReturn(Collections.singletonList(version1));
when(permissionCheckingService.hasPermission(USER, CREDENTIAL_NAME, PermissionOperation.READ))
.thenReturn(true);
Expand Down Expand Up @@ -322,7 +322,7 @@ public void getCurrentCredentialVersion_whenTheUserLacksPermission_throwsExcepti

@Test
public void getCurrentCredentialVersion_whenAclsDisabled_andWhenTheCredentialExists_doesNotCheckPermission_returnsDataResponse() {
when(credentialService.findActiveByName(eq(CREDENTIAL_NAME)))
when(credentialService.findActiveByName(CREDENTIAL_NAME))
.thenReturn(Collections.singletonList(version1));


Expand All @@ -338,7 +338,7 @@ public void getCurrentCredentialVersion_whenAclsDisabled_andWhenTheCredentialExi

@Test
public void getCurrentCredentialVersions_whenTheCredentialExists_addsToAuditRecord() {
when(credentialService.findActiveByName(eq(CREDENTIAL_NAME)))
when(credentialService.findActiveByName(CREDENTIAL_NAME))
.thenReturn(Collections.singletonList(version1));

subjectWithoutAcls.getCurrentCredentialVersions(CREDENTIAL_NAME);
Expand Down
8 changes: 6 additions & 2 deletions components/credentials/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ dependencies {
implementation('org.springframework.boot:spring-boot-starter-security')
implementation('org.springframework.boot:spring-boot-starter-validation')
testImplementation("org.springframework.boot:spring-boot-starter-test")
testImplementation("junit:junit")

testImplementation('org.junit.jupiter:junit-jupiter')
testRuntimeOnly('org.junit.platform:junit-platform-launcher')
implementation('com.fasterxml.jackson.module:jackson-module-kotlin')
implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8")
implementation("org.jetbrains.kotlin:kotlin-reflect")
Expand Down Expand Up @@ -93,6 +93,10 @@ test {
exceptionFormat "full"
}

test {
useJUnitPlatform {}
}

systemProperties = System.properties
systemProperties["spring.profiles.active"] = System.getProperty("spring.profiles.active", "unit-test-h2")
systemProperties["java.security.egd"] = System.getProperty("java.security.egd", "file:/dev/urandom")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.passay.CharacterRule;
import org.passay.EnglishCharacterData;

final public class CharacterRuleProvider {
public final class CharacterRuleProvider {

private CharacterRuleProvider() {
super();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import static java.nio.charset.StandardCharsets.UTF_8;

final public class CertificateFormatter {
public final class CertificateFormatter {
public static final String SSH_RSA = "ssh-rsa";

private CertificateFormatter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import java.nio.ByteBuffer;
import java.util.UUID;

final public class UuidUtil {
public final class UuidUtil {

private UuidUtil() {
super();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import com.google.common.net.InetAddresses
import com.google.common.net.InternetDomainName
import org.apache.commons.lang3.StringUtils.isEmpty
import org.cloudfoundry.credhub.ErrorMessages
import org.cloudfoundry.credhub.exceptions.InvalidAlternateNameCertificateException
import org.cloudfoundry.credhub.exceptions.InvalidDurationCertificateException
import org.cloudfoundry.credhub.exceptions.InvalidKeyLengthCertificateException
import org.cloudfoundry.credhub.exceptions.MissingSigningCACertificateException
import org.cloudfoundry.credhub.exceptions.NoSubjectCertificateException
import org.cloudfoundry.credhub.exceptions.ParameterizedValidationException
import org.cloudfoundry.credhub.exceptions.SelfSignedCACertificateException
import java.util.Arrays
import java.util.regex.Pattern

Expand Down Expand Up @@ -86,21 +92,25 @@ class CertificateGenerationRequestParameters {
isEmpty(commonName) &&
isEmpty(country)
) {
throw ParameterizedValidationException(ErrorMessages.MISSING_CERTIFICATE_PARAMETERS)
} else if (isEmpty(caName) && !selfSigned && !isCa) {
throw ParameterizedValidationException(ErrorMessages.MISSING_SIGNING_CA)
} else if (!isEmpty(caName) && selfSigned) {
throw ParameterizedValidationException(ErrorMessages.CA_AND_SELF_SIGN)
throw NoSubjectCertificateException()
}

if (isEmpty(caName) && !selfSigned && !isCa) {
hsinn0 marked this conversation as resolved.
Show resolved Hide resolved
throw MissingSigningCACertificateException()
}

if (!isEmpty(caName) && selfSigned) {
throw SelfSignedCACertificateException()
}

if (!validKeyLengths.contains(keyLength)) {
throw ParameterizedValidationException(ErrorMessages.INVALID_KEY_LENGTH)
throw InvalidKeyLengthCertificateException()
}

if (alternativeNames != null) {
for (name in alternativeNames!!) {
if (!InetAddresses.isInetAddress(name) && !(InternetDomainName.isValid(name) || DNS_WILDCARD_PATTERN.matcher(name).matches())) {
throw ParameterizedValidationException(ErrorMessages.INVALID_ALTERNATE_NAME)
throw InvalidAlternateNameCertificateException()
}
}
}
Expand Down Expand Up @@ -128,7 +138,7 @@ class CertificateGenerationRequestParameters {
}

if (duration < ONE_DAY || duration > TEN_YEARS) {
throw ParameterizedValidationException(ErrorMessages.INVALID_DURATION)
throw InvalidDurationCertificateException()
}

validateParameterLength(commonName, "common name", 64)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package org.cloudfoundry.credhub.credential;

import org.cloudfoundry.credhub.CryptSaltFactory;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsEqual.equalTo;

@RunWith(JUnit4.class)
public class UserCredentialValueTest {
@Test
public void getSalt_returnsAConsistentSalt() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
import org.cloudfoundry.credhub.TestHelper;
import org.cloudfoundry.credhub.entities.EncryptedValue;
import org.cloudfoundry.credhub.entity.CertificateCredentialVersionData;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.cloudfoundry.credhub.utils.CertificateStringConstants.PRIVATE_KEY;
Expand All @@ -20,7 +18,6 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

@RunWith(JUnit4.class)
public class CertificateCredentialVersionTest {

private CertificateCredentialVersion subject;
Expand All @@ -32,7 +29,7 @@ public class CertificateCredentialVersionTest {
private byte[] encryptedValue;
private byte[] nonce;

@Before
@BeforeEach
public void setup() {
TestHelper.getBouncyCastleFipsProvider();
encryptor = mock(Encryptor.class);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package org.cloudfoundry.credhub.domain;

import org.cloudfoundry.credhub.requests.CertificateGenerationRequestParameters;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsEqual.equalTo;

@RunWith(JUnit4.class)
public class CertificateGenerationParametersTest {

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@
import org.cloudfoundry.credhub.utils.BouncyCastleFipsConfigurer;
import org.cloudfoundry.credhub.utils.JsonObjectMapper;
import org.hamcrest.MatcherAssert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.cloudfoundry.credhub.utils.CertificateStringConstants.SELF_SIGNED_CA_CERT;
Expand All @@ -34,7 +32,6 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

@RunWith(JUnit4.class)
public class CredentialFactoryTest {

private static final String CREDENTIAL_NAME = "/test";
Expand All @@ -56,12 +53,12 @@ public class CredentialFactoryTest {
private JsonObjectMapper objectMapper;
private StringGenerationParameters generationParameters;

@BeforeClass
@BeforeAll
public static void setUpAll() {
BouncyCastleFipsConfigurer.configure();
}

@Before
@BeforeEach
public void setup() throws JsonProcessingException {

if (Security.getProvider(BouncyCastleFipsProvider.PROVIDER_NAME) == null) {
Expand Down
Loading