diff --git a/smp-angular/src/app/common/alert-message/alert-message.service.ts b/smp-angular/src/app/common/alert-message/alert-message.service.ts index 09e66f237c487b9c52ec8dcb887efb15c5e7d754..95f7ddbb8be69481059ada5c57b24bcf2428b373 100644 --- a/smp-angular/src/app/common/alert-message/alert-message.service.ts +++ b/smp-angular/src/app/common/alert-message/alert-message.service.ts @@ -128,6 +128,10 @@ export class AlertMessageService { this.showMessage(message, 'success', keepAfterNavigationChange, timeoutInSeconds); } + warning(message: string, keepAfterNavigationChange = false, timeoutInSeconds: number = null) { + this.showMessage(message, 'warning', keepAfterNavigationChange, timeoutInSeconds); + } + error(message: any, keepAfterNavigationChange = false, timeoutInSeconds: number = null) { this.showMessage(message, 'error', keepAfterNavigationChange, timeoutInSeconds); } diff --git a/smp-angular/src/app/common/model/keystore-result.model.ts b/smp-angular/src/app/common/model/keystore-result.model.ts index 352592c4fd44c3bb9ddc8741367d8722a29549e5..1f67bd075a4706372a1c46dba60d5f1aa382d4be 100644 --- a/smp-angular/src/app/common/model/keystore-result.model.ts +++ b/smp-angular/src/app/common/model/keystore-result.model.ts @@ -5,4 +5,6 @@ export interface KeystoreResult { errorMessage?: string; addedCertificates?: CertificateRo[]; + + ignoredAliases?: String[]; } diff --git a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts b/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts deleted file mode 100644 index 0e77656dd6511f32cca2081ae74428b7dcf18cbe..0000000000000000000000000000000000000000 --- a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { async, ComponentFixture, TestBed } from '@angular/core/testing'; - -import {KeystoreImportDialogComponent} from "./keystore-import-dialog.component"; - -describe('KeystoreImportDialogComponent', () => { - let component: KeystoreImportDialogComponent; - let fixture: ComponentFixture<KeystoreImportDialogComponent>; - - beforeEach(async(() => { - TestBed.configureTestingModule({ - declarations: [ KeystoreImportDialogComponent ] - }) - .compileComponents(); - })); - - beforeEach(() => { - fixture = TestBed.createComponent(KeystoreImportDialogComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - }); - - it('should be created', () => { - expect(component).toBeTruthy(); - }); -}); diff --git a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts b/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts index 9e872cb6b6b471ffe7c60ecd1e463f2eaaba1000..44d3b89108826a97bff91f740b380480727481ac 100644 --- a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts +++ b/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts @@ -2,8 +2,6 @@ import {Component, Inject} from '@angular/core'; import {MAT_DIALOG_DATA, MatDialogRef} from '@angular/material/dialog'; import {FormControl, FormGroup, UntypedFormBuilder, Validators} from "@angular/forms"; import {AlertMessageService} from "../../../common/alert-message/alert-message.service"; -import {HttpClient} from "@angular/common/http"; -import {SecurityService} from "../../../security/security.service"; import {AdminKeystoreService} from "../admin-keystore.service"; import {KeystoreResult} from "../../../common/model/keystore-result.model"; @@ -18,8 +16,6 @@ export class KeystoreImportDialogComponent { selectedFile: File; constructor(private keystoreService: AdminKeystoreService, - private securityService: SecurityService, - private http: HttpClient, private dialogRef: MatDialogRef<KeystoreImportDialogComponent>, private alertService: AlertMessageService, @Inject(MAT_DIALOG_DATA) public data: any, @@ -27,7 +23,7 @@ export class KeystoreImportDialogComponent { this.formTitle = "Keystore import dialog"; - this.dialogForm = fb.group({ + this.dialogForm = this.fb.group({ 'file': new FormControl({value: '', readonly: false}, [Validators.required]), 'keystoreType': new FormControl({value: '', readonly: false}, [Validators.required]), 'password': new FormControl({value: '', readonly: false}, [Validators.required]), @@ -44,11 +40,14 @@ export class KeystoreImportDialogComponent { importKeystore() { this.keystoreService.uploadKeystore(this.selectedFile, this.dialogForm.controls['keystoreType'].value, - this.dialogForm.controls['password'].value).subscribe((res: KeystoreResult) => { + this.dialogForm.controls['password'].value).subscribe({next: (res: KeystoreResult) => { if (res) { if (res.errorMessage) { this.alertService.exception("Error occurred while importing keystore:" + this.selectedFile.name, res.errorMessage, false); } else { + if (res.ignoredAliases) { + this.alertService.warning("The following aliases have been ignored because they were already present in the current keystore: " + res.ignoredAliases.join(","), false); + } this.keystoreService.notifyKeystoreEntriesUpdated(res.addedCertificates); this.dialogRef.close(); } @@ -56,9 +55,9 @@ export class KeystoreImportDialogComponent { this.alertService.exception("Error occurred while reading keystore.", "Check if uploaded file has valid keystore type.", false); } }, - err => { + error: (err) => { this.alertService.exception('Error uploading keystore file ' + this.selectedFile.name, err.error?.errorDescription); } - ) + }) } } diff --git a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java index 51ce5863f620aa62a4de37e3fd082ca384a4470d..18c0412b5aa6a2143a9edcc9f9a3cd3281c2489a 100644 --- a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java +++ b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java @@ -20,10 +20,15 @@ package eu.europa.ec.edelivery.smp.data.ui; import java.util.ArrayList; import java.util.List; +import java.util.Set; +import java.util.TreeSet; public class KeystoreImportResult { String errorMessage; + + Set<String> ignoredAliases = new TreeSet<>(); + List<CertificateRO> addedCertificates = new ArrayList<>(); public String getErrorMessage() { @@ -37,4 +42,8 @@ public class KeystoreImportResult { public List<CertificateRO> getAddedCertificates() { return addedCertificates; } + + public Set<String> getIgnoredAliases() { + return ignoredAliases; + } } diff --git a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java index 45db040bc63320b56982214bb58c2e2056ba678f..7ddd7fc2dfd5fbeb2ae9c3abda05c094eaa41731 100644 --- a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java +++ b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java @@ -260,7 +260,6 @@ public class UIKeystoreService extends BasicKeystoreService { * @param password password for new keystore file */ public List<CertificateRO> importKeys(KeyStore newKeystore, String password) throws UnrecoverableKeyException, NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException { - String keystoreSecToken = configurationService.getKeystoreCredentialToken(); KeyStore keyStore = loadKeystore(configurationService.getKeystoreFile(), keystoreSecToken); if (keyStore != null) { @@ -274,6 +273,36 @@ public class UIKeystoreService extends BasicKeystoreService { return Collections.emptyList(); } + /** + * Returns entries having certificates that are already present in the current keystore. + * + * @param newKeystore new keystore file to import + * @return the set of duplicate certificates + * @throws KeyStoreException when not able to read the keystore aliases + */ + public Set<String> findDuplicateCertificates(KeyStore newKeystore) throws KeyStoreException { + LOG.debug("Searching for entries with duplicate certificates"); + + String keystoreSecToken = configurationService.getKeystoreCredentialToken(); + KeyStore keyStore = loadKeystore(configurationService.getKeystoreFile(), keystoreSecToken); + if (keyStore != null) { + return list(newKeystore.aliases()) + .stream() + .filter(alias -> containsCertificate(keyStore, newKeystore, alias)) + .peek(alias -> LOG.debug("Found entry with duplicate certificate [{}]", alias)) + .collect(Collectors.toSet()); + } + return Collections.emptySet(); + } + + private boolean containsCertificate(KeyStore keyStore, KeyStore newKeystore, String alias) { + try { + return keyStore.getCertificateAlias(newKeystore.getCertificate(alias)) != null; + } catch (KeyStoreException e) { + throw new SMPRuntimeException(ErrorCode.CERTIFICATE_ERROR, "An error occurred while loading the entry " + alias, e); + } + } + /** * Delete keys smp keystore * diff --git a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java index 66726448264849c212f3fcbfd83fd9377c4f7ec4..88625ad60d25dccbec62a77f3e399fda6e838597 100644 --- a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java +++ b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java @@ -25,6 +25,8 @@ import eu.europa.ec.edelivery.smp.data.model.user.DBUser; import eu.europa.ec.edelivery.smp.data.ui.CertificateRO; import eu.europa.ec.edelivery.smp.exceptions.CertificateAlreadyRegisteredException; import eu.europa.ec.edelivery.smp.exceptions.CertificateNotTrustedException; +import eu.europa.ec.edelivery.smp.exceptions.ErrorCode; +import eu.europa.ec.edelivery.smp.exceptions.SMPRuntimeException; import eu.europa.ec.edelivery.smp.logging.SMPLogger; import eu.europa.ec.edelivery.smp.logging.SMPLoggerFactory; import eu.europa.ec.edelivery.smp.services.CRLVerifierService; @@ -502,9 +504,14 @@ public class UITruststoreService extends BasicKeystoreService { } public String addCertificate(String alias, X509Certificate certificate) throws NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException { - KeyStore truststore = loadTruststore(getTruststoreFile()); if (truststore != null) { + + String certificateAlias = truststore.getCertificateAlias(certificate); + if (certificateAlias != null) { + throw new SMPRuntimeException(ErrorCode.CERTIFICATE_ERROR, "duplicate", "The certificate you are trying to upload already exists under the [" + certificateAlias + "] entry"); + } + String aliasPrivate = StringUtils.isBlank(alias) ? createAliasFromCert(certificate, truststore) : alias.trim(); if (truststore.containsAlias(aliasPrivate)) { @@ -529,8 +536,6 @@ public class UITruststoreService extends BasicKeystoreService { } public String createAliasFromCert(X509Certificate x509cert, KeyStore truststore) { - - String dn = x509cert.getSubjectX500Principal().getName(); String alias = null; try { diff --git a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java index 7fcf7ada5c730aa6d942825561f9c97344ac52f7..72085836993ac1e81ae8ef9a2cf0cc2dfe4839bf 100644 --- a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java +++ b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java @@ -36,14 +36,16 @@ import org.springframework.test.util.ReflectionTestUtils; import javax.net.ssl.KeyManager; import javax.security.auth.x500.X500Principal; import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.security.*; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static org.junit.jupiter.api.Assertions.*; @@ -254,5 +256,17 @@ public class UIKeystoreServiceTest extends AbstractServiceIntegrationTest { assertNotEquals(km, testInstance.getKeyManagers()[0]); } + @Test + void testFindDuplicateCertificate() throws Exception { + // given + KeyStore keyStore = loadKeystore("test-import.jks", "NewPassword1234", "JKS"); + KeyStore sameKeyStore = keyStore; + testInstance.importKeys(keyStore, "NewPassword1234"); + + // when + Set<String> duplicateCertificates = testInstance.findDuplicateCertificates(keyStore); + // then + assertEquals(new HashSet<>(Arrays.asList("testcertificatea", "testcertificateb")), duplicateCertificates); + } } diff --git a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java index 48439490ee90592ae7dcab26a37835e2375c81d8..d8218b2e0f070bd62ac6085316ed30ee70777b67 100644 --- a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java +++ b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java @@ -22,12 +22,14 @@ import eu.europa.ec.edelivery.smp.data.dao.UserDao; import eu.europa.ec.edelivery.smp.data.model.user.DBUser; import eu.europa.ec.edelivery.smp.data.ui.CertificateRO; import eu.europa.ec.edelivery.smp.exceptions.CertificateNotTrustedException; +import eu.europa.ec.edelivery.smp.exceptions.SMPRuntimeException; import eu.europa.ec.edelivery.smp.services.CRLVerifierService; import eu.europa.ec.edelivery.smp.services.ConfigurationService; import eu.europa.ec.edelivery.smp.testutil.X509CertificateTestUtils; import org.apache.commons.io.FileUtils; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -401,6 +403,26 @@ class UITruststoreServiceTest { assertEquals(count + 1, testInstance.getNormalizedTrustedList().size()); } + @Test + void testAddCertificate_DuplicateCertificate() throws Exception { + String alias = "duplicate"; + String subject = "CN=Something,O=test,C=EU"; + X509Certificate certificate = X509CertificateTestUtils.createX509CertificateForTest(subject); + + doReturn(targetTruststore.toFile()).when(configurationService).getTruststoreFile(); + doReturn(targetTruststore.toFile()).when(configurationService).getTruststoreFile(); + doReturn(truststorePassword).when(configurationService).getTruststoreCredentialToken(); + testInstance.refreshData(); + int count = testInstance.getNormalizedTrustedList().size(); + + // when + testInstance.addCertificate(alias, certificate); + SMPRuntimeException smpRuntimeException = assertThrows(SMPRuntimeException.class, () -> testInstance.addCertificate(alias, certificate)); + + // then + Assertions.assertEquals("Certificate error [duplicate]. Error: The certificate you are trying to upload already exists under the [duplicate] entry!", smpRuntimeException.getMessage()); + } + @Test void testDeleteCertificate() throws Exception { String subject = "CN=Something,O=test,C=EU"; @@ -418,7 +440,6 @@ class UITruststoreServiceTest { assertEquals(count - 1, testInstance.getNormalizedTrustedList().size()); } - protected void resetKeystore() throws IOException { FileUtils.deleteDirectory(targetDirectory.toFile()); FileUtils.copyDirectory(resourceDirectory.toFile(), targetDirectory.toFile()); diff --git a/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java b/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java index a973ee80e5f4d63693e5502d1c6bccca97e79578..0c03e7d52638ec2dadd1b8c57d823a32fe8148e7 100644 --- a/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java +++ b/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java @@ -18,6 +18,7 @@ */ package eu.europa.ec.edelivery.smp.ui.internal; +import eu.europa.ec.edelivery.security.utils.KeystoreUtils; import eu.europa.ec.edelivery.smp.data.ui.CertificateRO; import eu.europa.ec.edelivery.smp.data.ui.KeystoreImportResult; import eu.europa.ec.edelivery.smp.data.ui.enums.EntityROStatus; @@ -38,7 +39,9 @@ import java.security.NoSuchAlgorithmException; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static eu.europa.ec.edelivery.smp.ui.ResourceConstants.*; import static org.springframework.util.MimeTypeUtils.APPLICATION_JSON_VALUE; @@ -90,9 +93,11 @@ public class KeystoreAdminController { try { KeyStore keyStore = KeyStore.getInstance(keystoreType); keyStore.load(new ByteArrayInputStream(fileBytes), password.toCharArray()); + Set<String> ignoredAliases = removeDuplicateCertificates(keyStore); List<CertificateRO> certificateROList = uiKeystoreService.importKeys(keyStore, password); certificateROList.forEach(cert -> cert.setStatus(EntityROStatus.NEW.getStatusNumber())); keystoreImportResult.getAddedCertificates().addAll(certificateROList); + keystoreImportResult.getIgnoredAliases().addAll(ignoredAliases); } catch (KeyStoreException | CertificateException | NoSuchAlgorithmException | IOException | UnrecoverableKeyException e) { String msg = e.getClass().getName() + " occurred while reading the keystore: " + e.getMessage(); @@ -102,6 +107,18 @@ public class KeystoreAdminController { return keystoreImportResult; } + private Set<String> removeDuplicateCertificates(KeyStore keyStore) throws KeyStoreException { + Set<String> duplicateAliases = new HashSet<>(); + + Set<String> duplicateCertificateAliases = uiKeystoreService.findDuplicateCertificates(keyStore); + for (String alias: duplicateCertificateAliases) { + KeystoreUtils.deleteCertificate(keyStore, alias); + duplicateAliases.add(alias); + } + + return duplicateAliases; + } + @PreAuthorize("@smpAuthorizationService.systemAdministrator AND @smpAuthorizationService.isCurrentlyLoggedIn(#userEncId)") @DeleteMapping(value = "/{user-id}/delete/{cert-alias}", produces = APPLICATION_JSON_VALUE) public CertificateRO deleteCertificate(@PathVariable(PATH_PARAM_ENC_USER_ID) String userEncId, diff --git a/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java b/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java index 30742c1cff1ff5781e856a5c4292eeaf0cb2c61b..9958cd26f210a229d3d8cbdf7d9177bd0b5dda43 100644 --- a/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java +++ b/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java @@ -36,7 +36,10 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.stream.Collectors; import static eu.europa.ec.edelivery.smp.test.testutils.MockMvcUtils.*; import static eu.europa.ec.edelivery.smp.ui.ResourceConstants.CONTEXT_PATH_INTERNAL_KEYSTORE; @@ -123,11 +126,13 @@ class KeystoreAdminControllerIT extends AbstractControllerTest { } @Test - void uploadKeystoreOK() throws Exception { - + void uploadKeystoreOK_removeDuplicateCertificates() throws Exception { MockHttpSession session = loginWithSystemAdmin(mvc); UserRO userRO = getLoggedUserData(mvc, session); - int countStart = uiKeystoreService.getKeystoreEntriesList().size(); + assertEquals(uiKeystoreService.getKeystoreEntriesList().stream() + .map(CertificateRO::getAlias) + .collect(Collectors.toSet()), new HashSet<>(Arrays.asList("second_domain_alias", "single_domain_key"))); + // given when MvcResult result = mvc.perform(post(PATH + "/" + userRO.getUserId() + "/upload/JKS/test123") .session(session) @@ -140,7 +145,11 @@ class KeystoreAdminControllerIT extends AbstractControllerTest { assertNotNull(res); assertNull(res.getErrorMessage()); - assertEquals(countStart + 1, uiKeystoreService.getKeystoreEntriesList().size()); + assertTrue(res.getAddedCertificates().isEmpty()); + assertEquals(res.getIgnoredAliases(), new HashSet(Arrays.asList("single_domain_key"))); + assertEquals(uiKeystoreService.getKeystoreEntriesList().stream() + .map(CertificateRO::getAlias) + .collect(Collectors.toSet()), new HashSet<>(Arrays.asList("second_domain_alias", "single_domain_key"))); } @Test