feat(openid-connect): add session.cookie configuration #10919
Conversation
|
Hello, I think we may need additional testing, and documentation to ensure it is available in the plugin |
|
|
@illidan33 you can find the keycloak service defined here: |
@shreemaan-abhishek hi, I updated the configuration to keep it the same as the official one. |
monkeyDluffy6017
changed the title
…_number and current_time"
t/plugin/openid-connect6.t
Outdated
| for part in string.gmatch(cookie_str, "[^|]+") do | ||
| table.insert(parts, part) | ||
| end | ||
| local target_number = tonumber(parts[2], 10) - 100 |
There was a problem hiding this comment.
If you don't set the cookie.lifetime, what will the target_number be?
If we want to use the condition if target_number >= current_time then, the default value of cookie.lifetime should be less than 100
There was a problem hiding this comment.
The default value of cookie.lifetime is 3600.
Yes, you are right. 100 is not the right condition, I will change it to a larger number than the default value.
| @@ -61,6 +61,8 @@ 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. | | |||
| | session.cookie | integer | False | | | | | |||
There was a problem hiding this comment.
missing description for session.cookie
There was a problem hiding this comment.
It's a map to hold the lifetime, does it need some description?
There was a problem hiding this comment.
missing description for
session.cookie
As @monkeyDluffy6017 said, it is a map. I fixed its type to object, does it still need some description?
|
@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 |
|
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:
|
|
@shreemaan-abhishek could you create a pr to change what @kayx23 mentioned? |
hi @kayx23 @monkeyDluffy6017 @shreemaan-abhishek , the documentation is sourced from https://github.com/bungle/lua-resty-session/tree/v3.10, where it describes the |
|
@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 |
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)
#10797
Checklist