-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
acmeserver: add policy field to define allow/deny rules #5796
Conversation
Cool, this looks great so far. Again, no real strong feedback yet -- it's similar to how I'd probably do it if I had need of this. |
@mohammed90 you could drop the URI domains from this. They're only used for when there are URIs in a certificate (request), and those are not a valid ACME identifier type. Let me know if you hit issues with the policy configuration / evaluation. We kept it fairly simple intentionally, and sometimes it can be too limiting (e.g. strict equality check of the common name), so happy to hear feedback about it. |
Thanks for the tip! I'll make the change when I circle back to this PR. Currently the progress on this PR is blocked #5794 since both touch the same file. I'm stuck on testing the ACME server due to my knowledge gap of how to use github.com/mholt/acmez 😶 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, looking good and clean and tested. Just one random thought but nothing of importance. Thank you Mohammed, this is great!
TODO:
To be merged after #5794