Skip to content

Commit

Permalink
Revert "Change search by certificate subject"
Browse files Browse the repository at this point in the history
This reverts commit a7d7498.
  • Loading branch information
rmiccoli committed Dec 1, 2023
1 parent 9f96d76 commit 5347c37
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ private Supplier<IllegalArgumentException> groupNotFoundError(String groupNameOr
}

@Override
public ScimListResponse<ScimUser> findAccountByCertificateSubjectAndIssuer(String certSubject, String certIssuer) {
Optional<IamAccount> account = repo.findByCertificateSubjectAndIssuer(certSubject, certIssuer);
public ScimListResponse<ScimUser> findAccountByCertificateSubject(String certSubject) {
Optional<IamAccount> account = repo.findByCertificateSubject(certSubject);
ScimListResponseBuilder<ScimUser> builder = ScimListResponse.builder();
account.ifPresent(a -> builder.singleResource(converter.dtoFromEntity(a)));
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class FindAccountController {
public static final String FIND_BY_LABEL_RESOURCE = "/iam/account/find/bylabel";
public static final String FIND_BY_EMAIL_RESOURCE = "/iam/account/find/byemail";
public static final String FIND_BY_USERNAME_RESOURCE = "/iam/account/find/byusername";
public static final String FIND_BY_CERT_SUBJECT_RESOURCE = "/iam/account/find/bycertsubjectandissuer";
public static final String FIND_BY_CERT_SUBJECT_RESOURCE = "/iam/account/find/bycertsubject";
public static final String FIND_BY_GROUP_RESOURCE = "/iam/account/find/bygroup/{groupUuid}";
public static final String FIND_NOT_IN_GROUP_RESOURCE =
"/iam/account/find/notingroup/{groupUuid}";
Expand Down Expand Up @@ -81,8 +81,8 @@ public ListResponseDTO<ScimUser> findByUsername(@RequestParam(required = true) S
@RequestMapping(method = GET, value = FIND_BY_CERT_SUBJECT_RESOURCE,
produces = ScimConstants.SCIM_CONTENT_TYPE)
public ListResponseDTO<ScimUser> findByCertSubject(
@RequestParam(required = true) String certificateSubject, @RequestParam(required = true) String certificateIssuer) {
return service.findAccountByCertificateSubjectAndIssuer(certificateSubject, certificateIssuer);
@RequestParam(required = true) String certificateSubject) {
return service.findAccountByCertificateSubject(certificateSubject);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public interface FindAccountService {

ScimListResponse<ScimUser> findInactiveAccounts(Pageable pageable);

ScimListResponse<ScimUser> findAccountByCertificateSubjectAndIssuer(String certSubject, String certIssuer);
ScimListResponse<ScimUser> findAccountByCertificateSubject(String certSubject);

ScimListResponse<ScimUser> findActiveAccounts(Pageable pageable);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void linkX509Certificate(Principal authenticatedUser,

IamAccount userAccount = findAccount(authenticatedUser);

iamAccountRepository.findByCertificateSubjectAndIssuer(x509Credential.getSubject(), x509Credential.getIssuer())
iamAccountRepository.findByCertificateSubject(x509Credential.getSubject())
.ifPresent(linkedAccount -> {
if (!linkedAccount.getUuid().equals(userAccount.getUuid())) {
throw new AccountAlreadyLinkedError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public Adders(IamAccountRepository repo, IamAccountService accountService,
findByOidcId = id -> repo.findByOidcId(id.getIssuer(), id.getSubject());
findBySamlId = repo::findBySamlId;
findBySshKey = key -> repo.findBySshKeyValue(key.getValue());
findByX509CertificateSubject = cert -> repo.findByCertificateSubjectAndIssuer(cert.getSubjectDn(), cert.getIssuerDn());
findByX509CertificateSubject = cert -> repo.findByCertificateSubject(cert.getSubjectDn());

oidcIdAddChecks = buildOidcIdsAddChecks();
samlIdAddChecks = buildSamlIdsAddChecks();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,10 @@ public UserDetails loadUserDetails(PreAuthenticatedAuthenticationToken token)
throws UsernameNotFoundException {

String principal = (String) token.getPrincipal();
IamX509AuthenticationCredential credentials = (IamX509AuthenticationCredential) token.getCredentials();

String issuerDn = credentials.getIssuer();

LOG.debug("Loading IAM account for X.509 principal '{}'", principal);

IamAccount account = accountRepository.findByCertificateSubjectAndIssuer(principal, issuerDn).orElseThrow(() -> {
IamAccount account = accountRepository.findByCertificateSubject(principal).orElseThrow(() -> {
final String msg = String.format("No IAM account found for X.509 principal '%s'", principal);
LOG.debug(msg);
return new UsernameNotFoundException(msg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ private void x509CertificateSanityCheck(IamX509Certificate cert) {
checkArgument(!isNullOrEmpty(cert.getIssuerDn()), "null or empty X.509 certificate issuer DN");
checkArgument(!isNullOrEmpty(cert.getLabel()), "null or empty X.509 certificate label");

accountRepo.findByCertificateSubjectAndIssuer(cert.getSubjectDn(), cert.getIssuerDn()).ifPresent(c -> {
accountRepo.findByCertificateSubject(cert.getSubjectDn()).ifPresent(c -> {
throw new CredentialAlreadyBoundException(
String.format("X509 certificate with subject '%s' is already bound to another user",
cert.getSubjectDn()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void testX509AuthenticationSuccessButNotRequestedLeadsToLoginPage() throw

iamAccountRepo.save(testAccount);

IamAccount resolvedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
IamAccount resolvedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(
() -> new AssertionError("Expected test user linked with subject " + TEST_0_SUBJECT));

Expand All @@ -114,7 +114,7 @@ public void testX509AuthenticationSuccessButNotRequestedLeadsToLoginPage() throw
.andExpect(redirectedUrl("/dashboard"))
.andExpect(authenticated().withUsername("test"));

resolvedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
resolvedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(
() -> new AssertionError("Expected test user linked with subject " + TEST_0_SUBJECT));

Expand All @@ -133,7 +133,7 @@ public void testX509AuthenticationVerifyFailedLeadsToLoginPage() throws Exceptio

iamAccountRepo.save(testAccount);

IamAccount resolvedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
IamAccount resolvedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(
() -> new AssertionError("Expected test user linked with subject " + TEST_0_SUBJECT));

Expand Down Expand Up @@ -183,7 +183,7 @@ public void testx509AccountLinking() throws Exception {
.andExpect(
flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMessage)));

IamAccount linkedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
IamAccount linkedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found"));

Date lastUpdateTime = linkedAccount.getLastUpdateTime();
Expand All @@ -196,7 +196,7 @@ public void testx509AccountLinking() throws Exception {
.andExpect(
flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMessage)));

linkedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
linkedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found"));

assertThat(linkedAccount.getLastUpdateTime().after(lastUpdateTime), is(true));
Expand All @@ -219,7 +219,7 @@ public void testx509AccountLinking() throws Exception {
.andExpect(
flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMsg)));

linkedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_NEW_ISSUER)
linkedAccount = iamAccountRepo.findByUsername("test")
.orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found"));

assertThat(linkedAccount.getX509Certificates().size(), is(2));
Expand All @@ -244,7 +244,7 @@ public void testx509AccountLinkingWithDifferentSubjectAndIssuer() throws Excepti
.andExpect(
flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMessage)));

IamAccount linkedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
IamAccount linkedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found"));

assertThat(linkedAccount.getUsername(), equalTo("test"));
Expand All @@ -266,9 +266,6 @@ public void testx509AccountLinkingWithDifferentSubjectAndIssuer() throws Excepti
.andExpect(
flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMsg)));

linkedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_1_SUBJECT, TEST_NEW_ISSUER)
.orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found"));

assertThat(linkedAccount.getX509Certificates().size(), is(2));
}

Expand All @@ -282,7 +279,7 @@ public void x509AccountUnlinkWorks() throws Exception {

iamAccountRepo.save(user);

IamAccount linkedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
IamAccount linkedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(() -> new AssertionError(
"Expected test user linked with certificate subject " + TEST_0_SUBJECT));

Expand All @@ -294,7 +291,7 @@ public void x509AccountUnlinkWorks() throws Exception {
.andDo(print())
.andExpect(status().isNoContent());

iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER).ifPresent(a -> {
iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT).ifPresent(a -> {
throw new AssertionError(
"Found unexpected user linked with certificate subject " + TEST_0_SUBJECT);
});
Expand All @@ -307,7 +304,7 @@ public void x509AccountUnlinkSuccedsSilentlyForUnlinkedAccount() throws Exceptio
iamAccountRepo.findByUsername("test")
.orElseThrow(() -> new AssertionError("Expected user not found"));

iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER).ifPresent(a -> {
iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT).ifPresent(a -> {
throw new AssertionError(
"Found unexpected user linked with certificate subject " + TEST_0_SUBJECT);
});
Expand All @@ -317,7 +314,7 @@ public void x509AccountUnlinkSuccedsSilentlyForUnlinkedAccount() throws Exceptio
.with(csrf().asHeader()))
.andExpect(status().isNoContent());

iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER).ifPresent(a -> {
iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT).ifPresent(a -> {
throw new AssertionError(
"Found unexpected user linked with certificate subject " + TEST_0_SUBJECT);
});
Expand All @@ -342,7 +339,7 @@ public void testx509AuthNFailsIfDisabledUser() throws Exception {

iamAccountRepo.save(testAccount);

IamAccount resolvedAccount = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
IamAccount resolvedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(
() -> new AssertionError("Expected test user linked with subject " + TEST_0_SUBJECT));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ private HttpHeaders test0SSLHeaders(boolean verified, String verificationError)
private HttpHeaders test2SSLHeaders(boolean verified, String verificationError) {
HttpHeaders headers = new HttpHeaders();
headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.CLIENT_CERT.getHeader(),
TEST_1_CERT_STRING_NGINX);
TEST_0_CERT_STRING_NGINX);

headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.SUBJECT.getHeader(),
TEST_1_SUBJECT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ public void testScimAddCertificateSuccess() throws Exception {
.content(mapper.writeValueAsString(patchRequest)).contentType(SCIM_CONTENT_TYPE))
.andExpect(status().isNoContent());

testUser = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
testUser = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(() -> new AssertionError("Expected test user not found"));

assertThat(testUser.getUsername(), equalTo(TEST_USERNAME));
Expand Down Expand Up @@ -408,7 +408,7 @@ public void testScimRemoveCertificateSuccess() throws Exception {
.contentType(SCIM_CONTENT_TYPE))
.andExpect(status().isCreated());

IamAccount account = iamAccountRepo.findByCertificateSubjectAndIssuer(TEST_0_SUBJECT, TEST_0_ISSUER)
IamAccount account = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT)
.orElseThrow(() -> new AssertionError(
"Expected account bound to '" + TEST_0_SUBJECT + "' certificate not found"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void setup() {

when(accountRepo.findProvisionedAccountsWithLastLoginTimeBeforeTimestamp(any()))
.thenReturn(emptyList());
when(accountRepo.findByCertificateSubjectAndIssuer(anyString(), anyString())).thenReturn(Optional.empty());
when(accountRepo.findByCertificateSubject(anyString())).thenReturn(Optional.empty());
when(accountRepo.findBySshKeyValue(anyString())).thenReturn(Optional.empty());
when(accountRepo.findBySamlId(any())).thenReturn(Optional.empty());
when(accountRepo.findByOidcId(anyString(), anyString())).thenReturn(Optional.empty());
Expand Down Expand Up @@ -636,7 +636,7 @@ public void testBoundX509CertificateIsNotAccepted() {
IamAccount account = cloneAccount(CICCIO_ACCOUNT);
account.linkX509Certificates(asList(TEST_X509_CERTIFICATE_1));

when(accountRepo.findByCertificateSubjectAndIssuer(TEST_X509_CERTIFICATE_SUBJECT_1, TEST_X509_CERTIFICATE_ISSUER_1))
when(accountRepo.findByCertificateSubject(TEST_X509_CERTIFICATE_SUBJECT_1))
.thenReturn(Optional.of(TEST_ACCOUNT));

accountService.createAccount(account);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ Optional<IamAccount> findByUsernameWithDifferentUUID(@Param("username") String u
Optional<IamAccount> findByEmailWithDifferentUUID(@Param("emailAddress") String emailAddress,
@Param("uuid") String uuid);

@Query("select a from IamAccount a join a.x509Certificates c where c.subjectDn = :subject and c.issuerDn = :issuer")
Optional<IamAccount> findByCertificateSubjectAndIssuer(@Param("subject") String subject, @Param("issuer") String issuer);
@Query("select a from IamAccount a join a.x509Certificates c where c.subjectDn = :subject")
Optional<IamAccount> findByCertificateSubject(@Param("subject") String subject);

@Query("select a from IamAccount a join a.x509Certificates c where c.certificate = :certificate")
Optional<IamAccount> findByCertificate(@Param("certificate") String certificate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,10 @@ protected User buildUnknownUser(PreAuthenticatedAuthenticationToken token) {
public UserDetails loadUserDetails(PreAuthenticatedAuthenticationToken token) {

String principal = (String) token.getPrincipal();
IamX509AuthenticationCredential credentials = (IamX509AuthenticationCredential) token.getCredentials();

String issuerDn = credentials.getIssuer();

LOG.debug("Loading IAM account for X.509 principal '{}'", principal);

Optional<IamAccount> account = accountRepository.findByCertificateSubjectAndIssuer(principal, issuerDn);
Optional<IamAccount> account = accountRepository.findByCertificateSubject(principal);

if (account.isPresent()) {
LOG.debug("Found IAM account {} linked to principal '{}'", account.get().getUuid(), principal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ public DefaultIamVomsAccountResolver(IamAccountRepository repo) {
public Optional<IamAccount> resolveAccountFromRequest(VOMSRequestContext requestContext) {

String certificateSubject = requestContext.getRequest().getRequesterSubject();
String certificateIssuer = requestContext.getRequest().getRequesterIssuer();

return accountRepo.findByCertificateSubjectAndIssuer(certificateSubject, certificateIssuer);
return accountRepo.findByCertificateSubject(certificateSubject);

}

Expand Down

0 comments on commit 5347c37

Please sign in to comment.