AlinsRan commented on PR #13346: URL: https://github.com/apache/apisix/pull/13346#issuecomment-4444822159
Fixed all CI failures and addressed all code review comments: **CI fixes (commit `fda4fed`):** - `t/admin/plugins.t`: Added `saml-auth` to the expected plugin list (it was registered but missing from the test snapshot) - `t/plugin/saml-auth.t` TEST 2 & 3: Updated regex patterns to handle quoted field names in jsonschema error messages (e.g. `"sp_issuer" is required`) - `t/plugin/saml-auth.t` preprocessor: Fixed the `no_error_log` guard to also check `error_log_like`, so TEST 8 no longer gets a conflicting `no_error_log "[error]"` injected **Code review fixes (commit `56d8268`):** - Schema: added `additionalProperties = false` to reject unknown/misspelled fields - `check_schema`: renamed unused `schema_type` parameter to `_` - `saml_lib.init`: changed `debug = true` to `debug = false` to prevent potential sensitive data exposure in logs; the `data_dir` path keeps `5.1` since APISIX requires LuaJIT which is Lua 5.1 compatible — the prefix is already dynamic via `constants.apisix_lua_home` - `saml:authenticate()`: added error return value check; returns HTTP 500 on explicit errors - Added TEST 9 that mocks `resty.saml` and verifies the normal rewrite flow sets `ctx.external_user` correctly Docs (`||→|`) were already correctly formatted in the current code — no changes needed there. -- 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]
