Fix broken signing algorithm configuration for token authentication #4578
Conversation
Signed-off-by: evanebb <git@evanus.nl>
… var Signed-off-by: evanebb <git@evanus.nl>
| for _, signingAlgorithmVal := range signingAlgorithmsVals { | ||
| signingAlgorithm, ok := signingAlgorithmVal.(string) | ||
| if !ok { | ||
| return opts, errors.New("signingalgorithms must be a list of signing algorithms") |
There was a problem hiding this comment.
I see we use return opts in other places, and we probably shouldn't as this may return a partially propagated struct. Now, callers of this method shouldn't use any of the results returned if it returns a non-nil error, but we should probably change all to explicitly return a zero value; return tokenAccessOptions{}, err.
For this one, we could either consider changing that ☝️ or to use an intermediate var;
var algs []stringThen append to that slice, and only after we completed, assign it to opts.signingAlgorithms
There was a problem hiding this comment.
I've changed the checkOptions function to explicitly return the zero value when an error occurs in 64f780f, which seems to be most in line with the rest of the package. Using the intermediate var would still result in a partially filled struct being returned, just not one with a partial signing algorithms list, so this seems better to me.
There was a problem hiding this comment.
Thanks! Yes, that looks better; it removes any ambiguity in what we return (i.e., only if it's valid, otherwise a zero-value that nobody would be able to depend on)
Signed-off-by: evanebb <git@evanus.nl>
Fixes #4577
Specifically, this seems to be something funky when unmarshalling a YAML file containing a list/slice into an
interface{}type, and then using a type assertion to directly convert the field into the slice of the desired type.A related issue is go-yaml/yaml#282.
This can be fixed by first converting the value to a
[]interface{}, then looping over it and converting each element into astringseparately.Also included is a small fix for the error message shown when you supply an unsupported signing algorithm, which doesn't currently show which algorithm was invalid, like so: