diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index cb4992e..84576f1 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -1,21 +1,11 @@ local saml = require "saml" local uuid = require 'resty.jit-uuid' uuid.seed() -local cjson = require "cjson" -local ck = require "resty.cookie" +local session = require "resty.session" local _M = {} - local RSA_SHA_512_HREF = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512" -local SESSION_COOKIE_NAME = "saml_session" - -local SESSION_SHM = "saml_sessions" - -local DEFAULT_COOKIE_LIFETIME = 300 -- in secs - -local EXPIRED_DATE = "Thu, 01 Jan 1970 00:00:01 GMT" - local function create_redirect(key, params) local saml_type if params.SAMLRequest then @@ -191,26 +181,34 @@ local function logout_request(opts, name_id, session_index) end local function login(self, opts) - local cookie, err = ck:new() - if not cookie then - ngx.log(ngx.ERR, "cookie:new(): ", err) - ngx.exit(500) + local sess = session.start(self.session_config) + + local authenticated = sess:get("authenticated") + local expires = sess:get("expires") + local expired = false + if type(expires) == "number" then + local delta = expires - ngx.time() + if delta < 0 then + expired = true + end end - local session_id = cookie:get(SESSION_COOKIE_NAME) - - if session_id then - local data = ngx.shared[SESSION_SHM]:get(session_id) - if data then - data = cjson.decode(data) - if data.authenticated then - return data - end - end + if authenticated and not expired then + return { + authenticated = authenticated, + name_id = sess:get("name_id"), + session_index = sess:get("session_index"), + attrs = sess:get("attrs"), + issuer = sess:get("issuer"), + } end - local request_uri = ngx.var.request_uri local state = uuid.generate_v4() + local request_uri = ngx.var.request_uri + + sess:set("saml_state", state) + sess:set("request_uri", request_uri) + sess:save() local query_str, err = create_redirect(self.sign_key, { SAMLRequest = authn_request(opts), @@ -222,30 +220,7 @@ local function login(self, opts) ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR) end - if not session_id then - session_id = uuid.generate_v4() - local ok, err = cookie:set({ - key = SESSION_COOKIE_NAME, - value = session_id, - path = "/", - httponly = true, - samesite = self.samesite, - secure = self.secure, - }) - if not ok then - ngx.log(ngx.ERR, "cookie:set(): ", err) - ngx.exit(500) - end - end - - local data = { - request_uri = request_uri, - state = state, - } - data = cjson.encode(data) - ngx.shared[SESSION_SHM]:set(session_id, data, DEFAULT_COOKIE_LIFETIME) - - ngx.log(ngx.INFO, "login start, request_uri=", data.request_uri) + ngx.log(ngx.INFO, "login start, request_uri=", request_uri) return ngx.redirect(opts.idp_uri .. "?" .. query_str) end @@ -284,30 +259,15 @@ local function parse_iso8601_utc_time(str) end local function login_callback(self, opts) - local cookie, err = ck:new() - if not cookie then - ngx.log(ngx.ERR, "cookie:new(): ", err) - ngx.exit(500) - end + local sess = session.start(self.session_config) - local session_id, err = cookie:get(SESSION_COOKIE_NAME) - if err then - ngx.log(ngx.ERR, "cookie:get(): ", err) - ngx.exit(500) - end - - if not session_id then - ngx.log(ngx.ERR, "no session found") + local saml_state = sess:get("saml_state") + if not saml_state then + ngx.log(ngx.ERR, "no session found or saml_state missing") ngx.exit(503) end - local data = ngx.shared[SESSION_SHM]:get(session_id) - if not data then - ngx.log(ngx.ERR, "no session found") - ngx.exit(503) - end - data = cjson.decode(data) - local request_uri = data.request_uri + local request_uri = sess:get("request_uri") local method = ngx.req.get_method() local doc, args, err @@ -331,8 +291,8 @@ local function login_callback(self, opts) end local state = args.RelayState - if state ~= data.state then - ngx.log(ngx.ERR, "state different: args.state=", state, ", state=", data.state) + if state ~= saml_state then + ngx.log(ngx.ERR, "state different: args.state=", state, ", state=", saml_state) ngx.exit(ngx.HTTP_UNAUTHORIZED) end @@ -341,7 +301,7 @@ local function login_callback(self, opts) local name_id = saml.doc_name_id(doc) local session_index = saml.doc_session_index(doc) local session_expires = saml.doc_session_expires(doc) - local expires, lifetime + local expires if session_expires then expires, err = parse_iso8601_utc_time(session_expires) ngx.log(ngx.INFO, "login callback: session_expires=", os.date("%Y-%m-%d %T %z", expires)) @@ -349,35 +309,22 @@ local function login_callback(self, opts) ngx.say(err) ngx.exit(500) end - lifetime = expires - ngx.time() - else - lifetime = DEFAULT_COOKIE_LIFETIME - expires = ngx.time() + DEFAULT_COOKIE_LIFETIME end - local data = { - authenticated = true, - name_id = name_id, - session_index = session_index, - attrs = attrs, - issuer = issuer, - } - data = cjson.encode(data) - ngx.shared[SESSION_SHM]:set(session_id, data, lifetime) - - local ok, err = cookie:set({ - key = SESSION_COOKIE_NAME, - value = session_id, - path = "/", - httponly = true, - expires = ngx.cookie_time(expires), - }) - if not ok then - ngx.log(ngx.ERR, "cookie:set(): ", err) - ngx.exit(500) - end - ngx.log(ngx.INFO, "login finish: data=", cjson.encode(data)) + sess:set("authenticated", true) + sess:set("name_id", name_id) + sess:set("session_index", session_index) + sess:set("attrs", attrs) + sess:set("issuer", issuer) + sess:set("expires", expires) + + -- clear temporary authentication state no longer needed after successful login + sess:set("saml_state", nil) + sess:set("request_uri", nil) + sess:save() + + ngx.log(ngx.INFO, "login finish: name_id=", name_id) return ngx.redirect(request_uri) end @@ -403,20 +350,11 @@ local function logout_response(destination, in_response_to, status, issuer) end local function logout_callback(self, opts) - local cookie, err = ck:new() - if not cookie then - ngx.log(ngx.ERR, "cookie:new(): ", err) - ngx.exit(500) - end + local sess = session.start(self.session_config) + local authenticated = sess:get("authenticated") - local session_id, err = cookie:get(SESSION_COOKIE_NAME) - if err then - ngx.log(ngx.ERR, "cookie:get(): ", err) - ngx.exit(500) - end - - if not session_id then - ngx.log(ngx.ERR, "no session found") + if not authenticated then + ngx.log(ngx.ERR, "no active session for logout") ngx.exit(ngx.HTTP_UNAUTHORIZED) end @@ -449,13 +387,6 @@ local function logout_callback(self, opts) ngx.exit(ngx.HTTP_BAD_REQUEST) end - local data = ngx.shared[SESSION_SHM]:get(session_id) - if not data then - ngx.log(ngx.ERR, "no session found") - ngx.exit(ngx.HTTP_UNAUTHORIZED) - end - data = cjson.decode(data) - if name == "LogoutRequest" then local issuer = saml.doc_issuer(doc) local request_id = saml.doc_id(doc) @@ -463,28 +394,25 @@ local function logout_callback(self, opts) local name_id = saml.doc_name_id(doc) local session_index = saml.doc_session_index(doc) - if issuer ~= data.issuer then + local saved_issuer = sess:get("issuer") + if issuer ~= saved_issuer then ngx.log(ngx.WARN, "issuer different: issuer=", issuer, - ", data.issuer=", data.issuer) + ", data.issuer=", saved_issuer) end - if name_id ~= data.name_id then + local saved_name_id = sess:get("name_id") + if name_id ~= saved_name_id then ngx.log(ngx.WARN, "name_id different: name_id=", name_id, - ", data.name_id=", data.name_id) + ", data.name_id=", saved_name_id) end - if session_index ~= data.session_index then + local saved_session_index = sess:get("session_index") + if session_index ~= saved_session_index then ngx.log(ngx.WARN, "session_index different: session_index=", - session_index, ", data.session_index=", data.session_index) + session_index, ", data.session_index=", saved_session_index) end - cookie:set({ - key = SESSION_COOKIE_NAME, - value = "", - expires = EXPIRED_DATE, - max_age = 0, - }) - ngx.shared[SESSION_SHM]:delete(session_id) + sess:destroy() local query_str, err = create_redirect(self.sign_key, { SAMLResponse = logout_response(opts.idp_uri, request_id, status, opts.sp_issuer), @@ -495,73 +423,33 @@ local function logout_callback(self, opts) ngx.log(ngx.ERR, err) ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR) end - ngx.log(ngx.INFO, "logout finish: data=", cjson.encode(data)) + ngx.log(ngx.INFO, "logout finish") return ngx.redirect(opts.idp_uri .. "?" .. query_str) - - --[[ - local body, err = create_post(self.sign_key, "SAMLResponse", - logout_response(opts.idp_uri, request_id, status, opts.sp_issuer), RSA_SHA_512_HREF, "", opts.idp_uri) - if err then - ngx.log(ngx.ERR, err) - ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR) - end - - ngx.log(ngx.INFO, "logout finish: data=", cjson.encode(data)) - - ngx.header.content_type = "text/html" - ngx.header.content_length = #body - ngx.say(body) - return ngx.exit(ngx.HTTP_OK) - --]] else local status_code = saml.doc_status_code(doc) if status_code ~= saml.STATUS_SUCCESS then ngx.log(ngx.ERR, "IdP returned non-success status: ", status_code) ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR) end - cookie:set({ - key = SESSION_COOKIE_NAME, - value = "", - expires = EXPIRED_DATE, - max_age = 0, - }) - ngx.shared[SESSION_SHM]:delete(session_id) - ngx.log(ngx.INFO, "logout finish: data=", cjson.encode(data)) + sess:destroy() + + ngx.log(ngx.INFO, "logout finish") return ngx.redirect(opts.logout_redirect_uri or "/") end end local function logout(self, opts) - local cookie, err = ck:new() - if not cookie then - ngx.log(ngx.ERR, "cookie:new(): ", err) - ngx.exit(500) - end + local sess = session.start(self.session_config) + local authenticated = sess:get("authenticated") - local session_id, err = cookie:get(SESSION_COOKIE_NAME) - if err then - ngx.log(ngx.ERR, "cookie:get(): ", err) - ngx.exit(500) - end - - local authenticated = false - if session_id then - local data = ngx.shared[SESSION_SHM]:get(session_id) - if data then - data = cjson.decode(data) - if data.authenticated then - authenticated = true - end - end - end if not authenticated then ngx.exit(ngx.HTTP_UNAUTHORIZED) end local query_str, err = create_redirect(self.sign_key, { - SAMLRequest = logout_request(opts, ngx.shared.name_id, ngx.shared.session_index), + SAMLRequest = logout_request(opts, sess:get("name_id"), sess:get("session_index")), SigAlg = RSA_SHA_512_HREF, RelayState = "", }) @@ -598,10 +486,18 @@ function _M.new(opts) obj.key_mngr_from_doc = function(doc) return obj.idp_cert_manager end obj.idp_cert_func = function(doc) return idp_cert end obj.auth_protocol_binding_method = opts.auth_protocol_binding_method - if obj.auth_protocol_binding_method == "HTTP-POST" then - obj.samesite = "None" - obj.secure = true - end + local cookie_secure, cookie_same_site + if opts.auth_protocol_binding_method == "HTTP-POST" then + cookie_secure = true + cookie_same_site = "None" + end + obj.session_config = { + cookie_name = "saml_session", + secret = opts.secret, + secret_fallbacks = opts.secret_fallbacks, + cookie_secure = cookie_secure, + cookie_same_site = cookie_same_site, + } return obj end diff --git a/rockspec/lua-resty-saml-0.2.3-0.rockspec b/rockspec/lua-resty-saml-0.2.3-0.rockspec index 3a51426..abbd915 100644 --- a/rockspec/lua-resty-saml-0.2.3-0.rockspec +++ b/rockspec/lua-resty-saml-0.2.3-0.rockspec @@ -14,7 +14,7 @@ description = { dependencies = { "api7-lua-resty-http = 0.2.0", "lua-resty-jit-uuid = 0.0.7", - "lua-resty-cookie = 0.4.1-1", + "lua-resty-cookie = 0.1.0", } build = { diff --git a/rockspec/lua-resty-saml-main-0-0.rockspec b/rockspec/lua-resty-saml-main-0-0.rockspec index 94d927c..af8926f 100644 --- a/rockspec/lua-resty-saml-main-0-0.rockspec +++ b/rockspec/lua-resty-saml-main-0-0.rockspec @@ -14,7 +14,7 @@ description = { dependencies = { "api7-lua-resty-http = 0.2.0", "lua-resty-jit-uuid = 0.0.7", - "lua-resty-cookie = 0.1.0", + "lua-resty-session = 4.1.5-1", } build = { diff --git a/t/lib/keycloak.lua b/t/lib/keycloak.lua index 75ff1b8..7daeac6 100644 --- a/t/lib/keycloak.lua +++ b/t/lib/keycloak.lua @@ -82,6 +82,7 @@ function _M.login_keycloak(uri, username, password) else local cookies = res.headers['Set-Cookie'] local cookie_str = _M.concatenate_cookies(cookies) + ngx.log(ngx.INFO, cookies) res, err = httpc:request_uri(res.headers['Location'], {method = "GET"}) if not res then diff --git a/t/saml-post.t b/t/saml-post.t index d33ab51..a93e78d 100644 --- a/t/saml-post.t +++ b/t/saml-post.t @@ -27,11 +27,9 @@ _EOC_ $block->set_value("main_config", $main_config); my $http_config = $block->http_config // <<_EOC_; - lua_package_path '$pwd/deps/share/lua/5.1/?.lua;$pwd/lua/?.lua;$pwd/t/?.lua;;'; + lua_package_path '$pwd/lua/?.lua;$pwd/deps/share/lua/5.1/?.lua;$pwd/t/?.lua;;'; lua_package_cpath '$pwd/?.so;;'; - lua_shared_dict saml_sessions 10m; - init_by_lua_block { local saml = require "resty.saml" local err = saml.init({ @@ -60,6 +58,9 @@ _EOC_ local opts = setmetatable({sp_issuer = sp_issuer}, {__index = kc.get_default_opts()}) opts.auth_protocol_binding_method = "HTTP-POST" ngx.log(ngx.INFO, "create sp_issuer=", sp_issuer) + opts.session_config = { + secret = "very-secret-key-that-is-32-bytes-long-exact", + } local saml = require("resty.saml").new(opts) samls[sp_issuer] = saml end @@ -136,6 +137,8 @@ __DATA__ --- error_code: 200 --- error_log login callback req with http post +; Path=/; SameSite=None; Secure; HttpOnly, + === TEST 2: login sp1 and sp2, then do single logout @@ -198,3 +201,4 @@ login callback req with http post --- error_code: 200 --- error_log login callback req with http post +; Path=/; SameSite=None; Secure; HttpOnly, diff --git a/t/saml.t b/t/saml.t index 45f88f9..2ba1d73 100644 --- a/t/saml.t +++ b/t/saml.t @@ -27,11 +27,9 @@ _EOC_ $block->set_value("main_config", $main_config); my $http_config = $block->http_config // <<_EOC_; - lua_package_path '$pwd/deps/share/lua/5.1/?.lua;$pwd/lua/?.lua;$pwd/t/?.lua;;'; + lua_package_path '$pwd/lua/?.lua;$pwd/deps/share/lua/5.1/?.lua;$pwd/t/?.lua;;'; lua_package_cpath '$pwd/?.so;;'; - lua_shared_dict saml_sessions 10m; - init_by_lua_block { local saml = require "resty.saml" local err = saml.init({ @@ -60,6 +58,9 @@ _EOC_ local opts = setmetatable({sp_issuer = sp_issuer}, {__index = kc.get_default_opts()}) ngx.log(ngx.INFO, "create sp_issuer=", sp_issuer) opts.auth_protocol_binding_method = "HTTP-Redirect" + opts.session_config = { + secret = "very-secret-key-that-is-32-bytes-long-exact", + } local saml = require("resty.saml").new(opts) samls[sp_issuer] = saml end @@ -136,6 +137,8 @@ __DATA__ --- error_code: 200 --- error_log login callback req with redirect +; Path=/; SameSite=Lax; HttpOnly, + === TEST 2: login sp1 and sp2, then do single logout @@ -198,3 +201,4 @@ login callback req with redirect --- error_code: 200 --- error_log login callback req with redirect +; Path=/; SameSite=Lax; HttpOnly,