mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(auth): keep demo visitors out of each other's sessions
On a demo instance every visitor signs in as the one shared demo account, so its sessions are other people's browsers. The session routes had no demo check, unlike the password, MFA and account changes, so any visitor could read the User-Agent of everyone else signed in and sign them all out with revoke-others. For the shared demo account in demo mode, GET /api/auth/sessions now lists only the session the request came with, and DELETE /api/auth/sessions/:id and POST /api/auth/sessions/revoke-others answer 403 "Sessions cannot be ended in demo mode." without ending anything. Logout still ends the visitor's own session. Other accounts on a demo instance keep the full routes. No MCP tool covers sessions, so there is no parallel tool to change. The demo sign-in now records the device's User-Agent on its session like every other sign-in, which the session list needs to tell the visitor's own entry apart.
This commit is contained in:
@@ -58,7 +58,7 @@ export class AuthPublicController {
|
||||
@Public('issues a session for the demo account; there is nothing to authenticate yet')
|
||||
@HttpCode(200)
|
||||
async demoLogin(@Req() req: Request, @Res({ passthrough: true }) res: Response) {
|
||||
const result = await this.auth.demoLogin();
|
||||
const result = await this.auth.demoLogin(sessionClientFrom(req));
|
||||
if (result.error) {
|
||||
throw new HttpException({ error: result.error }, result.status!);
|
||||
}
|
||||
|
||||
@@ -415,13 +415,13 @@ export class AuthService {
|
||||
// Auth: register, login, demo
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
async demoLogin(): Promise<{ error?: string; status?: number; token?: string; user?: Record<string, unknown> }> {
|
||||
async demoLogin(client?: SessionClient): Promise<{ error?: string; status?: number; token?: string; user?: Record<string, unknown> }> {
|
||||
if (!readEnv().demo.enabled) {
|
||||
return { error: 'Not found', status: 404 };
|
||||
}
|
||||
const user = await this.usersRepo.findByEmailExact(DEMO_EMAIL_PRIMARY);
|
||||
if (!user) return { error: 'Demo user not found', status: 500 };
|
||||
const token = await this.generateToken(user);
|
||||
const token = await this.generateToken(user, undefined, client);
|
||||
const safe = stripUserForClient(toClientUser(user)) as Record<string, unknown>;
|
||||
return { token, user: { ...safe, avatar_url: avatarUrl(user) } };
|
||||
}
|
||||
|
||||
@@ -12,6 +12,8 @@ import { clearAuthCookie } from '../common/cookie';
|
||||
import { getClientIp } from '../audit/client-ip';
|
||||
import { AuditService } from '../audit/audit.service';
|
||||
import { SessionsService } from '../sessions/sessions.service';
|
||||
import { isDemoEmail } from '../common/demo';
|
||||
import { readEnv } from '../../app-config';
|
||||
import type { User } from '../../types';
|
||||
import { CurrentUser } from './current-user.decorator';
|
||||
import { currentSessionId } from './jwt-verify';
|
||||
@@ -27,6 +29,11 @@ import { JwtAuthGuard } from './jwt-auth.guard';
|
||||
* current session (`current_tracked: false`). Signing out the others then
|
||||
* ends every tracked session, while that token itself runs on to its expiry.
|
||||
*
|
||||
* On a demo instance every visitor signs in as the one shared demo account,
|
||||
* so its sessions are other people's browsers: the list shows the caller only
|
||||
* their own session, and ending sessions is refused with the 403 the other
|
||||
* account changes answer in demo mode.
|
||||
*
|
||||
* No MCP tool mirrors these routes: MCP exposes no account or session tools,
|
||||
* and a session is a browser credential an assistant has no business ending.
|
||||
*/
|
||||
@@ -41,13 +48,18 @@ export class SessionsController {
|
||||
@Get()
|
||||
async list(@CurrentUser() user: User, @Req() req: Request): Promise<UserSessionListResponse> {
|
||||
const current = currentSessionId(req);
|
||||
return { sessions: await this.sessions.list(user.id, current), current_tracked: current !== undefined };
|
||||
const sessions = await this.sessions.list(user.id, current);
|
||||
return {
|
||||
sessions: isSharedDemoAccount(user) ? sessions.filter((session) => session.current) : sessions,
|
||||
current_tracked: current !== undefined,
|
||||
};
|
||||
}
|
||||
|
||||
// Static sub-route before the `:id` one.
|
||||
@Post('revoke-others')
|
||||
@HttpCode(200)
|
||||
async revokeOthers(@CurrentUser() user: User, @Req() req: Request): Promise<UserSessionRevokeOthersResponse> {
|
||||
refuseForSharedDemoAccount(user);
|
||||
const revoked = await this.sessions.revokeAll(user.id, currentSessionId(req));
|
||||
await this.audit.writeAudit({ userId: user.id, action: 'user.sessions_revoke_others', ip: getClientIp(req), details: { revoked } });
|
||||
return { success: true, revoked };
|
||||
@@ -60,6 +72,7 @@ export class SessionsController {
|
||||
@Req() req: Request,
|
||||
@Res({ passthrough: true }) res: Response,
|
||||
): Promise<UserSessionRevokeResponse> {
|
||||
refuseForSharedDemoAccount(user);
|
||||
if (!(await this.sessions.revoke(user.id, params.id))) {
|
||||
throw new HttpException({ error: 'Session not found' }, 404);
|
||||
}
|
||||
@@ -69,3 +82,14 @@ export class SessionsController {
|
||||
return { success: true };
|
||||
}
|
||||
}
|
||||
|
||||
/** The demo account every visitor of a demo instance shares. */
|
||||
function isSharedDemoAccount(user: User): boolean {
|
||||
return readEnv().demo.enabled && isDemoEmail(user.email);
|
||||
}
|
||||
|
||||
function refuseForSharedDemoAccount(user: User): void {
|
||||
if (isSharedDemoAccount(user)) {
|
||||
throw new HttpException({ error: 'Sessions cannot be ended in demo mode.' }, 403);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -229,4 +229,35 @@ describe('Sessions e2e (sign-in sessions that can be ended)', () => {
|
||||
expect(list.body.current_tracked).toBe(true);
|
||||
expect(list.body.sessions).toEqual([expect.objectContaining({ id: sessionIdOf(renewed), user_agent: 'Renewed', current: true })]);
|
||||
}, 10000);
|
||||
|
||||
it("on a demo instance, visitors of the shared demo account neither see nor end each other's sessions", async () => {
|
||||
createUser(db as never, { username: 'demo', email: 'demo@trek.app' });
|
||||
vi.stubEnv('DEMO_MODE', 'true');
|
||||
try {
|
||||
const visit = async (device: string) => {
|
||||
const res = await request(server).post('/api/auth/demo-login').set('User-Agent', device);
|
||||
expect(res.status).toBe(200);
|
||||
const cookie = (res.headers['set-cookie'] as unknown as string[]).find((c) => c.startsWith('trek_session='))!;
|
||||
return /^(trek_session=[^;]+)/.exec(cookie)![1];
|
||||
};
|
||||
const first = await visit('First visitor');
|
||||
const second = await visit('Second visitor');
|
||||
|
||||
const list = await request(server).get('/api/auth/sessions').set('Cookie', first);
|
||||
expect(list.status).toBe(200);
|
||||
expect(list.body.sessions).toEqual([expect.objectContaining({ id: sessionIdOf(first), user_agent: 'First visitor', current: true })]);
|
||||
|
||||
const one = await request(server).delete(`/api/auth/sessions/${sessionIdOf(second)}`).set('Cookie', first);
|
||||
expect(one.status).toBe(403);
|
||||
expect(one.body).toEqual({ error: 'Sessions cannot be ended in demo mode.' });
|
||||
const others = await request(server).post('/api/auth/sessions/revoke-others').set('Cookie', first);
|
||||
expect(others.status).toBe(403);
|
||||
expect(others.body).toEqual({ error: 'Sessions cannot be ended in demo mode.' });
|
||||
|
||||
expect((await me(second)).status).toBe(200);
|
||||
expect((await me(first)).status).toBe(200);
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
}
|
||||
}, 15000);
|
||||
});
|
||||
|
||||
@@ -92,9 +92,12 @@ describe('AuthPublicController', () => {
|
||||
it('demo-login maps error, else sets the cookie + returns token/user', async () => {
|
||||
expect(await thrownAsync(() => apc(asvc({ demoLogin: vi.fn().mockReturnValue({ error: 'Demo disabled', status: 403 }) } as Partial<AuthService>), rl()).demoLogin(req, res))).toEqual({ status: 403, body: { error: 'Demo disabled' } });
|
||||
const setAuthCookie = vi.fn();
|
||||
const c = apc(asvc({ demoLogin: vi.fn().mockReturnValue({ token: 'tk', user }), setAuthCookie } as Partial<AuthService>), rl());
|
||||
expect(await c.demoLogin(req, res)).toEqual({ token: 'tk', user });
|
||||
expect(setAuthCookie).toHaveBeenCalledWith(res, 'tk', req);
|
||||
const demoLogin = vi.fn().mockReturnValue({ token: 'tk', user });
|
||||
const c = apc(asvc({ demoLogin, setAuthCookie } as Partial<AuthService>), rl());
|
||||
expect(await c.demoLogin({ ...req, headers: { 'user-agent': 'Visitor' } } as Request, res)).toEqual({ token: 'tk', user });
|
||||
expect(setAuthCookie).toHaveBeenCalledWith(res, 'tk', expect.objectContaining({ ip: '9.9.9.9' }));
|
||||
// The session records the device like every other sign-in does.
|
||||
expect(demoLogin).toHaveBeenCalledWith({ userAgent: 'Visitor' });
|
||||
});
|
||||
|
||||
it('register audits + sets cookie; maps error', async () => {
|
||||
|
||||
@@ -783,10 +783,11 @@ describe('demoLogin', () => {
|
||||
vi.stubEnv('DEMO_MODE', 'true');
|
||||
expect(await svc.demoLogin()).toEqual({ error: 'Demo user not found', status: 500 });
|
||||
// demoLogin looks up DEMO_EMAIL_PRIMARY specifically (not any demo alias).
|
||||
createUser(testDb, { email: 'demo@trek.app' });
|
||||
const result = await svc.demoLogin();
|
||||
const { user } = createUser(testDb, { email: 'demo@trek.app' });
|
||||
const result = await svc.demoLogin({ userAgent: 'Demo visitor' });
|
||||
expect(typeof result.token).toBe('string');
|
||||
expect(result.user).not.toHaveProperty('password_hash');
|
||||
expect(sessionRows(testDb, user.id).map((r) => r.user_agent)).toEqual(['Demo visitor']);
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it, vi, beforeEach } from 'vitest';
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { HttpException } from '@nestjs/common';
|
||||
import jwt from 'jsonwebtoken';
|
||||
import type { Request, Response } from 'express';
|
||||
@@ -33,6 +33,7 @@ function controller(o: Partial<Record<'list' | 'revoke' | 'revokeAll', ReturnTyp
|
||||
}
|
||||
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
afterEach(() => vi.unstubAllEnvs());
|
||||
|
||||
describe('SessionsController', () => {
|
||||
it('GET lists the sessions with the current one flagged', async () => {
|
||||
@@ -86,3 +87,47 @@ describe('SessionsController', () => {
|
||||
expect(writeAudit).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('SessionsController on a demo instance', () => {
|
||||
const demo = { id: 3, username: 'demo', email: 'demo@trek.app', role: 'user' } as User;
|
||||
|
||||
it('GET shows the shared demo account only the session it was called with', async () => {
|
||||
vi.stubEnv('DEMO_MODE', 'true');
|
||||
const mine = { id: SID, created_at: 'a', last_seen_at: 'b', expires_at: 'c', user_agent: 'Mine', current: true };
|
||||
const theirs = { id: OTHER, created_at: 'a', last_seen_at: 'b', expires_at: 'c', user_agent: 'Another visitor', current: false };
|
||||
const { c } = controller({ list: vi.fn().mockResolvedValue([theirs, mine]) });
|
||||
|
||||
expect(await c.list(demo, reqWith(SID))).toEqual({ sessions: [mine], current_tracked: true });
|
||||
expect(await c.list(demo, reqWith())).toEqual({ sessions: [], current_tracked: false });
|
||||
});
|
||||
|
||||
it('DELETE and revoke-others answer 403 for the shared demo account and end nothing', async () => {
|
||||
vi.stubEnv('DEMO_MODE', 'true');
|
||||
const { c, sessions } = controller({ revoke: vi.fn(), revokeAll: vi.fn() });
|
||||
|
||||
for (const call of [() => c.revokeOthers(demo, reqWith(SID)), () => c.revoke(demo, { id: OTHER }, reqWith(SID), resStub())]) {
|
||||
const err = await call().catch((e: unknown) => e);
|
||||
expect(err).toBeInstanceOf(HttpException);
|
||||
expect((err as HttpException).getStatus()).toBe(403);
|
||||
expect((err as HttpException).getResponse()).toEqual({ error: 'Sessions cannot be ended in demo mode.' });
|
||||
}
|
||||
expect(sessions.revoke).not.toHaveBeenCalled();
|
||||
expect(sessions.revokeAll).not.toHaveBeenCalled();
|
||||
expect(writeAudit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('another account on a demo instance, and the demo address without demo mode, keep the full routes', async () => {
|
||||
vi.stubEnv('DEMO_MODE', 'true');
|
||||
const listed = [
|
||||
{ id: SID, created_at: 'a', last_seen_at: 'b', expires_at: 'c', user_agent: null, current: true },
|
||||
{ id: OTHER, created_at: 'a', last_seen_at: 'b', expires_at: 'c', user_agent: null, current: false },
|
||||
];
|
||||
const { c } = controller({ list: vi.fn().mockResolvedValue(listed), revokeAll: vi.fn().mockResolvedValue(1) });
|
||||
expect((await c.list(user, reqWith(SID))).sessions).toHaveLength(2);
|
||||
expect(await c.revokeOthers(user, reqWith(SID))).toEqual({ success: true, revoked: 1 });
|
||||
|
||||
vi.stubEnv('DEMO_MODE', '');
|
||||
expect((await c.list(demo, reqWith(SID))).sessions).toHaveLength(2);
|
||||
expect(await c.revokeOthers(demo, reqWith(SID))).toEqual({ success: true, revoked: 1 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -62,6 +62,7 @@ On first start with demo mode active, TREK seeds three example trips (Tokyo & Ky
|
||||
|
||||
- Demo mode is not for production use with real user data. The hourly reset deletes all visitor-created content.
|
||||
- All demo visitors share a single account — there is no isolation between sessions.
|
||||
- Because the demo account is shared, its session list (`GET /api/auth/sessions`) shows each visitor only their own session, and ending sessions (`DELETE /api/auth/sessions/{id}`, `POST /api/auth/sessions/revoke-others`) answers 403 `Sessions cannot be ended in demo mode.`, so one visitor cannot see or sign out the others. Logging out still ends the visitor's own session.
|
||||
- File uploads (photos, documents, trip covers, avatars) are disabled for the demo user.
|
||||
|
||||
## See also
|
||||
|
||||
@@ -50,7 +50,7 @@ A session ends when:
|
||||
| Admin clears a user's two-factor authentication | Every session of that user. |
|
||||
| Account deletion | Every session, with the account. |
|
||||
|
||||
`GET /api/auth/sessions` lists the active sessions of your account, most recently used first, with when each started, when it was last used (refreshed at most every few minutes), when it expires, the browser's User-Agent at sign-in (cut to 256 characters) and which one is the current one. No IP address is stored. The settings screen does not show this list yet; the routes are there for it and for API clients.
|
||||
`GET /api/auth/sessions` lists the active sessions of your account, most recently used first, with when each started, when it was last used (refreshed at most every few minutes), when it expires, the browser's User-Agent at sign-in (cut to 256 characters) and which one is the current one. No IP address is stored. The settings screen does not show this list yet; the routes are there for it and for API clients. On a demo instance the shared demo account sees only its own session in the list and cannot end sessions (403), since its sessions are other visitors' browsers.
|
||||
|
||||
Sessions issued before this was introduced carry no session id. They keep working until they expire (or until the password changes, which ends them through the password version), are not listed, and are not ended by **revoke-others**. Once such a session passes half its lifetime, the sliding renewal replaces it with a tracked one. Expired and ended session rows are removed by a nightly job.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user