feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms
Description
The underlying lua-resty-openidc module already supports the
symmetric_key option to specify the HMAC key for verifying HS???
tokens. However, note that lua-resty-openidc just passes the
symmetric_key value as-is to HMAC. So we choose to accept a
base64url-encoded secret, which is easier to obtain from OP and
configure, and then decode it before passing it to lua-resty-openidc.
Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)
NOTE: the lua-resty-openidc is having a bug in verifying HS??? tokens, and I've submitted a PR to fix it: https://github.com/zmartzone/lua-resty-openidc/pull/447
Could you add some tests for this feature? How can we test it?
Hmm, I'll have to first check out the existing tests (e.g., for RS256) and figure out one for the HS256/HS512 tokens.
NOTE: the
lua-resty-openidcis having a bug in verifying HS??? tokens, and I've submitted a PR to fix it: zmartzone/lua-resty-openidc#447
Well. lua-resty-openidc is actually good. It's the Keycloak (I was testing with) having the issue with HS256 (and other HS algorithms) tokens.
By the way, I'll have to consider how to best pass the symmetric_key option (e.g., whether to encode or not).
It's the Keycloak (I was testing with) having the issue with HS256 (and other HS algorithms) tokens.
Is there any link for this issue in Keycloak? Would you explain it in depth?
It's the Keycloak (I was testing with) having the issue with HS256 (and other HS algorithms) tokens.
Is there any link for this issue in Keycloak? Would you explain it in depth?
Solved it: https://github.com/keycloak/keycloak/issues/13823.
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
Re-opened as this is a meaningful contribution.
Helped resolve conflicts and update the explanation for the symmetric_key to be a bit more readable, because the PR author mentioned in a different PR they've changed jobs and might not be able to continue the effort here, especially with adding test cases.
@shreemaan-abhishek please help to finish this pr
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.
@liweitianux Can you no longer work on this PR?
@liweitianux Looks like you're busy. We will assign this issue to someone else.
@liweitianux Looks like you're busy. We will assign this issue to someone else.
Hi @Revolyssup, sorry that I haven't been using APISIX for some time, and I currently don't have the time for this PR. Please help reassign it. Thanks.
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.