fix(feishu-auth, dingtalk-auth): bind the authorization code to the session that started the login

Both plugins emitted the login redirect and consumed the callback without a
state parameter, so a code from the query string was accepted regardless of
which session it arrived on.

Generate a random state before redirecting, carry it on the redirect URL, and
require it back on codes taken from the query string. Codes taken from the
configured header are exempt, since those requests come from non-browser
clients.

Also replaces sess:delete(), which does not exist in lua-resty-session 4.x and
crashed the feishu-auth token refresh path.

Author: shreemaan-abhishek

apisix/plugins/dingtalk-auth.lua

@@ -17,9 +17,13 @@
 local core = require("apisix.core")
 local http = require("resty.http")
 local session = require("resty.session")
+local resty_random = require("resty.random")
+local resty_string = require("resty.string")
 
 local base64_encode = ngx.encode_base64
 
+local STATE_BYTES = 16
+
 -- the access token from dingtalk has a TTL of 7200 seconds,
 -- we set the cache TTL to 7000 seconds to avoid edge cases of token expiration during use.
 local access_token_cache = core.lrucache.new({
@@ -201,14 +205,46 @@ local function fetch_userinfo(conf, access_token, code)
 end
 
 
+local function session_opts(conf)
+    return {
+        secret = conf.secret,
+        secret_fallbacks = conf.secret_fallbacks,
+        cookie_name = "dingtalk_session",
+        absolute_timeout = conf.cookie_expires_in,
+    }
+end
+
+
+-- returns the code and whether it came from the request header
 local function get_code(conf, ctx)
     local code = core.request.header(ctx, conf.code_header)
-    if not code then
-        local uri_args = core.request.get_uri_args(ctx) or {}
-        code = uri_args[conf.code_query]
+    if code then
+        return code, true
+    end
+
+    local uri_args = core.request.get_uri_args(ctx) or {}
+    return uri_args[conf.code_query], false
+end
+
+
+-- bind a fresh state to the session and carry it along to the login page,
+-- so the code that comes back can be tied to the browser that started the flow
+local function redirect_to_login(conf, opts)
+    local bytes = resty_random.bytes(STATE_BYTES, true)
+                    or resty_random.bytes(STATE_BYTES)
+    local state = resty_string.to_hex(bytes)
+
+    local sess = session.start(opts)
+    sess:set("state", state)
+    local ok, err = sess:save()
+    if not ok then
+        core.log.error("failed to save session: ", err)
+        return 500, {message = "Failed to save session"}
     end
 
-    return code
+    local sep = core.string.find(conf.redirect_uri, "?") and "&" or "?"
+    core.response.set_header("Location", conf.redirect_uri .. sep .. "state=" .. state)
+    return 302
 end
 
 
@@ -218,14 +254,8 @@ function _M.rewrite(conf, ctx)
     -- clear any client-supplied X-Userinfo before authentication
     core.request.set_header(ctx, "X-Userinfo", nil)
 
-    local sess, sess_err = session.open(
-        {
-            secret = conf.secret,
-            secret_fallbacks = conf.secret_fallbacks,
-            cookie_name = "dingtalk_session",
-            absolute_timeout = conf.cookie_expires_in,
-        }
-    )
+    local opts = session_opts(conf)
+    local sess, sess_err = session.open(opts)
     if not sess then
         core.log.error("failed to open session: ", sess_err)
         return 500, {message = "Failed to open session"}
@@ -237,14 +267,26 @@ function _M.rewrite(conf, ctx)
         if not userinfo then
             sess:destroy()
             core.log.error("failed to decode userinfo in session: ", err)
-            core.response.set_header("Location", conf.redirect_uri)
-            return 302
+            return redirect_to_login(conf, opts)
         end
     else
-        local code = get_code(conf, ctx)
+        local code, from_header = get_code(conf, ctx)
         if not code then
-            core.response.set_header("Location", conf.redirect_uri)
-            return 302
+            return redirect_to_login(conf, opts)
+        end
+
+        -- a code in the query string comes back from the login redirect, so it must
+        -- carry the state bound to this session. a code in the header comes from a
+        -- non-browser client, which cannot be driven cross-site.
+        if not from_header then
+            local uri_args = core.request.get_uri_args(ctx) or {}
+            local state = sess:get("state")
+            if not state or uri_args.state ~= state then
+                sess:destroy()
+                core.log.warn("state does not match the one bound to the session")
+                return 401, {message = "Invalid state"}
+            end
+            sess:set("state", nil)
         end
 
         local key = core.table.concat({

apisix/plugins/feishu-auth.lua

@@ -17,11 +17,15 @@
 local core = require("apisix.core")
 local http = require("resty.http")
 local session = require("resty.session")
+local resty_random = require("resty.random")
+local resty_string = require("resty.string")
 
 local base64_encode = ngx.encode_base64
 local ngx_time = ngx.time
 local type = type
 
+local STATE_BYTES = 16
+
 local DEFAULT_TOKEN_URL = "https://open.feishu.cn/open-apis/authen/v2/oauth/token"
 local DEFAULT_USERINFO_URL = "https://open.feishu.cn/open-apis/authen/v1/user_info"
 
@@ -193,14 +197,46 @@ local function fetch_userinfo(conf, access_token)
 end
 
 
+local function session_opts(conf)
+    return {
+        secret = conf.secret,
+        secret_fallbacks = conf.secret_fallbacks,
+        cookie_name = "feishu_session",
+        absolute_timeout = conf.cookie_expires_in,
+    }
+end
+
+
+-- returns the code and whether it came from the request header
 local function get_code(conf, ctx)
     local code = core.request.header(ctx, conf.code_header)
-    if not code then
-        local uri_args = core.request.get_uri_args(ctx) or {}
-        code = uri_args[conf.code_query]
+    if code then
+        return code, true
+    end
+
+    local uri_args = core.request.get_uri_args(ctx) or {}
+    return uri_args[conf.code_query], false
+end
+
+
+-- bind a fresh state to the session and carry it along to the login page,
+-- so the code that comes back can be tied to the browser that started the flow
+local function redirect_to_login(conf, opts)
+    local bytes = resty_random.bytes(STATE_BYTES, true)
+                    or resty_random.bytes(STATE_BYTES)
+    local state = resty_string.to_hex(bytes)
+
+    local sess = session.start(opts)
+    sess:set("state", state)
+    local ok, err = sess:save()
+    if not ok then
+        core.log.error("failed to save session: ", err)
+        return 500, {message = "Failed to save session"}
     end
 
-    return code
+    local sep = core.string.find(conf.redirect_uri, "?") and "&" or "?"
+    core.response.set_header("Location", conf.redirect_uri .. sep .. "state=" .. state)
+    return 302
 end
 
 
@@ -210,14 +246,8 @@ function _M.rewrite(conf, ctx)
     -- clear any client-supplied X-Userinfo before authentication
     core.request.set_header(ctx, "X-Userinfo", nil)
 
-    local sess, sess_err = session.open(
-        {
-            secret = conf.secret,
-            secret_fallbacks = conf.secret_fallbacks,
-            cookie_name = "feishu_session",
-            absolute_timeout = conf.cookie_expires_in,
-        }
-    )
+    local opts = session_opts(conf)
+    local sess, sess_err = session.open(opts)
     if not sess then
         core.log.error("failed to open session: ", sess_err)
         return 500, {message = "Failed to open session"}
@@ -232,10 +262,23 @@ function _M.rewrite(conf, ctx)
             return 500, {message = "Invalid userinfo in session"}
         end
     else
-        local code = get_code(conf, ctx)
+        local code, from_header = get_code(conf, ctx)
         if not code then
-            core.response.set_header("Location", conf.redirect_uri)
-            return 302
+            return redirect_to_login(conf, opts)
+        end
+
+        -- a code in the query string comes back from the login redirect, so it must
+        -- carry the state bound to this session. a code in the header comes from a
+        -- non-browser client, which cannot be driven cross-site.
+        if not from_header then
+            local uri_args = core.request.get_uri_args(ctx) or {}
+            local state = sess:get("state")
+            if not state or uri_args.state ~= state then
+                sess:destroy()
+                core.log.warn("state does not match the one bound to the session")
+                return 401, {message = "Invalid state"}
+            end
+            sess:set("state", nil)
         end
 
         local refreshed = true
@@ -245,8 +288,8 @@ function _M.rewrite(conf, ctx)
             if expires_at and ngx_time() < expires_at then
                 refreshed = false
             else
-                sess:delete("access_token")
-                sess:delete("access_token_expires_at")
+                sess:set("access_token", nil)
+                sess:set("access_token_expires_at", nil)
             end
         end
 

docs/en/latest/plugins/dingtalk-auth.md

@@ -119,8 +119,8 @@ curl http://127.0.0.1:9180/apisix/admin/routes/1 \
 
 Once you have enabled the Plugin, incoming requests to the Route are processed as follows:
 
-1. **No session and no code**: The user is redirected to `redirect_uri` (typically a DingTalk OAuth login page) with a `302` response.
-2. **Authorization code present** (in the `code` query parameter or `X-DingTalk-Code` header): The Plugin exchanges the code for an access token via `access_token_url`, then retrieves user information from `userinfo_url`. On success, the user information is stored in an encrypted cookie session and the original request proceeds.
+1. **No session and no code**: The Plugin generates a random `state`, stores it in the session cookie, and redirects the user to `redirect_uri` (typically a DingTalk OAuth login page) with a `302` response, appending `state` to the query string. Pass `state` through to DingTalk so that it is returned on the callback.
+2. **Authorization code present** (in the `code` query parameter or `X-DingTalk-Code` header): A code taken from the query parameter must arrive with the `state` bound to the session, otherwise the Plugin responds with `401`. This ties the code to the browser that started the flow. A code taken from the `X-DingTalk-Code` header is exempt, since such requests come from non-browser clients. The Plugin then exchanges the code for an access token via `access_token_url`, then retrieves user information from `userinfo_url`. On success, the user information is stored in an encrypted cookie session and the original request proceeds.
 3. **Valid session cookie**: Subsequent requests carrying the session cookie bypass DingTalk API calls entirely and proceed directly to the upstream.
 
 When `set_userinfo_header` is `true` (the default), the upstream receives the DingTalk user information in the `X-Userinfo` header as a Base64-encoded JSON object.

docs/en/latest/plugins/feishu-auth.md

@@ -102,11 +102,12 @@ curl http://127.0.0.1:9180/apisix/admin/routes/1 -H "X-API-KEY: $admin_key" -X P
 The authentication flow proceeds as follows:
 
 1. A user visits a Route protected by `feishu-auth`.
-2. If no valid session cookie exists and no authorization `code` is present, the plugin redirects the user to `redirect_uri` with HTTP 302. Your application should then redirect the user to the Feishu OAuth authorization page.
+2. If no valid session cookie exists and no authorization `code` is present, the plugin generates a random `state`, stores it in the session cookie, and redirects the user to `redirect_uri` with HTTP 302, appending `state` to the query string. Your application should then redirect the user to the Feishu OAuth authorization page, passing `state` through so that Feishu returns it on the callback.
 3. After the user authorizes, Feishu redirects back to `auth_redirect_uri` with an authorization `code`. The plugin extracts the code either from the `code_query` query parameter or the `code_header` HTTP header.
-4. The plugin exchanges the code for an access token at `access_token_url`, then fetches user information from `userinfo_url`.
-5. User information is stored in an encrypted session cookie (`feishu_session`). Subsequent requests with a valid cookie bypass the OAuth flow.
-6. If `set_userinfo_header` is `true`, the plugin encodes the user information as Base64 JSON and sets it in the `X-Userinfo` request header before forwarding to the upstream.
+4. A code taken from the query string must arrive with the `state` bound to the session, otherwise the plugin responds with HTTP 401. This ties the code to the browser that started the flow. A code taken from the `code_header` HTTP header is exempt, since such requests come from non-browser clients.
+5. The plugin exchanges the code for an access token at `access_token_url`, then fetches user information from `userinfo_url`.
+6. User information is stored in an encrypted session cookie (`feishu_session`). Subsequent requests with a valid cookie bypass the OAuth flow.
+7. If `set_userinfo_header` is `true`, the plugin encodes the user information as Base64 JSON and sets it in the `X-Userinfo` request header before forwarding to the upstream.
 
 ## Delete Plugin
 

docs/zh/latest/plugins/feishu-auth.md

@@ -99,11 +99,12 @@ curl http://127.0.0.1:9180/apisix/admin/routes/1 -H "X-API-KEY: $admin_key" -X P
 认证流程如下：
 
 1. 用户访问受 `feishu-auth` 插件保护的路由。
-2. 若不存在有效的 session Cookie 且请求中不含授权 `code`，插件将以 HTTP 302 重定向用户至 `redirect_uri`。你的应用随后应将用户重定向到飞书 OAuth 授权页面。
+2. 若不存在有效的 session Cookie 且请求中不含授权 `code`，插件将生成随机 `state` 并存入 session Cookie，然后以 HTTP 302 重定向用户至 `redirect_uri`，并在其查询字符串中附加 `state`。你的应用随后应将用户重定向到飞书 OAuth 授权页面，并透传 `state`，以便飞书在回调时将其返回。
 3. 用户授权后，飞书将携带授权 `code` 重定向回 `auth_redirect_uri`。插件从 `code_query` 查询参数或 `code_header` 请求头中提取该授权码。
-4. 插件向 `access_token_url` 发起请求，使用授权码换取 access token，再从 `userinfo_url` 获取用户信息。
-5. 用户信息存储在加密的 session Cookie（`feishu_session`）中。后续携带有效 Cookie 的请求将跳过 OAuth 流程。
-6. 若 `set_userinfo_header` 为 `true`，插件将用户信息 Base64 编码后设置到 `X-Userinfo` 请求头，随请求转发至上游服务。
+4. 从查询参数中获取的授权码必须携带与当前 session 绑定的 `state`，否则插件返回 HTTP 401。该校验将授权码与发起流程的浏览器绑定。从 `code_header` 请求头中获取的授权码不做此校验，因为此类请求来自非浏览器客户端。
+5. 插件向 `access_token_url` 发起请求，使用授权码换取 access token，再从 `userinfo_url` 获取用户信息。
+6. 用户信息存储在加密的 session Cookie（`feishu_session`）中。后续携带有效 Cookie 的请求将跳过 OAuth 流程。
+7. 若 `set_userinfo_header` 为 `true`，插件将用户信息 Base64 编码后设置到 `X-Userinfo` 请求头，随请求转发至上游服务。
 
 ## 删除插件
 

t/lib/oauth_login.lua

@@ -0,0 +1,65 @@
+--
+-- 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.
+--
+local http = require("resty.http")
+local str_match = string.match
+
+local _M = {}
+
+
+-- follow the redirect to the login page and return the session cookie
+-- together with the state bound to it
+function _M.begin(port, path)
+    local httpc = http.new()
+    local uri = "http://127.0.0.1:" .. port .. path
+
+    local res, err = httpc:request_uri(uri, {method = "GET"})
+    if not res then
+        return nil, nil, err
+    end
+    if res.status ~= 302 then
+        return nil, nil, "expected 302 to the login page, got " .. res.status
+    end
+
+    local cookie = res.headers["Set-Cookie"]
+    local state = str_match(res.headers["Location"] or "", "state=([0-9a-f]+)")
+    if not cookie or not state then
+        return nil, nil, "redirect did not carry a session cookie and a state"
+    end
+
+    return cookie, state
+end
+
+
+-- drive a full login: pick up the state, then come back with the code
+function _M.login(port, path, code, code_query)
+    local cookie, state, err = _M.begin(port, path)
+    if not cookie then
+        return nil, err
+    end
+
+    local query = {state = state}
+    query[code_query or "code"] = code
+
+    return http.new():request_uri("http://127.0.0.1:" .. port .. path, {
+        method = "GET",
+        query = query,
+        headers = {["Cookie"] = cookie},
+    })
+end
+
+
+return _M