You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "baqianxin (via GitHub)" <gi...@apache.org> on 2023/12/07 05:05:35 UTC
[PR] #docs: update plugin-openid-connect attrbuites [apisix]
baqianxin opened a new pull request, #10609:
URL: https://github.com/apache/apisix/pull/10609
### Description
There are some attributes available in the [lua-resty-openidc](https://github.com/zmartzone/lua-resty-openidc) that are not mentioned in the plugin schema and docs. They are configurable, however, and passed down to [lua-resty-openidc](https://github.com/zmartzone/lua-resty-openidc) through opts.
Quick reference:
https://github.com/apache/apisix/blob/master/apisix/plugins/openid-connect.lua
https://github.com/zmartzone/lua-resty-openidc
https://github.com/apache/apisix/pull/10591
### Checklist
- [ ] I have explained the need for this PR and the problem it solves
- [ ] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [ ] 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)
--
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] docs: update `openid-connect` plugin `refresh_session_interval` attribute [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on PR #10609:
URL: https://github.com/apache/apisix/pull/10609#issuecomment-1845915412
You'd need to update the expected outcome of the test as well. If you check my previous PR, you'd see I did the same.
--
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] docs: update plugin-openid-connect attrbuites [apisix]
Posted by "baqianxin (via GitHub)" <gi...@apache.org>.
baqianxin commented on PR #10609:
URL: https://github.com/apache/apisix/pull/10609#issuecomment-1844871789
> But now that you mentioned and I happened to be double checking the code, it seems the default value is actually not 900... It doesn't have a default value: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1545
>
> Note that the attribute values in README for `lua-resty-openidc` aren't always the default value...
>
> Maybe you could re-position this PR as a fix to correct it (docs and schema in `openid-connect.lua`).
Your updates were very prompt, and I followed your suggestions to modify the default values and documentation description. Thanks.
I just checked the documentation site to see if the relevant parameters existed, without reviewing the latest code, and then modified the document content to submit a pull request.Thanks again for your reminder.
--
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] docs: update plugin-openid-connect attrbuites [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on PR #10609:
URL: https://github.com/apache/apisix/pull/10609#issuecomment-1844812638
This is already added in https://github.com/apache/apisix/pull/10591/files
--
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] docs: update plugin-openid-connect attrbuites [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on PR #10609:
URL: https://github.com/apache/apisix/pull/10609#issuecomment-1844823900
But now that you mentioned this and I happened to double check the code, it seems the default value is actually not 900... It doesn't have a default value: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1545
Note that the attribute values in README for `lua-resty-openidc` aren't always the default value...
Maybe you could re-position this PR as a fix to correct it (docs and schema in `openid-connect.lua`).
--
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] docs: update `openid-connect` plugin `refresh_session_interval` attribute [apisix]
Posted by "baqianxin (via GitHub)" <gi...@apache.org>.
baqianxin commented on PR #10609:
URL: https://github.com/apache/apisix/pull/10609#issuecomment-1845460935
@kayx23 the ci on centos7 was faild,because the `TEST 24` case
```bash
# Failed test 't/plugin/openid-connect.t TEST 24: Check defaults. - response_body - response is expected (repeated req 0, req 0)'
# at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1674.
# got: '{"accept_none_alg":false,"accept_unsupported_alg":true,"access_token_expires_leeway":0,"access_token_in_authorization_header":false,"bearer_only":false,"client_id":"kbyuFDidLLm280LIwVFiazOqjO3ty8KH","client_jwt_assertion_expires_in":60,"client_secret":"60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa","discovery":"http://127.0.0.1:1980/.well-known/openid-configuration","force_reauthorize":false,"iat_slack":120,"introspection_endpoint_auth_method":"client_secret_basic","introspection_interval":0,"jwk_expires_in":86400,"jwt_verification_cache_ignore":false,"logout_path":"/logout","realm":"apisix","renew_access_token_on_expiry":true,"revoke_tokens_on_logout":false,"scope":"openid","set_access_token_header":true,"set_id_token_header":true,"set_refresh_token_header":false,"set_userinfo_header":true,"ssl_verify":false,"timeout":3,"token_endpoint_auth_method":"client_secret_basic","unauth_action":"auth","use_nonce":false,"use_pkce":false}
# '
# expected: '{"accept_none_alg":false,"accept_unsupported_alg":true,"access_token_expires_leeway":0,"access_token_in_authorization_header":false,"bearer_only":false,"client_id":"kbyuFDidLLm280LIwVFiazOqjO3ty8KH","client_jwt_assertion_expires_in":60,"client_secret":"60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa","discovery":"http://127.0.0.1:1980/.well-known/openid-configuration","force_reauthorize":false,"iat_slack":120,"introspection_endpoint_auth_method":"client_secret_basic","introspection_interval":0,"jwk_expires_in":86400,"jwt_verification_cache_ignore":false,"logout_path":"/logout","realm":"apisix","refresh_session_interval":900,"renew_access_token_on_expiry":true,"revoke_tokens_on_logout":false,"scope":"openid","set_access_token_header":true,"set_id_token_header":true,"set_refresh_token_header":false,"set_userinfo_header":true,"ssl_verify":false,"timeout":3,"token_endpoint_auth_method":"client_secret_basic","unauth_action":"auth","use_nonce":false,"us
e_pkce":false}
# '
# Looks like you failed 1 test of 99.
[12:14:20] t/plugin/openid-connect.t ..................
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/99 subtests
```
--
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] docs: update `openid-connect` plugin `refresh_session_interval` attribute [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on PR #10609:
URL: https://github.com/apache/apisix/pull/10609#issuecomment-1846433296
Ok misunderstood. Re-running workflows now - should be good.
--
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] docs: update plugin-openid-connect attrbuites [apisix]
Posted by "SylviaBABY (via GitHub)" <gi...@apache.org>.
SylviaBABY commented on code in PR #10609:
URL: https://github.com/apache/apisix/pull/10609#discussion_r1418376357
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,7 @@ description: OpenID Connect allows the client to obtain user information from th
| set_refresh_token_header | boolean | False | false | | When set to true and a refresh token object is available, sets it in the `X-Refresh-Token` request header. |
| session | object | False | | | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
| session.secret | string | True | Automatic generation | 16 or more characters | The key used for session encrypt and HMAC operation. |
+| refresh_session_interval | integer | False | 900 | | Refresh the users id_token(or session in the browser) after 900 seconds without requiring re-authentication |
Review Comment:
```suggestion
| refresh_session_interval | integer | False | 900 | | Refresh the user's id_token(or session in the browser) after 900 seconds without requiring re-authentication. |
```
##########
docs/zh/latest/plugins/openid-connect.md:
##########
@@ -60,6 +60,7 @@ description: OpenID Connect（OIDC）是基于 OAuth 2.0 的身份认证协议
| set_refresh_token_header | boolean | 否 | false | | 当设置为 `true` 并且刷新令牌可用时，则会将该属性设置在`X-Refresh-Token`请求头中。 |
| session | object | 否 | | | 当设置 bearer_only 为 false 时，openid-connect 插件将使用 Authorization Code 在 IDP 上进行认证，因此你必须设置 session 相关设置。 |
| session.secret | string | 是 | 自动生成 | 16 个以上字符 | 用于 session 加密和 HMAC 计算的密钥。 |
+| refresh_session_interval | integer | 否 | 900 | | 900 秒后刷新用户存储的session，而无需进行重新授权流程 |
Review Comment:
```suggestion
| refresh_session_interval | integer | 否 | 900 | | 900 秒后刷新用户存储的 session，而无需进行重新授权流程。 |
```
##########
docs/zh/latest/plugins/openid-connect.md:
##########
@@ -60,6 +60,7 @@ description: OpenID Connect（OIDC）是基于 OAuth 2.0 的身份认证协议
| set_refresh_token_header | boolean | 否 | false | | 当设置为 `true` 并且刷新令牌可用时，则会将该属性设置在`X-Refresh-Token`请求头中。 |
| session | object | 否 | | | 当设置 bearer_only 为 false 时，openid-connect 插件将使用 Authorization Code 在 IDP 上进行认证，因此你必须设置 session 相关设置。 |
| session.secret | string | 是 | 自动生成 | 16 个以上字符 | 用于 session 加密和 HMAC 计算的密钥。 |
+| refresh_session_interval | integer | 否 | 900 | | 900 秒后刷新用户存储的session，而无需进行重新授权流程 |
Review Comment:
```suggestion
| refresh_session_interval | integer | 否 | 900 | | 900 秒后刷新用户存储的 session，而无需进行重新授权流程。 |
```
--
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] docs: update `openid-connect` plugin `refresh_session_interval` attribute [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #10609:
URL: https://github.com/apache/apisix/pull/10609
--
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] docs: update `openid-connect` plugin `refresh_session_interval` attribute [apisix]
Posted by "baqianxin (via GitHub)" <gi...@apache.org>.
baqianxin commented on PR #10609:
URL: https://github.com/apache/apisix/pull/10609#issuecomment-1846325476
> You'd need to update the expected outcome of the test as well. If you check my previous PR, you'd see I did the same.
Yes, I have already modified the test case [`t/openid-connect.t TEST 24` ](https://github.com/apache/apisix/pull/10609/commits/092223d2d508a2b6f61122b2a34d1cc50629e48b)
--
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