Skip to content

Commit f346eb4

Browse files
cycomacheadclaude
authored andcommitted
feat: implement remember_token for session management and harden security
- Add `remember_token` to users table to decouple sessions from usernames. - Implement server-side session invalidation via token rotation on password change, reset, email/username updates, and account banning. - Add `all_sessions=true` flag to logout and a new `force_logout` admin API. - Refactor auth logic into `establish_session` with JIT token migration for legacy cookies. - Fix "remember me" logic by removing incorrect session clearing in `/api/v1/init`. - Enhance clickjacking protection by adding `X-Frame-Options: DENY` and `frame-ancestors 'none'` to sensitive user forms. - Fix a typo in `persist_session` cookie handling. Co-authored-by: Claude Code <noreply@anthropic.com>
1 parent fdd9f3c commit f346eb4

8 files changed

Lines changed: 231 additions & 43 deletions

File tree

api.lua

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,13 @@ app:match(api_route('init'), respond_to({
8888
400)
8989
end),
9090
POST = capture_errors(function (self)
91-
if not self.session.username or
92-
(self.session.username and
93-
self.session.persist_session == 'false') then
94-
self.session.username = ''
95-
end
91+
-- Historically /init wiped any session whose persist_session flag
92+
-- was 'false', which forced users who hadn't checked "remember me"
93+
-- out every time the editor reloaded — defeating the whole point
94+
-- of session cookies. Cookie persistence is now driven entirely
95+
-- by the absence/presence of Max-Age in app.cookie_attributes:
96+
-- the browser handles "should this die on browser close?" itself,
97+
-- and we don't second-guess it here.
9698
return okResponse()
9799
end)
98100
}))
@@ -182,6 +184,10 @@ app:match(api_route('users/:username/become'), respond_to({
182184
POST = UserController.become
183185
}))
184186

187+
app:match(api_route('users/:username/force_logout'), respond_to({
188+
POST = UserController.force_logout
189+
}))
190+
185191
app:match(api_route('users/:username/verify'), respond_to({
186192
POST = UserController.verify
187193
}))

app.lua

Lines changed: 78 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,83 @@ end
8080
require 'models'
8181
require 'responses'
8282

83+
-- Resolve the user this request belongs to, based on the session cookie.
84+
--
85+
-- Priority:
86+
-- 1. session.remember_token: the source of truth. Look up by token; if
87+
-- it doesn't match a user the cookie is stale (token rotated by a
88+
-- password change, "log out all sessions", etc.) and we treat the
89+
-- request as logged-out.
90+
-- 2. JIT-migrate legacy cookies. A cookie minted before remember_token
91+
-- existed will only carry session.username. We accept it ONLY if the
92+
-- user's record has no remember_token yet — i.e. nobody has logged
93+
-- in for this account since the column was added. We then issue a
94+
-- token and pin it to the cookie.
95+
-- 3. If the user record DOES already have a remember_token, refuse the
96+
-- tokenless cookie. Accepting it would defeat the whole point of
97+
-- server-side invalidation: the legitimate owner has logged in
98+
-- somewhere and rotated the token; the bare-username cookie is
99+
-- either ours-but-stale or stolen, and either way it must lose.
100+
local function resolve_current_user(self)
101+
local Users = package.loaded.Users
102+
local remember_token = self.session.remember_token
103+
104+
if remember_token and remember_token ~= '' then
105+
return Users:find({ remember_token = remember_token })
106+
end
107+
108+
local username = self.session.username
109+
if not username or username == '' then return nil end
110+
111+
local user = Users:find({ username = username })
112+
if not user then return nil end
113+
114+
if user.remember_token and user.remember_token ~= '' then
115+
return nil
116+
end
117+
118+
self.session.remember_token = user:rotate_remember_token()
119+
return user
120+
end
121+
122+
-- Track a "session" (a 4-hour window of activity) for the user. Called
123+
-- after a request has been authenticated and current_user is set.
124+
local function track_session_activity(self)
125+
local should_count_session = false
126+
if not self.current_user.last_session_at then
127+
should_count_session = true
128+
else
129+
local hours_since = package.loaded.db.select(
130+
"extract(epoch from now() - ?::timestamptz) / 3600 as hours",
131+
self.current_user.last_session_at
132+
)[1]
133+
if hours_since.hours >= 4 then
134+
should_count_session = true
135+
end
136+
end
137+
if should_count_session then
138+
self.current_user:update({
139+
last_session_at = package.loaded.db.format_date(),
140+
session_count = self.current_user.session_count + 1
141+
})
142+
end
143+
end
144+
145+
-- Single entry point for "who is the user on this request?"
146+
-- Mutates self.current_user and the session in place.
147+
local function establish_session(self)
148+
self.current_user = resolve_current_user(self)
149+
if self.current_user then
150+
self.session.username = self.current_user.username
151+
self.session.last_access_at = date(true):fmt('${http}')
152+
track_session_activity(self)
153+
else
154+
self.session.username = ''
155+
self.session.remember_token = nil
156+
self.session.user_id = nil
157+
end
158+
end
159+
83160
app.cookie_attributes = function (self)
84161
-- Cookies are 'session cookies' unless they have an expiration date.
85162
-- Cookies have a Max-Age of 35 days, because this is continually reset
@@ -246,37 +323,7 @@ app:before_filter(function (self)
246323
end
247324
end
248325

249-
if self.session.username and self.session.username ~= '' then
250-
self.current_user =
251-
package.loaded.Users:find({ username = self.session.username })
252-
self.session.last_access_at = date(true):fmt('${http}')
253-
254-
-- Track distinct sessions: increment session_count and update
255-
-- last_session_at at most once every 4 hours.
256-
if self.current_user then
257-
local should_count_session = false
258-
if not self.current_user.last_session_at then
259-
should_count_session = true
260-
else
261-
local hours_since = package.loaded.db.select(
262-
"extract(epoch from now() - ?::timestamptz) / 3600 as hours",
263-
self.current_user.last_session_at
264-
)[1]
265-
if hours_since.hours >= 4 then
266-
should_count_session = true
267-
end
268-
end
269-
if should_count_session then
270-
self.current_user:update({
271-
last_session_at = package.loaded.db.format_date(),
272-
session_count = self.current_user.session_count + 1
273-
})
274-
end
275-
end
276-
else
277-
self.session.username = ''
278-
self.current_user = nil
279-
end
326+
establish_session(self)
280327

281328
if self.params.matchtext then
282329
self.params.matchtext = '%' .. self.params.matchtext .. '%'

controllers/user.lua

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ local validate_token = validations.validate_token
4949
local utils = require('lib.util')
5050
local escape_html = utils.escape_html
5151

52+
-- Rotate the target user's remember_token (kicking every other session
53+
-- for that account) and, if the target IS the requestor, slide the new
54+
-- token into the current cookie so the requestor stays logged in.
55+
local function rotate_token_and_sync_session(self, user)
56+
if not user then return end
57+
local new_token = user:rotate_remember_token()
58+
if self.current_user and user.id == self.current_user.id then
59+
self.session.remember_token = new_token
60+
end
61+
end
62+
5263
UserController = {
5364
run_query = function (self, query)
5465
if not self.params.page_number then self.params.page_number = 1 end
@@ -180,8 +191,9 @@ UserController = {
180191
token:delete()
181192
end
182193

183-
-- TODO: Create and store a remember token
184194
self.session.username = self.queried_user.username
195+
self.session.remember_token =
196+
self.queried_user:ensure_remember_token()
185197
self.session.persist_session = tostring(self.params.persist)
186198
self.queried_user:update({
187199
last_login_at = db.format_date()
@@ -204,12 +216,28 @@ UserController = {
204216
-- Admins can log in as other people
205217
assert_admin(self, err.wrong_password)
206218
self.session.username = self.queried_user.username
219+
self.session.remember_token =
220+
self.queried_user:ensure_remember_token()
207221
return jsonResponse({ redirect = self:build_url('/') })
208222
end
209223
end),
224+
-- POST /logout
225+
-- Optional param: all_sessions=true rotates the user's remember_token
226+
-- before clearing the cookie, which invalidates every other session
227+
-- that was authenticated with the previous token.
210228
logout = capture_errors(function (self)
229+
if self.params.all_sessions == true
230+
or tostring(self.params.all_sessions) == 'true' then
231+
local user = self.current_user
232+
or (self.session.remember_token and self.session.remember_token ~= ''
233+
and Users:find({ remember_token = self.session.remember_token }))
234+
if user then user:rotate_remember_token() end
235+
end
211236
self.session.username = ''
212237
self.session.user_id = nil
238+
self.session.remember_token = nil
239+
self.session.impersonator = nil
240+
self.session.impersonator_token = nil
213241
self.session.persist_session = 'false'
214242
return jsonResponse({
215243
redirect = (self.params.redirect or self:build_url('/'))
@@ -258,6 +286,10 @@ UserController = {
258286
)
259287
end
260288

289+
-- Email is the recovery channel — if it changed under an attacker,
290+
-- every existing session for this account must die.
291+
rotate_token_and_sync_session(self, user)
292+
261293
return jsonResponse({
262294
title = 'Email changed',
263295
message = 'Email has been updated.',
@@ -306,6 +338,10 @@ UserController = {
306338
)
307339
end
308340

341+
-- A username change is a strong account-takeover signal; rotate
342+
-- the token so any other live session for this account is dropped.
343+
rotate_token_and_sync_session(self, user)
344+
309345
return jsonResponse({
310346
title = 'Username changed',
311347
message = 'Username has been updated.',
@@ -330,7 +366,13 @@ UserController = {
330366
password = bcrypt_hash(self.params.new_password),
331367
password_version = PASSWORD_VERSION_BCRYPT,
332368
salt = '',
369+
password_changed_at = db.raw('now()'),
370+
updated_at = db.raw('now()')
333371
})
372+
-- Rotate the remember_token so every other device gets logged out.
373+
-- Refresh the current session's token so the caller stays logged in.
374+
self.session.remember_token =
375+
self.current_user:rotate_remember_token()
334376
return jsonResponse({
335377
title = 'Password changed',
336378
message = 'Your password has been changed.',
@@ -423,6 +465,7 @@ UserController = {
423465
-- we've deleted ourselves, let's log out
424466
self.session.username = ''
425467
self.session.user_id = nil
468+
self.session.remember_token = nil
426469
self.session.persist_session = 'false'
427470
end
428471
return jsonResponse({
@@ -690,9 +733,17 @@ UserController = {
690733
Users.roles[self.queried_user.role] then
691734
yield_error(err.auth)
692735
else
736+
-- Stash the admin's identity so unbecome can restore it.
737+
-- We save both username and remember_token so the admin's
738+
-- session keeps working even if we swap the cookie's token
739+
-- to the target user's.
693740
self.session.impersonator = self.current_user.username
741+
self.session.impersonator_token =
742+
self.session.remember_token
694743
self.current_user = self.queried_user
695744
self.session.username = self.queried_user.username
745+
self.session.remember_token =
746+
self.queried_user:ensure_remember_token()
696747
end
697748
end
698749
return jsonResponse({
@@ -704,9 +755,11 @@ UserController = {
704755
unbecome = capture_errors(function (self)
705756
if self.session.impersonator then
706757
self.session.username = self.session.impersonator
758+
self.session.remember_token = self.session.impersonator_token
707759
self.current_user =
708760
Users:find({ username = self.session.impersonator})
709761
self.session.impersonator = nil
762+
self.session.impersonator_token = nil
710763
return jsonResponse({
711764
message = 'You are now ' .. self.session.username .. ' again',
712765
title = 'Unimpersonation',
@@ -732,6 +785,11 @@ UserController = {
732785
if self.queried_user then
733786
assert_can_set_role(self, self.params.role)
734787
self.queried_user:update({ role = self.params.role })
788+
-- A banned user must lose every live session — otherwise an
789+
-- attacker who keeps an existing cookie keeps their access.
790+
if self.params.role == 'banned' then
791+
rotate_token_and_sync_session(self, self.queried_user)
792+
end
735793
end
736794
return jsonResponse({
737795
message =
@@ -741,6 +799,20 @@ UserController = {
741799
redirect = self.queried_user:url_for('site')
742800
})
743801
end),
802+
-- Admin/moderator action: rotate a user's remember_token, which
803+
-- invalidates every active session for that account on every device.
804+
-- Use this for suspected account takeover or as a manual "kick" tool.
805+
force_logout = capture_errors(function (self)
806+
assert_min_role(self, 'moderator')
807+
assert_user_exists(self)
808+
rotate_token_and_sync_session(self, self.queried_user)
809+
return jsonResponse({
810+
title = 'Sessions terminated',
811+
message = 'All sessions for ' ..
812+
self.queried_user.username .. ' have been terminated.',
813+
redirect = self.queried_user:url_for('site')
814+
})
815+
end),
744816
set_teacher = capture_errors(function (self)
745817
assert_min_role(self, 'moderator')
746818
if self.queried_user then
@@ -861,6 +933,7 @@ app:match(
861933
self.username = escape_html(token.username)
862934
self.csrf_token = csrf.generate_token(self)
863935
self.min_password_length = Users.MIN_PASSWORD_LENGTH
936+
utils.set_no_frame_headers(self)
864937
return { render = 'password_reset' }
865938
end
866939
),
@@ -869,6 +942,7 @@ app:match(
869942
-- Step 2: User submitted the new password form.
870943
-- Validate CSRF token first, without consuming
871944
-- the reset token.
945+
utils.set_no_frame_headers(self)
872946
local valid, csrf_err = csrf.validate_token(self)
873947
if not valid then
874948
return html_message_page(
@@ -906,13 +980,23 @@ app:match(
906980
'password_reset',
907981
function (user)
908982
-- Store as native bcrypt (version 2) on reset.
983+
-- Rotate the remember_token in the same UPDATE so
984+
-- every existing session for this user is logged
985+
-- out — including any session the attacker may
986+
-- still hold. The user re-logs in fresh.
909987
user:update({
910988
password = bcrypt_hash(prehash),
911989
password_version = PASSWORD_VERSION_BCRYPT,
912990
salt = '',
913991
password_changed_at = db.raw('now()'),
914-
updated_at = db.raw('now()')
992+
updated_at = db.raw('now()'),
993+
remember_token = secure_token()
915994
})
995+
-- Drop any session this browser still has so the
996+
-- "log in" link below is not a no-op.
997+
self.session.username = ''
998+
self.session.remember_token = nil
999+
self.session.user_id = nil
9161000
send_mail(
9171001
user.email,
9181002
mail_subjects.password_changed ..

db/schema.sql

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ CREATE TABLE public.users (
135135
password_changed_at timestamp with time zone,
136136
updated_at timestamp with time zone,
137137
last_session_at timestamp with time zone,
138-
password_version integer DEFAULT 0 NOT NULL
138+
password_version integer DEFAULT 0 NOT NULL,
139+
remember_token text
139140
);
140141

141142

@@ -164,7 +165,8 @@ CREATE VIEW public.active_users AS
164165
password_changed_at,
165166
updated_at,
166167
last_session_at,
167-
password_version
168+
password_version,
169+
remember_token
168170
FROM public.users
169171
WHERE (deleted IS NULL);
170172

@@ -610,6 +612,14 @@ ALTER TABLE ONLY public.users
610612
ADD CONSTRAINT users_unique_email_key UNIQUE (unique_email);
611613

612614

615+
--
616+
-- Name: users users_remember_token_key; Type: CONSTRAINT; Schema: public; Owner: -
617+
--
618+
619+
ALTER TABLE ONLY public.users
620+
ADD CONSTRAINT users_remember_token_key UNIQUE (remember_token);
621+
622+
613623
--
614624
-- Name: tokens value_pkey; Type: CONSTRAINT; Schema: public; Owner: -
615625
--

0 commit comments

Comments
 (0)