-
Notifications
You must be signed in to change notification settings - Fork 33
Fix PublicKey extraction from JWK #188
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Dionna Glaze <[email protected]>
setrofim
left a comment
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.
LGTM
| return alg, key, nil | ||
| } | ||
| switch v := key.(type) { | ||
| case *ecdsa.PrivateKey: |
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.
The linter bizarrely crashes around here with the error:
Error: corim/signer.go:156:7: previous case (typecheck)
case *ecdsa.PrivateKey:
^
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.
PrivateKey extends PublicKey in ecdsa, so maybe it doesn't like that they're both tested in the same switch. Even if it's legal to the Golang typechecker.
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.
🤷
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.
Thanks for the attempt (9a86440). Unfortunately, we now have two errors rather than one :-)
Error: corim/signer.go:158:7: previous case (typecheck)
case *ecdsa.PrivateKey:
^
Error: corim/signer.go:169:7: previous case (typecheck)
case *ecdsa.PublicKey:
^
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.
I suggest we use staticcheck instead.
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.
I've created a PR to fix all the linter problems except the documentation lint that I can rebase on top of when it's merged.
The linter does not appear to like that ecdsa.PrivateKey contains an unnamed ecdsa.PublicKey. Signed-off-by: Dionna Glaze <[email protected]>
aee3d11 to
e12b637
Compare
Signed-off-by: Dionna Glaze <[email protected]>
veraison/cocli#35