Delete UNIQUE constraint on subject_dn column of iam_x509_cert table #672
Conversation
of iam_x509_cert because it causes certificate duplication error with same dn but different issuer
considering subjectDn AND issuerDn that guarantee uniqueness
| @@ -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/bycertsubject"; | |||
| public static final String FIND_BY_CERT_SUBJECT_AND_ISSUER_RESOURCE = "/iam/account/find/bycertsubjectandissuer"; | |||
There was a problem hiding this comment.
Does this brake the UI? Surely a documentation fix is necessary here https://indigo-iam.github.io/v/v1.8.3/docs/reference/api/account-api/#account-filtering
There was a problem hiding this comment.
I don't think so, simply change the logic and two query parameters for the search, subjectDn and issuerDn are needed (URL-encoded). E.g.
http://localhost:8080/iam/account/find/bycertsubjectandissuer?certificateSubject=XXX&certificateIssuer=XXX
Obviously, as soon as it is approved, the documentation will also have to be modified accordingly.
There was a problem hiding this comment.
Does this mean that we can't search anymore only by cert subject?
There was a problem hiding this comment.
Yes, because since subjectDn no longer has the uniqueness constraint the search returns two records (in case one account has two certificates with same subject and different issuer) and thus gives an error.
There was a problem hiding this comment.
Ok, but searching for subject is useful. What's the problem if I get back two (or more) records? who gives the error?
There was a problem hiding this comment.
But we would get duplicate records, right? The resulting exception is IncorrectResultSizeDataAccessException and comes from spring framework.
However, if we want to keep the search by subject, we can revise the logic.
There was a problem hiding this comment.
I would expect that most of the times the search will be done using just the cert subject, so that has to work in my opinion. The we can add the possibility to search specifying also the issuer. It wouldn't be bad to search also by serial number and issuer (for example). I wonder if it wouldn't be better to leave /bycertsubject as it is for backwards compatibility (fixing possible bugs, of course) and add a new /bycert, with the new options
There was a problem hiding this comment.
But... I don't think we keep the serial number, so a search based on it doesn't make much sense, right?
There was a problem hiding this comment.
Exactly, that's how it is!
| .andExpect(jsonPath("$.Resources[0].userName", is("admin"))); | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
I'd add a "notFound" test with fake issuer and right subject, a fake subject and right issuer and both fake items.
|
Kudos, SonarCloud Quality Gate passed!
|
This is a fix for the already closed issue #550
There's still a UNIQUE constraints on
subject_dncolumn ofiam_x509_certtable which has to be removed.Also the search logic has to be fixed from finding a certificate through the subject to finding a certificate through both subject and issuer.