History

fix(server): prevent duplicate desktop clients after restart

Replace stale local desktop sessions in one transaction. Preserve paired clients and browser sessions, and keep the previous credential valid if replacement fails. Closes https://github.com/pingdotgg/t3code/issues/6283. Original implementation by seeb1337. Reviewed and verified with GPT-6 Astra (preview) in Codex.

seeb1337 committed 2 months ago

Commit eb776831 parent 14bf3f68 files · +284 −18Browse files

Unified · Split

8 files
  1. apps/server/src/auth/EnvironmentAuth.test.ts +77 −0
  2. apps/server/src/auth/EnvironmentAuth.ts +3 −0
  3. apps/server/src/auth/SessionStore.test.ts +74 −0
  4. apps/server/src/auth/SessionStore.ts +39 −18
  5. apps/server/src/persistence/AuthSessions.ts +49 −0
  6. apps/server/src/server.test.ts +32 −0
  7. docs/internals/environment-auth.md +6 −0
  8. docs/user/remote-access.md +4 −0
apps/server/src/auth/EnvironmentAuth.test.ts+77−0
@@ -52,6 +52,18 @@ const makeCookieRequest = ( · 51 unmodified lines
5252 EnvironmentAuth.EnvironmentAuth["Service"]["authenticateHttpRequest"]
5353 >[0];
5454
55+const makeBearerRequest = (
56+ token: string,
57+): Parameters<EnvironmentAuth.EnvironmentAuth["Service"]["authenticateHttpRequest"]>[0] =>
58+ ({
59+ cookies: {},
60+ headers: {
61+ authorization: `Bearer ${token}`,
62+ },
63+ }) as unknown as Parameters<
64+ EnvironmentAuth.EnvironmentAuth["Service"]["authenticateHttpRequest"]
65+ >[0];
66+
5567 const requestMetadata = {
5668 deviceType: "desktop" as const,
5769 os: "macOS",
@@ -159,6 +171,71 @@ it.layer(NodeServices.layer)("EnvironmentAuth.layer", (it) => { · 101 unmodified lines
159171 }).pipe(Effect.provide(makeEnvironmentAuthLayer())),
160172 );
161173
174+ it.effect("rotates desktop bearer sessions without accumulating authorized clients", () =>
175+ Effect.gen(function* () {
176+ const serverAuth = yield* EnvironmentAuth.EnvironmentAuth;
177+ const sessions = yield* SessionStore.SessionStore;
178+ const browser = yield* serverAuth.createBrowserSession(
179+ "desktop-bootstrap-token",
180+ requestMetadata,
181+ );
182+ const browserSession = yield* serverAuth.authenticateHttpRequest(
183+ makeCookieRequest(sessions.cookieName, browser.sessionToken),
184+ );
185+ const staleSessions = yield* Effect.forEach([1, 2, 3], () =>
186+ sessions.issue({ subject: "desktop-bootstrap", method: "bearer-access-token" }),
187+ );
188+ const pairing = yield* serverAuth.issuePairingCredential();
189+ const paired = yield* serverAuth.exchangeBootstrapCredentialForAccessToken(
190+ pairing.credential,
191+ undefined,
192+ { ...requestMetadata, label: "T3 Code Desktop" },
193+ );
194+ const first = yield* serverAuth.exchangeBootstrapCredentialForAccessToken(
195+ "desktop-bootstrap-token",
196+ undefined,
197+ requestMetadata,
198+ );
199+ const firstSession = yield* serverAuth.authenticateHttpRequest(
200+ makeBearerRequest(first.access_token),
201+ );
202+ const second = yield* serverAuth.exchangeBootstrapCredentialForAccessToken(
203+ "desktop-bootstrap-token",
204+ undefined,
205+ requestMetadata,
206+ );
207+
208+ const active = yield* serverAuth.listSessions();
209+ const firstError = yield* serverAuth
210+ .authenticateHttpRequest(makeBearerRequest(first.access_token))
211+ .pipe(Effect.flip);
212+ const secondSession = yield* serverAuth.authenticateHttpRequest(
213+ makeBearerRequest(second.access_token),
214+ );
215+
216+ expect(active).toHaveLength(3);
217+ expect(active.map((entry) => entry.sessionId)).toContain(browserSession.sessionId);
218+ expect(active.map((entry) => entry.sessionId)).toContain(secondSession.sessionId);
219+ expect(active.map((entry) => entry.sessionId)).not.toContain(firstSession.sessionId);
220+ expect(firstError._tag).toBe("ServerAuthInvalidCredentialError");
221+ for (const stale of staleSessions) {
222+ const error = yield* sessions.verify(stale.token).pipe(Effect.flip);
223+ expect(error._tag).toBe("SessionTokenRevokedError");
224+ }
225+ const pairedSession = yield* serverAuth.authenticateHttpRequest(
226+ makeBearerRequest(paired.access_token),
227+ );
228+ expect(pairedSession.subject).toBe("one-time-token");
229+ expect(active.map((entry) => entry.sessionId)).toContain(pairedSession.sessionId);
230+ }).pipe(
231+ Effect.provide(
232+ makeEnvironmentAuthLayer({
233+ desktopBootstrapToken: "desktop-bootstrap-token",
234+ }),
235+ ),
236+ ),
237+ );
238+
162239 it.effect("keeps user-issued administrative pairing links manageable", () =>
163240 Effect.gen(function* () {
164241 const serverAuth = yield* EnvironmentAuth.EnvironmentAuth;
apps/server/src/auth/EnvironmentAuth.ts+3−0
@@ -750,6 +750,9 @@ export const make = Effect.gen(function* () { · 749 unmodified lines
750750 ttl: Duration.hours(1),
751751 }
752752 : {}),
753+ // Desktop restarts forget the previous bearer token. Replace
754+ // its session, including stale entries left by older versions.
755+ replaceActiveForSubjectAndMethod: grant.method === "desktop-bootstrap",
753756 client: {
754757 ...requestMetadata,
755758 ...(grant.label ? { label: grant.label } : {}),
apps/server/src/auth/SessionStore.test.ts+74−0
@@ -4,6 +4,7 @@ import { expect, it } from "@effect/vitest"; · 3 unmodified lines
44 import * as Duration from "effect/Duration";
55 import * as Effect from "effect/Effect";
66 import * as Layer from "effect/Layer";
7+import * as Option from "effect/Option";
78 import * as TestClock from "effect/testing/TestClock";
89 import * as SqlClient from "effect/unstable/sql/SqlClient";
910
@@ -50,6 +51,7 @@ const repositoryFailure = new PersistenceSqlError({ · 40 unmodified lines
5051
5152 const failingSessionLookupRepositoryLayer = Layer.succeed(AuthSessions.AuthSessionRepository, {
5253 create: () => Effect.void,
54+ createReplacingActive: () => Effect.succeed([]),
5355 getById: () => Effect.fail(repositoryFailure),
5456 listActive: () => Effect.succeed([]),
5557 revoke: () => Effect.fail(repositoryFailure),
@@ -181,6 +183,78 @@ it.layer(NodeServices.layer)("SessionStore.layer", (it) => { · 125 unmodified lines
181183 }).pipe(Effect.provide(Layer.merge(makeSessionStoreLayer(), TestClock.layer()))),
182184 );
183185
186+ it.effect("atomically replaces active sessions with the same subject and method", () =>
187+ Effect.gen(function* () {
188+ const sessions = yield* SessionStore.SessionStore;
189+ const browser = yield* sessions.issue({
190+ subject: "desktop-bootstrap",
191+ method: "browser-session-cookie",
192+ });
193+ const [firstBearer, secondBearer] = yield* Effect.all(
194+ [
195+ sessions.issue({
196+ subject: "desktop-bootstrap",
197+ method: "bearer-access-token",
198+ replaceActiveForSubjectAndMethod: true,
199+ }),
200+ sessions.issue({
201+ subject: "desktop-bootstrap",
202+ method: "bearer-access-token",
203+ replaceActiveForSubjectAndMethod: true,
204+ }),
205+ ],
206+ { concurrency: "unbounded" },
207+ );
208+
209+ const active = yield* sessions.listActive();
210+ const bearerVerification = yield* Effect.all([
211+ sessions.verify(firstBearer.token).pipe(Effect.option),
212+ sessions.verify(secondBearer.token).pipe(Effect.option),
213+ ]);
214+
215+ expect(active).toHaveLength(2);
216+ expect(active.find((entry) => entry.sessionId === browser.sessionId)).toBeDefined();
217+ expect(
218+ active.filter(
219+ (entry) =>
220+ entry.subject === "desktop-bootstrap" && entry.method === "bearer-access-token",
221+ ),
222+ ).toHaveLength(1);
223+ expect(bearerVerification.filter(Option.isSome)).toHaveLength(1);
224+ }).pipe(Effect.provide(makeSessionStoreLayer())),
225+ );
226+
227+ it.effect("keeps the previous desktop session valid when replacement fails", () =>
228+ Effect.gen(function* () {
229+ const sessions = yield* SessionStore.SessionStore;
230+ const sql = yield* SqlClient.SqlClient;
231+ const previous = yield* sessions.issue({
232+ subject: "desktop-bootstrap",
233+ method: "bearer-access-token",
234+ });
235+ yield* sql`
236+ CREATE TRIGGER reject_auth_session_insert BEFORE INSERT ON auth_sessions
237+ BEGIN
238+ SELECT RAISE(ABORT, 'simulated insert failure');
239+ END
240+ `;
241+
242+ const error = yield* sessions
243+ .issue({
244+ subject: "desktop-bootstrap",
245+ method: "bearer-access-token",
246+ replaceActiveForSubjectAndMethod: true,
247+ })
248+ .pipe(Effect.flip);
249+
250+ expect(error._tag).toBe("SessionCredentialIssueError");
251+ expect((yield* sessions.verify(previous.token)).sessionId).toBe(previous.sessionId);
252+ expect((yield* sessions.listActive()).map((session) => session.sessionId)).toEqual([
253+ previous.sessionId,
254+ ]);
255+ }).pipe(Effect.provide(Layer.mergeAll(makeSessionStoreLayer(), SqlitePersistenceMemory))),
256+ );
257+
184258 it.effect("rejects websocket tokens once the parent session has expired", () =>
185259 Effect.gen(function* () {
186260 const sessions = yield* SessionStore.SessionStore;
apps/server/src/auth/SessionStore.ts+39−18
@@ -370,6 +370,11 @@ export class SessionStore extends Context.Service< · 369 unmodified lines
370370 readonly scopes?: ReadonlyArray<AuthEnvironmentScope>;
371371 readonly client?: AuthClientMetadata;
372372 readonly proofKeyThumbprint?: string;
373+ /**
374+ * Atomically revoke active sessions with the same subject and method
375+ * before storing this session.
376+ */
377+ readonly replaceActiveForSubjectAndMethod?: boolean;
373378 }) => Effect.Effect<IssuedSession, SessionCredentialInternalError>;
374379 readonly verify: (token: string) => Effect.Effect<VerifiedSession, SessionCredentialError>;
375380 readonly issueWebSocketToken: (
@@ -647,24 +652,40 @@ export const make = Effect.gen(function* () { · 271 unmodified lines
647652 );
648653 const signature = signPayload(encodedPayload, signingSecret);
649654 const client = input?.client ?? createDefaultClientMetadata();
650− yield* authSessions
651− .create({
652− sessionId,
653− subject: claims.sub,
654− scopes: claims.scopes,
655− method: claims.method,
656− client: {
657− label: client.label ?? null,
658− ipAddress: client.ipAddress ?? null,
659− userAgent: client.userAgent ?? null,
660− deviceType: client.deviceType,
661− os: client.os ?? null,
662− browser: client.browser ?? null,
663− },
664− issuedAt,
665− expiresAt,
666− })
667− .pipe(Effect.mapError((cause) => new SessionCredentialIssueError({ sessionId, cause })));
655+ const sessionRecord = {
656+ sessionId,
657+ subject: claims.sub,
658+ scopes: claims.scopes,
659+ method: claims.method,
660+ client: {
661+ label: client.label ?? null,
662+ ipAddress: client.ipAddress ?? null,
663+ userAgent: client.userAgent ?? null,
664+ deviceType: client.deviceType,
665+ os: client.os ?? null,
666+ browser: client.browser ?? null,
667+ },
668+ issuedAt,
669+ expiresAt,
670+ } satisfies AuthSessions.CreateAuthSessionInput;
671+ const replacedSessionIds = yield* (
672+ input?.replaceActiveForSubjectAndMethod
673+ ? authSessions.createReplacingActive({ session: sessionRecord, revokedAt: issuedAt })
674+ : authSessions.create(sessionRecord).pipe(Effect.as([] as ReadonlyArray<AuthSessionId>))
675+ ).pipe(Effect.mapError((cause) => new SessionCredentialIssueError({ sessionId, cause })));
676+ if (replacedSessionIds.length > 0) {
677+ yield* Ref.update(connectedSessionsRef, (current) => {
678+ const next = new Map(current);
679+ for (const replacedSessionId of replacedSessionIds) {
680+ next.delete(replacedSessionId);
681+ }
682+ return next;
683+ });
684+ yield* Effect.forEach(replacedSessionIds, emitRemoved, {
685+ concurrency: "unbounded",
686+ discard: true,
687+ });
688+ }
668689 yield* emitUpsert(
669690 toAuthClientSession({
670691 sessionId,
apps/server/src/persistence/AuthSessions.ts+49−0
@@ -55,6 +55,13 @@ export const CreateAuthSessionInput = Schema.Struct({ · 54 unmodified lines
5555 });
5656 export type CreateAuthSessionInput = typeof CreateAuthSessionInput.Type;
5757
58+export const CreateReplacingActiveAuthSessionInput = Schema.Struct({
59+ session: CreateAuthSessionInput,
60+ revokedAt: Schema.DateTimeUtcFromString,
61+});
62+export type CreateReplacingActiveAuthSessionInput =
63+ typeof CreateReplacingActiveAuthSessionInput.Type;
64+
5865 export const GetAuthSessionByIdInput = Schema.Struct({
5966 sessionId: AuthSessionId,
6067 });
@@ -96,6 +103,9 @@ export class AuthSessionRepository extends Context.Service< · 35 unmodified lines
96103 readonly create: (
97104 input: CreateAuthSessionInput,
98105 ) => Effect.Effect<void, AuthSessionRepositoryError>;
106+ readonly createReplacingActive: (
107+ input: CreateReplacingActiveAuthSessionInput,
108+ ) => Effect.Effect<ReadonlyArray<AuthSessionId>, AuthSessionRepositoryError>;
99109 readonly getById: (
100110 input: GetAuthSessionByIdInput,
101111 ) => Effect.Effect<Option.Option<AuthSessionRecord>, AuthSessionRepositoryError>;
@@ -254,6 +264,21 @@ export const make = Effect.gen(function* () { · 152 unmodified lines
254264 `,
255265 });
256266
267+ const revokeActiveSessionsForReplacement = SqlSchema.findAll({
268+ Request: CreateReplacingActiveAuthSessionInput,
269+ Result: Schema.Struct({ sessionId: AuthSessionId }),
270+ execute: ({ session, revokedAt }) =>
271+ sql`
272+ UPDATE auth_sessions
273+ SET revoked_at = ${revokedAt}
274+ WHERE subject = ${session.subject}
275+ AND method = ${session.method}
276+ AND revoked_at IS NULL
277+ AND expires_at > ${revokedAt}
278+ RETURNING session_id AS "sessionId"
279+ `,
280+ });
281+
257282 const listActiveSessionRows = SqlSchema.findAll({
258283 Request: ListActiveAuthSessionsInput,
259284 Result: AuthSessionRawDbRow,
@@ -343,6 +368,29 @@ export const make = Effect.gen(function* () { · 83 unmodified lines
343368 ),
344369 );
345370
371+ const createReplacingActive: AuthSessionRepository["Service"]["createReplacingActive"] = (
372+ input,
373+ ) =>
374+ sql
375+ .withTransaction(
376+ revokeActiveSessionsForReplacement(input).pipe(
377+ Effect.flatMap((revokedRows) =>
378+ createSessionRow(input.session).pipe(
379+ Effect.as(revokedRows.map((row) => row.sessionId)),
380+ ),
381+ ),
382+ ),
383+ )
384+ .pipe(
385+ Effect.mapError(
386+ toPersistenceSqlOrDecodeError(
387+ "AuthSessionRepository.createReplacingActive:query",
388+ "AuthSessionRepository.createReplacingActive:encodeRequest",
389+ { sessionId: input.session.sessionId },
390+ ),
391+ ),
392+ );
393+
346394 const getById: AuthSessionRepository["Service"]["getById"] = (input) =>
347395 getSessionRowById(input).pipe(
348396 Effect.mapError(
@@ -442,6 +490,7 @@ export const make = Effect.gen(function* () { · 93 unmodified lines
442490
443491 return {
444492 create,
493+ createReplacingActive,
445494 getById,
446495 listActive,
447496 revoke,
apps/server/src/server.test.ts+32−0
@@ -1905,6 +1905,38 @@ it.layer(NodeServices.layer)("server router seam", (it) => { · 1904 unmodified lines
19051905 }).pipe(Effect.provide(NodeHttpServer.layerTest)),
19061906 );
19071907
1908+ it.effect("replaces the local desktop credential on repeated bootstrap exchanges", () =>
1909+ Effect.gen(function* () {
1910+ yield* buildAppUnderTest();
1911+ const first = yield* exchangeAccessToken();
1912+ const second = yield* exchangeAccessToken();
1913+ const third = yield* exchangeAccessToken();
1914+ assert.equal(first.response.status, 200);
1915+ assert.equal(second.response.status, 200);
1916+ assert.equal(third.response.status, 200);
1917+
1918+ const clientsResponse = yield* HttpClient.get("/api/auth/clients", {
1919+ headers: { authorization: `Bearer ${third.body.access_token}` },
1920+ });
1921+ const clients = (yield* clientsResponse.json) as ReadonlyArray<{
1922+ readonly current: boolean;
1923+ readonly subject: string;
1924+ }>;
1925+ assert.equal(clientsResponse.status, 200);
1926+ assert.equal(clients.length, 1);
1927+ assert.equal(clients[0]?.current, true);
1928+ assert.equal(clients[0]?.subject, "desktop-bootstrap");
1929+
1930+ for (const previous of [first, second]) {
1931+ const response = yield* HttpClient.get("/api/auth/session", {
1932+ headers: { authorization: `Bearer ${previous.body.access_token}` },
1933+ });
1934+ const state = (yield* response.json) as { readonly authenticated: boolean };
1935+ assert.equal(state.authenticated, false);
1936+ }
1937+ }).pipe(Effect.provide(NodeHttpServer.layerTest)),
1938+ );
1939+
19081940 it.effect("persists token exchange client display metadata for authorized-client listings", () =>
19091941 Effect.gen(function* () {
19101942 yield* buildAppUnderTest({
docs/internals/environment-auth.md+6−0
@@ -121,6 +121,12 @@ Sessions issued from a plain bearer exchange use the store's · 120 unmodified lines
121121 only to DPoP-bound exchanges, where the token is additionally constrained by a
122122 proof key. See `SessionStore.ts` and `EnvironmentAuth.ts`.
123123
124+The reusable `desktop-bootstrap` grant replaces active sessions with the same
125+subject and authentication method. Revocation and insertion share one database
126+transaction, so a failed insertion preserves the previous credential. This also
127+removes stale local desktop entries from earlier launches. Browser-cookie sessions
128+and sessions issued through pairing links are not replaced.
129+
124130 Requested scopes must be a subset of the one-time bootstrap credential grant.
125131 An ordinary paired client therefore cannot exchange its grant for
126132 `access:read`, `access:write`, or `relay:write`.
docs/user/remote-access.md+4−0
@@ -80,6 +80,10 @@ and expiry, and can revoke it if they have access management permission. · 79 unmodified lines
8080
8181 The default endpoint controls the QR code and primary copy action for pairing links. You can change it from the expanded endpoint list. The preference is stored by endpoint type, so choosing the local LAN endpoint survives normal IP address changes when you move between networks.
8282
83+After an app restart, the desktop app replaces its previous
84+local credential. Old local desktop entries are removed from **Authorized clients**
85+automatically. Paired phones, browsers, and remote desktop clients keep their access.
86+
8387 When no user default is saved, the app uses the built-in LAN endpoint for pairing links when
8488 available. You can set another endpoint as the default from the expanded endpoint list.
8589

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t