diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/AccountLinkingController.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/AccountLinkingController.java index 43b8e9cd1..c917308f1 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/AccountLinkingController.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/AccountLinkingController.java @@ -25,15 +25,16 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpStatus; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.Authentication; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.CrossOrigin; +import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; @@ -54,14 +55,13 @@ public class AccountLinkingController extends ExternalAuthenticationHandlerSuppo @Value("${iam.account-linking.enable}") private Boolean accountLinkingEnabled; - @Autowired public AccountLinkingController(AccountLinkingService s) { linkingService = s; } @PreAuthorize("hasRole('USER')") - @RequestMapping(value = "/X509", method = RequestMethod.DELETE) + @DeleteMapping(value = "/X509") @ResponseStatus(value = HttpStatus.NO_CONTENT) public void unlinkX509Certificate(Principal principal, @RequestParam String certificateSubject, RedirectAttributes attributes) { @@ -72,7 +72,7 @@ public void unlinkX509Certificate(Principal principal, @RequestParam String cert @PreAuthorize("hasRole('USER')") - @RequestMapping(value = "/X509", method = RequestMethod.POST) + @PostMapping(value = "/X509") public String linkX509Certificate(HttpSession session, Principal principal, RedirectAttributes attributes) { @@ -104,7 +104,7 @@ private void checkAccountLinkingEnabled(RedirectAttributes attributes) { } @PreAuthorize("hasRole('USER')") - @RequestMapping(value = "/{type}", method = RequestMethod.POST) + @PostMapping(value = "/{type}") public void linkAccount(@PathVariable ExternalAuthenticationType type, @RequestParam(value = "id", required = false) String externalIdpId, Authentication authn, final RedirectAttributes redirectAttributes, HttpServletRequest request, @@ -163,7 +163,7 @@ public String finalizeAccountLinking(@PathVariable ExternalAuthenticationType ty } @PreAuthorize("hasRole('USER')") - @RequestMapping(value = "/{type}", method = RequestMethod.DELETE) + @DeleteMapping(value = "/{type}") @ResponseStatus(value = HttpStatus.NO_CONTENT) public void unlinkAccount(@PathVariable ExternalAuthenticationType type, Principal principal, @RequestParam("iss") String issuer, @RequestParam("sub") String subject, diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java index 15c06826b..a345a20ba 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java @@ -22,7 +22,6 @@ import java.util.Date; import java.util.Optional; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.security.core.userdetails.UsernameNotFoundException; @@ -30,9 +29,9 @@ import it.infn.mw.iam.audit.events.account.AccountLinkedEvent; import it.infn.mw.iam.audit.events.account.AccountUnlinkedEvent; -import it.infn.mw.iam.audit.events.account.X509CertificateLinkedEvent; -import it.infn.mw.iam.audit.events.account.X509CertificateUnlinkedEvent; import it.infn.mw.iam.audit.events.account.X509CertificateUpdatedEvent; +import it.infn.mw.iam.audit.events.account.x509.X509CertificateLinkedEvent; +import it.infn.mw.iam.audit.events.account.x509.X509CertificateUnlinkedEvent; import it.infn.mw.iam.authn.AbstractExternalAuthenticationToken; import it.infn.mw.iam.authn.ExternalAccountLinker; import it.infn.mw.iam.authn.ExternalAuthenticationRegistrationInfo.ExternalAuthenticationType; @@ -44,18 +43,21 @@ import it.infn.mw.iam.persistence.model.IamX509Certificate; import it.infn.mw.iam.persistence.model.IamX509ProxyCertificate; import it.infn.mw.iam.persistence.repository.IamAccountRepository; +import it.infn.mw.iam.persistence.repository.IamX509CertificateRepository; @Service public class DefaultAccountLinkingService implements AccountLinkingService, ApplicationEventPublisherAware { final IamAccountRepository iamAccountRepository; + final IamX509CertificateRepository certificateRepository; final ExternalAccountLinker externalAccountLinker; private ApplicationEventPublisher eventPublisher; - @Autowired - public DefaultAccountLinkingService(IamAccountRepository repo, ExternalAccountLinker linker) { + public DefaultAccountLinkingService(IamAccountRepository repo, + IamX509CertificateRepository certificateRepository, ExternalAccountLinker linker) { this.iamAccountRepository = repo; + this.certificateRepository = certificateRepository; this.externalAccountLinker = linker; } @@ -140,27 +142,25 @@ public void linkX509Certificate(Principal authenticatedUser, IamAccount userAccount = findAccount(authenticatedUser); - iamAccountRepository.findByCertificateSubject(x509Credential.getSubject()) - .ifPresent(linkedAccount -> { - if (!linkedAccount.getUuid().equals(userAccount.getUuid())) { - throw new AccountAlreadyLinkedError( - format("X.509 credential with subject '%s' is already linked to another user", - x509Credential.getSubject())); - } - }); + Optional linkedAccount = + certificateRepository.findBySubject(x509Credential.getSubject()).stream().findFirst(); - Optional linkedCert = userAccount.getX509Certificates() - .stream() - .filter(c -> c.getSubjectDn().equals(x509Credential.getSubject()) && c.getIssuerDn().equals(x509Credential.getIssuer())) - .findAny(); + // check if the x509Credential is linked to another user + if (linkedAccount.isPresent() && !linkedAccount.get().getUuid().equals(userAccount.getUuid())) { + throw new AccountAlreadyLinkedError( + format("X.509 credential with subject '%s' is already linked to another user", + x509Credential.getSubject())); + } - if (linkedCert.isPresent()) { + Optional linkedCertificate = certificateRepository + .findBySubjectAndIssuer(x509Credential.getSubject(), x509Credential.getIssuer()); - linkedCert.ifPresent(c -> { - c.setCertificate(x509Credential.getCertificateChainPemString()); - c.setLastUpdateTime(new Date()); - }); + if (linkedCertificate.isPresent()) { + linkedCertificate.get().setCertificate(x509Credential.getCertificateChainPemString()); + linkedCertificate.get().setLastUpdateTime(new Date()); + certificateRepository.save(linkedCertificate.get()); + userAccount.getX509Certificates().add(linkedCertificate.get()); userAccount.touch(); iamAccountRepository.save(userAccount); @@ -168,28 +168,23 @@ public void linkX509Certificate(Principal authenticatedUser, String.format("User '%s' has updated its linked certificate with subject '%s'", userAccount.getUsername(), x509Credential.getSubject()), x509Credential)); - } else { Date now = new Date(); IamX509Certificate newCert = x509Credential.asIamX509Certificate(); newCert.setLabel(String.format("cert-%d", userAccount.getX509Certificates().size())); - newCert.setCreationTime(now); newCert.setLastUpdateTime(now); - newCert.setPrimary(true); newCert.setAccount(userAccount); + certificateRepository.save(newCert); userAccount.getX509Certificates().add(newCert); userAccount.touch(); - iamAccountRepository.save(userAccount); - eventPublisher.publishEvent(new X509CertificateLinkedEvent(this, userAccount, String.format("User '%s' linked certificate with subject '%s' to his/her membership", userAccount.getUsername(), x509Credential.getSubject()), x509Credential)); - } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/X509CertificateUpdatedEvent.java b/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/X509CertificateUpdatedEvent.java index fee6c70c4..2efa5d343 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/X509CertificateUpdatedEvent.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/X509CertificateUpdatedEvent.java @@ -15,6 +15,7 @@ */ package it.infn.mw.iam.audit.events.account; +import it.infn.mw.iam.audit.events.account.x509.X509CertificateLinkedEvent; import it.infn.mw.iam.authn.x509.IamX509AuthenticationCredential; import it.infn.mw.iam.persistence.model.IamAccount; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/X509CertificateLinkedEvent.java b/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/x509/X509CertificateLinkedEvent.java similarity index 92% rename from iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/X509CertificateLinkedEvent.java rename to iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/x509/X509CertificateLinkedEvent.java index 0604a29c9..78d887cf0 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/X509CertificateLinkedEvent.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/x509/X509CertificateLinkedEvent.java @@ -13,8 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package it.infn.mw.iam.audit.events.account; +package it.infn.mw.iam.audit.events.account.x509; +import it.infn.mw.iam.audit.events.account.AccountEvent; import it.infn.mw.iam.authn.x509.IamX509AuthenticationCredential; import it.infn.mw.iam.persistence.model.IamAccount; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/x509/X509CertificateUnlinkedEvent.java b/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/x509/X509CertificateUnlinkedEvent.java new file mode 100644 index 000000000..a8bc47dbc --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/audit/events/account/x509/X509CertificateUnlinkedEvent.java @@ -0,0 +1,41 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.audit.events.account.x509; + +import it.infn.mw.iam.audit.events.account.AccountEvent; +import it.infn.mw.iam.persistence.model.IamAccount; + +public class X509CertificateUnlinkedEvent extends AccountEvent { + + /** + * + */ + private static final long serialVersionUID = 1L; + + + private final String certificateSubject; + + public X509CertificateUnlinkedEvent(Object source, IamAccount account, String message, + String certificateSubject) { + super(source, account, message); + this.certificateSubject = certificateSubject; + } + + public String getCertificateSubject() { + return certificateSubject; + } + +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java index a4cf1b9cd..26eaf1657 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java @@ -44,6 +44,7 @@ import java.util.Arrays; import java.util.Date; import java.util.HashSet; +import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -61,6 +62,7 @@ import it.infn.mw.iam.persistence.model.IamAccount; import it.infn.mw.iam.persistence.model.IamX509Certificate; import it.infn.mw.iam.persistence.repository.IamAccountRepository; +import it.infn.mw.iam.persistence.repository.IamX509CertificateRepository; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; import junit.framework.AssertionFailedError; @@ -73,6 +75,9 @@ public class X509AuthenticationIntegrationTests extends X509TestSupport { @Autowired private IamAccountRepository iamAccountRepo; + @Autowired + private IamX509CertificateRepository iamX509CertificateRepo; + @Autowired private MockMvc mvc; @@ -173,6 +178,7 @@ public void testx509AccountLinking() throws Exception { (IamX509AuthenticationCredential) session.getAttribute(X509_CREDENTIAL_SESSION_KEY); assertThat(credential.getSubject(), equalTo(TEST_0_SUBJECT)); + assertThat(credential.getIssuer(), equalTo(TEST_0_ISSUER)); String confirmationMessage = String.format("Certificate '%s' linked succesfully", credential.getSubject()); @@ -183,6 +189,13 @@ public void testx509AccountLinking() throws Exception { .andExpect( flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMessage))); + Optional linkedUser = iamX509CertificateRepo.findBySubject(TEST_0_SUBJECT).stream().findFirst(); + assertThat(linkedUser.isPresent(), is(true)); + assertThat(linkedUser.get().getUsername(), is("test")); + + Optional test0Cert = iamX509CertificateRepo.findBySubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER); + assertThat(test0Cert.isPresent(), is(true)); + IamAccount linkedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT) .orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found")); @@ -219,10 +232,13 @@ public void testx509AccountLinking() throws Exception { .andExpect( flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMsg))); - linkedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT) - .orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found")); - - assertThat(linkedAccount.getX509Certificates().size(), is(2)); + Optional testCert1 = iamX509CertificateRepo.findBySubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER); + assertThat(testCert1.isPresent(), is(true)); + assertThat(testCert1.get().getAccount().getUsername(), is("test")); + + Optional testCert2 = iamX509CertificateRepo.findBySubjectAndIssuer(TEST_0_SUBJECT, TEST_NEW_ISSUER); + assertThat(testCert2.isPresent(), is(true)); + assertThat(testCert2.get().getAccount().getUsername(), is("test")); } diff --git a/iam-persistence/src/main/java/it/infn/mw/iam/persistence/repository/IamX509CertificateRepository.java b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/repository/IamX509CertificateRepository.java new file mode 100644 index 000000000..7c4256319 --- /dev/null +++ b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/repository/IamX509CertificateRepository.java @@ -0,0 +1,23 @@ +package it.infn.mw.iam.persistence.repository; + +import java.util.List; +import java.util.Optional; + +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.PagingAndSortingRepository; +import org.springframework.data.repository.query.Param; + +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamX509Certificate; + +public interface IamX509CertificateRepository + extends PagingAndSortingRepository { + + @Query("select c.account from IamX509Certificate c where c.subjectDn = :subject") + List findBySubject(@Param("subject") String subject); + + @Query("select c from IamX509Certificate c where c.subjectDn = :subject and c.issuerDn = :issuer") + Optional findBySubjectAndIssuer(@Param("subject") String subject, + @Param("issuer") String issuer); + +} diff --git a/iam-persistence/src/main/resources/db/migration/h2/V97__delete_unique_subject_dn.sql b/iam-persistence/src/main/resources/db/migration/h2/V97__delete_unique_subject_dn.sql index b63dbad8d..8b31f38a2 100644 --- a/iam-persistence/src/main/resources/db/migration/h2/V97__delete_unique_subject_dn.sql +++ b/iam-persistence/src/main/resources/db/migration/h2/V97__delete_unique_subject_dn.sql @@ -1,2 +1,3 @@ -ALTER TABLE iam_x509_cert DROP CONSTRAINT CONSTRAINT_32; +ALTER TABLE iam_x509_cert DROP CONSTRAINT IF EXISTS CONSTRAINT_327; +ALTER TABLE iam_x509_cert DROP CONSTRAINT IF EXISTS UNIQ_IAM_X509_CERT_CERIFICATE; CREATE INDEX idx_subject_dn ON iam_x509_cert(subject_dn); \ No newline at end of file