juzhiyuan commented on code in PR #13651:
URL: https://github.com/apache/apisix/pull/13651#discussion_r4576402401
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -357,6 +361,28 @@ spec:
See [Implement Authorization Code
Grant](../tutorials/keycloak-oidc.md#implement-authorization-code-grant) for a
complete example to use the `openid-connect` Plugin to integrate with Keycloak
using the authorization code flow.
+### Cookie Session Revocation
Review Comment:
lua-resty-session generates a new sid on every `save()` (session.lua:1065),
and only `destroy()` writes a revocation mark (session.lua:1417).
openidc saves the session on token renewal and rolling refresh, so logout
only revokes the sid presented at logout time.
A stolen cookie that has already been refreshed by the attacker, or older
copies of the cookie, stay valid until their rolling/absolute timeout.
Could you document this limitation explicitly (e.g. "revocation applies to
the session cookie presented at logout")?
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -357,6 +361,28 @@ spec:
See [Implement Authorization Code
Grant](../tutorials/keycloak-oidc.md#implement-authorization-code-grant) for a
complete example to use the `openid-connect` Plugin to integrate with Keycloak
using the authorization code flow.
+### Cookie Session Revocation
+
+Cookie sessions are stored by the client and cannot normally be invalidated
individually by APISIX. To enable server-side revocation, set
`session.revocation` to `redis` and configure a Redis denylist:
+
+```json
+"session": {
+ "secret": "your-session-secret-min-16-chars",
+ "storage": "cookie",
+ "revocation": "redis",
+ "revocation_fail_mode": "closed",
+ "revocation_cache_ttl": 5,
+ "revocation_error_cache_ttl": 1,
+ "redis": {
+ "host": "127.0.0.1",
+ "port": 6379,
+ "prefix": "oidc:session:"
+ }
+}
+```
+
+`revocation_fail_mode` defaults to `open`, which continues accepting sessions
if the revocation store is unavailable. Use `closed` to reject session open and
destroy operations instead. Opening a cookie session checks the revocation
store, so each worker keeps a small cache of recent lookups to avoid one Redis
call per request. `revocation_cache_ttl` is the worker-local TTL for successful
lookups, and `revocation_error_cache_ttl` is the TTL for store errors. A logout
takes effect immediately on the worker that handled it and on other workers
once their cached entry expires, so `revocation_cache_ttl` is also the longest
a revoked session can still be accepted elsewhere. Set `revocation_cache_ttl`
to `0` to check Redis on every request; the error cache is then disabled as
well. Omit `revocation` to leave cookie-session revocation disabled.
Review Comment:
Also, worth noting for the default `open` mode: if Redis is unavailable
during logout, `destroy()` still succeeds and the browser cookie is cleared,
but no revocation mark is written and nothing retries it later.
A copied cookie therefore stays valid until it times out. Please mention
this and recommend `closed` for deployments that rely on revocation.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -68,6 +70,18 @@ local function build_session_opts(session_conf)
"use session.absolute_timeout instead")
end
end
+ if session_conf.revocation and session_conf.revocation_cache_ttl == 0 then
Review Comment:
This works around lua-resty-session allocating `REVOCATION_CACHE` only when
`revocation_cache_ttl > 0` (session.lua:2820), while `is_session_revoked` also
uses it when `revocation_error_cache_ttl > 0`.
Could you file or fix that upstream and link it in the comment so this can
be removed later?
Alternatively, reject `revocation_cache_ttl = 0` with
`revocation_error_cache_ttl > 0` in `check_schema`.
Also, since `core.table.clone` is shallow, this mutates the shared
`plugin_conf.session`; please say so explicitly in the comment.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
