From 0fa034a68eb21f7fdccdac861ac545580af430dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= Date: Fri, 28 Apr 2023 11:54:44 +0530 Subject: [PATCH 1/5] Migrate lua-resty-session to 4.0.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Oldřich Jedlička --- lib/resty/openidc.lua | 175 ++++++++++++++++------------- lua-resty-openidc-1.7.6-3.rockspec | 2 +- tests/Dockerfile | 2 +- tests/spec/logout_spec.lua | 34 +++--- 4 files changed, 118 insertions(+), 95 deletions(-) diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index e7b8872e..4a283b5e 100644 --- a/lib/resty/openidc.lua +++ b/lib/resty/openidc.lua @@ -46,12 +46,12 @@ local cjson_s = require("cjson.safe") local http = require("resty.http") local r_session = require("resty.session") local string = string +local table = table local ipairs = ipairs local pairs = pairs local type = type local ngx = ngx local b64 = ngx.encode_base64 -local unb64 = ngx.decode_base64 local b64url = require("ngx.base64").encode_base64url local unb64url = require("ngx.base64").decode_base64url @@ -351,11 +351,11 @@ local function openidc_authorize(opts, session, target_url, prompt) end -- store state in the session - session.data.original_url = target_url - session.data.state = state - session.data.nonce = nonce - session.data.code_verifier = code_verifier - session.data.last_authenticated = ngx.time() + session:set("original_url", target_url) + session:set("state", state) + session:set("nonce", nonce) + session:set("code_verifier", code_verifier) + session:set("last_authenticated", ngx.time()) if opts.lifecycle and opts.lifecycle.on_created then err = opts.lifecycle.on_created(session) @@ -365,7 +365,11 @@ local function openidc_authorize(opts, session, target_url, prompt) end end - session:save() + local res + res, err = session:save() + if err then + log(WARN, "unable to save session: " .. err) + end -- redirect to the /authorization endpoint ngx.header["Cache-Control"] = "no-cache, no-store, max-age=0" @@ -1072,7 +1076,7 @@ local function openidc_load_and_validate_jwt_id_token(opts, jwt_id_token, sessio log(DEBUG, "id_token payload: ", cjson.encode(jwt_obj.payload)) -- validate the id_token contents - if openidc_validate_id_token(opts, id_token, session.data.nonce) == false then + if openidc_validate_id_token(opts, id_token, session:get("nonce")) == false then err = "id_token validation failed" log(ERROR, err) return nil, err @@ -1095,20 +1099,21 @@ local function openidc_authorization_response(opts, session) if not args.code or not args.state then err = "unhandled request to the redirect_uri: " .. ngx.var.request_uri log(ERROR, err) - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end -- check that the state returned in the response against the session; prevents CSRF - if args.state ~= session.data.state then - log_err = "state from argument: " .. (args.state and args.state or "nil") .. " does not match state restored from session: " .. (session.data.state and session.data.state or "nil") + local state = session:get("state") + if args.state ~= state then + log_err = "state from argument: " .. (args.state and args.state or "nil") .. " does not match state restored from session: " .. (state and state or "nil") client_err = "state from argument does not match state restored from session" log(ERROR, log_err) - return nil, client_err, session.data.original_url, session + return nil, client_err, session:get("original_url"), session end err = ensure_config(opts) if err then - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end -- check the iss if returned from the OP @@ -1116,7 +1121,7 @@ local function openidc_authorization_response(opts, session) log_err = "iss from argument: " .. args.iss .. " does not match expected issuer: " .. opts.discovery.issuer client_err = "iss from argument does not match expected issuer" log(ERROR, log_err) - return nil, client_err, session.data.original_url, session + return nil, client_err, session:get("original_url"), session end -- check the client_id if returned from the OP @@ -1124,7 +1129,7 @@ local function openidc_authorization_response(opts, session) log_err = "client_id from argument: " .. args.client_id .. " does not match expected client_id: " .. opts.client_id client_err = "client_id from argument does not match expected client_id" log(ERROR, log_err) - return nil, client_err, session.data.original_url, session + return nil, client_err, session:get("original_url"), session end -- assemble the parameters to the token endpoint @@ -1132,8 +1137,8 @@ local function openidc_authorization_response(opts, session) grant_type = "authorization_code", code = args.code, redirect_uri = openidc_get_redirect_uri(opts, session), - state = session.data.state, - code_verifier = session.data.code_verifier + state = state, + code_verifier = session:get("code_verifier") } log(DEBUG, "Authentication with OP done -> Calling OP Token Endpoint to obtain tokens") @@ -1143,22 +1148,22 @@ local function openidc_authorization_response(opts, session) local json json, err = openidc.call_token_endpoint(opts, opts.discovery.token_endpoint, body, opts.token_endpoint_auth_method) if err then - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end local id_token, err = openidc_load_and_validate_jwt_id_token(opts, json.id_token, session); if err then - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end -- mark this sessions as authenticated - session.data.authenticated = true - -- clear state, nonce and code_verifier to protect against potential misuse - session.data.nonce = nil - session.data.state = nil - session.data.code_verifier = nil + session:set("authenticated", true) + session:set("nonce", nil) + session:set("state", nil) + session:set("code_verifier", nil) + if store_in_session(opts, 'id_token') then - session.data.id_token = id_token + session:set("id_token", id_token) end if store_in_session(opts, 'user') then @@ -1174,21 +1179,21 @@ local function openidc_authorization_response(opts, session) err = "\"sub\" claim in id_token (\"" .. (id_token.sub or "null") .. "\") is not equal to the \"sub\" claim returned from the userinfo endpoint (\"" .. (user.sub or "null") .. "\")" log(ERROR, err) else - session.data.user = user + session:set("user", user) end end end if store_in_session(opts, 'enc_id_token') then - session.data.enc_id_token = json.id_token + session:set("enc_id_token", json.id_token) end if store_in_session(opts, 'access_token') then - session.data.access_token = json.access_token - session.data.access_token_expiration = current_time - + openidc_access_token_expires_in(opts, json.expires_in) + session:set("access_token", json.access_token) + session:set("access_token_expiration", current_time + openidc_access_token_expires_in(opts, json.expires_in)) + if json.refresh_token ~= nil then - session.data.refresh_token = json.refresh_token + session:set("refresh_token", json.refresh_token) end end @@ -1196,17 +1201,22 @@ local function openidc_authorization_response(opts, session) err = opts.lifecycle.on_authenticated(session, id_token, json) if err then log(WARN, "failed in `on_authenticated` handler: " .. err) - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end end -- save the session with the obtained id_token - session:save() + local res + res, err = session:save() + if err then + log(WARN, "unable to save session: " .. err) + end -- redirect to the URL that was accessed originally - log(DEBUG, "OIDC Authorization Code Flow completed -> Redirecting to original URL (" .. session.data.original_url .. ")") - ngx.redirect(session.data.original_url) - return nil, nil, session.data.original_url, session + local original_url = session:get("original_url") + log(DEBUG, "OIDC Authorization Code Flow completed -> Redirecting to original URL (" .. original_url .. ")") + ngx.redirect(original_url) + return nil, nil, original_url, session end -- token revocation (RFC 7009) @@ -1261,8 +1271,8 @@ function openidc.revoke_tokens(opts, session) return false end - local access_token = session.data.access_token - local refresh_token = session.data.refresh_token + local access_token = session:get("access_token") + local refresh_token = session:get("refresh_token") local access_token_revoke, refresh_token_revoke if refresh_token then @@ -1282,9 +1292,9 @@ local openidc_transparent_pixel = "\137\080\078\071\013\010\026\010\000\000\000\ -- handle logout local function openidc_logout(opts, session) - local session_token = session.data.enc_id_token - local access_token = session.data.access_token - local refresh_token = session.data.refresh_token + local session_token = session:get("enc_id_token") + local access_token = session:get("access_token") + local refresh_token = session:get("refresh_token") local err if opts.lifecycle and opts.lifecycle.on_logout then @@ -1353,21 +1363,26 @@ local function openidc_access_token(opts, session, try_to_renew) local err - if session.data.access_token == nil then + local access_token = session:get("access_token") + if access_token == nil then return nil, err end + local current_time = ngx.time() - if current_time < session.data.access_token_expiration then - return session.data.access_token, err + if current_time < session:get("access_token_expiration") then + return access_token, err end + if not try_to_renew then return nil, "token expired" end - if session.data.refresh_token == nil then + + local refresh_token = session:get("refresh_token") + if refresh_token == nil then return nil, "token expired and no refresh token available" end - log(DEBUG, "refreshing expired access_token: ", session.data.access_token, " with: ", session.data.refresh_token) + log(DEBUG, "refreshing expired access_token: ", access_token, " with: ", refresh_token) -- retrieve token endpoint URL from discovery endpoint if necessary err = ensure_config(opts) @@ -1378,7 +1393,7 @@ local function openidc_access_token(opts, session, try_to_renew) -- assemble the parameters to the token endpoint local body = { grant_type = "refresh_token", - refresh_token = session.data.refresh_token, + refresh_token = refresh_token, scope = opts.scope and opts.scope or "openid email profile" } @@ -1397,30 +1412,23 @@ local function openidc_access_token(opts, session, try_to_renew) end log(DEBUG, "access_token refreshed: ", json.access_token, " updated refresh_token: ", json.refresh_token) - session.data.access_token = json.access_token - session.data.access_token_expiration = current_time + openidc_access_token_expires_in(opts, json.expires_in) + session:set("access_token", json.access_token) + session:set("access_token_expiration", current_time + openidc_access_token_expires_in(opts, json.expires_in)) if json.refresh_token then - session.data.refresh_token = json.refresh_token + session:set("refresh_token", json.refresh_token) end if json.id_token and (store_in_session(opts, 'enc_id_token') or store_in_session(opts, 'id_token')) then log(DEBUG, "id_token refreshed: ", json.id_token) if store_in_session(opts, 'enc_id_token') then - session.data.enc_id_token = json.id_token + session:set("enc_id_token", json.id_token) end if store_in_session(opts, 'id_token') then - session.data.id_token = id_token + session:set("id_token", id_token) end end - -- save the session with the new access_token and optionally the new refresh_token and id_token using a new sessionid - local regenerated - regenerated, err = session:regenerate() - if err then - log(ERROR, "failed to regenerate session: " .. err) - return nil, err - end if opts.lifecycle and opts.lifecycle.on_regenerated then err = opts.lifecycle.on_regenerated(session) if err then @@ -1429,7 +1437,15 @@ local function openidc_access_token(opts, session, try_to_renew) end end - return session.data.access_token, err + -- save the session with the new access_token and optionally the new refresh_token and id_token + local res + res, err = session:save() + if err then + log(ERROR, "failed to save session: " .. err) + return nil, err + end + + return session:get("access_token"), err end local function openidc_get_path(uri) @@ -1445,7 +1461,7 @@ local function openidc_get_redirect_uri_path(opts) end local function is_session(o) - return o ~= nil and o.start and type(o.start) == "function" + return o ~= nil and o.save and type(o.save) == "function" end -- main routine for OpenID Connect user authentication @@ -1469,6 +1485,8 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) end end + local session_present = next(session:get_data()) ~= nil + target_url = target_url or ngx.var.request_uri local access_token @@ -1478,7 +1496,7 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) if path == openidc_get_redirect_uri_path(opts) then log(DEBUG, "Redirect URI path (" .. path .. ") is currently navigated -> Processing authorization response coming from OP") - if not session.present then + if not session_present then err = "request to the redirect_uri path but there's no session state found" log(ERROR, err) return nil, err, target_url, session @@ -1493,7 +1511,7 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) err = ensure_config(opts) if err then - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end openidc_logout(opts, session) @@ -1502,7 +1520,9 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) local token_expired = false local try_to_renew = opts.renew_access_token_on_expiry == nil or opts.renew_access_token_on_expiry - if session.present and session.data.authenticated + local authenticated = session:get("authenticated") + + if session_present and authenticated and store_in_session(opts, 'access_token') then -- refresh access_token if necessary @@ -1516,10 +1536,12 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) end end + local id_token = session:get("id_token") + log(DEBUG, - "session.present=", session.present, - ", session.data.id_token=", session.data.id_token ~= nil, - ", session.data.authenticated=", session.data.authenticated, + "session_present=", session_present, + ", session.data.id_token=", id_token ~= nil, + ", session.data.authenticated=", authenticated, ", opts.force_reauthorize=", opts.force_reauthorize, ", opts.renew_access_token_on_expiry=", opts.renew_access_token_on_expiry, ", try_to_renew=", try_to_renew, @@ -1527,13 +1549,13 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) -- if we are not authenticated then redirect to the OP for authentication -- the presence of the id_token is check for backwards compatibility - if not session.present - or not (session.data.id_token or session.data.authenticated) + if not session_present + or not (id_token or authenticated) or opts.force_reauthorize or (try_to_renew and token_expired) then if unauth_action == "pass" then if token_expired then - session.data.authenticated = false + session:set("authenticated", false) return nil, 'token refresh failed', target_url, session end return nil, err, target_url, session @@ -1544,7 +1566,7 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) err = ensure_config(opts) if err then - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end log(DEBUG, "Authentication is required - Redirecting to OP Authorization endpoint") @@ -1554,10 +1576,11 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) -- silently reauthenticate if necessary (mainly used for session refresh/getting updated id_token data) if opts.refresh_session_interval ~= nil then - if session.data.last_authenticated == nil or (session.data.last_authenticated + opts.refresh_session_interval) < ngx.time() then + local last_authenticated = session:get("last_authenticated") + if last_authenticated == nil or (last_authenticated + opts.refresh_session_interval) < ngx.time() then err = ensure_config(opts) if err then - return nil, err, session.data.original_url, session + return nil, err, session:get("original_url"), session end log(DEBUG, "Silent authentication is required - Redirecting to OP Authorization endpoint") @@ -1568,15 +1591,15 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts) if store_in_session(opts, 'id_token') then -- log id_token contents - log(DEBUG, "id_token=", cjson.encode(session.data.id_token)) + log(DEBUG, "id_token=", cjson.encode(id_token)) end -- return the id_token to the caller Lua script for access control purposes return { - id_token = session.data.id_token, + id_token = id_token, access_token = access_token, - user = session.data.user + user = session:get("user") }, err, target_url, diff --git a/lua-resty-openidc-1.7.6-3.rockspec b/lua-resty-openidc-1.7.6-3.rockspec index 1114c621..ad4819d5 100644 --- a/lua-resty-openidc-1.7.6-3.rockspec +++ b/lua-resty-openidc-1.7.6-3.rockspec @@ -24,7 +24,7 @@ description = { dependencies = { "lua >= 5.1", "lua-resty-http >= 0.08", - "lua-resty-session >= 2.8, <= 3.10", + "lua-resty-session >= 4.0.3", "lua-resty-jwt >= 0.2.0" } build = { diff --git a/tests/Dockerfile b/tests/Dockerfile index cc3345d1..a287dc19 100644 --- a/tests/Dockerfile +++ b/tests/Dockerfile @@ -1,7 +1,7 @@ FROM openresty/openresty:focal # install dependencies -RUN ["luarocks", "install", "lua-resty-session", "3.10"] +RUN ["luarocks", "install", "lua-resty-session", "4.0.3"] RUN ["luarocks", "install", "lua-resty-http"] RUN ["luarocks", "install", "lua-resty-jwt"] diff --git a/tests/spec/logout_spec.lua b/tests/spec/logout_spec.lua index 7a0fd7d3..cc2cf2d0 100644 --- a/tests/spec/logout_spec.lua +++ b/tests/spec/logout_spec.lua @@ -18,7 +18,7 @@ describe("when the configured logout uri is invoked with a non-image request", f end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -45,7 +45,7 @@ describe("when the configured logout uri is invoked with a png request", functio end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -76,7 +76,7 @@ describe("when logout is invoked and a callback with hint has been configured", end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -107,7 +107,7 @@ describe("when logout is invoked and a callback with hint has been configured - end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -141,7 +141,7 @@ describe("when logout is invoked and a callback with hint has been configured bu end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -171,7 +171,7 @@ describe("when logout is invoked and a callback without hint has been configured end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -200,7 +200,7 @@ describe("when logout is invoked and discovery contains end_session_endpoint and end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -232,7 +232,7 @@ describe("when logout is invoked and discovery contains end_session_endpoint and end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -257,7 +257,7 @@ describe("when logout is invoked and discovery contains ping_end_session_endpoin end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -294,7 +294,7 @@ describe("when logout is invoked and a callback with hint and a post_logout_uri end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -330,7 +330,7 @@ describe("when logout is invoked and discovery contains end_session_endpoint and end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -361,7 +361,7 @@ describe("when logout is invoked and discovery contains ping_end_session_endpoin end) it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) end) @@ -390,7 +390,7 @@ describe("when revoke_tokens_on_logout is enabled and a valid revocation endpoin it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) it("authorization credentials have not been passed on as post parameters to the revocation endpoint", function() @@ -436,7 +436,7 @@ describe("when revoke_tokens_on_logout is enabled and a valid revocation endpoin it("the session cookie has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) it("authorization header has not been passed on to the revocation endpoint", function() @@ -480,7 +480,7 @@ describe("when revoke_tokens_on_logout is enabled and an invalid revocation endp it("the session cookie still has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) it("error messages concerning unseccussful revocation have been logged", function() @@ -512,7 +512,7 @@ describe("when revoke_tokens_on_logout is enabled but no revocation endpoint is it("the session cookie still has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) it("debug messages concerning unseccussful revocation have been logged", function() @@ -544,7 +544,7 @@ describe("when revoke_tokens_on_logout is not defined and a revocation_endpoint it("the session cookie still has been revoked", function() assert.truthy(string.match(headers["set-cookie"], - "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*")) + "session=; Path=/; SameSite=Lax; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:01 GMT; .*")) end) it("no messages concerning revocation have been logged", function() From baf2643b70afbbad66cdba4adb74362e6171237f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= Date: Thu, 28 Dec 2023 23:37:54 +0100 Subject: [PATCH 2/5] Allow changing of query parameters in on_created call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Oldřich Jedlička --- README.md | 4 +++- lib/resty/openidc.lua | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0179b40e..cc066bb3 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,9 @@ h2JHukolz9xf6qN61QMLSd83+kwoBr2drp6xg3eGDLIkQCQLrkY= -- } -- -- where `handle_created`, `handle_authenticated`, `handle_regenerated` and `handle_logout` are callables - -- accepting a single argument `session` + -- accepting argument `session`. `handle_created` accepts also second argument `params` which is a table + -- containing the query parameters of the authorization request used to redirect the user to the OpenID + -- Connect provider endpoint. -- -- -- `on_created` hook is invoked *after* a session has been created in -- `openidc_authorize` immediately prior to saving the session diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index 4a283b5e..28031e33 100644 --- a/lib/resty/openidc.lua +++ b/lib/resty/openidc.lua @@ -358,7 +358,7 @@ local function openidc_authorize(opts, session, target_url, prompt) session:set("last_authenticated", ngx.time()) if opts.lifecycle and opts.lifecycle.on_created then - err = opts.lifecycle.on_created(session) + err = opts.lifecycle.on_created(session, params) if err then log(WARN, "failed in `on_created` handler: " .. err) return err From 20aa0333d7c15fea60c616fb94548a08c9e99f9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= Date: Thu, 28 Dec 2023 23:29:38 +0100 Subject: [PATCH 3/5] Fix logout with no session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- lib/resty/openidc.lua | 18 ++++++++++++------ tests/spec/logout_spec.lua | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index 28031e33..84272e07 100644 --- a/lib/resty/openidc.lua +++ b/lib/resty/openidc.lua @@ -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] @@ -1305,7 +1313,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. " .. @@ -1460,10 +1470,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) @@ -1485,7 +1491,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 diff --git a/tests/spec/logout_spec.lua b/tests/spec/logout_spec.lua index cc2cf2d0..cabc335f 100644 --- a/tests/spec/logout_spec.lua +++ b/tests/spec/logout_spec.lua @@ -552,3 +552,20 @@ 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 + }) + 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) From 742236fc233458af27ff6e0bcf2e047ab089d789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= Date: Thu, 28 Dec 2023 16:06:02 +0100 Subject: [PATCH 4/5] Fix usage of RFC 5737 address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address range 192.0.2.0/24 is reserved for documentation according to RFC 5737. The recommendation is to reject routing of this address range on routers, but as this is not mandatory, it might happen that the address is really routed. The tests on Docker on Windows fail because of this it, the fail reason is different to the expected one. Fix this by configuring Nginx to listen on 127.0.0.1:80 (and not 0.0.0.0:80) and connecting to 127.1.2.3 instead of 192.0.2.1. Signed-off-by: Oldřich Jedlička --- tests/spec/access_token_access_spec.lua | 4 ++-- tests/spec/bearer_token_verification_spec.lua | 4 ++-- tests/spec/introspection_spec.lua | 4 ++-- tests/spec/redirect_to_op_spec.lua | 4 ++-- tests/spec/test_support.lua | 2 +- tests/spec/token_request_spec.lua | 4 ++-- tests/spec/userinfo_spec.lua | 4 ++-- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/spec/access_token_access_spec.lua b/tests/spec/access_token_access_spec.lua index 87128e31..bd2347b4 100644 --- a/tests/spec/access_token_access_spec.lua +++ b/tests/spec/access_token_access_spec.lua @@ -202,7 +202,7 @@ describe("when token endpoint is not reachable", function() access_token_opts = { timeout = 40000, discovery = { - token_endpoint = "http://192.0.2.1/" + token_endpoint = "http://127.1.2.3/" } }, token_response_expires_in = 0 @@ -219,7 +219,7 @@ describe("when token endpoint is not reachable", function() assert.are.equals(401, status) end) it("an error has been logged", function() - assert.error_log_contains("access_token error: accessing token endpoint.*%(http://192.0.2.1/%) failed") + assert.error_log_contains("access_token error: accessing token endpoint.*%(http://127.1.2.3/%) failed") end) end) diff --git a/tests/spec/bearer_token_verification_spec.lua b/tests/spec/bearer_token_verification_spec.lua index 7834acaf..6d1c67d3 100644 --- a/tests/spec/bearer_token_verification_spec.lua +++ b/tests/spec/bearer_token_verification_spec.lua @@ -464,7 +464,7 @@ describe("when jwks endpoint is not reachable", function() verify_opts = { timeout = 40000, discovery = { - jwks_uri = "http://192.0.2.1/" + jwks_uri = "http://127.1.2.3/" } }, }) @@ -478,7 +478,7 @@ describe("when jwks endpoint is not reachable", function() assert.are.equals(401, status) end) it("an error has been logged", function() - assert.error_log_contains("Invalid token: accessing jwks url.*%(http://192.0.2.1/%) failed") + assert.error_log_contains("Invalid token: accessing jwks url.*%(http://127.1.2.3/%) failed") end) end) diff --git a/tests/spec/introspection_spec.lua b/tests/spec/introspection_spec.lua index 3b5437b7..a22a3b89 100644 --- a/tests/spec/introspection_spec.lua +++ b/tests/spec/introspection_spec.lua @@ -396,7 +396,7 @@ describe("when introspection endpoint is not reachable", function() test_support.start_server({ introspection_opts = { timeout = 40000, - introspection_endpoint = "http://192.0.2.1/" + introspection_endpoint = "http://127.1.2.3/" }, }) teardown(test_support.stop_server) @@ -409,7 +409,7 @@ describe("when introspection endpoint is not reachable", function() assert.are.equals(401, status) end) it("an error has been logged", function() - assert.error_log_contains("Introspection error:.*accessing introspection endpoint %(http://192.0.2.1/%) failed") + assert.error_log_contains("Introspection error:.*accessing introspection endpoint %(http://127.1.2.3/%) failed") end) end) diff --git a/tests/spec/redirect_to_op_spec.lua b/tests/spec/redirect_to_op_spec.lua index efadd723..05257826 100644 --- a/tests/spec/redirect_to_op_spec.lua +++ b/tests/spec/redirect_to_op_spec.lua @@ -136,7 +136,7 @@ describe("when discovery endpoint is not reachable", function() test_support.start_server({ oidc_opts = { timeout = 40000, - discovery = "http://192.0.2.1/" + discovery = "http://127.1.2.3/" }, }) teardown(test_support.stop_server) @@ -148,7 +148,7 @@ describe("when discovery endpoint is not reachable", function() assert.are.equals(401, status) end) it("an error has been logged", function() - assert.error_log_contains("authenticate failed: accessing discovery url.*%(http://192.0.2.1/%) failed") + assert.error_log_contains("authenticate failed: accessing discovery url.*%(http://127.1.2.3/%) failed") end) end) diff --git a/tests/spec/test_support.lua b/tests/spec/test_support.lua index cca73e24..688a3a91 100644 --- a/tests/spec/test_support.lua +++ b/tests/spec/test_support.lua @@ -160,7 +160,7 @@ http { server { log_subrequest on; - listen 80; + listen 127.0.0.1:80; #listen 443 ssl; #ssl_certificate certificate-chain.crt; #ssl_certificate_key private.key; diff --git a/tests/spec/token_request_spec.lua b/tests/spec/token_request_spec.lua index a7dfd50e..08b49fbc 100644 --- a/tests/spec/token_request_spec.lua +++ b/tests/spec/token_request_spec.lua @@ -153,7 +153,7 @@ describe("if token endpoint is not reachable", function() oidc_opts = { timeout = 40000, discovery = { - token_endpoint = "http://192.0.2.1/" + token_endpoint = "http://127.1.2.3/" } }, }) @@ -163,7 +163,7 @@ describe("if token endpoint is not reachable", function() assert.are.equals(401, status) end) it("an error has been logged", function() - assert.error_log_contains("authenticate failed:.*accessing token endpoint %(http://192.0.2.1/%) failed") + assert.error_log_contains("authenticate failed:.*accessing token endpoint %(http://127.1.2.3/%) failed") end) end) diff --git a/tests/spec/userinfo_spec.lua b/tests/spec/userinfo_spec.lua index 856a2378..78f79c06 100644 --- a/tests/spec/userinfo_spec.lua +++ b/tests/spec/userinfo_spec.lua @@ -75,7 +75,7 @@ describe("when userinfo endpoint is not reachable", function() oidc_opts = { timeout = 40000, discovery = { - userinfo_endpoint = "http://192.0.2.1/" + userinfo_endpoint = "http://127.1.2.3/" } }, }) @@ -85,7 +85,7 @@ describe("when userinfo endpoint is not reachable", function() assert.are.equals(302, status) end) it("an error has been logged", function() - assert.error_log_contains(".*error calling userinfo endpoint: accessing %(http://192.0.2.1/%) failed") + assert.error_log_contains(".*error calling userinfo endpoint: accessing %(http://127.1.2.3/%) failed") end) end) From d0dc1fd6953715cf5774ca637a4467792785d935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= Date: Sat, 30 Dec 2023 00:09:36 +0100 Subject: [PATCH 5/5] Fix Base64Url decoding of JWT in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mime module expects padded Base64 value, so add missing padding. Signed-off-by: Oldřich Jedlička --- tests/spec/token_request_spec.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/spec/token_request_spec.lua b/tests/spec/token_request_spec.lua index 08b49fbc..c1839aec 100644 --- a/tests/spec/token_request_spec.lua +++ b/tests/spec/token_request_spec.lua @@ -267,7 +267,8 @@ local function extract_jwt_from_error_log() local enc_hdr, enc_payload, enc_sign = string.match(encoded_jwt, '^(.+)%.(.+)%.(.*)$') local base64_url_decode = function(s) local mime = require "mime" - return mime.unb64(s:gsub('-','+'):gsub('_','/')) + local padding = (4 - #s % 4) % 4 + return mime.unb64(s:gsub('-','+'):gsub('_','/') .. string.rep("=", padding)) end local dkjson = require "dkjson" return {