From e566b882ff83cdc26662ad99891dce589eee424a Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 9 Jul 2026 22:13:07 +0800 Subject: [PATCH] test: add missing regression coverage for data-mask and saml-auth Both cases are already-supported behavior that no test exercised. data-mask: the plugin handles multi-value query parameters (a table-valued arg, apisix/plugins/data-mask.lua:124-137) and skips the regex action for a non-string JSON field instead of crashing (:190-199). Neither path had a test. Adds four blocks covering both, renumbering the existing access-log tests to TEST 19/20. saml-auth: the HTTP-POST binding was only covered by schema validation; the end-to-end login, single-logout, wrong-callback and login-failure flows ran only against the default HTTP-Redirect binding. Adds saml-auth-post.t, which uses the existing t/lib/keycloak_saml.lua helper and the apisix_keycloak_saml CI service. --- t/plugin/data-mask.t | 181 +++++++++++++++++++++- t/plugin/saml-auth-post.t | 314 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 493 insertions(+), 2 deletions(-) create mode 100644 t/plugin/saml-auth-post.t diff --git a/t/plugin/data-mask.t b/t/plugin/data-mask.t index d00aae131129..75fe7f8f25c8 100644 --- a/t/plugin/data-mask.t +++ b/t/plugin/data-mask.t @@ -667,7 +667,184 @@ success -=== TEST 15: create route for access log masking test +=== TEST 15: create route for multi-value query param masking +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "data-mask": { + "request": [ + { + "action": "regex", + "name": "token", + "regex": ".", + "type": "query", + "value": "*" + } + ] + }, + "file-logger": { + "path": "mask-multi-query.log" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1982": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } + + + +=== TEST 16: verify multi-value query param regex masking does not crash and masks all values +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin").test + + -- send request with the same param appearing twice + local code = t("/hello?token=abc&token=def", ngx.HTTP_GET) + + local fd, err = io.open("mask-multi-query.log", "r") + if not fd then + core.log.error("failed to open file: ", err) + return + end + local line = fd:read() + local log = core.json.decode(line) + local token = log.request.querystring.token + os.remove("mask-multi-query.log") + + -- token should be masked: regex "." replaces the first char with "*" + local ok = false + if type(token) == "string" then + ok = token:sub(1, 1) == "*" + elseif type(token) == "table" then + ok = true + for _, v in ipairs(token) do + if v:sub(1, 1) ~= "*" then + ok = false + break + end + end + end + if ok then + ngx.say("success") + else + ngx.say("token not fully masked: " .. core.json.encode(token)) + end + } + } +--- response_body +success + + + +=== TEST 17: create route for non-string JSON field regex masking +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "data-mask": { + "request": [ + { + "action": "regex", + "body_format": "json", + "name": "$.count", + "regex": "\\d+", + "type": "body", + "value": "[REDACTED]" + }, + { + "action": "replace", + "body_format": "json", + "name": "$.name", + "type": "body", + "value": "***" + } + ] + }, + "file-logger": { + "include_req_body": true, + "path": "mask-nonstring-json.log" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1982": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } + + + +=== TEST 18: verify non-string JSON field regex is skipped without crash +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin").test + + local code = t("/hello", ngx.HTTP_POST, [[{"count": 42, "name": "Alice"}]]) + + local fd, err = io.open("mask-nonstring-json.log", "r") + if not fd then + core.log.error("failed to open file: ", err) + return + end + local line = fd:read() + local log = core.json.decode(line) + local body = core.json.decode(log.request.body) + os.remove("mask-nonstring-json.log") + + -- $.count is a number; regex should be skipped, value unchanged + if body.count ~= 42 then + ngx.say("expected count=42, got: " .. tostring(body.count)) + return + end + -- $.name is a string; replace should work + if body.name ~= "***" then + ngx.say("expected name=***, got: " .. tostring(body.name)) + return + end + ngx.say("success") + } + } +--- response_body +success + + + +=== TEST 19: create route for access log masking test --- config location /t { content_by_lua_block { @@ -711,7 +888,7 @@ success -=== TEST 16: verify access log masks sensitive query parameters +=== TEST 20: verify access log masks sensitive query parameters --- extra_yaml_config nginx_config: http: diff --git a/t/plugin/saml-auth-post.t b/t/plugin/saml-auth-post.t new file mode 100644 index 000000000000..2e3fcfd883bc --- /dev/null +++ b/t/plugin/saml-auth-post.t @@ -0,0 +1,314 @@ +# +# 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) = @_; + + # setup default conf.yaml + 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: Add route for sp1 +--- config + location /t { + content_by_lua_block { + local kc = require("lib.keycloak_saml") + local core = require("apisix.core") + + local default_opts = kc.get_default_opts() + local opts = core.table.deepcopy(default_opts) + opts.sp_issuer = "sp" + opts.auth_protocol_binding_method = "HTTP-POST" + local t = require("lib.test_admin").test + + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "host" : "127.0.0.1", + "plugins": { + "saml-auth": ]] .. core.json.encode(opts) .. [[ + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/*" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 2: login and logout ok +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local httpc = http.new() + local kc = require "lib.keycloak_saml" + + local path = "/uri" + local uri = "http://127.0.0.1:" .. ngx.var.server_port + local username = "test" + local password = "test" + + local res, err, saml_cookie, keycloak_cookie = kc.login_keycloak(uri .. path, username, password) + if err or res.headers['Location'] ~= path then + ngx.log(ngx.ERR, err) + ngx.exit(500) + end + res, err = httpc:request_uri(uri .. res.headers['Location'], { + method = "GET", + headers = { + ["Cookie"] = saml_cookie + } + }) + assert(res.status == 200) + ngx.say(res.body) + + res, err = kc.logout_keycloak(uri .. "/logout", saml_cookie, keycloak_cookie) + if err or res.headers['Location'] ~= "/logout_ok" then + ngx.log(ngx.ERR, err) + ngx.exit(500) + end + } + } +--- response_body_like +uri: /uri +cookie: .* +host: 127.0.0.1:1984 +user-agent: .* +x-real-ip: 127.0.0.1 +--- error_log +login callback req with http post + + + +=== TEST 3: Add route for sp2 +--- config + location /t { + content_by_lua_block { + local kc = require("lib.keycloak_saml") + local core = require("apisix.core") + + local default_opts = kc.get_default_opts() + local opts = core.table.deepcopy(default_opts) + opts.sp_issuer = "sp2" + opts.auth_protocol_binding_method = "HTTP-POST" + + local t = require("lib.test_admin").test + + local code, body = t('/apisix/admin/routes/2', + ngx.HTTP_PUT, + [[{ + "host" : "127.0.0.2", + "plugins": { + "saml-auth": ]] .. core.json.encode(opts) .. [[ + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/*" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 4: login sp1 and sp2, then do single logout +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local httpc = http.new() + local kc = require "lib.keycloak_saml" + + local path = "/uri" + + -- login to sp1 + local uri = "http://127.0.0.1:" .. ngx.var.server_port + local username = "test" + local password = "test" + + local res, err, saml_cookie, keycloak_cookie = kc.login_keycloak(uri .. path, username, password) + if err or res.headers['Location'] ~= path then + ngx.log(ngx.ERR, err) + ngx.exit(500) + end + res, err = httpc:request_uri(uri .. res.headers['Location'], { + method = "GET", + headers = { + ["Cookie"] = saml_cookie + } + }) + assert(res.status == 200) + + -- login to sp2, which would skip login at keycloak side + local uri2 = "http://127.0.0.2:" .. ngx.var.server_port + + local res, err, saml_cookie2 = kc.login_keycloak_for_second_sp(uri2 .. path, keycloak_cookie) + if err or res.headers['Location'] ~= path then + ngx.log(ngx.ERR, err) + ngx.exit(500) + end + res, err = httpc:request_uri(uri2 .. res.headers['Location'], { + method = "GET", + headers = { + ["Cookie"] = saml_cookie2 + } + }) + assert(res.status == 200) + + -- SLO (single logout) + res, err = kc.single_logout(uri .. "/logout", saml_cookie, saml_cookie2, keycloak_cookie) + if err or res.headers['Location'] ~= "/logout_ok" then + ngx.log(ngx.ERR, err) + ngx.exit(500) + end + + -- login to sp2, which would do normal login process at keycloak side + local res, err, saml_cookie2, keycloak_cookie = kc.login_keycloak(uri2 .. path, username, password) + if err or res.headers['Location'] ~= path then + ngx.log(ngx.ERR, err) + ngx.exit(500) + end + res, err = httpc:request_uri(uri2 .. res.headers['Location'], { + method = "GET", + headers = { + ["Cookie"] = saml_cookie2 + } + }) + assert(res.status == 200) + + -- logout sp2 + res, err = kc.logout_keycloak(uri2 .. "/logout", saml_cookie2, keycloak_cookie) + if err or res.headers['Location'] ~= "/logout_ok" then + ngx.log(ngx.ERR, err) + ngx.exit(500) + end + } + } +--- error_log +login callback req with http post + + + +=== TEST 5: Add route for sp1 with wrong login_callback_uri +--- config + location /t { + content_by_lua_block { + local kc = require("lib.keycloak_saml") + local core = require("apisix.core") + + local default_opts = kc.get_default_opts() + local opts = core.table.deepcopy(default_opts) + opts.sp_issuer = "sp" + opts.login_callback_uri = "/wrong_url" + opts.auth_protocol_binding_method = "HTTP-POST" + local t = require("lib.test_admin").test + + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "host" : "127.0.0.1", + "plugins": { + "saml-auth": ]] .. core.json.encode(opts) .. [[ + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/*" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 6: login failed +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local httpc = http.new() + local kc = require "lib.keycloak_saml" + + local path = "/uri" + local uri = "http://127.0.0.1:" .. ngx.var.server_port + local username = "test" + local password = "test" + + local res = kc.login_keycloak(uri .. path, username, password) + assert(res == nil) + } + }