-
Notifications
You must be signed in to change notification settings - Fork 456
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
Check cnf claim with CSR fingerprint #1660
Conversation
This commit allows tying tokens with the provided CSR or SSH public key. Tokens with a confirmation claim kid (cnf.kid) will validate that the provided fingerprint (kid) matches the CSR or SSH public key. This check will only be present in JWK and X5C provisioners. Fixes #1637
} | ||
for name, tt := range tests { | ||
t.Run(name, func(t *testing.T) { | ||
tc := tt(t) | ||
ctx := NewContextWithMethod(context.Background(), SignIdentityMethod) | ||
if opts, err := tc.p.AuthorizeSign(ctx, tc.token); err != nil { | ||
if assert.NotNil(t, tc.err) { | ||
if assert.NotNil(t, tc.err, err.Error()) { |
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.
A bit of a weird test case. The tc.err
is the expected error, and an assertion on that is unexpected. It was already there, but I don't think this is the right test logic now.
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.
It works as expected, showing the err.Error()
if the assertion fails, but with ad70982 I've changed this file to use testify's assert and require packages.
This commit allows tying tokens with the provided CSR or SSH public key. Tokens with a confirmation claim kid (
cnf.kid
) will validate that the provided fingerprint (kid) matches the CSR or SSH public key.This check will only be present in JWK and X5C provisioners.
Fixes #1637
Related PRs:
cnf
claim cli#1092