feat: add additional certificate field for the trustchain#109
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for configuring a custom root/CA certificate to be used for DigiD-related outbound TLS calls, so deployments can trust non-standard certificate chains without modifying the system truststore.
Changes:
- Add a new Gravity Forms plugin setting to select a root certificate file.
- Pass the selected root certificate into Guzzle TLS verification (
verify) for outbound HTTP calls. - Expose the new root certificate setting via
config/digid.php.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/DigiD/GravityFormsAddon.php |
Adds a new “root certificate” select field and a helper to list .pem certificates. |
src/DigiD/DigiDServiceProvider.php |
Builds a Guzzle client with optional verify pointing at the configured root certificate and injects it into metadata resolution. |
src/DigiD/Binding/Artifact.php |
Adds optional verify to the SOAP artifact resolve request options. |
config/digid.php |
Adds digid.certificate.root to configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e71a618 to
2f10838
Compare
|
|
||
| function rootCertificate(): ?String | ||
| { | ||
| $root = config('digid.certificate.root'); |
There was a problem hiding this comment.
De andere certificaten worden sinds jaar en dag niet gecontroleerd op het bestaan daarvan, denk wel mooi om toe te voegen:
$root = (string) config('digid.certificate.root', '');
if (! file_exists($root)) {
throw new Exception('')
}
Dan kun je het gebruik van deze functie wrappen in een try catch block om vervolgens in de klasse de logException methode gebruiken van de Logger trait.
Deze trait voert ook een do_action() uit om de exception te loggen naar Sentry.
| */ | ||
| private function getRootCertificates(): array | ||
| { | ||
| return $this->formatListOfCertificates(glob($this->getCertificateLocation() . '/*.{pem}', GLOB_BRACE)); |
There was a problem hiding this comment.
Voor de zekerheid, een .pem is correct? Ik kom ook wel eens tegen dat hiervoor een .cer/.crt voor wordt gebruikt.
There was a problem hiding this comment.
Yes, heb geprobeerd met een .cer, maar aangezien dit valt onder een hele keten moet je een .pem hebben (wat in principe gestackte .cers zijn van de chain).
b22f9b9 to
e11000c
Compare
Allows for adding a root certificate in the call, this way we don't have to add additional certificates to the truststore.