You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/08/16 04:35:07 UTC
[GitHub] [apisix] liweitianux opened a new pull request, #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
liweitianux opened a new pull request, #7691:
URL: https://github.com/apache/apisix/pull/7691
### 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](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] github-actions[bot] commented on pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1279936838
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 dev@apisix.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms [apisix]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1916486568
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 dev@apisix.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms [apisix]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1992214306
@liweitianux Can you no longer work on this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liweitianux commented on pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
liweitianux commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1216135013
**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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on code in PR #7691:
URL: https://github.com/apache/apisix/pull/7691#discussion_r1410063225
##########
apisix/plugins/openid-connect.lua:
##########
@@ -71,6 +72,10 @@ local schema = {
type = "string",
description = "the URI will be redirect when request logout_path",
},
+ symmetric_key = {
+ type = "string",
+ description = "base64url-encoded secret for verifying HS??? tokens",
Review Comment:
I updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7691:
URL: https://github.com/apache/apisix/pull/7691#discussion_r946367967
##########
apisix/plugins/openid-connect.lua:
##########
@@ -71,6 +72,10 @@ local schema = {
type = "string",
description = "the URI will be redirect when request logout_path",
},
+ symmetric_key = {
+ type = "string",
+ description = "base64url-encoded secret for verifying HS??? tokens",
Review Comment:
What's `HS???` token?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
spacewander commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1217506173
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1832983472
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1835801360
@shreemaan-abhishek please help to finish this pr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms [apisix]
Posted by "liweitianux (via GitHub)" <gi...@apache.org>.
liweitianux commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1996200486
> @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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7691:
URL: https://github.com/apache/apisix/pull/7691#discussion_r947399872
##########
apisix/plugins/openid-connect.lua:
##########
@@ -71,6 +72,10 @@ local schema = {
type = "string",
description = "the URI will be redirect when request logout_path",
},
+ symmetric_key = {
+ type = "string",
+ description = "base64url-encoded secret for verifying HS??? tokens",
Review Comment:
we can just use `HS256, HS512`, not `HS???`, it looks unclear
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liweitianux commented on a diff in pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
liweitianux commented on code in PR #7691:
URL: https://github.com/apache/apisix/pull/7691#discussion_r947404100
##########
apisix/plugins/openid-connect.lua:
##########
@@ -71,6 +72,10 @@ local schema = {
type = "string",
description = "the URI will be redirect when request logout_path",
},
+ symmetric_key = {
+ type = "string",
+ description = "base64url-encoded secret for verifying HS??? tokens",
Review Comment:
ok. sure :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liweitianux commented on pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
liweitianux commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1217434400
> **NOTE**: the `lua-resty-openidc` is having a bug in verifying HS??? tokens, and I've submitted a PR to fix it: [zmartzone/lua-resty-openidc#447](https://github.com/zmartzone/lua-resty-openidc/pull/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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liweitianux commented on pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
liweitianux commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1216191230
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
spacewander commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1217509341
> > 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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liweitianux commented on a diff in pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
liweitianux commented on code in PR #7691:
URL: https://github.com/apache/apisix/pull/7691#discussion_r946371485
##########
apisix/plugins/openid-connect.lua:
##########
@@ -71,6 +72,10 @@ local schema = {
type = "string",
description = "the URI will be redirect when request logout_path",
},
+ symmetric_key = {
+ type = "string",
+ description = "base64url-encoded secret for verifying HS??? tokens",
Review Comment:
like HS256, HS512
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] github-actions[bot] commented on pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1320850011
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] github-actions[bot] closed pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #7691: feat(openid-connect): Add 'symmetric_key' option to support HS??? tokens
URL: https://github.com/apache/apisix/pull/7691
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): Add `symmetric_key` option to support verifying tokens with symmetric algorithms [apisix]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on PR #7691:
URL: https://github.com/apache/apisix/pull/7691#issuecomment-1995030275
@liweitianux Looks like you're busy. We will assign this issue to someone else.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org