fix(openid-connect): redirect instead of 500 on a stale authorization callback

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), restarting the authentication flow")
+                core.response.set_header("Location", target_url)
+                return 302
+            end
+
             core.log.error("OIDC authentication failed: ", err)
             return 500
         end

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.

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` 和以下消息，可能有多种原因。

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