diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index a4b39603..d98eae52 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -17,7 +17,7 @@ without the prefix is server-only. All are read at **runtime** (via | `PUBLIC_INSTANCE_URL` | browser, server | **yes in production** | The Coves backend as reachable from the browser (e.g. `https://coves.social`). The server refuses to boot in production without it, even if `PUBLIC_INTERNAL_INSTANCE` is set, because the browser can only ever see this value. In dev the OAuth cookie is scoped to this host, so `hooks.server.ts` redirects any other hostname to it (RFC 8252 requires `127.0.0.1`, not `localhost`). | | `PUBLIC_INTERNAL_INSTANCE` | server | no | Server-only shortcut to the backend for `hooks.server.ts` (`/api/me` validation) and the `/api/proxy` upstream — e.g. `http://appview:8080` on a Docker network, or `http://127.0.0.1:8081` in dev to skip the Caddy loop. Falls back to `PUBLIC_INSTANCE_URL`. | | `ALLOW_HTTP_INTERNAL_INSTANCE` | server | no | `"true"` to let the production proxy talk plaintext `http://` **only** to the origin of `PUBLIC_INTERNAL_INSTANCE` (which must then carry an explicit `http://` scheme). Any other `http://` target is still rejected with 400. | -| `PUBLIC_LOCK_TO_INSTANCE` | browser | no (default `true`) | When `true`, the login UI is pinned to `PUBLIC_INSTANCE_URL` and users cannot type a different instance. Set `false` to allow arbitrary instances. | +| `PUBLIC_LOCK_TO_INSTANCE` | browser, server | no (default `true`) | When `true`, login is pinned to `PUBLIC_INSTANCE_URL`: the login UI hides the instance field and `POST /api/auth/login` rejects any other origin with 403. Set `false` to allow arbitrary instances. | Resolution precedence: diff --git a/src/lib/app/state/auth.svelte.test.ts b/src/lib/app/state/auth.svelte.test.ts index 2e358c9d..43067f40 100644 --- a/src/lib/app/state/auth.svelte.test.ts +++ b/src/lib/app/state/auth.svelte.test.ts @@ -17,6 +17,7 @@ vi.mock('$lib/server/session', () => ({ import { isAuthenticated, isGuest, + profile, type ProfileInfo, type GuestProfile, type AuthenticatedProfile, @@ -163,3 +164,65 @@ describe('LogoutResult interface', () => { expect(result.remoteLogoutError).toBe('Token revocation failed') }) }) + +describe('Profile.syncFromServer', () => { + const authed: AuthenticatedProfile = { + type: 'authenticated', + id: 'did:plc:abc123', + instance: 'https://coves.social' as any, + jwt: 'authenticated', + did: 'did:plc:abc123' as any, + handle: 'test.user' as any, + } + + const seedAuthenticated = () => { + profile.meta.profiles = [authed] + profile.meta.profile = authed.id + } + + it('adopts the server account when authenticated', () => { + profile.syncFromServer({ + authenticated: true, + activeAccountId: 'did:plc:xyz', + account: { + id: 'did:plc:xyz', + did: 'did:plc:xyz', + handle: 'other.user', + instance: 'https://coves.social', + }, + } as any) + + expect(profile.meta.profile).toBe('did:plc:xyz') + expect(profile.meta.profiles).toHaveLength(1) + expect(isAuthenticated(profile.meta.profiles[0])).toBe(true) + }) + + it('drops a persisted authenticated profile when the server has no session', () => { + seedAuthenticated() + + profile.syncFromServer({ authenticated: false } as any) + + expect(profile.meta.profile).toBe('guest') + expect(profile.meta.profiles).toHaveLength(1) + expect(isGuest(profile.meta.profiles[0])).toBe(true) + }) + + it('drops a persisted authenticated profile when session data is missing', () => { + seedAuthenticated() + + profile.syncFromServer(undefined) + + expect(profile.meta.profile).toBe('guest') + expect(isGuest(profile.meta.profiles[0])).toBe(true) + }) + + it('leaves an existing guest profile untouched when unauthenticated', () => { + profile.syncFromServer(undefined) + const before = profile.meta.profiles[0] + + profile.syncFromServer({ authenticated: false } as any) + + expect(profile.meta.profiles[0]).toBe(before) + expect(profile.meta.profile).toBe('guest') + }) +}) diff --git a/src/lib/app/state/auth.svelte.ts b/src/lib/app/state/auth.svelte.ts index 7bb199f7..77085f12 100644 --- a/src/lib/app/state/auth.svelte.ts +++ b/src/lib/app/state/auth.svelte.ts @@ -266,7 +266,17 @@ class Profile { * @param serverSession - The session data from the server (passed via page data) */ syncFromServer(serverSession: ServerSession | undefined): void { - if (!serverSession || !serverSession.authenticated) return + if (!serverSession || !serverSession.authenticated) { + // The server is the source of truth. If it reports no session (cookie + // expired, revoked, or cleared) drop any persisted authenticated + // profile so a shared device doesn't keep showing the previous user's + // handle and avatar as if they were still signed in. + if (this.meta.profiles.some(isAuthenticated)) { + this.meta.profiles = [createGuestProfile()] + this.meta.profile = 'guest' + } + return + } // Convert server account to client ProfileInfo format const serverProfile: AuthenticatedProfile = { diff --git a/src/lib/app/state/instance.svelte.ts b/src/lib/app/state/instance.svelte.ts index 884d3886..629b4887 100644 --- a/src/lib/app/state/instance.svelte.ts +++ b/src/lib/app/state/instance.svelte.ts @@ -3,6 +3,7 @@ import { env } from '$env/dynamic/public' import { profile } from './auth.svelte' import { hasRequiredInstanceConfig, + isLockedToInstance, MISSING_INSTANCE_MESSAGE, resolveInstanceUrl, } from './instance/resolve' @@ -17,10 +18,9 @@ class InstanceData { export const instance = new InstanceData() -export const LINKED_INSTANCE_URL = - (env.PUBLIC_LOCK_TO_INSTANCE ?? 'true').toLowerCase() == 'true' - ? env.PUBLIC_INSTANCE_URL - : undefined +export const LINKED_INSTANCE_URL = isLockedToInstance(env) + ? env.PUBLIC_INSTANCE_URL + : undefined const getDefaultInstance = (): string => { // The instance URL must never default to a third-party host. In production diff --git a/src/lib/app/state/instance/resolve.test.ts b/src/lib/app/state/instance/resolve.test.ts index 39b425d9..cfafe7c2 100644 --- a/src/lib/app/state/instance/resolve.test.ts +++ b/src/lib/app/state/instance/resolve.test.ts @@ -4,7 +4,9 @@ import { canonicalPublicHost, hasRequiredInstanceConfig, instanceOrigin, + isLockedToInstance, isUpstreamSchemeAllowed, + lockedInstanceOrigin, normalizeInstanceUrl, resolveInstanceUrl, } from './resolve' @@ -155,3 +157,39 @@ describe('addressHeaderWarning', () => { } }) }) + +describe('isLockedToInstance', () => { + it('defaults to locked when unset', () => { + expect(isLockedToInstance({})).toBe(true) + }) + + it('only the literal "false" unlocks, case-insensitively', () => { + expect(isLockedToInstance({ PUBLIC_LOCK_TO_INSTANCE: 'false' })).toBe(false) + expect(isLockedToInstance({ PUBLIC_LOCK_TO_INSTANCE: 'FALSE' })).toBe(false) + expect(isLockedToInstance({ PUBLIC_LOCK_TO_INSTANCE: 'true' })).toBe(true) + expect(isLockedToInstance({ PUBLIC_LOCK_TO_INSTANCE: 'no' })).toBe(false) + }) +}) + +describe('lockedInstanceOrigin', () => { + it('returns the public origin when locked', () => { + expect(lockedInstanceOrigin(BOTH)).toBe('https://coves.social') + }) + + it('normalises a bare host and drops any path', () => { + expect( + lockedInstanceOrigin({ PUBLIC_INSTANCE_URL: 'coves.social/app' }), + ).toBe('https://coves.social') + }) + + it('returns null when unlocked', () => { + expect( + lockedInstanceOrigin({ ...BOTH, PUBLIC_LOCK_TO_INSTANCE: 'false' }), + ).toBeNull() + }) + + it('returns null when there is nothing to pin to', () => { + expect(lockedInstanceOrigin({})).toBeNull() + expect(lockedInstanceOrigin({ PUBLIC_INSTANCE_URL: '::' })).toBeNull() + }) +}) diff --git a/src/lib/app/state/instance/resolve.ts b/src/lib/app/state/instance/resolve.ts index 3be9a1df..695db6c9 100644 --- a/src/lib/app/state/instance/resolve.ts +++ b/src/lib/app/state/instance/resolve.ts @@ -25,6 +25,7 @@ export interface InstanceEnv { readonly PUBLIC_INSTANCE_URL?: string readonly PUBLIC_INTERNAL_INSTANCE?: string + readonly PUBLIC_LOCK_TO_INSTANCE?: string } export type InstanceSide = 'browser' | 'server' @@ -85,6 +86,24 @@ export function instanceOrigin(raw: string | undefined): string | null { return new URL(normalized).origin } +/** + * Whether this deployment pins login to `PUBLIC_INSTANCE_URL`. + * Defaults to locked; only the literal `"false"` (any case) opens it up. + */ +export function isLockedToInstance(env: InstanceEnv): boolean { + return (env.PUBLIC_LOCK_TO_INSTANCE ?? 'true').toLowerCase() === 'true' +} + +/** + * The only origin login may target when the deployment is locked, or null + * when unlocked or when `PUBLIC_INSTANCE_URL` is unset/invalid (nothing to + * pin to, so callers must not enforce). + */ +export function lockedInstanceOrigin(env: InstanceEnv): string | null { + if (!isLockedToInstance(env)) return null + return instanceOrigin(env.PUBLIC_INSTANCE_URL) +} + /** * Host (with port) of `PUBLIC_INSTANCE_URL`, used in development to redirect * `localhost` to the `127.0.0.1` origin the OAuth cookie is scoped to. diff --git a/src/lib/server/instance.ts b/src/lib/server/instance.ts index ff0fd790..f0f58e82 100644 --- a/src/lib/server/instance.ts +++ b/src/lib/server/instance.ts @@ -10,6 +10,7 @@ import { addressHeaderWarning, canonicalPublicHost, isUpstreamSchemeAllowed, + lockedInstanceOrigin, MISSING_INSTANCE_MESSAGE, resolveInstanceUrl, } from '$lib/app/state/instance/resolve' @@ -55,6 +56,15 @@ export function addressHeaderConfigWarning(): string | null { ) } +/** + * When `PUBLIC_LOCK_TO_INSTANCE` is on, the sole origin login may target; + * null when unlocked or unconfigured. The login UI hides the instance field + * under the same flag — this is the server-side half of that policy. + */ +export function loginLockedOrigin(): string | null { + return lockedInstanceOrigin(publicEnv) +} + /** Applies the `ALLOW_HTTP_INTERNAL_INSTANCE` plaintext policy to a target URL. */ export function upstreamSchemeAllowed(target: string): boolean { return isUpstreamSchemeAllowed(target, { diff --git a/src/routes/api/auth/auth.test.ts b/src/routes/api/auth/auth.test.ts index ed7e88d2..ab50e004 100644 --- a/src/routes/api/auth/auth.test.ts +++ b/src/routes/api/auth/auth.test.ts @@ -14,6 +14,10 @@ vi.mock('$env/dynamic/private', () => ({ env: {}, })) +// Mutable so individual tests can flip the instance-lock policy. +const publicEnv = vi.hoisted((): Record => ({})) +vi.mock('$env/dynamic/public', () => ({ env: publicEnv })) + /** * Helper to create authenticated App.Locals with the new shape. */ @@ -46,6 +50,52 @@ global.fetch = mockFetch describe('POST /api/auth/login', () => { beforeEach(() => { vi.clearAllMocks() + for (const key of Object.keys(publicEnv)) delete publicEnv[key] + }) + + describe('PUBLIC_LOCK_TO_INSTANCE enforcement', () => { + const login = (instance: string) => + loginHandler( + createMockEvent({ + method: 'POST', + body: { handle: 'user.example.com', instance }, + cookies: createMockCookies(), + url: 'http://localhost:5173/api/auth/login', + }), + ) + + it('rejects a foreign instance with 403 when locked (the default)', async () => { + publicEnv.PUBLIC_INSTANCE_URL = 'https://coves.social' + + const response = await login('https://attacker.example') + + expect(response.status).toBe(403) + expect((await response.json()).error).toMatch(/own instance/) + }) + + it('compares origins, so a bare host or trailing path still matches', async () => { + publicEnv.PUBLIC_INSTANCE_URL = 'https://coves.social' + + expect((await login('coves.social')).status).toBe(200) + expect((await login('https://coves.social/')).status).toBe(200) + }) + + it('rejects a scheme downgrade of the locked instance', async () => { + publicEnv.PUBLIC_INSTANCE_URL = 'https://coves.social' + + expect((await login('http://coves.social')).status).toBe(403) + }) + + it('allows any instance when PUBLIC_LOCK_TO_INSTANCE=false', async () => { + publicEnv.PUBLIC_INSTANCE_URL = 'https://coves.social' + publicEnv.PUBLIC_LOCK_TO_INSTANCE = 'false' + + expect((await login('https://other.example')).status).toBe(200) + }) + + it('does not enforce when PUBLIC_INSTANCE_URL is unset', async () => { + expect((await login('https://other.example')).status).toBe(200) + }) }) it('returns OAuth redirect URL for valid handle/instance', async () => { diff --git a/src/routes/api/auth/login/+server.ts b/src/routes/api/auth/login/+server.ts index 3d290666..5cc0fb9a 100644 --- a/src/routes/api/auth/login/+server.ts +++ b/src/routes/api/auth/login/+server.ts @@ -4,6 +4,7 @@ import type { RequestHandler } from './$types' import { PENDING_AUTH_COOKIE_OPTIONS } from '$lib/server/cookies' import { generateOAuthState } from '$lib/server/csrf' import { normalizeInstanceUrl } from '$lib/app/state/instance/resolve' +import { loginLockedOrigin } from '$lib/server/instance' interface LoginRequest { handle: string @@ -51,13 +52,28 @@ export const POST: RequestHandler = async ({ } const instanceUrl = new URL(normalizedInstance) - // Built once: all four logged rejections below share this request context. + // Built once: every logged rejection below shares this request context. const logContext = { requestId: locals.requestId, method: request.method, path: url.pathname, } + // PUBLIC_LOCK_TO_INSTANCE is a deployment policy, not a UI preference: the + // login form hides the instance field, but this endpoint is reachable + // directly, so refuse to start OAuth against any other origin. + const lockedOrigin = loginLockedOrigin() + if (lockedOrigin !== null && instanceUrl.origin !== lockedOrigin) { + log.warn( + `[auth/login] Rejected login to non-locked instance: ${instanceUrl.origin}`, + logContext, + ) + return json( + { error: 'This deployment only allows login to its own instance' }, + { status: 403 }, + ) + } + // Validate redirect URL to prevent open redirect attacks // Only allow relative URLs (starting with /) or same-origin URLs let safeRedirect = '/'