Skip to content

Commit

Permalink
Fix logout with no session
Browse files Browse the repository at this point in the history
Bug in lua-resty-session does not permit calling session:destroy() on
freshly started session with unset "audience" feature, so check for empty
session before trying to destroy it.

Signed-off-by: Oldřich Jedlička <[email protected]>
  • Loading branch information
oldium committed Dec 28, 2023
1 parent e5ede9f commit 5d6670f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 6 deletions.
18 changes: 12 additions & 6 deletions lib/resty/openidc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ local function store_in_session(opts, feature)
return opts.session_contents[feature]
end

local function is_session(o)
return o ~= nil and o.save and type(o.save) == "function"
end

local function is_session_present(session)
return session ~= nil and next(session:get_data()) ~= nil
end

-- set value in server-wide cache if available
local function openidc_cache_set(type, key, value, exp)
local dict = ngx.shared[type]
Expand Down Expand Up @@ -1294,7 +1302,9 @@ local function openidc_logout(opts, session)
end
end

session:destroy()
if is_session_present(session) then
session:destroy()
end

if opts.revoke_tokens_on_logout then
log(DEBUG, "revoke_tokens_on_logout is enabled. " ..
Expand Down Expand Up @@ -1449,10 +1459,6 @@ local function openidc_get_redirect_uri_path(opts)
return opts.redirect_uri and openidc_get_path(opts.redirect_uri) or opts.redirect_uri_path
end

local function is_session(o)
return o ~= nil and o.save and type(o.save) == "function"
end

-- main routine for OpenID Connect user authentication
function openidc.authenticate(opts, target_url, unauth_action, session_or_opts)

Expand All @@ -1474,7 +1480,7 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts)
end
end

local session_present = next(session:get_data()) ~= nil
local session_present = is_session_present(session)

target_url = target_url or ngx.var.request_uri

Expand Down
19 changes: 19 additions & 0 deletions tests/spec/logout_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -552,3 +552,22 @@ describe("when revoke_tokens_on_logout is not defined and a revocation_endpoint
assert.is_not.error_log_contains("revoke")
end)
end)

describe("when the configured logout uri is invoked with no active session", function()
test_support.start_server()
teardown(test_support.stop_server)
local _, status, headers = http.request({
url = "http://127.0.0.1/default/logout",
redirect = false
})
local log = test_support.load("/tmp/server/logs/error.log")
print("Error log: \n" .. log)
it("the response contains a default HTML-page", function()
assert.are.equals(200, status)
assert.are.equals("text/html", headers["content-type"])
-- TODO should there be a Cache-Control header?
end)
it("the session cookie has been revoked", function()
assert.is.Nil(headers["set-cookie"])
end)
end)

0 comments on commit 5d6670f

Please sign in to comment.