diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua
index 859c523..1e94c97 100644
--- a/lua/resty/saml.lua
+++ b/lua/resty/saml.lua
@@ -224,7 +224,11 @@ local function login(self, opts, request_uri)
sess:set("saml_state", state)
sess:set("saml_request_id", request_id)
sess:set("request_uri", request_uri)
- sess:save()
+ local saved, save_err = sess:save()
+ if not saved then
+ ngx.log(ngx.ERR, "could not save login session: ", save_err)
+ return ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR)
+ end
local query_str, err = create_redirect(self.sign_key, {
SAMLRequest = authn_request(opts, request_id),
@@ -651,7 +655,7 @@ local function spend_assertions(dict, dict_name, opts, assertions, expected, now
w.ttl, " seconds")
end
end
- return true
+ return true, spent
end
@@ -782,13 +786,15 @@ local function login_callback(self, opts)
-- the last gate: everything that can still refuse this login has run, so
-- the assertion is spent only where it actually authenticates somebody
+ local spent
if self.replay_dict then
- local unused, used_reason = spend_assertions(self.replay_dict, self.replay_dict_name,
+ local spent_ok, spent_or_err = spend_assertions(self.replay_dict, self.replay_dict_name,
opts, assertions, expected, now)
- if not unused then
- ngx.log(ngx.ERR, "response from IdP rejected: ", loggable(used_reason))
+ if not spent_ok then
+ ngx.log(ngx.ERR, "response from IdP rejected: ", loggable(spent_or_err))
ngx.exit(ngx.HTTP_UNAUTHORIZED)
end
+ spent = spent_or_err
end
sess:set("authenticated", true)
@@ -802,7 +808,16 @@ local function login_callback(self, opts)
sess:set("saml_state", nil)
sess:set("saml_request_id", nil)
sess:set("request_uri", nil)
- sess:save()
+ local saved, save_err = sess:save()
+ if not saved then
+ if spent then
+ for _, key in ipairs(spent) do
+ self.replay_dict:delete(key)
+ end
+ end
+ ngx.log(ngx.ERR, "could not save authenticated session: ", save_err)
+ return ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR)
+ end
ngx.log(ngx.INFO, "login finish: name_id=", loggable(name_id))
diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t
index ea7f2bf..ca8a892 100644
--- a/t/assertion-conditions.t
+++ b/t/assertion-conditions.t
@@ -34,6 +34,9 @@ _EOC_
my $http_config = $block->http_config // <<_EOC_;
lua_package_path '$pwd/lua/?.lua;$pwd/deps/share/lua/5.1/?.lua;$pwd/t/?.lua;;';
lua_package_cpath '$pwd/?.so;$pwd/deps/lib/lua/5.1/?.so;;';
+ large_client_header_buffers 4 64k;
+ client_max_body_size 1m;
+ client_body_buffer_size 128k;
# a zone of the same name and size is reused across a reload, so entries
# outlive the block that made them under TEST_NGINX_USE_HUP=1. Blocks name
@@ -114,6 +117,8 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw==
replay_dict = "saml_replay",
idp_issuers = { "https://elsewhere.example.com" },
},
+ save_fails_start = {},
+ save_fails_replay = { replay_dict = "saml_replay" },
}
SPS = {}
@@ -133,6 +138,9 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw==
}
for k, v in pairs(OPTS[name]) do opts[k] = v end
SPS[name] = require("resty.saml").new(opts)
+ if name == "save_fails_start" or name == "save_fails_replay" then
+ SPS[name].session_config.compression_threshold = 0
+ end
end
return SPS[name]
end
@@ -206,7 +214,7 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw==
'%s%s%s%s',
spec.id or "a1", spec.issuer or IDP, spec.name_id or "signed\@example.com",
spec.confirmations or "", spec.conditions or "",
- authn_statement(spec.session_expires))
+ authn_statement(spec.session_expires) .. (spec.attributes or ""))
end
function response(body, destination, in_response_to)
@@ -302,6 +310,16 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw==
return res.status .. " " .. tostring(res.headers["Location"])
end
+ function login_start_with_large_uri()
+ local bits = {}
+ for i = 1, 1200 do bits[i] = ngx.md5(i) end
+ local res = assert(require("resty.http").new():request_uri(
+ "http://127.0.0.1:1984/?q=" .. table.concat(bits), {
+ headers = { ["X-Test-SP"] = "save_fails_start" },
+ }))
+ return res.status
+ end
+
-- hand a response to a session the old code would have left behind
function login_with_legacy(name, xml)
local httpc = require("resty.http").new()
@@ -1411,8 +1429,44 @@ earlier: true
dated one decides: true
+=== TEST 48: a failed login-session save does not redirect to the IdP
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.say(login_start_with_large_uri())
+ }
+ }
+--- response_body
+500
+--- error_log
+could not save login session: cookie size limit exceeded
+
+
+=== TEST 49: a failed authenticated-session save returns replay entries
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.shared.saml_replay:flush_all()
+ local bits = {}
+ for i = 1, 1200 do bits[i] = ngx.md5(i) end
+ local xml = saml_response({
+ id = "save-failed",
+ attributes = '' ..
+ '' .. table.concat(bits) ..
+ '',
+ })
+ ngx.say(login_with("save_fails_replay", xml))
+ ngx.say("remembered: ", ngx.shared.saml_replay:get(replay_key("save-failed")) ~= nil)
+ }
+ }
+--- response_body
+500 nil
+remembered: false
+--- error_log
+could not save authenticated session: cookie size limit exceeded
+
-=== TEST 48: with a record, an OneTimeUse assertion is treated like any other
+=== TEST 50: with a record, an OneTimeUse assertion is treated like any other
--- config
location /t {
content_by_lua_block {
@@ -1442,7 +1496,7 @@ OneTimeUse
-=== TEST 49: a full dict says when the untracked login asked for single use
+=== TEST 51: a full dict says when the untracked login asked for single use
--- config
location /t {
content_by_lua_block {
@@ -1476,7 +1530,7 @@ stays acceptable past its record
-=== TEST 50: an OneTimeUse assertion that outlives its record says so
+=== TEST 52: an OneTimeUse assertion that outlives its record says so
--- config
location /t {
content_by_lua_block {
@@ -1508,7 +1562,7 @@ qr/\[warn\] .* assertion stamped-forever from https:\/\/idp\.example\.com carrie
-=== TEST 51: OneTimeUse is read wherever it sits among the conditions
+=== TEST 53: OneTimeUse is read wherever it sits among the conditions
--- config
location /t {
content_by_lua_block {
@@ -1533,7 +1587,7 @@ one_time_use=false unknown_condition=Condition
-=== TEST 52: without a record, an OneTimeUse assertion is accepted again
+=== TEST 54: without a record, an OneTimeUse assertion is accepted again
--- config
location /t {
content_by_lua_block {
@@ -1558,7 +1612,7 @@ qr/\[warn\] .* assertion stamped-untracked from https:\/\/idp\.example\.com carr
-=== TEST 54: the operator's replay_ttl is taken as given, past the day too
+=== TEST 55: the operator's replay_ttl is taken as given, past the day too
--- config
location /t {
content_by_lua_block {