From 4040119d8ebcd35df3bc35e60e20dcbfa566f13b 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] 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 | 177 ++++++++++++++++------------- lua-resty-openidc-1.7.6-3.rockspec | 2 +- tests/Dockerfile | 2 +- tests/spec/logout_spec.lua | 34 +++--- 4 files changed, 120 insertions(+), 95 deletions(-) diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index 1f3854e7..271e3829 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" @@ -1061,7 +1065,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 @@ -1081,23 +1085,26 @@ local function openidc_authorization_response(opts, session) args = ngx.req.get_uri_args() end + local original_url = session:get("original_url") + 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, 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, original_url, session end err = ensure_config(opts) if err then - return nil, err, session.data.original_url, session + return nil, err, original_url, session end -- check the iss if returned from the OP @@ -1105,7 +1112,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, original_url, session end -- check the client_id if returned from the OP @@ -1113,7 +1120,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, original_url, session end -- assemble the parameters to the token endpoint @@ -1121,8 +1128,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") @@ -1132,22 +1139,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, 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, 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 @@ -1163,21 +1170,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 @@ -1185,17 +1192,21 @@ 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, 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 + 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) @@ -1250,8 +1261,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 @@ -1271,9 +1282,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 @@ -1342,21 +1353,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) @@ -1367,7 +1383,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" } @@ -1386,30 +1402,24 @@ 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) + access_token = json.access_token + session:set("access_token", 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 @@ -1418,7 +1428,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 access_token, err end local function openidc_get_path(uri) @@ -1434,7 +1452,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 @@ -1458,6 +1476,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 @@ -1467,7 +1487,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 @@ -1482,7 +1502,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) @@ -1491,7 +1511,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 @@ -1505,10 +1527,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, @@ -1516,13 +1540,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 @@ -1533,7 +1557,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") @@ -1543,10 +1567,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") @@ -1557,15 +1582,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()