From 805312cd1f5cf0c16c4fcbf758aeb4c7b1fb48fe Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 20 Jul 2026 12:54:29 +0800 Subject: [PATCH 1/2] fix(openid-connect): redirect instead of 500 on a stale auth callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resty.openidc keeps the state of a single login flow in the session, so a second flow started in another tab overwrites it. When the first tab's callback then arrives, its state no longer matches and the plugin turned that into a 500 — a dead end the user can only escape by manually navigating back to a protected page. The library already returns the original URL alongside that error, which is exactly what a caller needs to recover. Use it: on a GET callback with a state mismatch, respond 302 to that URL so a fresh flow starts from there. While the ID provider still holds an SSO session the user sees no interruption at all. Callbacks with other methods, and callbacks carrying no session, keep returning 500. The error string is matched verbatim because the library builds it without interpolation; it is identical in lua-resty-openidc 1.8.0 (the pinned version) and 1.9.0. There is no open-redirect exposure: the URL comes from the session, written from $request_uri, so it is always a path on this server and never client-controlled. Fixes #12360 --- apisix/plugins/openid-connect.lua | 28 +++- docs/en/latest/plugins/openid-connect.md | 10 ++ docs/zh/latest/plugins/openid-connect.md | 10 ++ t/plugin/openid-connect11.t | 174 +++++++++++++++++++++++ 4 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 t/plugin/openid-connect11.t diff --git a/apisix/plugins/openid-connect.lua b/apisix/plugins/openid-connect.lua index eb0d3b526c58..ce4651eb720f 100644 --- a/apisix/plugins/openid-connect.lua +++ b/apisix/plugins/openid-connect.lua @@ -32,6 +32,12 @@ local ngx_encode_base64 = ngx.encode_base64 local plugin_name = "openid-connect" +-- returned verbatim by resty.openidc when the state in the authorization +-- callback does not match the one restored from the session; the string is +-- identical in lua-resty-openidc 1.8.0 and 1.9.0 +local STATE_MISMATCH_ERR = + "state from argument does not match state restored from session" + -- Session config is passed as-is to resty.session.start(); the only -- translation is the legacy session.cookie.lifetime alias from the @@ -757,7 +763,7 @@ function _M.rewrite(plugin_conf, ctx) end end - local response, err, session, _ + local response, err, session if conf.bearer_only or conf.introspection_endpoint or conf.public_key or conf.use_jwks then -- An introspection endpoint or a public key has been configured. Try to @@ -860,7 +866,8 @@ function _M.rewrite(plugin_conf, ctx) -- provider's authorization endpoint to initiate the Relying Party flow. -- This code path also handles when the ID provider then redirects to -- the configured redirect URI after successful authentication. - response, err, _, session = openidc.authenticate(conf, nil, unauth_action, + local target_url + response, err, target_url, session = openidc.authenticate(conf, nil, unauth_action, build_session_opts(conf.session)) if err then @@ -873,6 +880,23 @@ function _M.rewrite(plugin_conf, ctx) end return 401 end + + -- Stale authorization callback: the state in the callback does not + -- match the one in the session, e.g. the same browser started + -- another login flow in a second tab and overwrote the state, or an + -- already completed callback was replayed. The client is a browser + -- mid-navigation, so instead of a dead-end 500, send it back to the + -- original URL that resty.openidc returns alongside the error: a + -- fresh flow starts from there and completes without any user + -- interaction while the ID provider still holds an SSO session. + if err == STATE_MISMATCH_ERR and target_url + and ngx.req.get_method() == "GET" then + core.log.warn("OIDC state mismatch (concurrent login flows or ", + "replayed callback), redirecting to: ", target_url) + core.response.set_header("Location", target_url) + return 302 + end + core.log.error("OIDC authentication failed: ", err) return 500 end diff --git a/docs/en/latest/plugins/openid-connect.md b/docs/en/latest/plugins/openid-connect.md index e196d6922206..5bb74efd3fad 100644 --- a/docs/en/latest/plugins/openid-connect.md +++ b/docs/en/latest/plugins/openid-connect.md @@ -412,6 +412,16 @@ This section covers a few commonly seen issues when working with this Plugin to If APISIX fails to resolve or cannot connect to the OpenID provider, double check the DNS settings in your configuration file `config.yaml` and modify as needed. +### State Mismatch in the Authorization Callback + +The session holds the state of a single login flow. If the same browser starts a second flow before the first one completes — for example by opening a protected page in another tab — the second flow overwrites the state, and the first tab's callback arrives with a state that no longer matches: + +```text +state from argument does not match state restored from session +``` + +For a `GET` callback, the Plugin responds with `302` back to the URL the user was originally trying to reach, so a fresh authentication flow starts from there. When the OpenID provider still holds an SSO session, this completes without any user interaction. Callbacks using other request methods, and callbacks carrying no session at all, still fail with `500`. + ### No Session State Found If you encounter a `500 internal server error` with the following message in the log when working with [authorization code flow](#authorization-code-flow), there could be a number of reasons. diff --git a/docs/zh/latest/plugins/openid-connect.md b/docs/zh/latest/plugins/openid-connect.md index 445d2a977f3e..5671cfb8f136 100644 --- a/docs/zh/latest/plugins/openid-connect.md +++ b/docs/zh/latest/plugins/openid-connect.md @@ -412,6 +412,16 @@ OpenID Connect (OIDC) 中的 UserInfo 端点在 [OpenID Connect Core 1.0 第 5.3 如果 APISIX 无法解析或连接到 OpenID 提供商，请检查配置文件 `config.yaml` 中的 DNS 设置并根据需要进行修改。 +### 授权回调中的 state 不匹配 + +会话中只保存一次登录流程的 state。如果同一浏览器在前一次流程完成之前又发起了新的流程（例如在另一个标签页中打开受保护页面），新流程会覆盖 state，此时第一个标签页的回调携带的 state 已不再匹配： + +```text +state from argument does not match state restored from session +``` + +对于 `GET` 回调，插件会返回 `302` 重定向回用户最初访问的地址，从那里重新发起一次认证流程。当身份提供商仍保有 SSO 会话时，整个过程对用户无感。使用其他请求方法的回调，以及完全不携带会话的回调，仍然返回 `500`。 + ### 未找到会话状态 如果在使用[授权码流程](#授权码流程)时，日志中出现 `500 internal server error` 和以下消息，可能有多种原因。 diff --git a/t/plugin/openid-connect11.t b/t/plugin/openid-connect11.t new file mode 100644 index 000000000000..c834b2abedde --- /dev/null +++ b/t/plugin/openid-connect11.t @@ -0,0 +1,174 @@ +# +# 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'; + +log_level('debug'); +repeat_each(1); +no_long_string(); +no_root_location(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } + + # every block here drives resty.openidc into an error path on purpose, + # which logs at [error]; assert on the specific message instead + if ((!defined $block->error_log) && (!defined $block->no_error_log)) { + $block->set_value("no_error_log", "no such assertion"); + } +}); + +run_tests(); + +__DATA__ + +=== TEST 1: create a route protected by openid-connect +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/oidc11', + ngx.HTTP_PUT, + [[{ + "uri": "/oidc11/*", + "plugins": { + "openid-connect": { + "client_id": "apisix", + "client_secret": "secret", + "discovery": "http://127.0.0.1:8080/realms/basic/.well-known/openid-configuration", + "redirect_uri": "http://127.0.0.1:1984/oidc11/callback", + "ssl_verify": false, + "session": { + "secret": "6S8IO+A+6KJsdazbjNyG7g==" + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + } + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 2: a callback whose state was overwritten by a second tab redirects back +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local base = "http://127.0.0.1:" .. ngx.var.server_port + + local function cookie_of(res) + local c = res.headers["Set-Cookie"] + if type(c) == "table" then + c = table.concat(c, "; ") + end + return c and c:match("^([^;]+)") + end + + -- first tab: start a login flow and keep the session cookie + local res_a = http.new():request_uri(base .. "/oidc11/page?tab=A") + local state_a = res_a.headers["Location"]:match("state=([^&]+)") + local jar = cookie_of(res_a) + + -- second tab in the same browser: overwrites the state in the session + local res_b = http.new():request_uri(base .. "/oidc11/page?tab=B", { + headers = {Cookie = jar} + }) + jar = cookie_of(res_b) + + -- the first tab's callback now carries a stale state + local res_c = http.new():request_uri( + base .. "/oidc11/callback?code=dummy&state=" .. state_a, { + headers = {Cookie = jar} + }) + ngx.say(res_c.status, " ", tostring(res_c.headers["Location"])) + } + } +--- response_body +302 /oidc11/page?tab=B +--- error_log +does not match state restored from session + + + +=== TEST 3: a non-GET callback with a stale state still fails with 500 +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local base = "http://127.0.0.1:" .. ngx.var.server_port + + local function cookie_of(res) + local c = res.headers["Set-Cookie"] + if type(c) == "table" then + c = table.concat(c, "; ") + end + return c and c:match("^([^;]+)") + end + + local res_a = http.new():request_uri(base .. "/oidc11/page?tab=A") + local state_a = res_a.headers["Location"]:match("state=([^&]+)") + local jar = cookie_of(res_a) + + local res_b = http.new():request_uri(base .. "/oidc11/page?tab=B", { + headers = {Cookie = jar} + }) + jar = cookie_of(res_b) + + local res_c = http.new():request_uri( + base .. "/oidc11/callback?code=dummy&state=" .. state_a, { + method = "POST", + body = "", + headers = {Cookie = jar} + }) + ngx.say(res_c.status) + } + } +--- response_body +500 + + + +=== TEST 4: a callback without a session cookie still fails with 500 +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local base = "http://127.0.0.1:" .. ngx.var.server_port + local res = http.new():request_uri( + base .. "/oidc11/callback?code=dummy&state=deadbeef") + ngx.say(res.status) + } + } +--- response_body +500 From 3c29ff1c70721a508ff1f5d4be44dc04a84a2466 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 20 Jul 2026 14:04:09 +0800 Subject: [PATCH 2/2] fix(openid-connect): do not log the redirect URL on state mismatch The original URL comes from the user's own request and can carry query string credentials or PII. The mismatch itself is what matters for diagnosis, and the URL is already in the access log. --- apisix/plugins/openid-connect.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/plugins/openid-connect.lua b/apisix/plugins/openid-connect.lua index ce4651eb720f..202cf7e2c846 100644 --- a/apisix/plugins/openid-connect.lua +++ b/apisix/plugins/openid-connect.lua @@ -892,7 +892,7 @@ function _M.rewrite(plugin_conf, ctx) if err == STATE_MISMATCH_ERR and target_url and ngx.req.get_method() == "GET" then core.log.warn("OIDC state mismatch (concurrent login flows or ", - "replayed callback), redirecting to: ", target_url) + "replayed callback), restarting the authentication flow") core.response.set_header("Location", target_url) return 302 end