apisix icon indicating copy to clipboard operation
apisix copied to clipboard

fix: Set the session configuration

Open illidan33 opened this issue 1 year ago • 2 comments

Description

Added the configuration items related to resty.session. You can set the behavior and expiration time of the session through the configuration items to avoid the problem that the access token does not expire but the session expires when the expiration time of the access token is longer than the session.

Fixes # (issue) https://github.com/apache/apisix/issues/10797

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
  • [x] 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)

illidan33 avatar Feb 06 '24 07:02 illidan33

Hello, I think we may need additional testing, and documentation to ensure it is available in the plugin

smileby avatar Feb 06 '24 15:02 smileby

Hello, I think we may need additional testing, and documentation to ensure it is available in the plugin Sure @smileby @shreemaan-abhishek I have added test code and documentation, and successfully executed it on my local machine. However, I have a question. I am using a Docker setup of Keycloak/etcd on my local machine to validate the test code, referencing test code files from other contributors that use the same realm and client. I am unsure if this approach is standard. Does the official provide standard testing support services? For example, does the official provide a designated Keycloak authentication service for testing purposes?

illidan33 avatar Feb 18 '24 10:02 illidan33

@illidan33 you can find the keycloak service defined here:

https://github.com/apache/apisix/blob/21d9ceeac9715dcdad37335fe402e325ee74a43d/ci/pod/docker-compose.plugin.yml#L40C1-L59C91

shreemaan-abhishek avatar Feb 19 '24 15:02 shreemaan-abhishek

@illidan33 you can find the keycloak service defined here:

https://github.com/apache/apisix/blob/21d9ceeac9715dcdad37335fe402e325ee74a43d/ci/pod/docker-compose.plugin.yml#L40C1-L59C91

@shreemaan-abhishek hi, I updated the configuration to keep it the same as the official one.

illidan33 avatar Feb 26 '24 02:02 illidan33

@shreemaan-abhishek @monkeyDluffy6017 hi, could you help to check the ci's error. It doesn't look like it failed because of the test code I submitted

illidan33 avatar Mar 04 '24 05:03 illidan33

image

The quality of the english doc is subpar and I cannot understand what it's saying. Please hold the review to a higher standard.

My questions:

  • This can be configured with Nginx set $session_cookie_lifetime 3600: does this mean this new configuration option just makes it feasible to configure session_cookie_lifetime dynamically at runtime?
  • The next sentence is grammatically incorrect and I cannot understand, @shreemaan-abhishek please rephrase it makes sense to you.
  • it is used if the cookies are configured persistent with session.cookie.persistent == true: where is this configuration? it is not in this plugin.
  • The Chinese doc also says for more information see ssl_session_timeout. Where is this ssl_session_timeout

kayx23 avatar Mar 05 '24 03:03 kayx23

@shreemaan-abhishek could you create a pr to change what @kayx23 mentioned?

monkeyDluffy6017 avatar Mar 05 '24 06:03 monkeyDluffy6017

image

The quality of the english doc is subpar and I cannot understand what it's saying. Please hold the review to a higher standard.

My questions:

  • This can be configured with Nginx set $session_cookie_lifetime 3600: does this mean this new configuration option just makes it feasible to configure session_cookie_lifetime dynamically at runtime?
  • The next sentence is grammatically incorrect and I cannot understand, @shreemaan-abhishek please rephrase it makes sense to you.
  • it is used if the cookies are configured persistent with session.cookie.persistent == true: where is this configuration? it is not in this plugin.
  • The Chinese doc also says for more information see ssl_session_timeout. Where is this ssl_session_timeout

hi @kayx23 @monkeyDluffy6017 @shreemaan-abhishek , the documentation is sourced from https://github.com/bungle/lua-resty-session/tree/v3.10, where it describes the session.cookie.lifetime. This update only adds the configuration for lifetime, and the description should only focus on the role of lifetime. I have modified the documentation. Do I need to submit another PR?

illidan33 avatar Mar 05 '24 06:03 illidan33

@illidan33 yeah, of course, another pr is needed, and i will invite @kayx23 to review

monkeyDluffy6017 avatar Mar 05 '24 06:03 monkeyDluffy6017

@illidan33 yeah, of course, another pr is needed, and i will invite @kayx23 to review

hi @monkeyDluffy6017 @kayx23 , the new pr's link is 10994

illidan33 avatar Mar 05 '24 06:03 illidan33