diff --git a/ChangeLog b/ChangeLog index 054b65e..dac83a6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -30,6 +30,13 @@ unreleased with client_jwt_assertion_alg. - rebuild client assertion JWTs when retrying token endpoint calls after a DPoP nonce challenge. +- don't duplicate introspection client credentials in both the + Authorization header and POST body when using client_secret_basic; + the legacy implicit POST body credentials behavior when + introspection_endpoint_auth_method is unset is now deprecated, so + configure introspection_endpoint_auth_method = "client_secret_post" + explicitly to keep sending credentials in the POST body in a future + release; see #556 - keep pending authorization states separate within the same browser session so concurrent tabs do not overwrite state, nonce, PKCE verifier, or original redirect URI values; see #553. diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index d458f01..f62205f 100644 --- a/lib/resty/openidc.lua +++ b/lib/resty/openidc.lua @@ -60,6 +60,8 @@ local DEBUG = ngx.DEBUG local ERROR = ngx.ERR local WARN = ngx.WARN +local has_logged_legacy_introspection_body_auth_warning = false + local function token_auth_method_precondition(method, required_field) return function(opts) if not opts[required_field] then @@ -2332,11 +2334,20 @@ function openidc.introspect(opts) body[token_param_name] = access_token - if opts.client_id then - body.client_id = opts.client_id - end - if opts.client_secret then - body.client_secret = opts.client_secret + local use_legacy_introspection_body_auth = opts.introspection_endpoint_auth_method == nil + if use_legacy_introspection_body_auth then + if (opts.client_id or opts.client_secret) and not has_logged_legacy_introspection_body_auth_warning then + log(WARN, "introspection_endpoint_auth_method is not set; sending introspection client credentials " .. + "in the POST body is deprecated and will require explicit introspection_endpoint_auth_method = " .. + "\"client_secret_post\" in a future release") + has_logged_legacy_introspection_body_auth_warning = true + end + if opts.client_id then + body.client_id = opts.client_id + end + if opts.client_secret then + body.client_secret = opts.client_secret + end end -- merge any provided extra parameters diff --git a/tests/spec/introspection_spec.lua b/tests/spec/introspection_spec.lua index a22a3b8..695bde6 100644 --- a/tests/spec/introspection_spec.lua +++ b/tests/spec/introspection_spec.lua @@ -4,9 +4,31 @@ require 'busted.runner'() local function assert_introspection_endpoint_call_contains(s, case_insensitive) assert.error_log_contains("Received introspection request: .*" .. s .. ".*", - case_insensitive) + case_insensitive) end +local function assert_introspection_endpoint_call_doesnt_contain(s, case_insensitive) + assert.is_not.error_log_contains("Received introspection request: .*" .. s .. ".*", + case_insensitive) +end + +local function error_log_occurrences(s) + local count = 0 + local pos = 1 + local log = test_support.load("/tmp/server/logs/error.log") + while true do + pos = log:find(s, pos, true) + if not pos then + return count + end + count = count + 1 + pos = pos + #s + end +end + +local legacy_introspection_body_auth_warning = "introspection_endpoint_auth_method is not set; " .. + "sending introspection client credentials in the POST body is deprecated" + describe("when the introspection endpoint is invoked", function() test_support.start_server() teardown(test_support.stop_server) @@ -39,12 +61,87 @@ describe("when the introspection endpoint is invoked", function() it("no cookies are sent with the introspection request", function() assert.error_log_contains("no cookie in introspection call") end) + it("the legacy body credentials behavior is deprecated", function() + assert.error_log_contains(legacy_introspection_body_auth_warning) + end) it("the response is valid", function() assert.are.equals(200, status) end) end) end) +describe("when legacy introspection body credentials are used without caching", function() + test_support.start_server({ + introspection_opts = { + introspection_cache_ignore = true + } + }) + teardown(test_support.stop_server) + local jwt = test_support.trim(http.request("http://127.0.0.1/jwt")) + local _, first_status = http.request({ + url = "http://127.0.0.1/introspect", + headers = { authorization = "Bearer " .. jwt } + }) + local _, second_status = http.request({ + url = "http://127.0.0.1/introspect", + headers = { authorization = "Bearer " .. jwt } + }) + it("logs the deprecation warning only once", function() + assert.are.equals(1, error_log_occurrences(legacy_introspection_body_auth_warning)) + end) + it("the responses are valid", function() + assert.are.equals(200, first_status) + assert.are.equals(200, second_status) + end) +end) + +describe("when legacy introspection body auth is used without client credentials", function() + test_support.start_server({ + remove_introspection_config_keys = { "client_id", "client_secret" } + }) + teardown(test_support.stop_server) + local jwt = test_support.trim(http.request("http://127.0.0.1/jwt")) + local _, status = http.request({ + url = "http://127.0.0.1/introspect", + headers = { authorization = "Bearer " .. jwt } + }) + it("doesn't log the deprecation warning", function() + assert.is_not.error_log_contains(legacy_introspection_body_auth_warning) + end) + it("the response is valid", function() + assert.are.equals(200, status) + end) +end) + +describe("when the introspection endpoint is invoked using client_secret_basic", function() + test_support.start_server({ + introspection_opts = { + introspection_endpoint_auth_method = "client_secret_basic" + } + }) + teardown(test_support.stop_server) + local jwt = test_support.trim(http.request("http://127.0.0.1/jwt")) + local _, status = http.request({ + url = "http://127.0.0.1/introspect", + headers = { authorization = "Bearer " .. jwt } + }) + it("the request doesn't contain the client_id parameter", function() + assert_introspection_endpoint_call_doesnt_contain("client_id=client_id") + end) + it("the request doesn't contain the client_secret parameter", function() + assert_introspection_endpoint_call_doesnt_contain("client_secret=client_secret") + end) + it("the request contains the token parameter", function() + assert_introspection_endpoint_call_contains("token=" .. jwt:gsub("%-", "%%%-")) + end) + it("the request contains a basic auth header", function() + assert.error_log_contains("introspection authorization header: Basic") + end) + it("the response is valid", function() + assert.are.equals(200, status) + end) +end) + describe("when a different token parameter name is configured", function() test_support.start_server({ introspection_opts = {