Skip to content

Commit 40ab171

Browse files
authored
Merge pull request #120 from devartifex/fix/auth-persistence-and-notifications
fix: auth persistence via encrypted cookie + notification system hardening
2 parents 5b93421 + 99ea26f commit 40ab171

24 files changed

Lines changed: 497 additions & 31 deletions

infra/main.parameters.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
"githubClientId": { "value": "${GITHUB_CLIENT_ID}" },
88
"containerAppImage": { "value": "${SERVICE_WEB_IMAGE_NAME}" },
99
"resourceGroupName": { "value": "${AZURE_RESOURCE_GROUP}" },
10-
"deployerIpAddress": { "value": "${DEPLOYER_IP_ADDRESS}" }
10+
"deployerIpAddress": { "value": "${DEPLOYER_IP_ADDRESS}" },
11+
"vapidPublicKey": { "value": "${VAPID_PUBLIC_KEY}" },
12+
"vapidPrivateKey": { "value": "${VAPID_PRIVATE_KEY}" },
13+
"vapidSubject": { "value": "${VAPID_SUBJECT}" }
1114
}
1215
}

server.js

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ import session from 'express-session';
33
import FileStoreFactory from 'session-file-store';
44
import { setupWebSocket } from './dist/ws/handler.js';
55
import { registerSession, deleteSessionById } from './dist/session-store.js';
6+
import { unsealAuth, parseCookieValue, AUTH_COOKIE_NAME } from './dist/auth/auth-cookie.js';
67

78
const FileStore = FileStoreFactory(session);
89
const isDev = process.env.NODE_ENV !== 'production';
910
const port = parseInt(process.env.PORT || '3000');
11+
const sessionSecret = process.env.SESSION_SECRET || 'dev-secret-change-me';
12+
const tokenMaxAge = parseInt(process.env.TOKEN_MAX_AGE_MS || String(7 * 24 * 60 * 60 * 1000));
1013

1114
// Set ORIGIN for SvelteKit adapter-node CSRF check before importing handler.
1215
// Without this, adapter-node defaults protocol to 'https', causing origin mismatch on plain HTTP.
@@ -22,7 +25,7 @@ if (!isDev && !process.env.SESSION_SECRET) {
2225
const sessionStorePath = process.env.SESSION_STORE_PATH || (isDev ? '.sessions' : '/data/sessions');
2326
const sessionMiddleware = session({
2427
store: new FileStore({ path: sessionStorePath, ttl: 86400, retries: 0, logFn: () => {} }),
25-
secret: process.env.SESSION_SECRET || 'dev-secret-change-me',
28+
secret: sessionSecret,
2629
resave: false,
2730
saveUninitialized: false,
2831
rolling: true,
@@ -31,12 +34,30 @@ const sessionMiddleware = session({
3134
httpOnly: true,
3235
secure: !isDev,
3336
sameSite: 'lax',
34-
maxAge: parseInt(process.env.TOKEN_MAX_AGE_MS || String(7 * 24 * 60 * 60 * 1000)),
37+
maxAge: tokenMaxAge,
3538
},
3639
});
3740

41+
/** Restore auth from encrypted cookie when session file is missing (e.g. after EmptyDir wipe). */
42+
function restoreAuthFromCookie(req) {
43+
if (req.session && !req.session.githubToken) {
44+
const sealed = parseCookieValue(req.headers.cookie, AUTH_COOKIE_NAME);
45+
if (sealed) {
46+
const data = unsealAuth(sealed, sessionSecret, tokenMaxAge);
47+
if (data) {
48+
req.session.githubToken = data.githubToken;
49+
req.session.githubUser = data.githubUser;
50+
req.session.githubAuthTime = data.githubAuthTime;
51+
req.session.save(() => {});
52+
console.log(`[AUTH-COOKIE] Restored auth for user=${data.githubUser.login}`);
53+
}
54+
}
55+
}
56+
}
57+
3858
const server = createServer((req, res) => {
3959
sessionMiddleware(req, res, () => {
60+
restoreAuthFromCookie(req);
4061
const sessionId = registerSession(req.session);
4162
req.headers['x-session-id'] = sessionId;
4263

src/hooks.server.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ interface MockEvent {
1515
locals: {
1616
session?: SessionLike | null;
1717
};
18+
cookies: {
19+
delete: ReturnType<typeof vi.fn>;
20+
};
1821
getClientAddress: () => string;
1922
}
2023

@@ -56,6 +59,19 @@ vi.mock('$lib/server/auth/guard.js', () => ({
5659
revalidateTokenIfStale: mockRevalidateTokenIfStale,
5760
}));
5861

62+
vi.mock('$lib/server/auth/auth-cookie.js', () => ({
63+
unsealAuth: vi.fn(() => null),
64+
parseCookieValue: vi.fn(() => undefined),
65+
AUTH_COOKIE_NAME: '__copilot_auth',
66+
}));
67+
68+
vi.mock('$lib/server/config.js', () => ({
69+
config: {
70+
sessionSecret: 'test-secret',
71+
tokenMaxAge: 7 * 24 * 60 * 60 * 1000,
72+
},
73+
}));
74+
5975
const originalNodeEnv = process.env.NODE_ENV;
6076
const originalBaseUrl = process.env.BASE_URL;
6177

@@ -93,6 +109,9 @@ function createEvent({
93109
}),
94110
url: requestUrl,
95111
locals: {},
112+
cookies: {
113+
delete: vi.fn(),
114+
},
96115
getClientAddress: () => ip,
97116
};
98117
}

src/hooks.server.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import type { Handle } from '@sveltejs/kit';
22
import { sequence } from '@sveltejs/kit/hooks';
33
import { getSessionById } from '$lib/server/session-store';
44
import { checkAuth, revalidateTokenIfStale } from '$lib/server/auth/guard.js';
5+
import { unsealAuth, parseCookieValue, AUTH_COOKIE_NAME } from '$lib/server/auth/auth-cookie.js';
6+
import { config } from '$lib/server/config.js';
57
import { initServerSideEffects } from '$lib/server/init.js';
68

79
initServerSideEffects();
@@ -13,6 +15,22 @@ const sessionHandle: Handle = async ({ event, resolve }) => {
1315
if (sessionId) {
1416
const session = getSessionById(sessionId) ?? null;
1517
console.log(`[HOOKS] session resolved: hasSession=${!!session} hasToken=${!!session?.githubToken} user=${session?.githubUser?.login ?? 'none'}`);
18+
19+
// Restore auth from encrypted cookie when session file was lost (e.g. deploy wipe)
20+
if (session && !session.githubToken) {
21+
const sealed = parseCookieValue(event.request.headers.get('cookie') ?? undefined, AUTH_COOKIE_NAME);
22+
if (sealed) {
23+
const data = unsealAuth(sealed, config.sessionSecret, config.tokenMaxAge);
24+
if (data) {
25+
session.githubToken = data.githubToken;
26+
session.githubUser = data.githubUser;
27+
session.githubAuthTime = data.githubAuthTime;
28+
session.save(() => {});
29+
console.log(`[HOOKS] Restored auth from cookie for user=${data.githubUser.login}`);
30+
}
31+
}
32+
}
33+
1634
event.locals.session = session;
1735
} else {
1836
console.log(`[HOOKS] NO x-session-id header — session will be null`);
@@ -140,6 +158,8 @@ const tokenRevalidation: Handle = async ({ event, resolve }) => {
140158

141159
const result = await revalidateTokenIfStale(session);
142160
if (!result.valid) {
161+
// Token was revoked — clear encrypted auth cookie so it doesn't restore a dead token
162+
event.cookies.delete(AUTH_COOKIE_NAME, { path: '/' });
143163
return new Response(JSON.stringify({ error: 'Token revoked. Please sign in again.' }), {
144164
status: 401,
145165
headers: { 'Content-Type': 'application/json' },

src/lib/components/SettingsModal.svelte

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
onFetchAgents: () => void;
4040
onFetchQuota: () => void;
4141
onFetchSkills: () => void;
42+
notificationsEnabled: boolean;
43+
onToggleNotifications: (enabled: boolean) => void;
4244
}
4345
4446
const {
@@ -68,6 +70,8 @@
6870
onFetchAgents,
6971
onFetchQuota,
7072
onFetchSkills,
73+
notificationsEnabled,
74+
onToggleNotifications,
7175
}: Props = $props();
7276
7377
import {
@@ -134,10 +138,39 @@
134138
}
135139
if (typeof Notification !== 'undefined' && Notification.permission === 'denied') {
136140
notificationStatus = 'denied';
141+
if (notificationsEnabled) onToggleNotifications(false);
137142
return;
138143
}
139144
const sub = await getPushSubscription();
140-
if (sub) { notificationStatus = 'subscribed'; return; }
145+
if (sub) {
146+
notificationStatus = 'subscribed';
147+
if (!notificationsEnabled) onToggleNotifications(true);
148+
return;
149+
}
150+
// Settings say enabled but browser has no subscription — auto-re-subscribe.
151+
// First confirm the server has VAPID configured; if not (503), treat push as
152+
// unsupported and clear the stored preference so we don't keep retrying.
153+
if (notificationsEnabled && typeof Notification !== 'undefined' && Notification.permission === 'granted') {
154+
try {
155+
const vapidRes = await fetch('/api/push/vapid-key');
156+
if (!vapidRes.ok) {
157+
onToggleNotifications(false);
158+
notificationStatus = 'unsupported';
159+
return;
160+
}
161+
} catch {
162+
notificationStatus = 'granted-no-push';
163+
return;
164+
}
165+
notificationBusy = true;
166+
try {
167+
const newSub = await subscribeToPush();
168+
notificationStatus = newSub ? 'subscribed' : 'granted-no-push';
169+
} finally {
170+
notificationBusy = false;
171+
}
172+
return;
173+
}
141174
if (typeof Notification !== 'undefined' && Notification.permission === 'granted') {
142175
notificationStatus = 'granted-no-push';
143176
return;
@@ -151,8 +184,10 @@
151184
const sub = await subscribeToPush();
152185
if (sub) {
153186
notificationStatus = 'subscribed';
187+
onToggleNotifications(true);
154188
} else if (typeof Notification !== 'undefined' && Notification.permission === 'denied') {
155189
notificationStatus = 'denied';
190+
onToggleNotifications(false);
156191
}
157192
} finally {
158193
notificationBusy = false;
@@ -164,6 +199,7 @@
164199
try {
165200
await unsubscribeFromPush();
166201
notificationStatus = 'prompt';
202+
onToggleNotifications(false);
167203
} finally {
168204
notificationBusy = false;
169205
}
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
// @vitest-environment node
2+
3+
import { describe, expect, it } from 'vitest';
4+
import { sealAuth, unsealAuth, parseCookieValue, AUTH_COOKIE_NAME, type AuthCookiePayload } from './auth-cookie';
5+
6+
const SECRET = 'test-session-secret-at-least-32-chars-long';
7+
8+
const VALID_PAYLOAD: AuthCookiePayload = {
9+
githubToken: 'gho_abc123def456',
10+
githubUser: { login: 'octocat', name: 'The Octocat' },
11+
githubAuthTime: Date.now(),
12+
};
13+
14+
describe('auth-cookie', () => {
15+
describe('sealAuth / unsealAuth', () => {
16+
it('round-trips a valid payload', () => {
17+
const sealed = sealAuth(VALID_PAYLOAD, SECRET);
18+
const result = unsealAuth(sealed, SECRET);
19+
20+
expect(result).toEqual(VALID_PAYLOAD);
21+
});
22+
23+
it('produces different ciphertexts for the same payload (random IV)', () => {
24+
const a = sealAuth(VALID_PAYLOAD, SECRET);
25+
const b = sealAuth(VALID_PAYLOAD, SECRET);
26+
27+
expect(a).not.toBe(b);
28+
});
29+
30+
it('returns null for a wrong secret', () => {
31+
const sealed = sealAuth(VALID_PAYLOAD, SECRET);
32+
const result = unsealAuth(sealed, 'wrong-secret-that-is-long-enough');
33+
34+
expect(result).toBeNull();
35+
});
36+
37+
it('returns null for tampered ciphertext', () => {
38+
const sealed = sealAuth(VALID_PAYLOAD, SECRET);
39+
// Flip a character in the middle
40+
const tampered = sealed.slice(0, 10) + (sealed[10] === 'a' ? 'b' : 'a') + sealed.slice(11);
41+
const result = unsealAuth(tampered, SECRET);
42+
43+
expect(result).toBeNull();
44+
});
45+
46+
it('returns null for truncated data', () => {
47+
const sealed = sealAuth(VALID_PAYLOAD, SECRET);
48+
const result = unsealAuth(sealed.slice(0, 10), SECRET);
49+
50+
expect(result).toBeNull();
51+
});
52+
53+
it('returns null for empty string', () => {
54+
expect(unsealAuth('', SECRET)).toBeNull();
55+
});
56+
57+
it('returns null for non-base64 garbage', () => {
58+
expect(unsealAuth('not-valid-base64!!!', SECRET)).toBeNull();
59+
});
60+
61+
it('returns null when authTime exceeds maxAgeMs', () => {
62+
const oldPayload: AuthCookiePayload = {
63+
...VALID_PAYLOAD,
64+
githubAuthTime: Date.now() - 8 * 24 * 60 * 60 * 1000, // 8 days ago
65+
};
66+
const sealed = sealAuth(oldPayload, SECRET);
67+
const result = unsealAuth(sealed, SECRET, 7 * 24 * 60 * 60 * 1000); // 7-day max
68+
69+
expect(result).toBeNull();
70+
});
71+
72+
it('accepts payload when authTime is within maxAgeMs', () => {
73+
const recentPayload: AuthCookiePayload = {
74+
...VALID_PAYLOAD,
75+
githubAuthTime: Date.now() - 1000, // 1 second ago
76+
};
77+
const sealed = sealAuth(recentPayload, SECRET);
78+
const result = unsealAuth(sealed, SECRET, 7 * 24 * 60 * 60 * 1000);
79+
80+
expect(result).toEqual(recentPayload);
81+
});
82+
83+
it('accepts payload when maxAgeMs is not provided', () => {
84+
const oldPayload: AuthCookiePayload = {
85+
...VALID_PAYLOAD,
86+
githubAuthTime: Date.now() - 30 * 24 * 60 * 60 * 1000, // 30 days ago
87+
};
88+
const sealed = sealAuth(oldPayload, SECRET);
89+
const result = unsealAuth(sealed, SECRET);
90+
91+
expect(result).toEqual(oldPayload);
92+
});
93+
94+
it('returns null for payload missing githubToken', () => {
95+
const badPayload = { githubUser: { login: 'octocat', name: 'Octo' }, githubAuthTime: Date.now() };
96+
// Manually seal a bad payload by casting
97+
const sealed = sealAuth(badPayload as AuthCookiePayload, SECRET);
98+
const result = unsealAuth(sealed, SECRET);
99+
100+
expect(result).toBeNull();
101+
});
102+
103+
it('returns null for payload missing githubUser.login', () => {
104+
const badPayload = { githubToken: 'tok', githubUser: { name: 'Octo' }, githubAuthTime: Date.now() };
105+
const sealed = sealAuth(badPayload as unknown as AuthCookiePayload, SECRET);
106+
const result = unsealAuth(sealed, SECRET);
107+
108+
expect(result).toBeNull();
109+
});
110+
111+
it('returns null for payload with non-number githubAuthTime', () => {
112+
const badPayload = { githubToken: 'tok', githubUser: { login: 'x', name: 'X' }, githubAuthTime: 'not-a-number' };
113+
const sealed = sealAuth(badPayload as unknown as AuthCookiePayload, SECRET);
114+
const result = unsealAuth(sealed, SECRET);
115+
116+
expect(result).toBeNull();
117+
});
118+
});
119+
120+
describe('parseCookieValue', () => {
121+
it('extracts a cookie value from a header string', () => {
122+
const header = 'connect.sid=s%3Aabc; __copilot_auth=sealed123; other=val';
123+
expect(parseCookieValue(header, '__copilot_auth')).toBe('sealed123');
124+
});
125+
126+
it('returns undefined for a missing cookie', () => {
127+
const header = 'connect.sid=s%3Aabc; other=val';
128+
expect(parseCookieValue(header, '__copilot_auth')).toBeUndefined();
129+
});
130+
131+
it('returns undefined for undefined header', () => {
132+
expect(parseCookieValue(undefined, '__copilot_auth')).toBeUndefined();
133+
});
134+
135+
it('handles cookie value with equals signs', () => {
136+
const header = '__copilot_auth=abc=def=ghi';
137+
expect(parseCookieValue(header, '__copilot_auth')).toBe('abc=def=ghi');
138+
});
139+
140+
it('handles whitespace around cookie pairs', () => {
141+
const header = ' __copilot_auth = value123 ; other=x';
142+
expect(parseCookieValue(header, '__copilot_auth')).toBeUndefined(); // space before = means the prefix doesn't match
143+
});
144+
145+
it('returns first match for duplicate cookie names', () => {
146+
const header = '__copilot_auth=first; __copilot_auth=second';
147+
expect(parseCookieValue(header, '__copilot_auth')).toBe('first');
148+
});
149+
});
150+
151+
describe('AUTH_COOKIE_NAME', () => {
152+
it('is the expected value', () => {
153+
expect(AUTH_COOKIE_NAME).toBe('__copilot_auth');
154+
});
155+
});
156+
});

0 commit comments

Comments
 (0)