Copilot commented on code in PR #13346:
URL: https://github.com/apache/apisix/pull/13346#discussion_r3232581631


##########
apisix/plugins/saml-auth.lua:
##########
@@ -0,0 +1,136 @@
+--
+-- 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 require = require
+local pcall = pcall
+local core = require("apisix.core")
+local constants = require("apisix.constants")
+
+local is_resty_saml_init = false
+local resty_saml
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 512
+})
+
+local schema = {
+    type = "object",
+    properties = {
+        sp_issuer = { type = "string" },
+        idp_uri = { type = "string" },
+        idp_cert = { type = "string" },
+        login_callback_uri = { type = "string" },
+        logout_uri = { type = "string" },
+        logout_callback_uri = { type = "string" },
+        logout_redirect_uri = { type = "string" },
+        sp_cert = { type = "string" },
+        sp_private_key = { type = "string" },
+        auth_protocol_binding_method = {
+            type = "string",
+            default = "HTTP-Redirect",
+            enum = {"HTTP-Redirect", "HTTP-POST",},
+            description = "Binding method for authentication protocol, setting 
to HTTP-POST " ..
+                           "will set cookie samesite to None and cookie secure 
to true"
+        },
+        secret = {
+            type = "string",
+            description = "Secret used for key derivation.",
+            minLength = 8,
+            maxLength = 32,
+        },
+        secret_fallbacks = {
+            type = "array",
+            items = {
+                type = "string",
+                minLength = 8,
+                maxLength = 32,
+            },
+            description = "List of secrets for alternative secrets used when 
doing key rotation"
+        }
+    },
+    encrypt_fields = {"sp_private_key", "secret", "secret_fallbacks"},
+    required = {
+        "sp_issuer",
+        "idp_uri",
+        "idp_cert",
+        "login_callback_uri",
+        "logout_uri",
+        "logout_callback_uri",
+        "logout_redirect_uri",
+        "sp_cert",
+        "sp_private_key",
+    }
+}
+
+local plugin_name = "saml-auth"
+
+local _M = {
+    version = 0.1,
+    priority = 2598,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+local function load_resty_saml()
+    if resty_saml then
+        return resty_saml
+    end
+
+    local ok, saml = pcall(require, "resty.saml")
+    if not ok then
+        return nil, saml
+    end
+
+    resty_saml = saml
+    return resty_saml
+end
+
+function _M.check_schema(conf, schema_type)
+    return core.schema.check(schema, conf)
+end
+
+function _M.rewrite(conf, ctx)
+    local saml_lib, err = load_resty_saml()
+    if not saml_lib then
+        core.log.error("failed to load lua-resty-saml: ", err)
+        return 503, {message = "lua-resty-saml is required for saml-auth"}
+    end
+
+    if not is_resty_saml_init then
+        local err = saml_lib.init({
+            debug = true,
+            data_dir = constants.apisix_lua_home .. 
"/deps/share/lua/5.1/resty/saml"

Review Comment:
   `debug = true` in SAML libraries commonly increases log verbosity and can 
risk leaking sensitive data (assertions, identifiers, potentially key material 
depending on the lib). Please default this to `false` and (optionally) gate it 
behind an explicit plugin config flag or APISIX log level. Also, hardcoding 
`.../lua/5.1/...` in `data_dir` is fragile across packaging / Lua version 
layouts; prefer a library default, a dynamically discovered path, or a 
configurable `data_dir`.
   



##########
apisix/plugins/saml-auth.lua:
##########
@@ -0,0 +1,136 @@
+--
+-- 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 require = require
+local pcall = pcall
+local core = require("apisix.core")
+local constants = require("apisix.constants")
+
+local is_resty_saml_init = false
+local resty_saml
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 512
+})
+
+local schema = {
+    type = "object",
+    properties = {
+        sp_issuer = { type = "string" },
+        idp_uri = { type = "string" },
+        idp_cert = { type = "string" },
+        login_callback_uri = { type = "string" },

Review Comment:
   The plugin schema does not set `additionalProperties = false`, which means 
unknown/misspelled fields may be silently accepted. For plugin configs, 
rejecting unknown fields is generally preferable to catch misconfiguration 
early. Consider adding `additionalProperties = false` at the top level of the 
schema.



##########
apisix/plugins/saml-auth.lua:
##########
@@ -0,0 +1,136 @@
+--
+-- 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 require = require
+local pcall = pcall
+local core = require("apisix.core")
+local constants = require("apisix.constants")
+
+local is_resty_saml_init = false
+local resty_saml
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 512
+})
+
+local schema = {
+    type = "object",
+    properties = {
+        sp_issuer = { type = "string" },
+        idp_uri = { type = "string" },
+        idp_cert = { type = "string" },
+        login_callback_uri = { type = "string" },
+        logout_uri = { type = "string" },
+        logout_callback_uri = { type = "string" },
+        logout_redirect_uri = { type = "string" },
+        sp_cert = { type = "string" },
+        sp_private_key = { type = "string" },
+        auth_protocol_binding_method = {
+            type = "string",
+            default = "HTTP-Redirect",
+            enum = {"HTTP-Redirect", "HTTP-POST",},
+            description = "Binding method for authentication protocol, setting 
to HTTP-POST " ..
+                           "will set cookie samesite to None and cookie secure 
to true"
+        },
+        secret = {
+            type = "string",
+            description = "Secret used for key derivation.",
+            minLength = 8,
+            maxLength = 32,
+        },
+        secret_fallbacks = {
+            type = "array",
+            items = {
+                type = "string",
+                minLength = 8,
+                maxLength = 32,
+            },
+            description = "List of secrets for alternative secrets used when 
doing key rotation"
+        }
+    },
+    encrypt_fields = {"sp_private_key", "secret", "secret_fallbacks"},
+    required = {
+        "sp_issuer",
+        "idp_uri",
+        "idp_cert",
+        "login_callback_uri",
+        "logout_uri",
+        "logout_callback_uri",
+        "logout_redirect_uri",
+        "sp_cert",
+        "sp_private_key",
+    }
+}
+
+local plugin_name = "saml-auth"
+
+local _M = {
+    version = 0.1,
+    priority = 2598,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+local function load_resty_saml()
+    if resty_saml then
+        return resty_saml
+    end
+
+    local ok, saml = pcall(require, "resty.saml")
+    if not ok then
+        return nil, saml
+    end
+
+    resty_saml = saml
+    return resty_saml
+end
+
+function _M.check_schema(conf, schema_type)
+    return core.schema.check(schema, conf)
+end
+
+function _M.rewrite(conf, ctx)
+    local saml_lib, err = load_resty_saml()
+    if not saml_lib then
+        core.log.error("failed to load lua-resty-saml: ", err)
+        return 503, {message = "lua-resty-saml is required for saml-auth"}
+    end
+
+    if not is_resty_saml_init then
+        local err = saml_lib.init({
+            debug = true,
+            data_dir = constants.apisix_lua_home .. 
"/deps/share/lua/5.1/resty/saml"
+        })
+        if err then
+            core.log.error("saml init: ", err)
+            return 503, {message = "saml init failed"}
+        end
+        is_resty_saml_init = true
+    end
+
+    local saml = core.lrucache.plugin_ctx(lrucache, ctx, nil, saml_lib.new, 
conf)
+    if not saml then
+        core.log.error("saml new failed")
+        return 500, {message = "create saml object failed"}
+    end
+
+    local data = saml:authenticate()

Review Comment:
   The result of `saml:authenticate()` is not checked. If it returns 
`nil`/`false` (or returns `nil, err` in a common Lua pattern) without exiting 
internally, the request may proceed without an authenticated user while 
`ctx.external_user` remains unset. Handle explicit failure by checking the 
return value(s) and returning an appropriate status/response (or calling the 
standard APISIX exit helper) when authentication is not established.
   



##########
apisix/plugins/saml-auth.lua:
##########
@@ -0,0 +1,136 @@
+--
+-- 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 require = require
+local pcall = pcall
+local core = require("apisix.core")
+local constants = require("apisix.constants")
+
+local is_resty_saml_init = false
+local resty_saml
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 512
+})
+
+local schema = {
+    type = "object",
+    properties = {
+        sp_issuer = { type = "string" },
+        idp_uri = { type = "string" },
+        idp_cert = { type = "string" },
+        login_callback_uri = { type = "string" },
+        logout_uri = { type = "string" },
+        logout_callback_uri = { type = "string" },
+        logout_redirect_uri = { type = "string" },
+        sp_cert = { type = "string" },
+        sp_private_key = { type = "string" },
+        auth_protocol_binding_method = {
+            type = "string",
+            default = "HTTP-Redirect",
+            enum = {"HTTP-Redirect", "HTTP-POST",},
+            description = "Binding method for authentication protocol, setting 
to HTTP-POST " ..
+                           "will set cookie samesite to None and cookie secure 
to true"
+        },
+        secret = {
+            type = "string",
+            description = "Secret used for key derivation.",
+            minLength = 8,
+            maxLength = 32,
+        },
+        secret_fallbacks = {
+            type = "array",
+            items = {
+                type = "string",
+                minLength = 8,
+                maxLength = 32,
+            },
+            description = "List of secrets for alternative secrets used when 
doing key rotation"
+        }
+    },
+    encrypt_fields = {"sp_private_key", "secret", "secret_fallbacks"},
+    required = {
+        "sp_issuer",
+        "idp_uri",
+        "idp_cert",
+        "login_callback_uri",
+        "logout_uri",
+        "logout_callback_uri",
+        "logout_redirect_uri",
+        "sp_cert",
+        "sp_private_key",
+    }
+}
+
+local plugin_name = "saml-auth"
+
+local _M = {
+    version = 0.1,
+    priority = 2598,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+local function load_resty_saml()
+    if resty_saml then
+        return resty_saml
+    end
+
+    local ok, saml = pcall(require, "resty.saml")
+    if not ok then
+        return nil, saml
+    end
+
+    resty_saml = saml
+    return resty_saml
+end
+
+function _M.check_schema(conf, schema_type)

Review Comment:
   `schema_type` is unused. If APISIX calls this with a second argument as part 
of a standard interface, consider renaming it to `_` (or removing it if not 
needed) to make the intent explicit and avoid lint warnings.
   



##########
t/plugin/saml-auth.t:
##########
@@ -0,0 +1,302 @@
+#
+# 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('info');
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $extra_yaml_config = $block->extra_yaml_config // '';
+    $extra_yaml_config .= <<_EOC_;
+plugins:
+  - saml-auth                      # priority: 2598
+_EOC_
+
+    $block->set_value("extra_yaml_config", $extra_yaml_config);
+
+    if (!defined $block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+        $block->set_value("no_error_log", "[error]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: schema validation - valid config with all required fields
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: schema validation - missing required field sp_issuer
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*sp_issuer is required/
+
+
+
+=== TEST 3: schema validation - missing required field idp_uri
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*idp_uri is required/
+
+
+
+=== TEST 4: schema validation - invalid auth_protocol_binding_method
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+                auth_protocol_binding_method = "HTTP-INVALID",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*auth_protocol_binding_method/
+
+
+
+=== TEST 5: schema validation - secret too short (< 8 chars)
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+                secret = "short",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*secret/
+
+
+
+=== TEST 6: schema validation - valid config with optional fields
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+                auth_protocol_binding_method = "HTTP-POST",
+                secret = "mysecret1",
+                secret_fallbacks = {"oldsecret1", "oldsecret2"},
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 7: schema validation works when lua-resty-saml is unavailable

Review Comment:
   The test suite validates schema behavior and the missing-dependency failure 
path, but it doesn't cover the normal `rewrite` flow (e.g., that `rewrite` 
correctly sets `ctx.external_user` / triggers redirect behaviors when the SAML 
library is present). Add at least one test that stubs `resty.saml.new` + 
`:authenticate()` to return a known user payload and assert the plugin sets the 
expected context/outputs, so regressions in the main code path are caught.



##########
docs/en/latest/plugins/saml-auth.md:
##########
@@ -0,0 +1,153 @@
+---
+title: saml-auth
+keywords:
+  - Apache APISIX
+  - API Gateway
+  - SAML
+  - SAML 2.0
+  - SSO
+  - Single Sign-On
+description: The saml-auth Plugin enables SAML 2.0 authentication for API 
routes, integrating with external Identity Providers (IdP) such as Keycloak, 
Okta, and Azure Active Directory.
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+<head>
+  <link rel="canonical" href="https://docs.api7.ai/hub/saml-auth"; />
+</head>
+
+## Description
+
+The `saml-auth` Plugin enables [SAML 
2.0](https://docs.oasis-open.org/security/saml/Post2.0/sstc-saml-tech-overview-2.0.html)
 (Security Assertion Markup Language) authentication for API routes. It acts as 
a SAML Service Provider (SP) and integrates with external Identity Providers 
(IdP) such as Keycloak, Okta, and Azure Active Directory to authenticate users 
before allowing access to upstream resources.
+
+When a request arrives at a protected route, the Plugin checks for a valid 
SAML session. If no session exists, it redirects the user to the IdP for 
authentication. After the user authenticates at the IdP, the IdP posts a signed 
SAML assertion back to the SP's Assertion Consumer Service (ACS) URL. The 
Plugin validates the assertion and establishes a session for the user.
+
+The Plugin supports:
+
+- **HTTP-Redirect binding** (default) — SAML messages are transmitted as URL 
query parameters.
+- **HTTP-POST binding** — SAML messages are transmitted as HTML form values.
+- **Single Logout (SLO)** — logout requests can be initiated by the SP or the 
IdP.
+- **Session key rotation** via `secret_fallbacks`.
+
+Authenticated user data is stored in `ctx.external_user` and can be used by 
downstream authorization plugins such as `acl`.
+
+## Attributes
+
+| Name | Type | Required | Encrypted | Default | Valid Values | Description |
+|------|------|----------|-----------|---------|--------------|-------------|
+| sp_issuer | string | True | | | | Service Provider (SP) entity ID/issuer 
URI. Must match the SP entity ID registered with the IdP. |

Review Comment:
   The attributes table uses `||` at the start of each row, which renders as an 
extra empty column in standard Markdown table syntax. Replace the leading `||` 
with a single `|` on the header/separator/data rows so the table renders 
correctly.



##########
docs/zh/latest/plugins/saml-auth.md:
##########
@@ -0,0 +1,153 @@
+---
+title: saml-auth
+keywords:
+  - Apache APISIX
+  - API 网关
+  - SAML
+  - SAML 2.0
+  - SSO
+  - 单点登录
+description: saml-auth 插件为 API 路由提供 SAML 2.0 身份验证,可与 Keycloak、Okta、Azure 
Active Directory 等外部身份提供商集成。
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+<head>
+  <link rel="canonical" href="https://docs.api7.ai/hub/saml-auth"; />
+</head>
+
+## 描述
+
+`saml-auth` 插件为 API 路由提供 [SAML 
2.0](https://docs.oasis-open.org/security/saml/Post2.0/sstc-saml-tech-overview-2.0.html)(安全断言标记语言)身份验证。该插件充当
 SAML 服务提供商(SP),并与 Keycloak、Okta、Azure Active Directory 
等外部身份提供商(IdP)集成,在允许访问上游资源之前对用户进行身份验证。
+
+当请求到达受保护的路由时,插件会检查是否存在有效的 SAML 会话。若没有会话,则将用户重定向到 IdP 进行身份验证。用户在 IdP 完成认证后,IdP 
会将签名的 SAML 断言以 POST 方式发送到 SP 的断言消费者服务(ACS)URL。插件验证断言后,为用户建立会话。
+
+该插件支持:
+
+- **HTTP-Redirect 绑定**(默认)— SAML 消息以 URL 查询参数形式传输。
+- **HTTP-POST 绑定** — SAML 消息以 HTML 表单值形式传输。
+- **单点注销(SLO)** — 注销请求可由 SP 或 IdP 发起。
+- 通过 `secret_fallbacks` 实现**会话密钥轮换**。
+
+经过身份验证的用户数据存储在 `ctx.external_user` 中,可供 `acl` 等下游授权插件使用。
+
+## 属性
+
+| 名称 | 类型 | 必填 | 加密 | 默认值 | 有效值 | 描述 |
+|------|------|------|------|--------|--------|------|
+| sp_issuer | string | 是 | | | | 服务提供商(SP)实体 ID/颁发者 URI,必须与在 IdP 中注册的 SP 实体 ID 
一致。 |

Review Comment:
   属性表格行首使用了 `||`,在标准 Markdown 表格语法下会多出一个空列导致渲染错位。建议将表头/分隔行/数据行的行首 `||` 改为单个 
`|`,确保表格正常显示。



##########
t/plugin/saml-auth.t:
##########
@@ -0,0 +1,302 @@
+#
+# 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('info');
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $extra_yaml_config = $block->extra_yaml_config // '';
+    $extra_yaml_config .= <<_EOC_;
+plugins:
+  - saml-auth                      # priority: 2598
+_EOC_
+
+    $block->set_value("extra_yaml_config", $extra_yaml_config);
+
+    if (!defined $block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+        $block->set_value("no_error_log", "[error]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: schema validation - valid config with all required fields
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: schema validation - missing required field sp_issuer
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*sp_issuer is required/
+
+
+
+=== TEST 3: schema validation - missing required field idp_uri
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*idp_uri is required/
+
+
+
+=== TEST 4: schema validation - invalid auth_protocol_binding_method
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+                auth_protocol_binding_method = "HTTP-INVALID",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*auth_protocol_binding_method/
+
+
+
+=== TEST 5: schema validation - secret too short (< 8 chars)
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+                secret = "short",
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body_like eval
+qr/failed: .*secret/
+
+
+
+=== TEST 6: schema validation - valid config with optional fields
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+                auth_protocol_binding_method = "HTTP-POST",
+                secret = "mysecret1",
+                secret_fallbacks = {"oldsecret1", "oldsecret2"},
+            })
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 7: schema validation works when lua-resty-saml is unavailable
+--- config
+    location /t {
+        content_by_lua_block {
+            local old_plugin = package.loaded["apisix.plugins.saml-auth"]
+            local old_saml = package.loaded["resty.saml"]
+            local old_preload = package.preload["resty.saml"]
+
+            package.loaded["apisix.plugins.saml-auth"] = nil
+            package.loaded["resty.saml"] = nil
+            package.preload["resty.saml"] = function()
+                error("mock missing resty.saml")
+            end
+
+            local plugin = require("apisix.plugins.saml-auth")
+            local ok, err = plugin.check_schema({
+                sp_issuer = "https://sp.example.com";,
+                idp_uri = "https://idp.example.com/sso";,
+                idp_cert = "MIIC...",
+                login_callback_uri = "https://sp.example.com/login/callback";,
+                logout_uri = "https://sp.example.com/logout";,
+                logout_callback_uri = "https://sp.example.com/logout/callback";,
+                logout_redirect_uri = "https://sp.example.com/logout/done";,
+                sp_cert = "MIIC...",
+                sp_private_key = "MIIE...",
+            })
+
+            package.loaded["apisix.plugins.saml-auth"] = old_plugin
+            package.loaded["resty.saml"] = old_saml
+            package.preload["resty.saml"] = old_preload
+
+            if not ok then
+                ngx.say("failed: ", err)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 8: rewrite fails gracefully when lua-resty-saml is unavailable

Review Comment:
   The test suite validates schema behavior and the missing-dependency failure 
path, but it doesn't cover the normal `rewrite` flow (e.g., that `rewrite` 
correctly sets `ctx.external_user` / triggers redirect behaviors when the SAML 
library is present). Add at least one test that stubs `resty.saml.new` + 
`:authenticate()` to return a known user payload and assert the plugin sets the 
expected context/outputs, so regressions in the main code path are caught.



-- 
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]


Reply via email to