Wire ACME extension configuration refactor #1671
Conversation
github-actions
bot
added
the
needs triage
acme/challenge.go
Outdated
| key crypto.PublicKey | ||
| accountJWK *jose.JSONWebKey |
There was a problem hiding this comment.
Can you clarify the name of these variables? Looking at the code below, I think they can be something like tokenKey and dpopKey.
It's also quite weird to see two types here. I assume these keys come from some configuration, and you should be able to create the verifyParams with two crypto.PublicKey
| func (o *DPOPOptions) validate() error { | ||
| if _, err := pemutil.Parse(o.SigningKey); err != nil { | ||
| return fmt.Errorf("failed parsing key: %w", err) | ||
| } | ||
| if _, err := template.New("DeviceID").Parse(o.GetTarget()); err != nil { | ||
| return fmt.Errorf("failed parsing template: %w", err) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
This validation can also be part of the // TODO(hs): do this once?. We can validate and set hidden fields in the DPOPOptions struct.
I'm ok if you want to do this later.
| ClientID string `json:"client_id,omitempty"` | ||
| SupportedSigningAlgs []string `json:"supported_signing_algs,omitempty"` |
There was a problem hiding this comment.
I've already commented about this before, but unless this is something coming from a well-known file, what about using clientId and supportedSigningAlgs or even a simpler name like signatureAlgorithms, you can assume that those are the supported ones.
|
|
||
| return &oidc.Config{ | ||
| ClientID: o.Config.ClientID, | ||
| SupportedSigningAlgs: o.Config.SupportedSigningAlgs, |
There was a problem hiding this comment.
See previous comment, we can also name this SignatureAlgorithms
| if _, err := url.Parse(o.Provider.IssuerURL); err != nil { | ||
| return fmt.Errorf("failed parsing issuer URL: %w", err) | ||
| } | ||
| if _, err := template.New("DeviceID").Parse(o.Provider.IssuerURL); err != nil { | ||
| return fmt.Errorf("failed parsing template: %w", err) | ||
| } |
There was a problem hiding this comment.
You can probably do this once
| panic(err) // config error, it's ok to panic here | ||
| } | ||
| func toOIDCProviderConfig(in *Provider) *oidc.ProviderConfig { | ||
| issuerURL, _ := url.Parse(in.IssuerURL) // NOTE: validation is performed in validate() |
There was a problem hiding this comment.
You can set the issuerURL in the Provider and just parse once.
No description provided.