From 6b3fbe7c5d3b806efd89bd4f3aec9c5111071f1d Mon Sep 17 00:00:00 2001 From: Austin Hockenberry Date: Mon, 10 Aug 2026 13:50:03 -0400 Subject: [PATCH 1/2] switched to storage interface --- README.md | 56 ++- lib/resty/session.lua | 15 +- lib/resty/session/file/thread.lua | 4 +- lib/resty/session/mysql.lua | 6 +- lib/resty/session/utils.lua | 73 ---- spec/06-revocation-1_spec.lua | 543 +++++++++++++++-------------- spec/07-revocation-2_spec.lua | 548 ++++++++++++++++++++---------- 7 files changed, 710 insertions(+), 535 deletions(-) diff --git a/README.md b/README.md index 3a5ddb18..a5e0e931 100644 --- a/README.md +++ b/README.md @@ -328,7 +328,7 @@ Here are the possible session configuration options: | `request_headers` | `nil` | Set of headers to send to upstream, use `id`, `audience`, `subject`, `timeout`, `idling-timeout`, `rolling-timeout`, `absolute-timeout`. E.g. `{ "id", "timeout" }` will set `Session-Id` and `Session-Timeout` request headers when `set_headers` is called. | | `response_headers` | `nil` | Set of headers to send to downstream, use `id`, `audience`, `subject`, `timeout`, `idling-timeout`, `rolling-timeout`, `absolute-timeout`. E.g. `{ "id", "timeout" }` will set `Session-Id` and `Session-Timeout` response headers when `set_headers` is called. | | `storage` | `nil` | Storage is responsible of storing session data, use `nil` or `"cookie"` (data is stored in cookie), `"dshm"`, `"file"`, `"memcached"`, `"mysql"`, `"postgres"`, `"redis"`, or `"shm"`, or give a name of custom module (`"custom-storage"`), or a `table` that implements session storage interface. | -| `revocation` | `nil` | Enable Redis-backed session revocation for cookie (stateless) sessions, use `nil`, `true`, `false`, a Redis configuration `table`, or a `table` that implements the revocation store interface (see below). | +| `revocation` | `nil` | Storage used for cookie session revocation records. Use `nil` or `false` to disable, a storage name such as `"shm"`, `"redis"`, `"mysql"`, or `"postgres"`, a custom storage module name, or a storage `table` with `set`/`get` methods. | | `revocation_fail_mode` | `"open"` | Behavior when the revocation store is unreachable, use `"open"` (treat as not revoked) or `"closed"` (reject the session). | | `dshm` | `nil` | Configuration for dshm storage, e.g. `{ prefix = "sessions" }` (see below) | | `file` | `nil` | Configuration for file storage, e.g. `{ path = "/tmp", suffix = "session" }` (see below) | @@ -350,22 +350,24 @@ just set the `storage` to `nil` or `"cookie"`. Cookie (stateless) sessions are self-contained: once issued, a cookie remains valid until it expires according to the configured timeouts. Revocation adds -an optional Redis-backed denylist so that destroyed sessions are rejected +an optional storage-backed denylist so that destroyed sessions are rejected immediately, without waiting for the cookie to expire. Revocation is only available when session data is stored in the cookie -(`storage` is `nil` or `"cookie"`). It must be enabled explicitly with -`revocation = true` (using the `redis` configuration) or -`revocation = { ... }` (inline Redis or custom store settings). Setting -`revocation = false` disables it. +(`storage` is `nil` or `"cookie"`). Select the backend explicitly with +`revocation = "dshm"`, `"file"`, `"memcached"`, `"mysql"`, `"postgres"`, +`"redis"`, or `"shm"`. The backend uses its normal configuration section and +the same storage `set`/`get` contract used for session data. Custom storage +module names and pre-built storage tables are also supported. Setting +`revocation = false` or leaving it unset disables revocation. On every `session:open`, the library checks whether the session identifier is -revoked. On `session:destroy`, the identifier is written to Redis with a TTL -equal to the remaining session lifetime (rolling and absolute timeouts). The -revocation mark is a lightweight sentinel; no session payload is stored in -Redis. +revoked. On `session:destroy`, the identifier is written to the selected +storage with a TTL equal to the remaining session lifetime (rolling and +absolute timeouts). The revocation mark is a lightweight sentinel; no session +payload is stored. -Use `revocation_fail_mode` to control behavior when Redis is unreachable: +Use `revocation_fail_mode` to control behavior when the storage is unavailable: - `"open"` (default): log a warning and treat the session as not revoked. Destroy still clears the cookie even if the revocation write fails. @@ -377,23 +379,44 @@ the last audience). It does not revoke the previous session identifier on audiences). After rotation or partial logout, the previous cookie remains usable until its `stale_ttl` or timeout elapses. -Example: +Examples: ```lua +-- Redis denylist require("resty.session").init({ storage = "cookie", - revocation = true, + revocation = "redis", redis = { host = "127.0.0.1", password = "secret", prefix = "sessions", }, }) + +-- Shared memory denylist +require("resty.session").init({ + storage = "cookie", + revocation = "shm", + shm = { + zone = "sessions", + prefix = "revocations", + }, +}) + +-- MySQL denylist +require("resty.session").init({ + storage = "cookie", + revocation = "mysql", + mysql = { + host = "127.0.0.1", + database = "sessions", + username = "session", + password = "secret", + }, +}) ``` -The `redis.mode` setting selects whether a Redis connection is used for -session data (`"storage"`) or for revocation (`"revocation"`). When unset, -it defaults to `"revocation"` for cookie storage and `"storage"` otherwise. +The same pattern works for `"dshm"`, `"file"`, `"memcached"`, and `"postgres"`. ## DSHM Storage Configuration @@ -582,7 +605,6 @@ connections. Common configuration settings among them all: | Option | Default | Description | |---------------------|:-------:|----------------------------------------------------------------------------------------------| -| `mode` | `nil` | Role of this Redis connection: `"storage"` for session data or `"revocation"` for the session denylist. Defaults to `"revocation"` when `storage` is `nil` or `"cookie"`, otherwise `"storage"`. | | `prefix` | `nil` | Prefix for the keys stored in Redis. | | `suffix` | `nil` | Suffix for the keys stored in Redis. | | `username` | `nil` | The database username to authenticate. | diff --git a/lib/resty/session.lua b/lib/resty/session.lua index 2e565afc..d187683e 100644 --- a/lib/resty/session.lua +++ b/lib/resty/session.lua @@ -44,7 +44,6 @@ local encode_base64url = utils.encode_base64url local decode_base64url = utils.decode_base64url local table_is_empty = utils.is_empty_table local load_storage = utils.load_storage -local load_revocation = utils.load_revocation local encode_json = utils.encode_json local decode_json = utils.decode_json local base64_size = utils.base64_size @@ -2434,7 +2433,7 @@ local session = { -- @field request_headers Set of headers to send to upstream, use `id`, `audience`, `subject`, `timeout`, `idling-timeout`, `rolling-timeout`, `absolute-timeout`. E.g. `{ "id", "timeout" }` will set `Session-Id` and `Session-Timeout` request headers when `set_headers` is called. -- @field response_headers Set of headers to send to downstream, use `id`, `audience`, `subject`, `timeout`, `idling-timeout`, `rolling-timeout`, `absolute-timeout`. E.g. `{ "id", "timeout" }` will set `Session-Id` and `Session-Timeout` response headers when `set_headers` is called. -- @field storage Storage is responsible of storing session data, use `nil` or `"cookie"` (data is stored in cookie), `"dshm"`, `"file"`, `"memcached"`, `"mysql"`, `"postgres"`, `"redis"`, or `"shm"`, or give a name of custom module (`"custom-storage"`), or a `table` that implements session storage interface (defaults to `nil`) --- @field revocation Session revocation backend for cookie (stateless) sessions, use `nil` (auto-load from `redis` when configured), `false` to disable, `"redis"`, `true` (alias for `"redis"`), or a pre-built store `table` with `set`/`get` methods (defaults to `nil`) +-- @field revocation Storage used for cookie session revocation records, use `nil` or `false` to disable, `"dshm"`, `"file"`, `"memcached"`, `"mysql"`, `"postgres"`, `"redis"`, or `"shm"`, a custom storage module name, or a storage `table` with `set`/`get` methods (defaults to `nil`) -- @field revocation_fail_mode Behavior when the revocation store is unreachable, use `"open"` (treat as not revoked) or `"closed"` (reject the session) (defaults to `"open"`) -- @field dshm Configuration for dshm storage, e.g. `{ prefix = "sessions" }` -- @field file Configuration for file storage, e.g. `{ path = "/tmp", suffix = "session" }` @@ -2497,9 +2496,6 @@ local function opt(configuration, name, default) end end - elseif name == "revocation" then - value = load_revocation(nil, configuration) - end else @@ -2558,10 +2554,7 @@ local function opt(configuration, name, default) else local t = type(value) if t == "string" then - value = assert(load_revocation(value, configuration), "unable to load session revocation") - - elseif value == true then - value = assert(load_revocation("redis", configuration), "unable to load session revocation") + value = assert(load_storage(value, configuration), "unable to load session revocation storage") elseif t == "table" then if type(value.set) ~= "function" or type(value.get) ~= "function" then @@ -2682,6 +2675,10 @@ function session.new(configuration) local revocation = opt(configuration, "revocation", DEFAULT_REVOCATION) local revocation_fail_mode = opt(configuration, "revocation_fail_mode", DEFAULT_REVOCATION_FAIL_MODE) + if storage then + revocation = nil + end + if cookie_prefix == "__Host-" then cookie_name = cookie_prefix .. cookie_name remember_cookie_name = cookie_prefix .. remember_cookie_name diff --git a/lib/resty/session/file/thread.lua b/lib/resty/session/file/thread.lua index 11d2346f..d42c6889 100644 --- a/lib/resty/session/file/thread.lua +++ b/lib/resty/session/file/thread.lua @@ -128,8 +128,8 @@ local function get(path, prefix, suffix, name, key, current_time) -- TODO: do we want to check expiry here? -- The cookie header already has the info and has a MAC too. local exp = get_modification(file_path) - if exp and exp < current_time then - return nil, "expired" + if not exp or exp < current_time then + return nil end return file_read(file_path) diff --git a/lib/resty/session/mysql.lua b/lib/resty/session/mysql.lua index 4dfe93ef..034bc9cc 100644 --- a/lib/resty/session/mysql.lua +++ b/lib/resty/session/mysql.lua @@ -58,7 +58,7 @@ local DEFAULT_TABLE = "sessions" local DEFAULT_CHARSET = "ascii" -local SET = "INSERT INTO %s (sid, name, data, exp) VALUES ('%s', '%s', '%s', FROM_UNIXTIME(%d)) AS new ON DUPLICATE KEY UPDATE data = new.data" +local SET = "INSERT INTO %s (sid, name, data, exp) VALUES ('%s', '%s', '%s', FROM_UNIXTIME(%d)) AS new ON DUPLICATE KEY UPDATE data = new.data, exp = new.exp" local SET_META_PREFIX = "INSERT INTO %s (aud, sub, sid) VALUES " local SET_META_VALUES = "('%s', '%s', '%s')" local SET_META_SUFFIX = " ON DUPLICATE KEY UPDATE sid = sid" @@ -193,12 +193,12 @@ function metatable:get(name, key, current_time) -- luacheck: ignore local row = res[1] if not row then - return nil, "session not found" + return nil end local data = row.data if not row.data then - return nil, "session not found" + return nil end return data diff --git a/lib/resty/session/utils.lua b/lib/resty/session/utils.lua index f401ae57..b5b88749 100644 --- a/lib/resty/session/utils.lua +++ b/lib/resty/session/utils.lua @@ -923,8 +923,6 @@ local load_storage do elseif storage == "redis" then local cfg = configuration and configuration.redis if cfg then - assert(cfg.mode ~= "revocation", "invalid redis mode for session storage") - if cfg.nodes then if not REDIS_CLUSTER then REDIS_CLUSTER = require("resty.session.redis.cluster") @@ -961,76 +959,6 @@ local load_storage do end - -local load_revocation do - local REDIS - local CUSTOM = {} - - --- - -- Loads session revocation store and creates a new instance using session configuration. - -- - -- @function utils.load_revocation - -- @tparam nil|boolean|string revocation revocation store name, `nil` to auto-load from - -- `redis` when configured for revocation, `true` for `"redis"`, or `false` to disable - -- @tparam[opt] table configuration session configuration - -- @treturn table|nil instance of session revocation store - -- @treturn string|nil error message - -- - -- @usage - -- local redis = require("resty.session.utils").load_revocation("redis", { - -- redis = { - -- host = "127.0.0.1", - -- } - -- }) - load_revocation = function(revocation, configuration) - if revocation == false or revocation == "cookie" then - return nil - end - - if revocation == true then - revocation = "redis" - end - - local session_storage = configuration and configuration.storage - if session_storage and session_storage ~= "cookie" then - return nil - end - - if not revocation then - local redis_cfg = configuration and configuration.redis - if not redis_cfg or not redis_cfg.host or redis_cfg.mode == "storage" then - return nil - end - - revocation = "redis" - end - - if type(revocation) ~= "string" then - error("invalid session revocation") - end - - if revocation == "redis" then - local cfg = configuration and configuration.redis - if not cfg or not cfg.host or cfg.mode == "storage" then - return nil - end - - if not REDIS then - REDIS = require("resty.session.redis") - end - return REDIS.new(cfg) - - else - if not CUSTOM[revocation] then - CUSTOM[revocation] = require(revocation) - end - return CUSTOM[revocation].new(configuration and configuration[revocation]) - end - end -end - - - --- -- Helper to format error messages. -- @@ -1267,7 +1195,6 @@ return { decrypt_aes_256_gcm = decrypt_aes_256_gcm, hmac_sha256 = hmac_sha256, load_storage = load_storage, - load_revocation = load_revocation, errmsg = errmsg, get_name = get_name, set_flag = set_flag, diff --git a/spec/06-revocation-1_spec.lua b/spec/06-revocation-1_spec.lua index a6d0af05..3fdb24a7 100644 --- a/spec/06-revocation-1_spec.lua +++ b/spec/06-revocation-1_spec.lua @@ -1,20 +1,36 @@ +--- +-- Ensure to keep the tests consistent with those in 07-revocation-2_spec.lua + + local session = require "resty.session" -local redis_storage = require "resty.session.redis" -local encode_base64url = require("resty.session.utils").encode_base64url +local utils = require "resty.session.utils" local before_each = before_each +local after_each = after_each local lazy_setup = lazy_setup local describe = describe -local assert = assert local ipairs = ipairs +local assert = assert local sleep = ngx.sleep +local time = ngx.time local it = it -local redis_config = { - host = "127.0.0.1", - password = "password", +local storage_configs = { + file = { + suffix = "revocation", + }, + shm = { + prefix = "revocations", + }, + redis = { + prefix = "revocations", + password = "password", + }, + memcached = { + prefix = "revocations", + }, } @@ -34,272 +50,289 @@ local function extract_cookie(cookie_name, cookies) end -describe("Revocation tests 1", function() - local store - local long_ttl = 60 - local short_ttl = 2 - local id = "test_id_1iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" - local id1 = "test_id_2iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" - local id2 = "test_id_3iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" - local cookie = "session_cookie" - - lazy_setup(function() - store = redis_storage.new(redis_config) - assert.is_not_nil(store) - end) - - describe("session: normal use", function() - local cookie_name = "session_cookie" - local test_key = "test_key" - local value = "test_data" - - local function save_session(s, cookies) - session.__set_ngx_header(cookies) - s:set(test_key, value) - local ok, err = s:save() - assert.is_true(ok) - assert.is_nil(err) - return extract_cookie(cookie_name, cookies["Set-Cookie"]) - end - - local function open_session(session_cookie) - local s = session.new() - session.__set_ngx_var({ - ["cookie_" .. cookie_name] = session_cookie, - }) - - local ok, err = s:open() - if not ok then - return nil, err - end - - return s - end - - before_each(function() - session.init({ - cookie_name = cookie_name, +for _, st in ipairs({ + "file", + "shm", + "redis", + "memcached", +}) do + describe("Revocation tests 1", function() + local current_time + local store + local long_ttl = 60 + local short_ttl = 2 + local key = "test_key_1iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + local key1 = "test_key_2iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + local key2 = "test_key_3iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + local name = "session_cookie" + local mark = "1" + + lazy_setup(function() + local conf = { storage = "cookie", - redis = { - host = redis_config.host, - password = redis_config.password, - mode = "revocation", - }, - }) - end) - - it("open succeeds for a valid session with revocation enabled", function() - local cookies = {} - local s = session.new() - local session_cookie = save_session(s, cookies) - s:close() - - local s2, err = open_session(session_cookie) - assert.is_not_nil(s2) - assert.is_nil(err) - assert.equals(value, s2:get(test_key)) - s2:close() - end) - - it("destroy: rejected cookie cannot be reopened", function() - local cookies = {} - local s = session.new() - local session_cookie = save_session(s, cookies) - assert.is_not_equal("", session_cookie) - - s:close() - - local s2, err = open_session(session_cookie) - assert.is_not_nil(s2) - assert.is_nil(err) - assert.equals(value, s2:get(test_key)) - - session.__set_ngx_header(cookies) - local ok - ok, err = s2:destroy() - assert.is_true(ok) - assert.is_nil(err) - - local s3 - s3, err = open_session(session_cookie) - assert.is_nil(s3) - assert.equals("session revoked", err) + revocation = st, + } + conf[st] = storage_configs[st] + store = utils.load_storage(st, conf) + assert.is_not_nil(store) end) - it("save rotation does not revoke the previous cookie", function() - local cookies = {} - local s = session.new() - local session_cookie = save_session(s, cookies) - s:close() - - local s2, err = open_session(session_cookie) - assert.is_not_nil(s2) - assert.is_nil(err) - - s2:set(test_key, "rotated") - session.__set_ngx_header(cookies) - local ok - ok, err = s2:save() - assert.is_true(ok) - assert.is_nil(err) - s2:close() - - local s3 - s3, err = open_session(session_cookie) - assert.is_not_nil(s3) - assert.is_nil(err) - assert.equals(value, s3:get(test_key)) - s3:close() + before_each(function() + current_time = time() end) - it("cookie session without revocation clears cookie on destroy", function() - session.init({ - cookie_name = cookie_name, - storage = "cookie", - }) - - local cookies = {} - local s = session.new() - local session_cookie = save_session(s, cookies) - s:close() - - local s2, err = open_session(session_cookie) - assert.is_not_nil(s2) - assert.is_nil(err) - - session.__set_ngx_header(cookies) - local ok - ok, err = s2:destroy() - assert.is_true(ok) - assert.is_nil(err) - - local s3 - s3, err = open_session(session_cookie) - assert.is_nil(s3) - assert.is_not_equal("session revoked", err) + describe("[#" .. st .. "] revocation storage: SET + GET", function() + after_each(function() + store:delete(name, key, current_time) + store:delete(name, key1, current_time) + store:delete(name, key2, current_time) + end) + + it("SET: stores revocation mark and GET observes it", function() + local ok, err = store:set(name, key, mark, long_ttl, current_time) + assert.is_not_nil(ok) + assert.is_nil(err) + + local data + data, err = store:get(name, key, current_time) + assert.is_nil(err) + assert.equals(mark, data) + end) + + it("GET: missing revocation key returns not revoked", function() + local data, err = store:get(name, key1, current_time) + assert.is_nil(data) + assert.is_nil(err) + end) + + it("SET: ttl expires revocation entry", function() + local ok, err = store:set(name, key2, mark, short_ttl, current_time) + assert.is_not_nil(ok) + assert.is_nil(err) + + local data + data, err = store:get(name, key2, current_time) + assert.is_nil(err) + assert.equals(mark, data) + + sleep(short_ttl + 1) + + data, err = store:get(name, key2, time()) + assert.is_nil(data) + assert.is_nil(err) + end) + + it("SET: re-mark refreshes ttl", function() + local ok, err = store:set(name, key, mark, short_ttl, current_time) + assert.is_not_nil(ok) + assert.is_nil(err) + + sleep(1) + + ok, err = store:set(name, key, mark, long_ttl, time()) + assert.is_not_nil(ok) + assert.is_nil(err) + + sleep(short_ttl + 1) + + local data + data, err = store:get(name, key, time()) + assert.is_nil(err) + assert.equals(mark, data) + end) end) - end) - - describe("[#redis] revocation: SET + GET", function() - it("SET: stores revocation mark and GET observes it", function() - local ok, err = store:set(cookie, encode_base64url(id1), "1", long_ttl, ngx.time()) - assert.is_not_nil(ok) - assert.is_nil(err) - local data - data, err = store:get(cookie, encode_base64url(id1), ngx.time()) - assert.is_nil(err) - assert.equals("1", data) - end) - - it("GET: missing revocation key returns not revoked marker", function() - local data, err = store:get(cookie, encode_base64url(id), ngx.time()) - assert.is_nil(err) - assert.is_not_equal("1", data) - end) + describe("[#" .. st .. "] session: revocation lifecycle", function() + local cookie_name = "session_cookie" + local test_key = "test_key" + local value = "test_data" + + local function save_session(s, cookies) + session.__set_ngx_header(cookies) + s:set(test_key, value) + local ok, err = s:save() + assert.is_true(ok) + assert.is_nil(err) + return extract_cookie(cookie_name, cookies["Set-Cookie"]) + end - it("SET: ttl expires revocation entry", function() - local ok, err = store:set(cookie, encode_base64url(id2), "1", short_ttl, ngx.time()) - assert.is_not_nil(ok) - assert.is_nil(err) + local function open_session(session_cookie) + local s = session.new() + session.__set_ngx_var({ + ["cookie_" .. cookie_name] = session_cookie, + }) - local data - data, err = store:get(cookie, encode_base64url(id2), ngx.time()) - assert.is_nil(err) - assert.equals("1", data) + local ok, err = s:open() + if not ok then + return nil, err + end - sleep(short_ttl + 1) + return s + end - data, err = store:get(cookie, encode_base64url(id2), ngx.time()) - assert.is_nil(err) - assert.is_not_equal("1", data) + before_each(function() + local conf = { + cookie_name = cookie_name, + storage = "cookie", + revocation = st, + } + conf[st] = storage_configs[st] + session.init(conf) + end) + + it("open succeeds for a valid session with revocation enabled", function() + local cookies = {} + local s = session.new() + local session_cookie = save_session(s, cookies) + s:close() + + local s2, err = open_session(session_cookie) + assert.is_not_nil(s2) + assert.is_nil(err) + assert.equals(value, s2:get(test_key)) + s2:close() + end) + + it("destroy: rejected cookie cannot be reopened", function() + local cookies = {} + local s = session.new() + local session_cookie = save_session(s, cookies) + assert.is_not_equal("", session_cookie) + s:close() + + local s2, err = open_session(session_cookie) + assert.is_not_nil(s2) + assert.is_nil(err) + assert.equals(value, s2:get(test_key)) + + session.__set_ngx_header(cookies) + local ok + ok, err = s2:destroy() + assert.is_true(ok) + assert.is_nil(err) + + local s3 + s3, err = open_session(session_cookie) + assert.is_nil(s3) + assert.equals("session revoked", err) + end) + + it("save rotation does not revoke the previous cookie", function() + local cookies = {} + local s = session.new() + local session_cookie = save_session(s, cookies) + s:close() + + local s2, err = open_session(session_cookie) + assert.is_not_nil(s2) + assert.is_nil(err) + + s2:set(test_key, "rotated") + session.__set_ngx_header(cookies) + local ok + ok, err = s2:save() + assert.is_true(ok) + assert.is_nil(err) + s2:close() + + local s3 + s3, err = open_session(session_cookie) + assert.is_not_nil(s3) + assert.is_nil(err) + assert.equals(value, s3:get(test_key)) + s3:close() + end) end) end) +end - describe("session: configuration", function() - local configuration = {} - local cookie_name = "session_cookie" - - before_each(function() - configuration = { - cookie_name = cookie_name, - redis = redis_config, - } - session.init(configuration) - end) - - it("loads revocation when storage is cookie and redis mode is revocation", function() - session.init({ - cookie_name = cookie_name, - storage = "cookie", - redis = { - host = redis_config.host, - password = redis_config.password, - mode = "revocation", - }, - }) - - local s = session.new() - assert.is_not_nil(s.revocation) - assert.is_function(s.revocation.set) - assert.is_function(s.revocation.get) - end) - - it("loads revocation when cookie storage has redis without storage mode", function() - local s = session.new() - assert.is_not_nil(s.revocation) - assert.is_function(s.revocation.set) - assert.is_function(s.revocation.get) - end) - - it("skips revocation when storage backend is configured", function() - session.init({ - cookie_name = cookie_name, - storage = "redis", - redis = { - prefix = "sessions", - password = "password", - }, - }) - - local s = session.new() - assert.is_nil(s.revocation) - end) - - it("skips revocation when storage is cookie and redis mode is storage", function() - session.init({ - cookie_name = cookie_name, - storage = "cookie", - redis = { - host = redis_config.host, - password = redis_config.password, - mode = "storage", - }, - }) - - local s = session.new() - assert.is_nil(s.revocation) - end) - it("does not load revocation without a redis host", function() - session.init({ - cookie_name = cookie_name, - redis = { password = "password" }, - }) +describe("Revocation tests 1 session: configuration", function() + local cookie_name = "session_cookie" + + it("cookie session without revocation remains usable after destroy", function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + }) + + local cookies = {} + local s = session.new() + session.__set_ngx_header(cookies) + s:set("test_key", "test_data") + local ok, err = s:save() + assert.is_true(ok) + assert.is_nil(err) + local session_cookie = extract_cookie(cookie_name, cookies["Set-Cookie"]) + s:close() + + session.__set_ngx_var({ + ["cookie_" .. cookie_name] = session_cookie, + }) + local s2 = session.new() + ok, err = s2:open() + assert.is_true(ok) + assert.is_nil(err) + + session.__set_ngx_header(cookies) + ok, err = s2:destroy() + assert.is_true(ok) + assert.is_nil(err) + + session.__set_ngx_var({ + ["cookie_" .. cookie_name] = session_cookie, + }) + local s3 = session.new() + ok, err = s3:open() + assert.is_true(ok) + assert.is_nil(err) + assert.is_not_equal("session revoked", err) + end) - local s = session.new() - assert.is_nil(s.revocation) - end) + it("skips revocation when storage backend is configured", function() + session.init({ + cookie_name = cookie_name, + storage = "redis", + revocation = "shm", + redis = { + prefix = "sessions", + password = "password", + }, + shm = { + prefix = "revocations", + }, + }) + + local s = session.new() + assert.is_nil(s.revocation) + end) - it("skips revocation when revocation is explicitly false", function() - session.init({ - cookie_name = cookie_name, - redis = redis_config, - revocation = false, - }) + it("does not infer revocation storage from backend configuration", function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + redis = { + host = "127.0.0.1", + password = "password", + }, + }) + + local s = session.new() + assert.is_nil(s.revocation) + end) - local s = session.new() - assert.is_nil(s.revocation) - end) + it("skips revocation when revocation is explicitly false", function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + redis = { + host = "127.0.0.1", + password = "password", + }, + revocation = false, + }) + + local s = session.new() + assert.is_nil(s.revocation) end) end) diff --git a/spec/07-revocation-2_spec.lua b/spec/07-revocation-2_spec.lua index ae0a55f9..843f0307 100644 --- a/spec/07-revocation-2_spec.lua +++ b/spec/07-revocation-2_spec.lua @@ -1,30 +1,63 @@ +--- +-- For now these tests don't run on CI. +-- Ensure to keep the tests consistent with those in 06-revocation-1_spec.lua + + local session = require "resty.session" -local redis_storage = require "resty.session.redis" -local encode_base64url = require("resty.session.utils").encode_base64url +local utils = require "resty.session.utils" local before_each = before_each +local after_each = after_each +local lazy_setup = lazy_setup local describe = describe +local ipairs = ipairs local assert = assert local pcall = pcall -local ipairs = ipairs +local sleep = ngx.sleep +local time = ngx.time local it = it -local redis_config = { - host = "127.0.0.1", - password = "password", +local storage_configs = { + mysql = { + username = "root", + password = "password", + database = "test", + }, + postgres = { + username = "postgres", + password = "password", + database = "test", + }, + redis_sentinel = { + prefix = "revocations", + password = "password", + sentinels = { + { host = "127.0.0.1", port = "26379" } + }, + }, + redis_cluster = { + prefix = "revocations", + password = "password", + nodes = { + { ip = "127.0.0.1", port = "6380" } + }, + name = "somecluster", + lock_zone = "sessions", + }, + dshm = { + prefix = "revocations", + }, } -local bad_redis_config = { - host = "127.0.0.1", - port = 1, - password = "password", - connect_timeout = 100, - send_timeout = 100, - read_timeout = 100, -} +local function storage_type(ty) + if ty == "redis_cluster" or ty == "redis_sentinel" then + return "redis" + end + return ty +end local function extract_cookie(cookie_name, cookies) @@ -43,191 +76,354 @@ local function extract_cookie(cookie_name, cookies) end -describe("Revocation tests 2", function() - local long_ttl = 60 - local id = "test_id_1iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" - local id1 = "test_id_2iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" - local cookie = "session_cookie" - - describe("session: revocation_fail_mode", function() - local configuration = {} - local cookie_name = "session_cookie" - local test_key = "test_key" - local value = "test_data" - local session_cookie - local cookies - - local function save_session(s, cookies) - session.__set_ngx_header(cookies) - s:set(test_key, value) - local ok, err = s:save() - assert.is_true(ok) - assert.is_nil(err) - return extract_cookie(cookie_name, cookies["Set-Cookie"]) - end +for _, st in ipairs({ + "mysql", + "postgres", + "redis_cluster", + "redis_sentinel", + "dshm", +}) do + describe("Revocation tests 2 #noci", function() + local current_time + local store + local long_ttl = 60 + local short_ttl = 2 + local key = "test_key_1iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + local key1 = "test_key_2iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + local key2 = "test_key_3iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + local name = "session_cookie" + local mark = "1" + local ty = storage_type(st) + + lazy_setup(function() + local conf = { + storage = "cookie", + revocation = ty, + } + conf[ty] = storage_configs[st] + store = utils.load_storage(ty, conf) + assert.is_not_nil(store) + end) - local function open_session(session_cookie) - local s = session.new() - session.__set_ngx_var({ - ["cookie_" .. cookie_name] = session_cookie, - }) + before_each(function() + current_time = time() + end) - local ok, err = s:open() - if not ok then - return nil, err + describe("[#" .. st .. "] revocation storage: SET + GET", function() + after_each(function() + current_time = time() + store:delete(name, key, current_time) + store:delete(name, key1, current_time) + store:delete(name, key2, current_time) + end) + + it("SET: stores revocation mark and GET observes it", function() + local ok, err = store:set(name, key, mark, long_ttl, current_time) + assert.is_not_nil(ok) + assert.is_nil(err) + + local data + data, err = store:get(name, key, current_time) + assert.is_nil(err) + assert.equals(mark, data) + end) + + it("GET: missing revocation key returns not revoked", function() + local data, err = store:get(name, key1, current_time) + assert.is_nil(data) + assert.is_nil(err) + end) + + it("SET: ttl expires revocation entry", function() + local ok, err = store:set(name, key2, mark, short_ttl, current_time) + assert.is_not_nil(ok) + assert.is_nil(err) + + local data + data, err = store:get(name, key2, current_time) + assert.is_nil(err) + assert.equals(mark, data) + + sleep(short_ttl + 1) + + data, err = store:get(name, key2, time()) + assert.is_nil(data) + assert.is_nil(err) + end) + + it("SET: re-mark refreshes ttl", function() + local ok, err = store:set(name, key, mark, short_ttl, current_time) + assert.is_not_nil(ok) + assert.is_nil(err) + + sleep(1) + + ok, err = store:set(name, key, mark, long_ttl, time()) + assert.is_not_nil(ok) + assert.is_nil(err) + + sleep(short_ttl + 1) + + local data + data, err = store:get(name, key, time()) + assert.is_nil(err) + assert.equals(mark, data) + end) + end) + + describe("[#" .. st .. "] session: revocation lifecycle", function() + local cookie_name = "session_cookie" + local test_key = "test_key" + local value = "test_data" + + local function save_session(s, cookies) + session.__set_ngx_header(cookies) + s:set(test_key, value) + local ok, err = s:save() + assert.is_true(ok) + assert.is_nil(err) + return extract_cookie(cookie_name, cookies["Set-Cookie"]) end - return s - end + local function open_session(session_cookie) + local s = session.new() + session.__set_ngx_var({ + ["cookie_" .. cookie_name] = session_cookie, + }) - before_each(function() - configuration = { - cookie_name = cookie_name, - redis = redis_config, - } - session.init(configuration) + local ok, err = s:open() + if not ok then + return nil, err + end - cookies = {} - local s = session.new() - session_cookie = save_session(s, cookies) - s:close() - end) + return s + end - it("open: default fail mode allows open when redis is unreachable", function() - session.init({ - cookie_name = cookie_name, - redis = bad_redis_config, - }) - - local opened, err = open_session(session_cookie) - assert.is_true(opened) - assert.is_nil(err) - assert.equals("open", opened.revocation_fail_mode) - assert.equals(value, opened:get(test_key)) + before_each(function() + local conf = { + cookie_name = cookie_name, + storage = "cookie", + revocation = ty, + } + conf[ty] = storage_configs[st] + session.init(conf) + end) + + it("destroy: rejected cookie cannot be reopened", function() + local cookies = {} + local s = session.new() + local session_cookie = save_session(s, cookies) + assert.is_not_equal("", session_cookie) + s:close() + + local s2, err = open_session(session_cookie) + assert.is_not_nil(s2) + assert.is_nil(err) + assert.equals(value, s2:get(test_key)) + + session.__set_ngx_header(cookies) + local ok + ok, err = s2:destroy() + assert.is_true(ok) + assert.is_nil(err) + + local s3 + s3, err = open_session(session_cookie) + assert.is_nil(s3) + assert.equals("session revoked", err) + end) end) + end) +end - it("destroy: open fail mode succeeds when marking revoked fails", function() - session.init({ - cookie_name = cookie_name, - redis = bad_redis_config, - revocation_fail_mode = "open", - }) - - local s = session.new() - session.__set_ngx_var({ - ["cookie_" .. cookie_name] = session_cookie, - }) - - local ok, err = s:open() - assert.is_true(ok) - assert.is_nil(err) - - session.__set_ngx_header(cookies) - ok, err = s:destroy() - assert.is_true(ok) - assert.is_nil(err) - assert.equals("closed", s.state) - end) - it("open: closed fail mode rejects when redis is unreachable", function() - session.init({ - cookie_name = cookie_name, - redis = bad_redis_config, - revocation_fail_mode = "closed", - }) +describe("Revocation tests 2 session: revocation_fail_mode", function() + local cookie_name = "session_cookie" + local test_key = "test_key" + local value = "test_data" + local session_cookie + local cookies + + local function save_session(s, header_cookies) + session.__set_ngx_header(header_cookies) + s:set(test_key, value) + local ok, err = s:save() + assert.is_true(ok) + assert.is_nil(err) + return extract_cookie(cookie_name, header_cookies["Set-Cookie"]) + end - local opened, err = open_session(session_cookie) - assert.is_nil(opened) - assert.matches("unable to check session revocation", err) - end) + local function open_session(cookie_value) + local s = session.new() + session.__set_ngx_var({ + ["cookie_" .. cookie_name] = cookie_value, + }) - it("destroy: closed fail mode fails when marking revoked fails", function() - local s = session.new({ - revocation = { - set = function() - return nil, "connection refused" - end, - get = function() - return nil - end, - }, - revocation_fail_mode = "closed", - }) - session.__set_ngx_var({ - ["cookie_" .. cookie_name] = session_cookie, - }) - - local ok, err = s:open() - assert.is_true(ok) - assert.is_nil(err) - - session.__set_ngx_header(cookies) - ok, err = s:destroy() - assert.is_nil(ok) - assert.matches("unable to mark session revoked", err) - assert.equals("open", s.state) - end) + local ok, err = s:open() + if not ok then + return nil, err + end + + return s + end + + before_each(function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + }) + + cookies = {} + local s = session.new() + session_cookie = save_session(s, cookies) + s:close() end) - describe("session: Fields validation", function() - local configuration = {} - local cookie_name = "session_cookie" + it("open: default fail mode allows open when store is unreachable", function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + revocation = { + set = function() + return nil, "connection refused" + end, + get = function() + return nil, "connection refused" + end, + }, + }) + + local opened, err = open_session(session_cookie) + assert.is_not_nil(opened) + assert.is_nil(err) + assert.equals("open", opened.revocation_fail_mode) + assert.equals(value, opened:get(test_key)) + end) - before_each(function() - configuration = { - cookie_name = cookie_name, - redis = redis_config, - } - session.init(configuration) - end) + it("destroy: open fail mode succeeds when marking revoked fails", function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + revocation = { + set = function() + return nil, "connection refused" + end, + get = function() + return nil + end, + }, + revocation_fail_mode = "open", + }) + + local s = session.new() + session.__set_ngx_var({ + ["cookie_" .. cookie_name] = session_cookie, + }) + + local ok, err = s:open() + assert.is_true(ok) + assert.is_nil(err) + + session.__set_ngx_header(cookies) + ok, err = s:destroy() + assert.is_true(ok) + assert.is_nil(err) + assert.equals("closed", s.state) + end) + + it("open: closed fail mode rejects when store is unreachable", function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + revocation = { + set = function() + return nil, "connection refused" + end, + get = function() + return nil, "connection refused" + end, + }, + revocation_fail_mode = "closed", + }) + + local opened, err = open_session(session_cookie) + assert.is_nil(opened) + assert.matches("unable to check session revocation", err) + end) - it("new defaults revocation_fail_mode to open", function() - session.init({ - cookie_name = cookie_name, - redis = redis_config, - }) + it("destroy: closed fail mode fails when marking revoked fails", function() + local s = session.new({ + revocation = { + set = function() + return nil, "connection refused" + end, + get = function() + return nil + end, + }, + revocation_fail_mode = "closed", + }) + session.__set_ngx_var({ + ["cookie_" .. cookie_name] = session_cookie, + }) + + local ok, err = s:open() + assert.is_true(ok) + assert.is_nil(err) + + session.__set_ngx_header(cookies) + ok, err = s:destroy() + assert.is_nil(ok) + assert.matches("unable to mark session revoked", err) + assert.equals("open", s.state) + end) +end) - local s = session.new() - assert.equals("open", s.revocation_fail_mode) - end) - it("new rejects redis storage with revocation mode", function() - local ok, err = pcall(session.new, { - storage = "redis", - redis = { - host = redis_config.host, - password = redis_config.password, - mode = "revocation", - }, - }) - assert.is_false(ok) - assert.matches("invalid redis mode for session storage", err) - end) +describe("Revocation tests 2 session: Fields validation", function() + local cookie_name = "session_cookie" - it("new validates revocation configuration", function() - local ok, err = pcall(session.new, { - revocation = 123, - }) - assert.is_false(ok) - assert.matches("invalid session revocation", err) - end) + it("new defaults revocation_fail_mode to open", function() + session.init({ + cookie_name = cookie_name, + storage = "cookie", + }) + + local s = session.new() + assert.equals("open", s.revocation_fail_mode) end) - describe("[#redis] revocation: failures", function() - it("SET: connection failure returns error", function() - local bad_store = redis_storage.new(bad_redis_config) + it("new requires an explicit revocation storage name", function() + local ok, err = pcall(session.new, { + revocation = true, + }) + assert.is_false(ok) + assert.matches("invalid session revocation", err) + end) - local ok, err = bad_store:set(cookie, encode_base64url(id), "1", long_ttl, ngx.time()) - assert.is_nil(ok) - assert.is_not_nil(err) - end) + it("new validates revocation configuration", function() + local ok, err = pcall(session.new, { + revocation = 123, + }) + assert.is_false(ok) + assert.matches("invalid session revocation", err) + end) - it("GET: connection failure returns error", function() - local bad_store = redis_storage.new(bad_redis_config) + it("new rejects a store table without set and get", function() + local ok, err = pcall(session.new, { + revocation = { + set = function() end, + }, + }) + assert.is_false(ok) + assert.matches("invalid session revocation", err) + end) - local data, err = bad_store:get(cookie, encode_base64url(id1), ngx.time()) - assert.is_nil(data) - assert.is_not_nil(err) - end) + it("new rejects an invalid fail mode", function() + local ok, err = pcall(session.new, { + revocation_fail_mode = "deny", + }) + assert.is_false(ok) + assert.matches("invalid revocation fail mode", err) end) end) From 7e76f09f067dd97a299121d2aa2e170ebeac5107 Mon Sep 17 00:00:00 2001 From: Austin Hockenberry Date: Mon, 10 Aug 2026 13:50:03 -0400 Subject: [PATCH 2/2] Fix test --- spec/06-revocation-1_spec.lua | 21 +++++++-------------- spec/07-revocation-2_spec.lua | 21 +++++++-------------- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/spec/06-revocation-1_spec.lua b/spec/06-revocation-1_spec.lua index 3fdb24a7..4fdcbafb 100644 --- a/spec/06-revocation-1_spec.lua +++ b/spec/06-revocation-1_spec.lua @@ -89,12 +89,10 @@ for _, st in ipairs({ end) it("SET: stores revocation mark and GET observes it", function() - local ok, err = store:set(name, key, mark, long_ttl, current_time) + local ok = store:set(name, key, mark, long_ttl, current_time) assert.is_not_nil(ok) - assert.is_nil(err) - local data - data, err = store:get(name, key, current_time) + local data, err = store:get(name, key, current_time) assert.is_nil(err) assert.equals(mark, data) end) @@ -106,12 +104,10 @@ for _, st in ipairs({ end) it("SET: ttl expires revocation entry", function() - local ok, err = store:set(name, key2, mark, short_ttl, current_time) + local ok = store:set(name, key2, mark, short_ttl, current_time) assert.is_not_nil(ok) - assert.is_nil(err) - local data - data, err = store:get(name, key2, current_time) + local data, err = store:get(name, key2, current_time) assert.is_nil(err) assert.equals(mark, data) @@ -123,20 +119,17 @@ for _, st in ipairs({ end) it("SET: re-mark refreshes ttl", function() - local ok, err = store:set(name, key, mark, short_ttl, current_time) + local ok = store:set(name, key, mark, short_ttl, current_time) assert.is_not_nil(ok) - assert.is_nil(err) sleep(1) - ok, err = store:set(name, key, mark, long_ttl, time()) + ok = store:set(name, key, mark, long_ttl, time()) assert.is_not_nil(ok) - assert.is_nil(err) sleep(short_ttl + 1) - local data - data, err = store:get(name, key, time()) + local data, err = store:get(name, key, time()) assert.is_nil(err) assert.equals(mark, data) end) diff --git a/spec/07-revocation-2_spec.lua b/spec/07-revocation-2_spec.lua index 843f0307..87e5bd8d 100644 --- a/spec/07-revocation-2_spec.lua +++ b/spec/07-revocation-2_spec.lua @@ -118,12 +118,10 @@ for _, st in ipairs({ end) it("SET: stores revocation mark and GET observes it", function() - local ok, err = store:set(name, key, mark, long_ttl, current_time) + local ok = store:set(name, key, mark, long_ttl, current_time) assert.is_not_nil(ok) - assert.is_nil(err) - local data - data, err = store:get(name, key, current_time) + local data, err = store:get(name, key, current_time) assert.is_nil(err) assert.equals(mark, data) end) @@ -135,12 +133,10 @@ for _, st in ipairs({ end) it("SET: ttl expires revocation entry", function() - local ok, err = store:set(name, key2, mark, short_ttl, current_time) + local ok = store:set(name, key2, mark, short_ttl, current_time) assert.is_not_nil(ok) - assert.is_nil(err) - local data - data, err = store:get(name, key2, current_time) + local data, err = store:get(name, key2, current_time) assert.is_nil(err) assert.equals(mark, data) @@ -152,20 +148,17 @@ for _, st in ipairs({ end) it("SET: re-mark refreshes ttl", function() - local ok, err = store:set(name, key, mark, short_ttl, current_time) + local ok = store:set(name, key, mark, short_ttl, current_time) assert.is_not_nil(ok) - assert.is_nil(err) sleep(1) - ok, err = store:set(name, key, mark, long_ttl, time()) + ok = store:set(name, key, mark, long_ttl, time()) assert.is_not_nil(ok) - assert.is_nil(err) sleep(short_ttl + 1) - local data - data, err = store:get(name, key, time()) + local data, err = store:get(name, key, time()) assert.is_nil(err) assert.equals(mark, data) end)