diff --git a/web/src/lib/auth.svelte.ts b/web/src/lib/auth.svelte.ts index 79df96a2d..a38430326 100644 --- a/web/src/lib/auth.svelte.ts +++ b/web/src/lib/auth.svelte.ts @@ -32,7 +32,7 @@ import { saveAccounts, upsertAccount } from "$lib/auth/accounts"; -import { writePendingLogin } from "$lib/auth/pending"; +import { consumePendingCallback, writePendingLogin } from "$lib/auth/pending"; import { safeReturnTo } from "$lib/auth/returnTo"; export const AUTH_KEY = Symbol("auth"); @@ -86,6 +86,12 @@ export type AuthState = export type { AuthAccount } from "$lib/auth/accounts"; +export interface AgentOptions { + // a dead session sends the browser off to the pds, so only ask for this from + // something the user clicked + relogin?: boolean; +} + export interface Auth { readonly state: AuthState; readonly agent: OAuthUserAgent | null; @@ -104,7 +110,7 @@ export interface Auth { completeSignIn(): Promise; completePendingSignIn(hash: string): Promise; switchAccount(did: Did, returnTo?: string): Promise; - agentFor(did: Did): Promise; + agentFor(did: Did, options?: AgentOptions): Promise; removeAccount(did: Did): Promise; signOut(): Promise; signOutAll(): Promise; @@ -295,7 +301,10 @@ export const createAuth = ( }; // allowStale would let a dead session through, the write then fails with the raw pds error - const agentFor = async (did: Did): Promise => { + const agentFor = async ( + did: Did, + { relogin = true }: AgentOptions = {} + ): Promise => { configure(); // a sign-in exchange may still be storing this session if (pendingExchange) await pendingExchange.catch(() => {}); @@ -318,7 +327,7 @@ export const createAuth = ( forgetSession(did); syncAccounts(); } - if (browser) { + if (browser && relogin) { try { await signIn(did, location.pathname + location.search); // the page is navigating away, stay pending so the form keeps its spinner @@ -552,3 +561,23 @@ export const createAuth = ( }; export const getAuth = () => getContext(AUTH_KEY); + +export interface ResumedSignIn { + handle: string; + returnTo: string; + done: Promise; +} + +// the layout calls this while it initialises. child effects mount before its +// onMount, and one that asks for an agent before the exchange finishes sees a +// revoked session and starts oauth over +export const resumeSignIn = (auth: Auth, pathname: string): ResumedSignIn | null => { + if (pathname === "/oauth/callback") return null; + const pending = consumePendingCallback(); + if (!pending) return null; + + const done = auth.completePendingSignIn(pending.hash); + // onMount reports it, this only keeps the rejection from going unhandled + done.catch(() => {}); + return { handle: pending.login.handle, returnTo: pending.login.returnTo, done }; +}; diff --git a/web/src/lib/auth.test.ts b/web/src/lib/auth.test.ts index 905d33d43..4f4ff08b0 100644 --- a/web/src/lib/auth.test.ts +++ b/web/src/lib/auth.test.ts @@ -10,7 +10,8 @@ import { getSession } from "@atcute/oauth-browser-client"; import { type Mock, beforeEach, describe, expect, it, vi } from "vitest"; -import { createAuth } from "$lib/auth.svelte"; +import { createAuth, resumeSignIn } from "$lib/auth.svelte"; +import { consumePendingCallback } from "$lib/auth/pending"; interface MockAgent { sub: string; @@ -41,12 +42,18 @@ vi.mock("@atcute/oauth-browser-client", async (importOriginal) => { }; }); +vi.mock("$lib/auth/pending", () => ({ + consumePendingCallback: vi.fn(), + writePendingLogin: vi.fn() +})); + // the vi.mock above swaps in a class that records its instances const MockedUserAgent = OAuthUserAgent as unknown as { instances: MockAgent[] }; const mockedGetSession = vi.mocked(getSession); const mockedDeleteStoredSession = vi.mocked(deleteStoredSession); const mockedFinalize = vi.mocked(finalizeAuthorization); +const mockedConsumePending = vi.mocked(consumePendingCallback); const alice = "did:plc:alice" as Did; const liveSession = (did: Did): Session => ({ info: { sub: did } }) as unknown as Session; @@ -160,3 +167,49 @@ describe("completePendingSignIn", () => { expect(mockedGetSession).not.toHaveBeenCalled(); }); }); + +describe("resumeSignIn", () => { + const seeded = { did: alice, handle: "alice.example" }; + const stash = { + hash: "#state=sid&code=c", + login: { did: alice, handle: "alice.example", returnTo: "/", at: Date.now() } + }; + + it("starts the exchange before it returns, so agentFor has something to park on", () => { + mockedConsumePending.mockReturnValue(stash); + mockedFinalize.mockReturnValue(Promise.withResolvers().promise); + const auth = createAuth("http://127.0.0.1:1", seeded); + + const resumed = resumeSignIn(auth, "/"); + + expect(mockedFinalize).toHaveBeenCalledTimes(1); + expect(resumed).toMatchObject({ handle: "alice.example", returnTo: "/" }); + }); + + it("leaves the callback route to run its own exchange", () => { + mockedConsumePending.mockReturnValue(stash); + const auth = createAuth("http://127.0.0.1:1", seeded); + + expect(resumeSignIn(auth, "/oauth/callback")).toBeNull(); + expect(mockedConsumePending).not.toHaveBeenCalled(); + }); + + it("does nothing without a stashed callback", () => { + mockedConsumePending.mockReturnValue(null); + const auth = createAuth("http://127.0.0.1:1", null); + + expect(resumeSignIn(auth, "/")).toBeNull(); + expect(mockedFinalize).not.toHaveBeenCalled(); + }); + + it("hands a failed exchange to the caller without an unhandled rejection", async () => { + mockedConsumePending.mockReturnValue(stash); + mockedFinalize.mockRejectedValue(new LoginError("unknown state provided")); + const auth = createAuth("http://127.0.0.1:1", seeded); + + const resumed = resumeSignIn(auth, "/"); + + await expect(resumed?.done).rejects.toThrow(/unknown state/); + expect(auth.state.kind).toBe("logged-out"); + }); +}); diff --git a/web/src/lib/notifications.svelte.ts b/web/src/lib/notifications.svelte.ts index 68bf65795..8ce3e0724 100644 --- a/web/src/lib/notifications.svelte.ts +++ b/web/src/lib/notifications.svelte.ts @@ -42,7 +42,8 @@ export const createNotifications = ( const preview = async (limit: number): Promise => { const did = auth.currentDid; if (!did || !deliberiUrl) return null; - const agent = auth.agent ?? (await auth.agentFor(did).catch(() => null)); + const agent = + auth.agent ?? (await auth.agentFor(did, { relogin: false }).catch(() => null)); if (!agent) return null; const deliberi = createDeliberiClient({ deliberiUrl, agent }); const hydrate = (items: NotificationItem[]): Promise => diff --git a/web/src/routes/+layout.svelte b/web/src/routes/+layout.svelte index 1d541913b..d4ce621f1 100644 --- a/web/src/routes/+layout.svelte +++ b/web/src/routes/+layout.svelte @@ -4,9 +4,8 @@ import { resolve } from "$app/paths"; import type { Did } from "@atcute/lexicons/syntax"; import { AuthorizationError } from "@atcute/oauth-browser-client"; - import { createAuth, AUTH_KEY } from "$lib/auth.svelte"; + import { createAuth, resumeSignIn, AUTH_KEY } from "$lib/auth.svelte"; import { createNotifications, NOTIFICATIONS_KEY } from "$lib/notifications.svelte"; - import { consumePendingCallback } from "$lib/auth/pending"; import Footer from "$lib/components/shell/Footer.svelte"; import Topbar from "$lib/components/shell/Topbar.svelte"; import { onMount, setContext, untrack } from "svelte"; @@ -22,26 +21,23 @@ ); setContext(NOTIFICATIONS_KEY, notifications); + const resumed = untrack(() => resumeSignIn(auth, page.url.pathname)); + onMount(() => { if (page.url.pathname === "/oauth/callback") return; - const pending = consumePendingCallback(); - if (!pending) { + if (!resumed) { void auth.refresh(); return; } - void (async () => { - try { - await auth.completePendingSignIn(pending.hash); - } catch (cause) { - // a previous account may still be live, recover it first - await auth.refresh(); - await loginErrorRedirect( - cause instanceof AuthorizationError ? "access_denied" : "exchange", - pending.login.handle, - pending.login.returnTo - ); - } - })(); + void resumed.done.catch(async (cause) => { + // a previous account may still be live, recover it first + await auth.refresh(); + await loginErrorRedirect( + cause instanceof AuthorizationError ? "access_denied" : "exchange", + resumed.handle, + resumed.returnTo + ); + }); }); const loginErrorRedirect = (error: string, identifier: string, returnTo: string) => {