WIP! A BB-style forum, on the ATmosphere!
We're still working... we'll be back soon when we have something to show off!
node
typescript
hono
htmx
atproto
refactor: unify duplicate session restoration logic (#21)
* refactor: unify duplicate session restoration logic
Extract shared restoreOAuthSession() into lib/session.ts to eliminate
duplicate cookie-lookup + OAuth-restore logic from middleware/auth.ts
and routes/auth.ts. The auth middleware now wraps the shared function
to produce AuthenticatedUser with Agent/handle/pdsUrl enrichment.
* fix: eliminate redundant cookie store query in session restoration
Changes:
- `restoreOAuthSession()` now returns both oauth + cookie sessions
- Removes duplicate `cookieSessionStore.get()` call in auth middleware
- Adds `handle` field to `/api/auth/session` response (bonus improvement)
- Updates tests to match new return structure
Before: Cookie store queried twice per authenticated request
After: Single query, both sessions returned together
Addresses PR #21 review feedback (Option A - recommended approach)
---------
Co-authored-by: Claude <noreply@anthropic.com>
+298
-73
+212
apps/appview/src/lib/__tests__/session.test.ts
+212
apps/appview/src/lib/__tests__/session.test.ts
···
1
+
import { describe, it, expect, vi, beforeEach } from "vitest";
2
+
import { restoreOAuthSession } from "../session.js";
3
+
import type { AppContext } from "../app-context.js";
4
+
5
+
/**
6
+
* Create a minimal mock AppContext with controllable cookieSessionStore
7
+
* and oauthClient behavior for testing restoreOAuthSession.
8
+
*/
9
+
function createMockAppContext(overrides?: {
10
+
cookieSession?: { did: string; handle?: string; expiresAt: Date; createdAt: Date } | null;
11
+
restoreResult?: unknown;
12
+
restoreError?: Error;
13
+
}): AppContext {
14
+
const cookieSessionStore = {
15
+
get: vi.fn().mockReturnValue(overrides?.cookieSession ?? null),
16
+
set: vi.fn(),
17
+
delete: vi.fn(),
18
+
destroy: vi.fn(),
19
+
};
20
+
21
+
const oauthClient = {
22
+
restore: vi.fn(),
23
+
};
24
+
25
+
if (overrides?.restoreError) {
26
+
oauthClient.restore.mockRejectedValue(overrides.restoreError);
27
+
} else if (overrides?.restoreResult !== undefined) {
28
+
oauthClient.restore.mockResolvedValue(overrides.restoreResult);
29
+
}
30
+
31
+
return {
32
+
cookieSessionStore,
33
+
oauthClient,
34
+
// Remaining fields are not used by restoreOAuthSession
35
+
config: {} as any,
36
+
db: {} as any,
37
+
firehose: {} as any,
38
+
oauthStateStore: {} as any,
39
+
oauthSessionStore: {} as any,
40
+
} as unknown as AppContext;
41
+
}
42
+
43
+
describe("restoreOAuthSession", () => {
44
+
beforeEach(() => {
45
+
vi.restoreAllMocks();
46
+
});
47
+
48
+
it("returns null when cookie session does not exist", async () => {
49
+
const ctx = createMockAppContext({ cookieSession: null });
50
+
51
+
const result = await restoreOAuthSession(ctx, "nonexistent-token");
52
+
53
+
expect(result).toBeNull();
54
+
expect(ctx.oauthClient.restore).not.toHaveBeenCalled();
55
+
});
56
+
57
+
it("returns both OAuth and cookie sessions when restore succeeds", async () => {
58
+
const mockOAuthSession = {
59
+
did: "did:plc:test-user",
60
+
serverMetadata: { issuer: "https://test.pds" },
61
+
};
62
+
63
+
const mockCookieSession = {
64
+
did: "did:plc:test-user",
65
+
handle: "testuser.test",
66
+
expiresAt: new Date(Date.now() + 3600_000),
67
+
createdAt: new Date(),
68
+
};
69
+
70
+
const ctx = createMockAppContext({
71
+
cookieSession: mockCookieSession,
72
+
restoreResult: mockOAuthSession,
73
+
});
74
+
75
+
const result = await restoreOAuthSession(ctx, "valid-token");
76
+
77
+
expect(result).toEqual({
78
+
oauthSession: mockOAuthSession,
79
+
cookieSession: mockCookieSession,
80
+
});
81
+
expect(ctx.oauthClient.restore).toHaveBeenCalledWith("did:plc:test-user");
82
+
});
83
+
84
+
it("returns null when OAuth restore returns null", async () => {
85
+
const ctx = createMockAppContext({
86
+
cookieSession: {
87
+
did: "did:plc:test-user",
88
+
expiresAt: new Date(Date.now() + 3600_000),
89
+
createdAt: new Date(),
90
+
},
91
+
restoreResult: null,
92
+
});
93
+
94
+
const result = await restoreOAuthSession(ctx, "expired-oauth-token");
95
+
96
+
expect(result).toBeNull();
97
+
});
98
+
99
+
it("returns null when OAuth restore throws a 'not found' error", async () => {
100
+
const ctx = createMockAppContext({
101
+
cookieSession: {
102
+
did: "did:plc:test-user",
103
+
expiresAt: new Date(Date.now() + 3600_000),
104
+
createdAt: new Date(),
105
+
},
106
+
restoreError: new Error("Session not found for DID"),
107
+
});
108
+
109
+
const result = await restoreOAuthSession(ctx, "expired-oauth-token");
110
+
111
+
expect(result).toBeNull();
112
+
});
113
+
114
+
it("re-throws unexpected errors from OAuth restore", async () => {
115
+
const networkError = new Error("fetch failed: ECONNREFUSED");
116
+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
117
+
118
+
const ctx = createMockAppContext({
119
+
cookieSession: {
120
+
did: "did:plc:test-user",
121
+
expiresAt: new Date(Date.now() + 3600_000),
122
+
createdAt: new Date(),
123
+
},
124
+
restoreError: networkError,
125
+
});
126
+
127
+
await expect(restoreOAuthSession(ctx, "valid-token")).rejects.toThrow(
128
+
"fetch failed: ECONNREFUSED"
129
+
);
130
+
131
+
// Verify the error was logged before re-throwing
132
+
expect(consoleSpy).toHaveBeenCalledWith(
133
+
"Unexpected error restoring OAuth session",
134
+
expect.objectContaining({
135
+
operation: "restoreOAuthSession",
136
+
did: "did:plc:test-user",
137
+
error: "fetch failed: ECONNREFUSED",
138
+
})
139
+
);
140
+
141
+
consoleSpy.mockRestore();
142
+
});
143
+
144
+
it("logs structured context when unexpected error occurs", async () => {
145
+
const dbError = new Error("Database connection lost");
146
+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
147
+
148
+
const ctx = createMockAppContext({
149
+
cookieSession: {
150
+
did: "did:plc:another-user",
151
+
handle: "another.test",
152
+
expiresAt: new Date(Date.now() + 3600_000),
153
+
createdAt: new Date(),
154
+
},
155
+
restoreError: dbError,
156
+
});
157
+
158
+
await expect(restoreOAuthSession(ctx, "some-token")).rejects.toThrow(
159
+
"Database connection lost"
160
+
);
161
+
162
+
expect(consoleSpy).toHaveBeenCalledWith(
163
+
"Unexpected error restoring OAuth session",
164
+
{
165
+
operation: "restoreOAuthSession",
166
+
did: "did:plc:another-user",
167
+
error: "Database connection lost",
168
+
}
169
+
);
170
+
171
+
consoleSpy.mockRestore();
172
+
});
173
+
174
+
it("handles non-Error thrown values", async () => {
175
+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
176
+
177
+
const cookieSessionStore = {
178
+
get: vi.fn().mockReturnValue({
179
+
did: "did:plc:test-user",
180
+
expiresAt: new Date(Date.now() + 3600_000),
181
+
createdAt: new Date(),
182
+
}),
183
+
};
184
+
const oauthClient = {
185
+
restore: vi.fn().mockRejectedValue("string error"),
186
+
};
187
+
188
+
const ctx = {
189
+
cookieSessionStore,
190
+
oauthClient,
191
+
config: {} as any,
192
+
db: {} as any,
193
+
firehose: {} as any,
194
+
oauthStateStore: {} as any,
195
+
oauthSessionStore: {} as any,
196
+
} as unknown as AppContext;
197
+
198
+
await expect(restoreOAuthSession(ctx, "valid-token")).rejects.toBe(
199
+
"string error"
200
+
);
201
+
202
+
// Non-Error values should be stringified in the log
203
+
expect(consoleSpy).toHaveBeenCalledWith(
204
+
"Unexpected error restoring OAuth session",
205
+
expect.objectContaining({
206
+
error: "string error",
207
+
})
208
+
);
209
+
210
+
consoleSpy.mockRestore();
211
+
});
212
+
});
+59
apps/appview/src/lib/session.ts
+59
apps/appview/src/lib/session.ts
···
1
+
import type { OAuthSession } from "@atproto/oauth-client-node";
2
+
import type { AppContext } from "./app-context.js";
3
+
import type { CookieSession } from "./cookie-session-store.js";
4
+
5
+
/**
6
+
* Result of restoring an OAuth session from a cookie token.
7
+
*/
8
+
export interface RestoredSession {
9
+
oauthSession: OAuthSession;
10
+
cookieSession: CookieSession;
11
+
}
12
+
13
+
/**
14
+
* Restore an OAuth session from a cookie token.
15
+
*
16
+
* Looks up the cookie token in the CookieSessionStore, then restores
17
+
* the OAuth session from the library's session store (with automatic
18
+
* token refresh).
19
+
*
20
+
* Returns both the OAuth session and cookie session if successful,
21
+
* allowing callers to access handle and other cookie metadata without
22
+
* redundant store queries.
23
+
*
24
+
* Returns null if the cookie session doesn't exist, is expired,
25
+
* or the OAuth session is not found (expected cases).
26
+
* Throws on unexpected errors (network failures, etc.) that should bubble up.
27
+
*/
28
+
export async function restoreOAuthSession(
29
+
ctx: AppContext,
30
+
cookieToken: string
31
+
): Promise<RestoredSession | null> {
32
+
const cookieSession = ctx.cookieSessionStore.get(cookieToken);
33
+
if (!cookieSession) {
34
+
return null;
35
+
}
36
+
37
+
try {
38
+
// Restore OAuth session from library's session store
39
+
// This will automatically refresh tokens if needed
40
+
const oauthSession = await ctx.oauthClient.restore(cookieSession.did);
41
+
if (!oauthSession) {
42
+
return null;
43
+
}
44
+
return { oauthSession, cookieSession };
45
+
} catch (error) {
46
+
// Check if this is an expected "session not found" error
47
+
if (error instanceof Error && error.message.includes("not found")) {
48
+
return null;
49
+
}
50
+
51
+
// Unexpected error - log and re-throw
52
+
console.error("Unexpected error restoring OAuth session", {
53
+
operation: "restoreOAuthSession",
54
+
did: cookieSession.did,
55
+
error: error instanceof Error ? error.message : String(error),
56
+
});
57
+
throw error;
58
+
}
59
+
}
+21
-32
apps/appview/src/middleware/auth.ts
+21
-32
apps/appview/src/middleware/auth.ts
···
2
2
import type { Context, Next } from "hono";
3
3
import { Agent } from "@atproto/api";
4
4
import type { AppContext } from "../lib/app-context.js";
5
+
import { restoreOAuthSession } from "../lib/session.js";
5
6
import type { AuthenticatedUser, Variables } from "../types.js";
6
7
7
8
/**
8
9
* Helper to restore OAuth session from cookie and create an Agent.
9
10
*
11
+
* Delegates to the shared `restoreOAuthSession` for cookie lookup and
12
+
* OAuth session restoration, then enriches the result with an Agent,
13
+
* handle, and PDS URL to produce an AuthenticatedUser.
14
+
*
10
15
* Returns null if session doesn't exist or is expired (expected).
11
16
* Throws on unexpected errors (network failures, etc.) that should bubble up.
12
17
*/
13
18
async function restoreSession(ctx: AppContext, cookieToken: string): Promise<AuthenticatedUser | null> {
14
-
const cookieSession = ctx.cookieSessionStore.get(cookieToken);
15
-
if (!cookieSession) {
19
+
const result = await restoreOAuthSession(ctx, cookieToken);
20
+
if (!result) {
16
21
return null;
17
22
}
18
23
19
-
try {
20
-
// Restore OAuth session from library (automatic token refresh)
21
-
const oauthSession = await ctx.oauthClient.restore(cookieSession.did);
24
+
const { oauthSession, cookieSession } = result;
22
25
23
-
// Create Agent from OAuth session
24
-
// The library's OAuthSession implements the fetch handler with DPoP
25
-
const agent = new Agent(oauthSession);
26
+
// Create Agent from OAuth session
27
+
// The library's OAuthSession implements the fetch handler with DPoP
28
+
const agent = new Agent(oauthSession);
26
29
27
-
// Get handle from cookie session (fetched during login callback)
28
-
// Fall back to DID if handle wasn't stored
29
-
const handle = cookieSession.handle || oauthSession.did;
30
+
// Get handle from cookie session (fetched during login callback)
31
+
// Fall back to DID if handle wasn't stored
32
+
const handle = cookieSession.handle || oauthSession.did;
30
33
31
-
const user: AuthenticatedUser = {
32
-
did: oauthSession.did,
33
-
handle,
34
-
pdsUrl: oauthSession.serverMetadata.issuer, // PDS URL from server metadata
35
-
agent,
36
-
};
37
-
38
-
return user;
39
-
} catch (error) {
40
-
// Check if this is an expected "session not found" error
41
-
if (error instanceof Error && error.message.includes("not found")) {
42
-
return null;
43
-
}
34
+
const user: AuthenticatedUser = {
35
+
did: oauthSession.did,
36
+
handle,
37
+
pdsUrl: oauthSession.serverMetadata.issuer, // PDS URL from server metadata
38
+
agent,
39
+
};
44
40
45
-
// Unexpected error - log and re-throw
46
-
console.error("Unexpected error restoring OAuth session", {
47
-
operation: "restoreSession",
48
-
did: cookieSession.did,
49
-
error: error instanceof Error ? error.message : String(error),
50
-
});
51
-
throw error;
52
-
}
41
+
return user;
53
42
}
54
43
55
44
/**
+6
-41
apps/appview/src/routes/auth.ts
+6
-41
apps/appview/src/routes/auth.ts
···
3
3
import { randomBytes } from "crypto";
4
4
import { Agent } from "@atproto/api";
5
5
import type { AppContext } from "../lib/app-context.js";
6
-
import type { OAuthSession } from "@atproto/oauth-client-node";
7
-
8
-
/**
9
-
* Helper to restore OAuth session from cookie.
10
-
*
11
-
* Returns null if session doesn't exist or is expired (expected).
12
-
* Throws on unexpected errors (network failures, etc.) that should bubble up.
13
-
*/
14
-
async function restoreOAuthSession(
15
-
ctx: AppContext,
16
-
cookieToken: string
17
-
): Promise<OAuthSession | null> {
18
-
const cookieSession = ctx.cookieSessionStore.get(cookieToken);
19
-
if (!cookieSession) {
20
-
return null;
21
-
}
22
-
23
-
try {
24
-
// Restore OAuth session from library's session store
25
-
// This will automatically refresh tokens if needed
26
-
const oauthSession = await ctx.oauthClient.restore(cookieSession.did);
27
-
return oauthSession;
28
-
} catch (error) {
29
-
// Check if this is an expected "session not found" error
30
-
if (error instanceof Error && error.message.includes("not found")) {
31
-
return null;
32
-
}
33
-
34
-
// Unexpected error - log and re-throw
35
-
console.error("Unexpected error restoring OAuth session", {
36
-
operation: "restoreOAuthSession",
37
-
did: cookieSession.did,
38
-
error: error instanceof Error ? error.message : String(error),
39
-
});
40
-
throw error;
41
-
}
42
-
}
6
+
import { restoreOAuthSession } from "../lib/session.js";
43
7
44
8
/**
45
9
* Authentication routes for OAuth flow using @atproto/oauth-client-node.
···
275
239
}
276
240
277
241
try {
278
-
const oauthSession = await restoreOAuthSession(ctx, cookieToken);
242
+
const result = await restoreOAuthSession(ctx, cookieToken);
279
243
280
-
if (!oauthSession) {
244
+
if (!result) {
281
245
deleteCookie(c, "atbb_session");
282
246
return c.json({ authenticated: false }, 401);
283
247
}
284
248
249
+
const { oauthSession, cookieSession } = result;
250
+
285
251
// Get token info (library handles expiration checks and refresh)
286
252
const tokenInfo = await oauthSession.getTokenInfo();
287
253
288
254
return c.json({
289
255
authenticated: true,
290
256
did: oauthSession.did,
291
-
// Note: handle is not directly available from OAuthSession
292
-
// We'd need to fetch the profile or store it separately
257
+
handle: cookieSession.handle || oauthSession.did,
293
258
sub: tokenInfo.sub,
294
259
});
295
260
} catch (error) {