You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "francescodedomenico (via GitHub)" <gi...@apache.org> on 2026/04/07 20:55:15 UTC
[PR] feat(openid-connect): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
francescodedomenico opened a new pull request, #13178:
URL: https://github.com/apache/apisix/pull/13178
### Description
APISIX bundles `lua-resty-session 4.1.5`, but the `openid-connect` plugin's session schema still exposed the legacy `session.cookie.lifetime` property from the 3.x API. Since `lua-resty-openidc` passes the session configuration **as-is** to `resty.session.start()`, only properties recognized by the 4.x library actually take effect.
This PR removes the deprecated `session.cookie.lifetime` and replaces it with the full set of flat configuration keys that `lua-resty-session` 4.x supports.
### New session properties
| Category | Properties |
|---|---|
| **Cookie** | `cookie_name`, `cookie_prefix`, `cookie_path`, `cookie_domain`, `cookie_http_only`, `cookie_secure`, `cookie_priority`, `cookie_same_site`, `cookie_same_party`, `cookie_partitioned` |
| **Timeouts** | `idling_timeout` (default 900s), `rolling_timeout` (default 3600s), `absolute_timeout` (default 86400s) |
| **Remember / persistent sessions** | `remember`, `remember_cookie_name`, `remember_rolling_timeout`, `remember_absolute_timeout`, `remember_safety` |
| **Other** | `audience`, `subject`, `enforce_same_subject`, `stale_ttl`, `touch_threshold`, `compression_threshold`, `hash_storage_key`, `hash_subject`, `store_metadata` |
### Migration from `session.cookie.lifetime`
The old `session.cookie.lifetime` (default 3600s) mapped loosely to a single cookie expiry. In `lua-resty-session` 4.x this is replaced by three independent timeouts that give much finer-grained control:
| Old property | New equivalent(s) | Default |
|---|---|---|
| `session.cookie.lifetime` | `idling_timeout` — invalidate after inactivity | 900s |
| | `rolling_timeout` — force renewal after duration | 3600s |
| | `absolute_timeout` — hard session max lifetime | 86400s |
### Breaking change
___Please note that even if I am mentioning this as a breaking change, the `session.cookie.lifetime` is already ignored by the resty.session plugin in APISIX main branch and 3.15 release, possibly happening on older versions as well___
`session.cookie.lifetime` is no longer accepted. Existing configurations using this property will be rejected by schema validation (`additionalProperties = false`). Users should migrate to `idling_timeout`, `rolling_timeout`, and/or `absolute_timeout`.
### Files changed
- **`apisix/plugins/openid-connect.lua`** — replaced session schema with `lua-resty-session` 4.x properties
- **`docs/en/latest/plugins/openid-connect.md`** — updated English documentation
- **`docs/zh/latest/plugins/openid-connect.md`** — updated Chinese documentation
- **`t/plugin/openid-connect10.t`** — 14 new schema validation tests covering valid configs, invalid enum/type values, deprecated property rejection, and unknown property rejection
#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes https://github.com/apache/apisix/issues/13177
### 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
- [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
Re: [PR] feat(openid-connect): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4265152442
Hi @francescodedomenico, another issue with the current PR is that the `session.cookie.lifetime` configuration option has been removed outright, without providing a migration path or backward compatibility. This constitutes a breaking change. As for the additional configurations in `lua-resty-session`, I think we can make them available by setting `additionalProperties` instead of cramming them all into the current schema.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4204211796
Hi @francescodedomenico, thank you for your contribution, but we need to evaluate whether it’s necessary to expose all the underlying library’s configurations. I’ll discuss this with the other maintainers. I look forward to hearing others’ thoughts.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4266157586
hello @Baoyuantop
> ...without providing a migration path or backward compatibility. This constitutes a breaking change.
I did add a migration note in the PR, the closest valid alternative for session.cookie.lifetime is the absolute_timeout parameter ([gathered from the lifetime description of the 3.10 resty session](https://github.com/bungle/lua-resty-session/tree/v3.10#number-sessioncookielifetime)). I think it would be more maintainable if we continue to support 1:1 mapping rather than keeping a deprecated parameter and to map it internally. However if you tell me where I am supposed to also add the migration notes I will add them in a docs: commit.
> As for the additional configurations in `lua-resty-session`, I think we can make them available by setting `additionalProperties` instead of cramming them all into the current schema.
I do agree that many parameters could be added in an session.cookie.additionalProperties field and describe the field as being mapped against the resty session configuration.
However I would like to suggest to at least expose the:
- cookie-name
- cookie-path
These two are a game changer for the user, we can have multiple oidc sessions, avoid "session" cookie conflict and have specific sessions being forwarded only to specific cookie-path.
Let me know, thank you-
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4660205285
@francescodedomenico glad you're keen on this — I think we're closer than it sounds, but let me clear up one thing, because I suspect a version-number mix-up.
When I wrote "3.x" and "4.x" I meant the **lua-resty-session library** versions, not APISIX versions. #12862 bumped the bundled lua-resty-session from `3.10` to `4.1.5`, and the flat `cookie_*` / `*_timeout` keys are simply that library's 4.x config surface. None of this is tied to an APISIX major version.
And the key point: **the approach I proposed is not a breaking change** — it's purely additive, so it can ship in APISIX 3.x. Nothing needs to wait for a hypothetical APISIX 4.0:
- `session.cookie.lifetime` stays accepted (just marked deprecated) and is mapped to `absolute_timeout`. Existing configs keep validating and keep working.
- The new flat keys (`cookie_name`, `cookie_secure`, `idling_timeout`, …) are new optional properties. Nothing is removed, no types change, no new required fields.
Concretely, both of these stay valid. Existing config — still works, and `lifetime` now actually takes effect (that's the bug fix):
```json
{
"session": {
"secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
"cookie": { "lifetime": 7200 }
}
}
```
New config — the flat lua-resty-session 4.x keys:
```json
{
"session": {
"secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
"cookie_name": "oidc_session",
"cookie_path": "/app",
"cookie_same_site": "Strict",
"cookie_secure": true,
"absolute_timeout": 7200,
"idling_timeout": 600
}
}
```
On your point that APISIX shouldn't be opinionated about how the library is used — I actually agree, and that's exactly why I suggested the flat names: they're lua-resty-session's *own* config keys, with no remapping or aliasing in between. The only difference from a full `additionalProperties` pass-through is that we list the supported keys explicitly, so an unknown or misspelled key fails validation instead of being silently dropped — which is precisely the failure mode that let `cookie.lifetime` quietly stop working. You still get direct access to the library's config; you just get it validated.
Once we agree on this shape, I'll help bring in the other maintainers to review so the PR doesn't stall on the three-reviewer requirement. Does this clear up the breaking-change concern? If so, I think you're good to update the PR along these lines.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4659033649
Thanks for picking this up — fixing `cookie.lifetime` is overdue. One suggestion on the schema direction before it lands.
Instead of keeping the nested `session.cookie` object with `name`/`path`/`lifetime` aliases mapped onto lua-resty-session keys, I'd lean toward exposing the 4.x options **flat, under their real names** — `cookie_name`, `cookie_path`, `cookie_domain`, `cookie_secure`, `cookie_http_only`, `cookie_same_site`, `idling_timeout`, `rolling_timeout`, `absolute_timeout`, etc. That drops the alias layer and `build_session_opts()` entirely: the keys are identical to what `resty.session.start()` already consumes, so they pass straight through with no mapping and no alias-precedence rules to reason about. The only thing left to translate is the legacy `session.cookie.lifetime` — we keep accepting it (deprecated) and map it to `absolute_timeout` so existing configs keep working, even though, as you noted, it's been silently ignored since the 4.x bump in #12862.
I'd also drop the `additionalProperties` pass-through and enumerate the supported options explicitly, the way Kong's session and OIDC plugins do. A few reasons:
- Silent pass-through is exactly how `cookie.lifetime` broke in the first place: a key the bundled lua-resty-session version doesn't recognize is happily accepted by the schema and then quietly dropped at runtime. An explicit allowlist turns that into a validation error instead of a silent no-op.
- lua-resty-session is a pinned dependency, not a fast-moving external API. APISIX already enumerates stable-library config explicitly — see the `redis` block in this very plugin, or `limit-count`'s redis schema. `additionalProperties = true` is reserved for volatile surfaces (AI provider options, protobuf decoder flags, OTel attributes), which this isn't.
- Explicit fields are documentable and discoverable; with pass-through, users have to read the lua-resty-session source for the exact pinned version to know what's actually valid.
The tradeoff is that adding a new option later is a one-line schema addition — but that's cheap, and it keeps the surface honest about what the bundled library actually supports.
One more data point that reinforces this: the same #12862 upgrade left `authz-casdoor` broken in the same way. It still builds its session with `session.new{ cookie = {lifetime = ...} }` in `apisix/plugins/authz-casdoor.lua`, so the post-login session ignores the token's `expires_in` and falls back to the 4.x default `absolute_timeout`. Probably worth fixing in the same sweep, or at least tracking as a separate issue.
Happy to send a PR with the flat-schema version if this direction sounds good. What do you think?
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "mbsjed (via GitHub)" <gi...@apache.org>.
mbsjed commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4658359514
Since these functionalities would also be quite important for us, i was wondering what the timeline looks like. Is there any plan in place or any indication of when a release can be expected?
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4659085400
@francescodedomenico thanks a lot for taking this on — it's an important fix, the broken `cookie.lifetime` has been silently biting users since the 4.x bump and the missing session options are a real gap. Would you be up for pushing the PR forward along the lines above? Happy to review quickly and help get it merged.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "kovasaurus (via GitHub)" <gi...@apache.org>.
kovasaurus commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4258840665
I would like to chip into this discussion.
Prior to lua-resty-session 4.x we could use the nginx variables to setup the various features from lua-resty-session which were not exposed directly through openid-connect plugin in Apisix. After 4.x version, due to dependency chain of openid-connect apisix plugin -> lua-resty-openidc -> lua-resty-session it is only possible to configure those features by configuration which is sent from openid-connect plugin.
Effectively, the limited schema capabilities are now blocking us from using the features that exist in lua-resty-session.
This PR from OP addresses the cookie configuration, but I have additional example.
lua-resty-session supports Redis with sentinel configuration -> https://github.com/bungle/lua-resty-session/blob/master/README.md#redis-sentinels-configuration
I have tried to play around with it for a bit, and I've managed to create custom docker image from apisix:3.16.0 with lua-resty-redis-connector installed, adjusted the schema for openid-connect to support sentinel properties and were able to successfully connect to Redis sentinel. The setup is still being tested but so far looks 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] feat(openid-connect): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #13178:
URL: https://github.com/apache/apisix/pull/13178#discussion_r3381495843
##########
apisix/plugins/openid-connect.lua:
##########
@@ -33,6 +34,33 @@ local ngx_encode_base64 = ngx.encode_base64
local plugin_name = "openid-connect"
+-- Translate session config to lua-resty-session 4.x options.
+-- Most keys (cookie_name, cookie_path, *_timeout, etc.) are already named
+-- after lua-resty-session and pass straight through. The only translation
+-- is the legacy session.cookie.lifetime alias from the lua-resty-session 3.x
+-- schema, which is mapped to absolute_timeout when the latter is unset.
+local function build_session_opts(session_conf)
+ if not session_conf then
+ return nil
+ end
+ local opts = {}
+ for k, v in pairs(session_conf) do
+ if k ~= "cookie" then
+ opts[k] = v
+ end
+ end
+ local cookie = session_conf.cookie
+ if cookie and cookie.lifetime ~= nil then
+ if opts.absolute_timeout == nil then
+ opts.absolute_timeout = cookie.lifetime
+ core.log.warn("session.cookie.lifetime is deprecated; ",
+ "use session.absolute_timeout instead")
+ end
+ end
+ return opts
Review Comment:
```suggestion
if session_conf.cookie and session_conf.cookie.lifetime then
if not session_conf.absolute_timeout then
session_conf.absolute_timeout = session_conf.cookie.lifetime
core.log.warn("session.cookie.lifetime is deprecated; ",
"use session.absolute_timeout instead")
end
end
return session_conf
```
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #13178:
URL: https://github.com/apache/apisix/pull/13178#discussion_r3381620112
##########
apisix/plugins/openid-connect.lua:
##########
@@ -76,14 +104,59 @@ local schema = {
description = "the key used for the encrypt and HMAC calculation",
minLength = 16,
},
+ cookie_name = {
+ type = "string",
+ description = "session cookie name",
+ },
+ cookie_path = {
+ type = "string",
+ description = "cookie path scope",
+ },
+ cookie_domain = {
+ type = "string",
+ description = "cookie domain scope",
+ },
+ cookie_secure = {
+ type = "boolean",
+ description = "if true, set the Secure cookie attribute",
+ },
+ cookie_http_only = {
+ type = "boolean",
+ description = "if true, set the HttpOnly cookie attribute",
+ },
+ cookie_same_site = {
+ type = "string",
+ enum = {"Strict", "Lax", "None", "Default"},
+ description = "SameSite cookie attribute",
+ },
+ idling_timeout = {
+ type = "integer",
+ description = "idling timeout in seconds",
+ },
+ rolling_timeout = {
+ type = "integer",
+ description = "rolling timeout in seconds",
+ },
+ absolute_timeout = {
+ type = "integer",
+ description = "absolute session lifetime in seconds",
+ },
cookie = {
type = "object",
+ description =
+ "Deprecated. Kept for backward compatibility with "
+ .. "the lua-resty-session 3.x schema. Use the flat "
+ .. "session.* options (cookie_name, absolute_timeout, "
+ .. "etc.) instead.",
properties = {
lifetime = {
type = "integer",
- description = "it holds the cookie lifetime in seconds in the future",
- }
- }
+ description =
+ "Deprecated. Mapped to absolute_timeout at "
+ .. "runtime when absolute_timeout is not set.",
+ },
+ },
+ additionalProperties = false,
Review Comment:
Heads up — `TEST 9` in `openid-connect10.t` asserts exactly this rejection (an unknown key under `session.cookie`), so when you drop `additionalProperties = false` here it'll need to be removed too, otherwise it'll start failing.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -33,6 +34,33 @@ local ngx_encode_base64 = ngx.encode_base64
local plugin_name = "openid-connect"
+-- Translate session config to lua-resty-session 4.x options.
+-- Most keys (cookie_name, cookie_path, *_timeout, etc.) are already named
+-- after lua-resty-session and pass straight through. The only translation
+-- is the legacy session.cookie.lifetime alias from the lua-resty-session 3.x
+-- schema, which is mapped to absolute_timeout when the latter is unset.
+local function build_session_opts(session_conf)
+ if not session_conf then
+ return nil
+ end
+ local opts = {}
+ for k, v in pairs(session_conf) do
+ if k ~= "cookie" then
+ opts[k] = v
+ end
+ end
+ local cookie = session_conf.cookie
+ if cookie and cookie.lifetime ~= nil then
+ if opts.absolute_timeout == nil then
+ opts.absolute_timeout = cookie.lifetime
+ core.log.warn("session.cookie.lifetime is deprecated; ",
+ "use session.absolute_timeout instead")
+ end
+ end
+ return opts
Review Comment:
One follow-on if you take this version: it no longer strips the `cookie` key from the returned table, so `TEST 4` and `TEST 5` (which assert `cookie=nil`) will fail — they should instead expect the `cookie` table to still be present. Leaving it there is harmless since lua-resty-session 4.x only reads the keys it knows. Also `local pairs = pairs` becomes unused and luacheck will flag it, so that import can go.
##########
t/plugin/openid-connect10.t:
##########
@@ -0,0 +1,393 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: valid session with flat cookie_name, cookie_path, absolute_timeout
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ cookie_name = "my_session",
+ cookie_path = "/app",
+ absolute_timeout = 7200,
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 2: valid session with flat cookie_secure, cookie_same_site, idling/rolling timeouts
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ cookie_name = "oidc_session",
+ cookie_secure = true,
+ cookie_http_only = true,
+ cookie_same_site = "Strict",
+ cookie_domain = "example.com",
+ idling_timeout = 600,
+ rolling_timeout = 1800,
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 3: backward-compatible session.cookie.lifetime still accepted
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ cookie = {
+ lifetime = 3600,
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 4: build_session_opts passes flat keys through untouched
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local build = plugin._build_session_opts
+ local opts = build({
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ storage = "cookie",
+ cookie_name = "my_session",
+ cookie_path = "/app",
+ cookie_secure = true,
+ cookie_same_site = "Strict",
+ idling_timeout = 600,
+ rolling_timeout = 1800,
+ absolute_timeout = 7200,
+ })
+ ngx.say("cookie_name=", opts.cookie_name)
+ ngx.say("cookie_path=", opts.cookie_path)
+ ngx.say("cookie_secure=", tostring(opts.cookie_secure))
+ ngx.say("cookie_same_site=", opts.cookie_same_site)
+ ngx.say("idling_timeout=", opts.idling_timeout)
+ ngx.say("rolling_timeout=", opts.rolling_timeout)
+ ngx.say("absolute_timeout=", opts.absolute_timeout)
+ ngx.say("storage=", opts.storage)
+ ngx.say("cookie=", tostring(opts.cookie))
+ }
+ }
+--- response_body
+cookie_name=my_session
+cookie_path=/app
+cookie_secure=true
+cookie_same_site=Strict
+idling_timeout=600
+rolling_timeout=1800
+absolute_timeout=7200
+storage=cookie
+cookie=nil
+
+
+
+=== TEST 5: build_session_opts maps deprecated cookie.lifetime to absolute_timeout
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local build = plugin._build_session_opts
+ local opts = build({
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ cookie = {
+ lifetime = 7200,
+ }
+ })
+ ngx.say("absolute_timeout=", opts.absolute_timeout)
+ ngx.say("cookie=", tostring(opts.cookie))
+ }
+ }
+--- response_body
+absolute_timeout=7200
+cookie=nil
+--- error_log
+session.cookie.lifetime is deprecated
+
+
+
+=== TEST 6: build_session_opts returns nil for nil input
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local build = plugin._build_session_opts
+ ngx.say(tostring(build(nil)))
+ }
+ }
+--- response_body
+nil
+
+
+
+=== TEST 7: build_session_opts works when cookie field is absent
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local build = plugin._build_session_opts
+ local opts = build({
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ storage = "redis",
+ redis = { host = "127.0.0.1", port = 6379 },
+ })
+ ngx.say("secret=", opts.secret)
+ ngx.say("storage=", opts.storage)
+ ngx.say("redis.host=", opts.redis.host)
+ ngx.say("redis.port=", opts.redis.port)
+ }
+ }
+--- response_body
+secret=jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+storage=redis
+redis.host=127.0.0.1
+redis.port=6379
+
+
+
+=== TEST 8: invalid type for cookie.lifetime (string instead of integer)
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ cookie = {
+ lifetime = "invalid",
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body_like
+property "lifetime" validation failed: wrong type: expected integer, got string.*
+
+
+
+=== TEST 9: unknown key under session.cookie is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ cookie = {
+ cookie_secure = true,
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body_like
+.*additional property.*cookie_secure.*
+
+
+
+=== TEST 10: valid session with redis storage and flat cookie options
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ storage = "redis",
+ redis = {
+ host = "127.0.0.1",
+ port = 6379,
+ },
+ cookie_name = "oidc_session",
+ absolute_timeout = 7200,
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 11: absolute_timeout wins when both it and cookie.lifetime are set
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local build = plugin._build_session_opts
+ local opts = build({
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ absolute_timeout = 1800,
+ cookie = {
+ lifetime = 7200,
+ }
+ })
+ ngx.say("absolute_timeout=", opts.absolute_timeout)
+ }
+ }
+--- response_body
+absolute_timeout=1800
+--- no_error_log
+session.cookie.lifetime is deprecated
+
+
+
+=== TEST 12: unknown key directly under session is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ not_a_real_option = true,
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body_like
+.*additional property.*not_a_real_option.*
+
+
+
+=== TEST 13: invalid cookie_same_site value is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.openid-connect")
+ local ok, err = plugin.check_schema({
+ client_id = "a",
+ client_secret = "b",
+ discovery = "c",
+ session = {
+ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ cookie_same_site = "bogus",
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- response_body_like
+.*cookie_same_site.*
+session.cookie: both
Review Comment:
This test will fail as-is. The `session.cookie: both` line is leftover from the earlier alias implementation and is never emitted now — and since this is a schema-validation case, the response body is just the `cookie_same_site` enum error. That second line should be dropped.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4285711008
1. **Breaking change handling**: `session.cookie.lifetime` was removed outright without a backward-compatible migration path. Please add an internal mapping (e.g., map `cookie.lifetime` to `absolute_timeout`) or at least keep the old field during a deprecation period.
2. **Schema design**: Rather than hardcoding all ~30 lua-resty-session 4.x properties into the schema, consider explicitly defining only the essential ones (e.g., `cookie_name`, `cookie_path`) and allowing the rest via `additionalProperties`. Benefits:
- No APISIX schema changes needed when lua-resty-session adds new config options
- Reduced maintenance burden
- Cleaner schema
Looking forward to the code update for further review.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 merged PR #13178:
URL: https://github.com/apache/apisix/pull/13178
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4294161441
The updated approach aligns well with our earlier discussion — clean schema with explicit `name`/`path`/`lifetime` plus `additionalProperties` pass-through. Tests are comprehensive. Thank you for the iteration.
One concern about **key conflict in `build_session_opts`**: if a user sets both `lifetime` and `absolute_timeout` (or `name`/`cookie_name`, `path`/`cookie_path`) under `session.cookie`, the `pairs()` iteration order is non-deterministic, so the final value of `opts.absolute_timeout` depends on the internal Lua hash table traversal order.
Suggestion — add a conflict warning or precedence rule, e.g.:
```lua
local cookie = session_conf.cookie
if cookie then
for k, v in pairs(cookie) do
local mapped = cookie_key_map[k]
local target_key = mapped or k
if mapped and cookie[target_key] then
core.log.warn("session.cookie: both '", k, "' and '", target_key,
"' are set; using '", k, "' (mapped to '", target_key, "')")
end
opts[target_key] = v
end
end
```
Alternatively, document clearly that users should not set both a mapped alias and its corresponding flat key simultaneously.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4659967715
Hello @nic-6443 and @mbsjed
I started working on this because I really needed it myself (especially being able to declare different OIDC clients and forward cookies only to specific subpaths).
My initial take was to directly expose the resty session configuration in order to take full advantage of the third party library, in my opinion apisix should not have an opinionated approach on how this library is used by mapping/remapping it's configuration.
That said, I changed the initial implementation due to the @Baoyuantop suggestion that also makes sense, deprecating completely the session.lifetime and exposing through the schema resty.session configuration would definitely be a breaking change, meaning that this won't reach Apisix until the 4.x release (if apisix uses semantic versioning).
In order to avoid a breaking change I suggest to expose the resty.session parameters like you suggester, keep the session.lifetime parameter marking as deprecated for the 3.x and to remove it completely when apisix goes 4.x.
This PR needs at least 3 reviewer also, before I move on I would love to reach some kind of consensus, I am not sure who else can/should express what kind of approach we want to follow.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #13178:
URL: https://github.com/apache/apisix/pull/13178#discussion_r3381474748
##########
apisix/plugins/openid-connect.lua:
##########
@@ -76,14 +104,59 @@ local schema = {
description = "the key used for the encrypt and HMAC calculation",
minLength = 16,
},
+ cookie_name = {
+ type = "string",
+ description = "session cookie name",
+ },
+ cookie_path = {
+ type = "string",
+ description = "cookie path scope",
+ },
+ cookie_domain = {
+ type = "string",
+ description = "cookie domain scope",
+ },
+ cookie_secure = {
+ type = "boolean",
+ description = "if true, set the Secure cookie attribute",
+ },
+ cookie_http_only = {
+ type = "boolean",
+ description = "if true, set the HttpOnly cookie attribute",
+ },
+ cookie_same_site = {
+ type = "string",
+ enum = {"Strict", "Lax", "None", "Default"},
+ description = "SameSite cookie attribute",
+ },
+ idling_timeout = {
+ type = "integer",
+ description = "idling timeout in seconds",
+ },
+ rolling_timeout = {
+ type = "integer",
+ description = "rolling timeout in seconds",
+ },
+ absolute_timeout = {
+ type = "integer",
+ description = "absolute session lifetime in seconds",
+ },
cookie = {
type = "object",
+ description =
+ "Deprecated. Kept for backward compatibility with "
+ .. "the lua-resty-session 3.x schema. Use the flat "
+ .. "session.* options (cookie_name, absolute_timeout, "
+ .. "etc.) instead.",
properties = {
lifetime = {
type = "integer",
- description = "it holds the cookie lifetime in seconds in the future",
- }
- }
+ description =
+ "Deprecated. Mapped to absolute_timeout at "
+ .. "runtime when absolute_timeout is not set.",
+ },
+ },
+ additionalProperties = false,
Review Comment:
This change is redundant because it makes the schema stricter and thus has certain breaking impacts.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "kovasaurus (via GitHub)" <gi...@apache.org>.
kovasaurus commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4294361005
@Baoyuantop I would like to point out that this addresses only cookie properties in the session object.
However the lua-resty-session has matured enough to support additional storage options for session object.
Redis storage is only partially addressed in the the openid-connect plugin's schema.
Would it make sense to allow additional options to entire session object ( to support the rest of the storage options which are already exposed by lua-resty-session ) and existing session.redis to support redis cluster/sentinels?
All configuration options from lua-resty-session -> [here](https://github.com/bungle/lua-resty-session#session-configuration)
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on code in PR #13178:
URL: https://github.com/apache/apisix/pull/13178#discussion_r3381548184
##########
apisix/plugins/openid-connect.lua:
##########
@@ -76,14 +104,59 @@ local schema = {
description = "the key used for the encrypt and HMAC calculation",
minLength = 16,
},
+ cookie_name = {
+ type = "string",
+ description = "session cookie name",
+ },
+ cookie_path = {
+ type = "string",
+ description = "cookie path scope",
+ },
+ cookie_domain = {
+ type = "string",
+ description = "cookie domain scope",
+ },
+ cookie_secure = {
+ type = "boolean",
+ description = "if true, set the Secure cookie attribute",
+ },
+ cookie_http_only = {
+ type = "boolean",
+ description = "if true, set the HttpOnly cookie attribute",
+ },
+ cookie_same_site = {
+ type = "string",
+ enum = {"Strict", "Lax", "None", "Default"},
+ description = "SameSite cookie attribute",
+ },
+ idling_timeout = {
+ type = "integer",
+ description = "idling timeout in seconds",
+ },
+ rolling_timeout = {
+ type = "integer",
+ description = "rolling timeout in seconds",
+ },
+ absolute_timeout = {
+ type = "integer",
+ description = "absolute session lifetime in seconds",
+ },
cookie = {
type = "object",
+ description =
+ "Deprecated. Kept for backward compatibility with "
+ .. "the lua-resty-session 3.x schema. Use the flat "
+ .. "session.* options (cookie_name, absolute_timeout, "
+ .. "etc.) instead.",
properties = {
lifetime = {
type = "integer",
- description = "it holds the cookie lifetime in seconds in the future",
- }
- }
+ description =
+ "Deprecated. Mapped to absolute_timeout at "
+ .. "runtime when absolute_timeout is not set.",
+ },
+ },
+ additionalProperties = false,
Review Comment:
line 159? didn't you initially propose to set it as false [explicitly in this comment](https://github.com/apache/apisix/pull/13178#issuecomment-4659033649)?
I wasn't aware of the breaking impacts, want me to wipe it out?
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4661599508
thank you for the review @nic-6443 -> I have applied you reccomendations and I did bootstrap devcontainers to run the tests locally, I'll let you to mark the open 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
Re: [PR] feat(openid-connect): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "mbsjed (via GitHub)" <gi...@apache.org>.
mbsjed commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4659139185
@nic-6443 sounds great! We are currently preparing for a large scale rollout and have chosen APISIX. For security reasons we would very much like to use this feature (it has effectively become a requirement for us at this point :-)).
If there is any way i can help or contribute, i would be happy to do so.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4660245516
@francescodedomenico one more thing — no pressure either way: if you have the bandwidth, this is your PR and I'm happy to just review and help it land. But if you're stretched thin, I'm glad to pick up the implementation and carry it forward with you so it doesn't stall — your work and authorship stay on it, I'd just help push it over the line. A few users here clearly need this, so let's make sure it lands one way or another. Just let me know which you'd prefer.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4294547385
@kovasaurus while I do believe it would be a great addition to support other storage options I think this should be addressed in a whole different PR -> I'd like not to broad the scope of this one.
@Baoyuantop good observation, I made the the aliases explicitly declared in the schema to have precedence over the ones in additionalProperties, warning is logged when both are set so the user has evidence of what's happening.
Documentation has been updated and order of precedence explained in the openid markdown under the session.cookie.<other> parameter.
I have noticed that the previous workflow had a couple of jobs failed but they didn't seem related to openid-connect.md to me.
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4287631738
hello @Baoyuantop
I like this approach, I pushed changes and updated both the openid-connect.md and the description of this PR to match the current state of the branch.
- session.cookie.lifetime is mapped to absolute_timeout
- cookie_name and cookie_path are mapped to session.cookie.name and session.cookie.path to respect the schema structure
thank you
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on PR #13178:
URL: https://github.com/apache/apisix/pull/13178#issuecomment-4660505551
@nic-6443 I am on board with this, and luckily I had time to update the PR, like you I am interested in having this fixed for my personal and professional projects.
PR and description updated, the code diff from main is very little tbh , I hope review won't be difficult, let me know!
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "francescodedomenico (via GitHub)" <gi...@apache.org>.
francescodedomenico commented on code in PR #13178:
URL: https://github.com/apache/apisix/pull/13178#discussion_r3381548184
##########
apisix/plugins/openid-connect.lua:
##########
@@ -76,14 +104,59 @@ local schema = {
description = "the key used for the encrypt and HMAC calculation",
minLength = 16,
},
+ cookie_name = {
+ type = "string",
+ description = "session cookie name",
+ },
+ cookie_path = {
+ type = "string",
+ description = "cookie path scope",
+ },
+ cookie_domain = {
+ type = "string",
+ description = "cookie domain scope",
+ },
+ cookie_secure = {
+ type = "boolean",
+ description = "if true, set the Secure cookie attribute",
+ },
+ cookie_http_only = {
+ type = "boolean",
+ description = "if true, set the HttpOnly cookie attribute",
+ },
+ cookie_same_site = {
+ type = "string",
+ enum = {"Strict", "Lax", "None", "Default"},
+ description = "SameSite cookie attribute",
+ },
+ idling_timeout = {
+ type = "integer",
+ description = "idling timeout in seconds",
+ },
+ rolling_timeout = {
+ type = "integer",
+ description = "rolling timeout in seconds",
+ },
+ absolute_timeout = {
+ type = "integer",
+ description = "absolute session lifetime in seconds",
+ },
cookie = {
type = "object",
+ description =
+ "Deprecated. Kept for backward compatibility with "
+ .. "the lua-resty-session 3.x schema. Use the flat "
+ .. "session.* options (cookie_name, absolute_timeout, "
+ .. "etc.) instead.",
properties = {
lifetime = {
type = "integer",
- description = "it holds the cookie lifetime in seconds in the future",
- }
- }
+ description =
+ "Deprecated. Mapped to absolute_timeout at "
+ .. "runtime when absolute_timeout is not set.",
+ },
+ },
+ additionalProperties = false,
Review Comment:
@nic-6443 line 159? didn't you initially propose to set it as false [explicitly in this comment](https://github.com/apache/apisix/pull/13178#issuecomment-4659033649)?
I wasn't aware of the breaking impacts, want me to wipe it out?
--
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): update session config to support lua-resty-session, fixes deprecated session.cookie.lifetime [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #13178:
URL: https://github.com/apache/apisix/pull/13178#discussion_r3381593110
##########
apisix/plugins/openid-connect.lua:
##########
@@ -76,14 +104,59 @@ local schema = {
description = "the key used for the encrypt and HMAC calculation",
minLength = 16,
},
+ cookie_name = {
+ type = "string",
+ description = "session cookie name",
+ },
+ cookie_path = {
+ type = "string",
+ description = "cookie path scope",
+ },
+ cookie_domain = {
+ type = "string",
+ description = "cookie domain scope",
+ },
+ cookie_secure = {
+ type = "boolean",
+ description = "if true, set the Secure cookie attribute",
+ },
+ cookie_http_only = {
+ type = "boolean",
+ description = "if true, set the HttpOnly cookie attribute",
+ },
+ cookie_same_site = {
+ type = "string",
+ enum = {"Strict", "Lax", "None", "Default"},
+ description = "SameSite cookie attribute",
+ },
+ idling_timeout = {
+ type = "integer",
+ description = "idling timeout in seconds",
+ },
+ rolling_timeout = {
+ type = "integer",
+ description = "rolling timeout in seconds",
+ },
+ absolute_timeout = {
+ type = "integer",
+ description = "absolute session lifetime in seconds",
+ },
cookie = {
type = "object",
+ description =
+ "Deprecated. Kept for backward compatibility with "
+ .. "the lua-resty-session 3.x schema. Use the flat "
+ .. "session.* options (cookie_name, absolute_timeout, "
+ .. "etc.) instead.",
properties = {
lifetime = {
type = "integer",
- description = "it holds the cookie lifetime in seconds in the future",
- }
- }
+ description =
+ "Deprecated. Mapped to absolute_timeout at "
+ .. "runtime when absolute_timeout is not set.",
+ },
+ },
+ additionalProperties = false,
Review Comment:
Breaking means that previously, extra fields could be configured under the key "cookie". Although this behavior was undefined, after we added `additionalProperties = false`, APISIX will directly reject the old configurations. This offers no benefit to our current PR. I will revise the examples in my previous comment and remove the redundant `additionalProperties = false`.
--
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