cosign: Allow use of regex in CertSubjectEmailVerifier #300
Conversation
dave-tucker
force-pushed
the
cert-regex
branch
from
ce043ee
to
e93beff
Compare
There was a problem hiding this comment.
I like the idea, I'm in favor of implementing this change.
The code looks good, but I have some suggestions. These might be a matter of personal taste, hence I'm open to have a discussion about how to proceed.
My proposal would be to change the StringVerifier from being a trait to be a simple enum. We could have something like StringVerifier::ExactMatch(String) and StringVerifier::Regex(Regex). This would make the code easier to understand for our end consumers and we could get rid of the dynamic dispatch introduced by the usage of the trait.
What do you think?
This allows for either an exact match [StringVerifier::ExactMatch] or it allows for a regular expression [StringVerifier::Regex] This supports the use case of trusting signatures from a collection of email addresses e.g .*@redhat.com and or from a collection of issuers. Fixes: sigstore#299 Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
dave-tucker
force-pushed
the
cert-regex
branch
from
e93beff
to
c67f5cb
Compare
Makes sense to me. I've updated the patch with your proposal. |
| pub enum StringVerifier { | ||
| ExactMatch(String), | ||
| Regex(Regex), | ||
| } |
There was a problem hiding this comment.
Can you add some inline docs to be included on docs.rs please?
| } | ||
|
|
||
| #[test] | ||
| fn cert_email_verifier_only_email_regex() { |
There was a problem hiding this comment.
nitpicking, using something like rstest could reduce a lot of this repetitions
| issuer: None, | ||
| }; | ||
| assert!(!vc.verify(&sl).unwrap()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn cert_email_verifier_email_and_issuer_regex() { |
Summary
This allows for either an exact match [StringVerifier::ExactMatch]
or it allows for a regular expression [StringVerifier::Regex]
This supports the use case of trusting signatures from a
collection of email addresses e.g .*@redhat.com and or from a
collection of issuers.
Fixes: #299
Release Note
CertSubjectEmailVerifieris now constructed using either aStringVeriferenum. This supports exact string matches or regular expressions.Documentation
Docstrings and examples are updated to show how this is used.