You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by sp...@apache.org on 2022/10/17 05:38:06 UTC

[apisix] branch master updated: feat(openid-connect): make session_secret support configurable (#8068)

This is an automated email from the ASF dual-hosted git repository.

spacewander pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git


The following commit(s) were added to refs/heads/master by this push:
     new 8f390621c feat(openid-connect): make session_secret support configurable (#8068)
8f390621c is described below

commit 8f390621cefbf84dc03b756919243e1277c6b01b
Author: Zeping Bai <bz...@apache.org>
AuthorDate: Mon Oct 17 13:37:57 2022 +0800

    feat(openid-connect): make session_secret support configurable (#8068)
---
 apisix/plugins/openid-connect.lua        |  36 +++++++--
 docs/en/latest/plugins/openid-connect.md |   4 +-
 docs/zh/latest/plugins/openid-connect.md |   4 +-
 t/plugin/openid-connect.t                | 126 +++++--------------------------
 t/plugin/openid-connect2.t               |  80 ++++++++++++++++++++
 5 files changed, 135 insertions(+), 115 deletions(-)

diff --git a/apisix/plugins/openid-connect.lua b/apisix/plugins/openid-connect.lua
index b472feca0..e7b96a34a 100644
--- a/apisix/plugins/openid-connect.lua
+++ b/apisix/plugins/openid-connect.lua
@@ -14,11 +14,14 @@
 -- See the License for the specific language governing permissions and
 -- limitations under the License.
 --
-local string = string
-local core = require("apisix.core")
-local ngx_re = require("ngx.re")
+
+local core    = require("apisix.core")
+local ngx_re  = require("ngx.re")
 local openidc = require("resty.openidc")
-local ngx = ngx
+local random  = require("resty.random")
+local string  = string
+local ngx     = ngx
+
 local ngx_encode_base64 = ngx.encode_base64
 
 local plugin_name = "openid-connect"
@@ -55,6 +58,18 @@ local schema = {
             type = "boolean",
             default = false,
         },
+        session = {
+            type = "object",
+            properties = {
+                secret = {
+                    type = "string",
+                    description = "the key used for the encrypt and HMAC calculation",
+                    minLength = 16,
+                },
+            },
+            required = {"secret"},
+            additionalProperties = false,
+        },
         realm = {
             type = "string",
             default = "apisix",
@@ -114,7 +129,7 @@ local schema = {
 
 
 local _M = {
-    version = 0.1,
+    version = 0.2,
     priority = 2599,
     name = plugin_name,
     schema = schema,
@@ -127,6 +142,15 @@ function _M.check_schema(conf)
         conf.ssl_verify = false
     end
 
+    if not conf.bearer_only and not conf.session then
+        core.log.warn("when bearer_only = false, " ..
+                       "you'd better complete the session configuration manually")
+        conf.session = {
+            -- generate a secret when bearer_only = false and no secret is configured
+            secret = ngx_encode_base64(random.bytes(32, true) or random.bytes(32))
+        }
+    end
+
     local ok, err = core.schema.check(schema, conf)
     if not ok then
         return false, err
@@ -309,7 +333,7 @@ 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)
+        response, err, _, session  = openidc.authenticate(conf, nil, nil, conf.session)
 
         if err then
             core.log.error("OIDC authentication failed: ", err)
diff --git a/docs/en/latest/plugins/openid-connect.md b/docs/en/latest/plugins/openid-connect.md
index 56f20e75b..965cf7474 100644
--- a/docs/en/latest/plugins/openid-connect.md
+++ b/docs/en/latest/plugins/openid-connect.md
@@ -58,6 +58,8 @@ description: OpenID Connect allows the client to obtain user information from th
 | set_id_token_header                  | boolean | False    | true                  |              | When set to true and the ID token is available, sets the ID token in the `X-ID-Token` request header.                    |
 | set_userinfo_header                  | boolean | False    | true                  |              | When set to true and the UserInfo object is available, sets it in the `X-Userinfo` request header.                       |
 | set_refresh_token_header             | boolean | False    | false                 |              | When set to true and a refresh token object is available, sets it in the `X-Refresh-Token` request header.               |
+| session                              | object  | False    |                       |              | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
+| session.secret                       | string  | True     | Automatic generation  | 16 or more characters | The key used for session encrypt and HMAC operation. |
 
 ## Scenarios
 
@@ -71,7 +73,7 @@ This plugin offers two scenorios:
 
 1. Authentication between Services: Set `bearer_only` to `true` and configure the `introspection_endpoint` or `public_key` attribute. In this scenario, APISIX will reject requests without a token or invalid token in the request header.
 
-2. Authentication between Browser and Identity Providers: Set `bearer_only` to `false.` After successful authentication, this plugin can obtain and manage the token in the cookie, and subsequent requests will use the token.
+2. Authentication between Browser and Identity Providers: Set `bearer_only` to `false.` After successful authentication, this plugin can obtain and manage the token in the cookie, and subsequent requests will use the token. In this mode, the user session will be stored in the browser as a cookie and this data is encrypted, so you have to set a key for encryption via `session.secret`.
 
 ### Token introspection
 
diff --git a/docs/zh/latest/plugins/openid-connect.md b/docs/zh/latest/plugins/openid-connect.md
index d8b69fd63..5d5a11c30 100644
--- a/docs/zh/latest/plugins/openid-connect.md
+++ b/docs/zh/latest/plugins/openid-connect.md
@@ -58,6 +58,8 @@ description: OpenID Connect(OIDC)是基于 OAuth 2.0 的身份认证协议
 | set_id_token_header                  | boolean | 否     | true                  | [true, false] | 是否将 ID 令牌设置到请求头参数 `X-ID-Token`。                                                       |
 | set_userinfo_header                  | boolean | 否     | true                  | [true, false] | 是否将用户信息对象设置到请求头参数 `X-Userinfo`。                                                    |
 | set_refresh_token_header             | boolean | 否     | false                 |               | 当设置为 `true` 并且刷新令牌可用时,则会将该属性设置在`X-Refresh-Token`请求头中。                      |
+| session                              | object  | 否     |                       |               | 当设置 bearer_only 为 false 时,openid-connect 插件将使用 Authorization Code 在 IDP 上进行认证,因此你必须设置 session 相关设置。 |
+| session.secret                       | string  | 是     | 自动生成               | 16 个以上字符  | 用于 session 加密和 HMAC 计算的密钥。 |
 
 ## 使用场景
 
@@ -71,7 +73,7 @@ description: OpenID Connect(OIDC)是基于 OAuth 2.0 的身份认证协议
 
 1. 应用之间认证授权:将 `bearer_only` 设置为 `true`,并配置 `introspection_endpoint` 或 `public_key` 属性。该场景下,请求头(Header)中没有令牌或无效令牌的请求将被拒绝。
 
-2. 浏览器中认证授权:将 `bearer_only` 设置为 `false`。认证成功后,该插件可获得并管理 Cookie 中的令牌,后续请求将使用该令牌。
+2. 浏览器中认证授权:将 `bearer_only` 设置为 `false`。认证成功后,该插件可获得并管理 Cookie 中的令牌,后续请求将使用该令牌。在这种模式中,用户会话将作为 Cookie 存储在浏览器中,这些数据是加密的,因此你必须通过 `session.secret` 设置一个密钥用于加密。
 
 ### 令牌内省
 
diff --git a/t/plugin/openid-connect.t b/t/plugin/openid-connect.t
index 9337e4235..27fda1983 100644
--- a/t/plugin/openid-connect.t
+++ b/t/plugin/openid-connect.t
@@ -21,7 +21,20 @@ repeat_each(1);
 no_long_string();
 no_root_location();
 no_shuffle();
-run_tests;
+
+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__
 
@@ -38,12 +51,8 @@ __DATA__
             ngx.say("done")
         }
     }
---- request
-GET /t
 --- response_body
 done
---- no_error_log
-[error]
 
 
 
@@ -60,13 +69,9 @@ done
             ngx.say("done")
         }
     }
---- request
-GET /t
 --- response_body
 property "client_id" is required
 done
---- no_error_log
-[error]
 
 
 
@@ -83,13 +88,9 @@ done
             ngx.say("done")
         }
     }
---- request
-GET /t
 --- response_body
 property "client_id" validation failed: wrong type: expected string, got number
 done
---- no_error_log
-[error]
 
 
 
@@ -129,12 +130,8 @@ done
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -157,14 +154,10 @@ passed
             end
         }
     }
---- request
-GET /t
 --- timeout: 10s
 --- response_body
 true
 --- error_code: 302
---- no_error_log
-[error]
 
 
 
@@ -210,12 +203,8 @@ true
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -264,8 +253,6 @@ passed
             ngx.say(res.body)
         }
     }
---- request
-GET /t
 --- response_body_like
 uri: /uri
 cookie: .*
@@ -276,8 +263,6 @@ x-id-token: ey.*
 x-real-ip: 127.0.0.1
 x-refresh-token: ey.*
 x-userinfo: ey.*
---- no_error_log
-[error]
 
 
 
@@ -322,12 +307,8 @@ x-userinfo: ey.*
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -376,8 +357,6 @@ passed
             ngx.say(res.body)
         }
     }
---- request
-GET /t
 --- response_body_like
 uri: /uri
 authorization: Bearer ey.*
@@ -385,8 +364,6 @@ cookie: .*
 host: 127.0.0.1:1984
 user-agent: .*
 x-real-ip: 127.0.0.1
---- no_error_log
-[error]
 
 
 
@@ -426,12 +403,8 @@ x-real-ip: 127.0.0.1
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -499,12 +472,8 @@ OIDC introspection failed: Invalid Authorization header format.
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -531,12 +500,8 @@ passed
             end
         }
     }
---- request
-GET /t
 --- response_body
 true
---- no_error_log
-[error]
 
 
 
@@ -580,12 +545,8 @@ true
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -600,8 +561,6 @@ authorization: Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJkYXRhMSI6IkRhdGEgM
 host: localhost
 x-access-token: eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJkYXRhMSI6IkRhdGEgMSIsImlhdCI6MTU4NTEyMjUwMiwiZXhwIjoxOTAwNjk4NTAyLCJhdWQiOiJodHRwOi8vbXlzb2Z0Y29ycC5pbiIsImlzcyI6Ik15c29mdCBjb3JwIiwic3ViIjoic29tZUB1c2VyLmNvbSJ9.u1ISx7JbuK_GFRIUqIMP175FqXRyF9V7y86480Q4N3jNxs3ePbc51TFtIHDrKttstU4Tub28PYVSlr-HXfjo7w
 x-real-ip: 127.0.0.1
---- no_error_log
-[error]
 --- error_code: 200
 
 
@@ -650,12 +609,8 @@ x-real-ip: 127.0.0.1
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -669,8 +624,6 @@ uri: /uri
 authorization: Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJkYXRhMSI6IkRhdGEgMSIsImlhdCI6MTU4NTEyMjUwMiwiZXhwIjoxOTAwNjk4NTAyLCJhdWQiOiJodHRwOi8vbXlzb2Z0Y29ycC5pbiIsImlzcyI6Ik15c29mdCBjb3JwIiwic3ViIjoic29tZUB1c2VyLmNvbSJ9.u1ISx7JbuK_GFRIUqIMP175FqXRyF9V7y86480Q4N3jNxs3ePbc51TFtIHDrKttstU4Tub28PYVSlr-HXfjo7w
 host: localhost
 x-real-ip: 127.0.0.1
---- no_error_log
-[error]
 --- error_code: 200
 
 
@@ -715,12 +668,8 @@ x-real-ip: 127.0.0.1
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -747,8 +696,6 @@ passed
             end
         }
     }
---- request
-GET /t
 --- error_code: 401
 --- error_log
 jwt signature verification failed
@@ -793,12 +740,8 @@ jwt signature verification failed
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -853,16 +796,12 @@ passed
             end
         }
     }
---- request
-GET /t
 --- response_body
 true
 --- grep_error_log eval
 qr/token validate successfully by \w+/
 --- grep_error_log_out
 token validate successfully by introspection
---- no_error_log
-[error]
 
 
 
@@ -888,8 +827,6 @@ token validate successfully by introspection
             end
         }
     }
---- request
-GET /t
 --- response_body
 false
 --- error_log
@@ -913,15 +850,16 @@ OIDC introspection failed: invalid token
                 ngx.say(err)
             end
 
+            -- ensure session secret generated when bearer_only = false
+            -- then remove it from table, because it's a random value that I cannot verify it by response body
+            assert(s.session and s.session.secret, "no session secret generated")
+            s.session = nil
+
             ngx.say(json.encode(s))
         }
     }
---- request
-GET /t
 --- response_body
 {"access_token_in_authorization_header":false,"bearer_only":false,"client_id":"kbyuFDidLLm280LIwVFiazOqjO3ty8KH","client_secret":"60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa","discovery":"http://127.0.0.1:1980/.well-known/openid-configuration","introspection_endpoint_auth_method":"client_secret_basic","logout_path":"/logout","realm":"apisix","scope":"openid","set_access_token_header":true,"set_id_token_header":true,"set_refresh_token_header":false,"set_userinfo_heade [...]
---- no_error_log
-[error]
 
 
 
@@ -964,12 +902,8 @@ GET /t
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -1024,16 +958,12 @@ passed
             end
         }
     }
---- request
-GET /t
 --- response_body
 true
 --- grep_error_log eval
 qr/token validate successfully by \w+/
 --- grep_error_log_out
 token validate successfully by jwks
---- no_error_log
-[error]
 
 
 
@@ -1059,8 +989,6 @@ token validate successfully by jwks
             end
         }
     }
---- request
-GET /t
 --- response_body
 false
 --- error_log
@@ -1110,12 +1038,8 @@ OIDC introspection failed: invalid jwt: invalid jwt string
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -1180,12 +1104,8 @@ passed
             ngx.say(res.headers["Location"])
         }
     }
---- request
-GET /t
 --- response_body_like
 http://127.0.0.1:.*/hello
---- no_error_log
-[error]
 
 
 
@@ -1225,12 +1145,8 @@ http://127.0.0.1:.*/hello
             ngx.say(body)
         }
     }
---- request
-GET /t
 --- response_body
 passed
---- no_error_log
-[error]
 
 
 
@@ -1255,11 +1171,7 @@ passed
             end
         }
     }
---- request
-GET /t
 --- timeout: 10s
 --- response_body
 true
 --- error_code: 302
---- no_error_log
-[error]
diff --git a/t/plugin/openid-connect2.t b/t/plugin/openid-connect2.t
new file mode 100644
index 000000000..810df9775
--- /dev/null
+++ b/t/plugin/openid-connect2.t
@@ -0,0 +1,80 @@
+#
+# 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();
+
+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: sanity
+--- config
+    location /t {
+        content_by_lua_block {
+            local test_cases = {
+                {
+                    name = "sanity (bearer_only = true)",
+                    data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = true},
+                    cb = function(ok, err, case)
+                        assert(ok and not case.session, "not expect session was generated")
+                    end,
+                },
+                {
+                    name = "sanity (bearer_only = false)",
+                    data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = false},
+                    cb = function(ok, err, case)
+                        assert(ok and case.session and case.session.secret, "no session secret generated")
+                    end,
+                },
+                {
+                    name = "sanity (bearer_only = false, user-set secret, less than 16 characters)",
+                    data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = false, session = {secret = "test"}},
+                    cb = function(ok, err, case)
+                        assert(not ok and err == "property \"session\" validation failed: property \"secret\" validation failed: string too short, expected at least 16, got 4", "too short key passes validation")
+                    end,
+                },
+                {
+                    name = "sanity (bearer_only = false, user-set secret, more than 16 characters)",
+                    data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = false, session = {secret = "test_secret_more_than_16"}},
+                    cb = function(ok, err, case)
+                        assert(ok and case.session and case.session.secret and case.session.secret == "test_secret_more_than_16", "user-set secret is incorrect")
+                    end,
+                },
+            }
+
+            local plugin = require("apisix.plugins.openid-connect")
+            for _, case in ipairs(test_cases) do
+                local ok, err = plugin.check_schema(case.data)
+                case.cb(ok, err, case.data)
+            end
+        }
+    }