diff --git a/.env.example b/.env.example index 35d62af..89b009c 100644 --- a/.env.example +++ b/.env.example @@ -216,6 +216,7 @@ DOCKER_TAG=latest # Login page: tab that opens first when both email and SSO are available # AUTH_DEFAULT_METHOD=email # email or sso +# AUTH_ALLOW_SIGNUP=false # true keeps email sign-up open after the first account # Header Authentication (for Reverse Proxy SSO) # Enable automatic authentication via reverse proxy headers diff --git a/CHANGELOG.md b/CHANGELOG.md index cbcd659..a1ce23b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Security +- Fixed four privately reported vulnerabilities (GHSA-9m33-xfrc-5jxw, GHSA-6m23-28hh-gjh2, GHSA-2hpx-83vg-gm45, GHSA-5pp8-r7f5-6q8p) + - `POST /api/gitea/test-connection` and `POST /api/github/test-connection` now require a signed-in user; they were the only non-public API routes without a guard and made a server side request to any URL in the body + - The email sign-up endpoint is closed server side once an account exists (`AUTH_ALLOW_SIGNUP=true` reopens it); only the signup page redirected before, so anyone reaching the instance could create an account through the API + - SSO providers are scoped to the user who created them for listing, updating and deleting, and the OIDC client secret is no longer returned by the API (the form keeps the stored secret when the field is left blank) + - HTTP errors from Gitea and other hosts no longer carry the upstream response body in the message, so an error can no longer echo the content of a host the caller chose + - Requests to user supplied URLs (connection tests, OIDC discovery, ntfy, Gotify, Apprise and webhook notifications) refuse link local and cloud metadata addresses and no longer follow redirects; private networks stay allowed because mirroring to a LAN Gitea is the normal deployment - Raised the devalue floor to 5.9.2 in the application and the documentation site (GHSA-9rgm-9g3h-6x36, denial of service through malformed input); both lockfiles resolve 5.9.4 - Raised the dependency floors for the advisories published on 2026-09-08: Astro 7.2.8 (remote code execution through AVIF image optimization, and an authorization bypass when stripping the configured base), @xmldom/xmldom 0.8.15 (eight parser and serializer issues), sharp 0.35.4 (libheif), svgo 4.1.0 (removeScripts sanitization) and js-yaml 4.3.2 (merge-key CPU use). Applied to both the application and the documentation site. diff --git a/docker-compose.yml b/docker-compose.yml index 82232d9..014871f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -81,6 +81,7 @@ services: # - GITEA_SKIP_TLS_VERIFY=${GITEA_SKIP_TLS_VERIFY:-false} # Login page: which tab opens first when both methods are available - AUTH_DEFAULT_METHOD=${AUTH_DEFAULT_METHOD:-email} + - AUTH_ALLOW_SIGNUP=${AUTH_ALLOW_SIGNUP:-false} # Header Authentication (for Reverse Proxy SSO) - HEADER_AUTH_ENABLED=${HEADER_AUTH_ENABLED:-false} - HEADER_AUTH_USER_HEADER=${HEADER_AUTH_USER_HEADER:-X-Authentik-Username} diff --git a/docs/ENVIRONMENT_VARIABLES.md b/docs/ENVIRONMENT_VARIABLES.md index 33cf2f6..25b540f 100644 --- a/docs/ENVIRONMENT_VARIABLES.md +++ b/docs/ENVIRONMENT_VARIABLES.md @@ -347,6 +347,7 @@ Configure authentication methods and SSO. | Variable | Description | Default | Options | |----------|-------------|---------|---------| | `AUTH_DEFAULT_METHOD` | Tab the login page opens on when both email and SSO are available. Any other value falls back to `email`. | `email` | `email`, `sso` | +| `AUTH_ALLOW_SIGNUP` | Allow email and password sign-up after the first account exists. Off, the sign-up endpoint answers 403 once a user exists; the first account can always be created. | `false` | `true`, `false` | The login page also remembers the method last used in that browser, which wins over this default. Anything unrecognised falls back to `email`. diff --git a/src/components/config/SSOSettings.tsx b/src/components/config/SSOSettings.tsx index 40a9764..195d4d7 100644 --- a/src/components/config/SSOSettings.tsx +++ b/src/components/config/SSOSettings.tsx @@ -454,6 +454,7 @@ export function SSOSettings() { setProviderForm(prev => ({ ...prev, clientSecret: e.target.value }))} /> diff --git a/src/lib/auth-signup-guard.test.ts b/src/lib/auth-signup-guard.test.ts new file mode 100644 index 0000000..6d470c3 --- /dev/null +++ b/src/lib/auth-signup-guard.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "bun:test"; +import { + isOpenRegistration, + isSignupClosed, + isSignupRequest, + signupClosedError, + signupGuardPlugin, +} from "./auth-signup-guard"; + +describe("signup guard", () => { + it("only matches the email sign-up endpoint", () => { + expect(isSignupRequest("/sign-up/email")).toBe(true); + expect(isSignupRequest("/sign-in/email")).toBe(false); + expect(isSignupRequest("/sign-up/email/extra")).toBe(false); + expect(isSignupRequest(undefined)).toBe(false); + }); + + it("opens registration only for the literal true", () => { + expect(isOpenRegistration({ AUTH_ALLOW_SIGNUP: "true" } as NodeJS.ProcessEnv)).toBe(true); + expect(isOpenRegistration({ AUTH_ALLOW_SIGNUP: "TRUE" } as NodeJS.ProcessEnv)).toBe(false); + expect(isOpenRegistration({ AUTH_ALLOW_SIGNUP: "1" } as NodeJS.ProcessEnv)).toBe(false); + expect(isOpenRegistration({} as NodeJS.ProcessEnv)).toBe(false); + }); + + it("closes sign-up once a user exists unless registration is open", () => { + expect(isSignupClosed({ userCount: 0, allowSignup: false })).toBe(false); + expect(isSignupClosed({ userCount: 1, allowSignup: false })).toBe(true); + expect(isSignupClosed({ userCount: 5, allowSignup: true })).toBe(false); + }); + + it("answers 403 with a stable code", () => { + const error = signupClosedError(); + expect(error.status).toBe("FORBIDDEN"); + expect(error.body?.code).toBe("SIGNUP_CLOSED"); + }); + + it("registers a before hook on the sign-up path that counts users", async () => { + let counted = 0; + const plugin = signupGuardPlugin({ + countUsers: async () => { + counted += 1; + return 1; + }, + }); + const hook = plugin.hooks.before[0]; + expect(hook.matcher({ path: "/sign-up/email" } as any)).toBe(true); + expect(hook.matcher({ path: "/sign-in/email" } as any)).toBe(false); + + const previous = process.env.AUTH_ALLOW_SIGNUP; + delete process.env.AUTH_ALLOW_SIGNUP; + try { + await expect(hook.handler({ path: "/sign-up/email", context: {} } as any)).rejects.toMatchObject({ + body: { code: "SIGNUP_CLOSED" }, + }); + expect(counted).toBe(1); + + process.env.AUTH_ALLOW_SIGNUP = "true"; + await hook.handler({ path: "/sign-up/email", context: {} } as any); + expect(counted).toBe(1); + } finally { + if (previous === undefined) delete process.env.AUTH_ALLOW_SIGNUP; + else process.env.AUTH_ALLOW_SIGNUP = previous; + } + }); +}); diff --git a/src/lib/auth-signup-guard.ts b/src/lib/auth-signup-guard.ts new file mode 100644 index 0000000..25277f8 --- /dev/null +++ b/src/lib/auth-signup-guard.ts @@ -0,0 +1,68 @@ +import type { BetterAuthPlugin } from "better-auth"; +import { APIError, createAuthMiddleware } from "better-auth/api"; + +/** + * Closes email and password sign-up once the first account exists. + * + * The signup page already redirects to the login page when a user exists, + * but that check lived only in the page. The Better Auth endpoint behind + * it, POST /api/auth/sign-up/email, accepted new accounts from anyone who + * could reach the instance, which turned every "signed in users only" API + * route into a public one. This plugin refuses the endpoint server side + * once a user exists, unless the operator opts into open registration with + * AUTH_ALLOW_SIGNUP=true. + * + * SSO sign-ins through a registered identity provider and header auth + * provisioning (HEADER_AUTH_AUTO_PROVISION) are separate flows and are not + * affected: the operator chose those providers. + */ + +/** Endpoint path (without the auth base path) of email sign-up. */ +export const SIGNUP_PATH = "/sign-up/email"; + +export function isSignupRequest(path: string | undefined): boolean { + return path === SIGNUP_PATH; +} + +/** Reads the opt-in from the environment: only the literal "true" opens sign-up. */ +export function isOpenRegistration(env: NodeJS.ProcessEnv = process.env): boolean { + return env.AUTH_ALLOW_SIGNUP === "true"; +} + +/** True when a sign-up must be refused: an account exists and registration is closed. */ +export function isSignupClosed({ + userCount, + allowSignup, +}: { + userCount: number; + allowSignup: boolean; +}): boolean { + return userCount > 0 && !allowSignup; +} + +export function signupClosedError(): APIError { + return new APIError("FORBIDDEN", { + message: + "Sign-up is closed: an account already exists on this instance. Set AUTH_ALLOW_SIGNUP=true to allow more accounts.", + code: "SIGNUP_CLOSED", + }); +} + +export const signupGuardPlugin = (options: { countUsers: () => Promise }) => + ({ + id: "signup-guard", + hooks: { + before: [ + { + matcher: (ctx) => isSignupRequest(ctx.path), + handler: createAuthMiddleware(async () => { + if (isOpenRegistration()) return; + const userCount = await options.countUsers(); + if (isSignupClosed({ userCount, allowSignup: false })) { + throw signupClosedError(); + } + }), + }, + ], + }, + }) satisfies BetterAuthPlugin; diff --git a/src/lib/auth.ts b/src/lib/auth.ts index b1f24e4..8dd0d97 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -11,6 +11,7 @@ import { eq } from "drizzle-orm"; import { withBase } from "./base-path"; import { headerAuthPlugin } from "./auth-header-plugin"; import { apiKeyGuardPlugin } from "./auth-api-key-guard"; +import { signupGuardPlugin } from "./auth-signup-guard"; /** * Extracts the origins implied by registered SSO identity providers. @@ -402,6 +403,18 @@ export const auth = betterAuth({ // can never create, list or revoke keys. See auth-api-key-guard.ts. apiKeyGuardPlugin(), + // Refuses email sign-up once an account exists unless + // AUTH_ALLOW_SIGNUP=true. The signup page only redirected; the endpoint + // itself was open. See auth-signup-guard.ts. + signupGuardPlugin({ + countUsers: async () => { + const { db, users } = await import("./db"); + const { sql } = await import("drizzle-orm"); + const [row] = await db.select({ count: sql`count(*)` }).from(users); + return Number(row?.count ?? 0); + }, + }), + // Header / forward authentication bridge. Exposes // POST /api/auth/sign-in/header so the middleware can mint a real // Better Auth session from trusted upstream headers (Authentik / diff --git a/src/lib/http-client.ts b/src/lib/http-client.ts index 5736754..dfb2d73 100644 --- a/src/lib/http-client.ts +++ b/src/lib/http-client.ts @@ -41,25 +41,17 @@ export async function httpRequest( const responseClone = response.clone(); if (!response.ok) { - let errorMessage = `HTTP ${response.status}: ${response.statusText}`; + // The message carries the status only. The upstream body is kept on + // `HttpError.response` for logs and for callers that inspect it, but + // it never goes into the message: messages can reach API clients + // through createSecureErrorResponse, and a URL under the user's + // control would otherwise turn any error into a readable proxy of + // whatever host it pointed at. + const errorMessage = `HTTP ${response.status}: ${response.statusText}`; let responseText = ''; try { responseText = await responseClone.text(); - if (responseText) { - // Try to parse as JSON for better error messages - try { - const errorData = JSON.parse(responseText); - if (errorData.message) { - errorMessage = `HTTP ${response.status}: ${errorData.message}`; - } else { - errorMessage += ` - ${responseText}`; - } - } catch { - // Not JSON, use as-is - errorMessage += ` - ${responseText}`; - } - } } catch { // Ignore text parsing errors } @@ -104,7 +96,7 @@ export async function httpRequest( } throw new HttpError( - `Failed to parse JSON response from ${url}: ${jsonError instanceof Error ? jsonError.message : String(jsonError)}. Response: ${responseText.substring(0, 200)}${responseText.length > 200 ? '...' : ''}`, + `Failed to parse JSON response from ${url}: ${jsonError instanceof Error ? jsonError.message : String(jsonError)}`, response.status, response.statusText, responseText diff --git a/src/lib/providers/apprise.ts b/src/lib/providers/apprise.ts index 7a747ae..acc924b 100644 --- a/src/lib/providers/apprise.ts +++ b/src/lib/providers/apprise.ts @@ -1,4 +1,5 @@ import type { AppriseConfig } from "@/types/config"; +import { safeFetch } from "@/lib/utils/outbound-url"; import type { NotificationEvent } from "./ntfy"; export async function sendAppriseNotification(config: AppriseConfig, event: NotificationEvent): Promise { @@ -10,6 +11,6 @@ export async function sendAppriseNotification(config: AppriseConfig, event: Noti type: event.type === "sync_error" ? "failure" : "success", tag: config.tag || undefined, }); - const resp = await fetch(url, { method: "POST", body, headers }); - if (!resp.ok) throw new Error(`Apprise error: ${resp.status} ${await resp.text()}`); + const resp = await safeFetch(url, { method: "POST", body, headers }); + if (!resp.ok) throw new Error(`Apprise error: ${resp.status}`); } diff --git a/src/lib/providers/gotify.ts b/src/lib/providers/gotify.ts index 8e227ec..48d620a 100644 --- a/src/lib/providers/gotify.ts +++ b/src/lib/providers/gotify.ts @@ -1,4 +1,5 @@ import type { GotifyConfig } from "@/types/config"; +import { safeFetch } from "@/lib/utils/outbound-url"; import type { NotificationEvent } from "./ntfy"; export async function sendGotifyNotification(config: GotifyConfig, event: NotificationEvent): Promise { @@ -12,6 +13,6 @@ export async function sendGotifyNotification(config: GotifyConfig, event: Notifi message: event.message, priority: event.type === "sync_error" ? 8 : (config.priority ?? 5), }); - const resp = await fetch(url, { method: "POST", body, headers }); - if (!resp.ok) throw new Error(`Gotify error: ${resp.status} ${await resp.text()}`); + const resp = await safeFetch(url, { method: "POST", body, headers }); + if (!resp.ok) throw new Error(`Gotify error: ${resp.status}`); } diff --git a/src/lib/providers/ntfy.ts b/src/lib/providers/ntfy.ts index 8c1e8db..117b41e 100644 --- a/src/lib/providers/ntfy.ts +++ b/src/lib/providers/ntfy.ts @@ -1,4 +1,5 @@ import type { NtfyConfig } from "@/types/config"; +import { safeFetch } from "@/lib/utils/outbound-url"; export interface NotificationEvent { title: string; @@ -16,6 +17,6 @@ export async function sendNtfyNotification(config: NtfyConfig, event: Notificati if (config.token) { headers["Authorization"] = `Bearer ${config.token}`; } - const resp = await fetch(url, { method: "POST", body: event.message, headers }); - if (!resp.ok) throw new Error(`Ntfy error: ${resp.status} ${await resp.text()}`); + const resp = await safeFetch(url, { method: "POST", body: event.message, headers }); + if (!resp.ok) throw new Error(`Ntfy error: ${resp.status}`); } diff --git a/src/lib/providers/webhook.ts b/src/lib/providers/webhook.ts index 036ff5a..12a796a 100644 --- a/src/lib/providers/webhook.ts +++ b/src/lib/providers/webhook.ts @@ -1,4 +1,5 @@ import { createHmac } from "node:crypto"; +import { safeFetch } from "@/lib/utils/outbound-url"; import type { WebhookConfig } from "@/types/config"; import type { NotificationEvent } from "./ntfy"; @@ -16,6 +17,6 @@ export async function sendWebhookNotification(config: WebhookConfig, event: Noti const signature = createHmac("sha256", config.secret).update(body).digest("hex"); headers["X-Webhook-Signature"] = `sha256=${signature}`; } - const resp = await fetch(config.url, { method: "POST", body, headers }); - if (!resp.ok) throw new Error(`Webhook error: ${resp.status} ${await resp.text()}`); + const resp = await safeFetch(config.url, { method: "POST", body, headers }); + if (!resp.ok) throw new Error(`Webhook error: ${resp.status}`); } diff --git a/src/lib/utils/outbound-url.test.ts b/src/lib/utils/outbound-url.test.ts new file mode 100644 index 0000000..9e95c21 --- /dev/null +++ b/src/lib/utils/outbound-url.test.ts @@ -0,0 +1,145 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { + assertSafeOutboundUrl, + isLinkLocalAddress, + OutboundUrlError, + resolveOutboundTarget, + safeFetch, +} from "./outbound-url"; + +const noResolve = async () => []; + +describe("isLinkLocalAddress", () => { + it("matches the IPv4 link local range", () => { + expect(isLinkLocalAddress("169.254.169.254")).toBe(true); + expect(isLinkLocalAddress("169.254.0.1")).toBe(true); + expect(isLinkLocalAddress("169.253.1.1")).toBe(false); + expect(isLinkLocalAddress("10.0.0.1")).toBe(false); + expect(isLinkLocalAddress("192.168.1.10")).toBe(false); + expect(isLinkLocalAddress("127.0.0.1")).toBe(false); + }); + + it("matches IPv6 link local and the mapped IPv4 forms", () => { + expect(isLinkLocalAddress("fe80::1")).toBe(true); + expect(isLinkLocalAddress("FE9A::1")).toBe(true); + expect(isLinkLocalAddress("febf::1")).toBe(true); + expect(isLinkLocalAddress("fec0::1")).toBe(false); + expect(isLinkLocalAddress("::ffff:169.254.169.254")).toBe(true); + expect(isLinkLocalAddress("::ffff:a9fe:a9fe")).toBe(true); + expect(isLinkLocalAddress("::ffff:192.168.1.1")).toBe(false); + expect(isLinkLocalAddress("::1")).toBe(false); + expect(isLinkLocalAddress("not-an-ip")).toBe(false); + }); +}); + +describe("assertSafeOutboundUrl", () => { + it("allows private networks, localhost and Docker service names", async () => { + for (const url of [ + "http://192.168.1.10:3000", + "http://10.0.0.5", + "http://172.16.4.4:8080/api", + "http://localhost:3000", + "http://127.0.0.1:3000", + "http://gitea:3000", + "https://codeberg.org", + ]) { + await expect(assertSafeOutboundUrl(url, noResolve)).resolves.toBeInstanceOf(URL); + } + }); + + it("refuses the metadata address in every spelling", async () => { + for (const url of [ + "http://169.254.169.254/latest/meta-data/", + "http://[fe80::1]/", + "http://[::ffff:169.254.169.254]/", + "http://metadata.google.internal/computeMetadata/v1/", + "http://METADATA/", + ]) { + await expect(assertSafeOutboundUrl(url, noResolve)).rejects.toBeInstanceOf(OutboundUrlError); + } + }); + + it("refuses a host name that resolves to a link local address", async () => { + const resolver = async () => ["93.184.216.34", "169.254.169.254"]; + await expect(assertSafeOutboundUrl("http://evil.example/", resolver)).rejects.toThrow( + /link local/ + ); + }); + + it("leaves unresolvable names to fetch", async () => { + await expect(assertSafeOutboundUrl("http://nope.invalid/", noResolve)).resolves.toBeInstanceOf( + URL + ); + }); + + it("refuses other schemes and malformed URLs", async () => { + await expect(assertSafeOutboundUrl("file:///etc/passwd", noResolve)).rejects.toThrow( + /http and https/ + ); + await expect(assertSafeOutboundUrl("gopher://x/", noResolve)).rejects.toThrow(/http and https/); + await expect(assertSafeOutboundUrl("not a url", noResolve)).rejects.toThrow(/Invalid URL/); + }); +}); + +describe("resolveOutboundTarget", () => { + it("pins plain http to the checked address and keeps the name in Host", async () => { + const resolver = async () => ["10.0.0.5", "10.0.0.6"]; + const target = await resolveOutboundTarget("http://ntfy.local:8080/topic?x=1", resolver); + expect(target.requestUrl).toBe("http://10.0.0.5:8080/topic?x=1"); + expect(target.headers).toEqual({ Host: "ntfy.local:8080" }); + }); + + it("brackets a pinned IPv6 address", async () => { + const resolver = async () => ["fd00::10"]; + const target = await resolveOutboundTarget("http://gotify.local/message", resolver); + expect(target.requestUrl).toBe("http://[fd00::10]/message"); + expect(target.headers).toEqual({ Host: "gotify.local" }); + }); + + it("does not pin https, IP literals or unresolvable names", async () => { + const resolver = async () => ["10.0.0.5"]; + expect((await resolveOutboundTarget("https://idp.example.com/x", resolver)).headers).toEqual({}); + expect((await resolveOutboundTarget("http://192.168.1.10:3000/", resolver)).requestUrl).toBe( + "http://192.168.1.10:3000/" + ); + const unresolved = await resolveOutboundTarget("http://nope.invalid/", noResolve); + expect(unresolved.requestUrl).toBe("http://nope.invalid/"); + expect(unresolved.headers).toEqual({}); + }); +}); + +describe("safeFetch", () => { + const realFetch = globalThis.fetch; + afterEach(() => { + globalThis.fetch = realFetch; + }); + + it("never follows redirects, pins the address and refuses blocked hosts before fetching", async () => { + let seen: any = null; + globalThis.fetch = (async (input: any, init?: RequestInit) => { + const headers = new Headers(init?.headers); + seen = { url: String(input), redirect: init?.redirect, host: headers.get("host"), auth: headers.get("authorization") }; + return new Response("", { status: 302, headers: { location: "http://169.254.169.254/" } }); + }) as typeof fetch; + + const resolver = async () => ["10.0.0.5"]; + const response = await safeFetch( + "http://ntfy.local/topic", + { method: "POST", headers: { Authorization: "Bearer t" } }, + resolver + ); + expect(response.status).toBe(302); + expect(seen).toEqual({ + url: "http://10.0.0.5/topic", + redirect: "manual", + host: "ntfy.local", + auth: "Bearer t", + }); + + seen = null; + await expect(safeFetch("http://169.254.169.254/", {}, noResolve)).rejects.toBeInstanceOf( + OutboundUrlError + ); + expect(seen).toBeNull(); + }); +}); diff --git a/src/lib/utils/outbound-url.ts b/src/lib/utils/outbound-url.ts new file mode 100644 index 0000000..94059cf --- /dev/null +++ b/src/lib/utils/outbound-url.ts @@ -0,0 +1,186 @@ +/** + * Guard for outbound requests to user supplied URLs. + * + * Gitea Mirror talks to hosts on private networks by design: the Gitea it + * mirrors to usually lives on a LAN address, a Docker service name or + * localhost, and so do ntfy and Gotify. Blocking private ranges would break + * the normal deployment, so this guard is deliberately narrow. It refuses + * the addresses that are never a legitimate destination for this app and + * are the classic SSRF targets: + * + * - the IPv4 link local range 169.254.0.0/16, which carries the cloud + * instance metadata service on AWS, GCP, Azure, DigitalOcean and others + * - the IPv6 link local range fe80::/10 and the IPv4 mapped form of the + * same range + * - the well known metadata host names + * - anything that is not plain http or https + * + * Host names are resolved and every address they resolve to is checked, so + * a DNS name pointing at the metadata range is refused too. A plain http + * request is then sent to the address that was checked, with the original + * name in the Host header, so a name that changes its answer between the + * check and the request (DNS rebinding) gains nothing. An https request + * keeps the name: the certificate is validated against it, and the + * metadata services this guard exists for speak no TLS at all. Callers + * that follow redirects must run the check on every hop; `safeFetch` below + * does not follow redirects at all. + */ + +import { isIP } from "node:net"; +import { lookup } from "node:dns/promises"; + +export class OutboundUrlError extends Error { + constructor(message: string) { + super(message); + this.name = "OutboundUrlError"; + } +} + +const BLOCKED_HOSTNAMES = new Set([ + "metadata.google.internal", + "metadata.goog", + "metadata", + "instance-data", + "instance-data.ec2.internal", +]); + +/** True for an address in 169.254.0.0/16 or fe80::/10 (including v4 mapped). */ +export function isLinkLocalAddress(address: string): boolean { + const version = isIP(address); + if (version === 4) { + const [a, b] = address.split(".").map(Number); + return a === 169 && b === 254; + } + if (version === 6) { + const lower = address.toLowerCase(); + // ::ffff:169.254.x.y (dotted form) + const mapped = lower.match(/^(?:0*:)*ffff:(\d+\.\d+\.\d+\.\d+)$/); + if (mapped) return isLinkLocalAddress(mapped[1]); + // ::ffff:a9fe:xxxx (hex form of 169.254.0.0/16) + if (/^(?:0*:)*ffff:a9fe:[0-9a-f]{1,4}$/.test(lower)) return true; + // fe80::/10 covers fe80 to febf + return /^fe[89ab][0-9a-f]:/.test(lower); + } + return false; +} + +function stripBrackets(hostname: string): string { + return hostname.startsWith("[") && hostname.endsWith("]") + ? hostname.slice(1, -1) + : hostname; +} + +export type AddressResolver = (hostname: string) => Promise; + +async function defaultResolver(hostname: string): Promise { + try { + const records = await lookup(hostname, { all: true, verbatim: true }); + return records.map((record) => record.address); + } catch { + // Unresolvable names are left to fetch, which fails on them anyway. + return []; + } +} + +/** + * Parse a user supplied URL and refuse it when it points at a blocked + * destination. Returns the parsed URL on success. + */ +export async function assertSafeOutboundUrl( + rawUrl: string, + resolver: AddressResolver = defaultResolver +): Promise { + let url: URL; + try { + url = new URL(rawUrl); + } catch { + throw new OutboundUrlError("Invalid URL format"); + } + + if (url.protocol !== "http:" && url.protocol !== "https:") { + throw new OutboundUrlError("Only http and https URLs are allowed"); + } + + const hostname = stripBrackets(url.hostname).toLowerCase(); + if (!hostname) { + throw new OutboundUrlError("Invalid URL format"); + } + + if (BLOCKED_HOSTNAMES.has(hostname)) { + throw new OutboundUrlError("Requests to the instance metadata service are not allowed"); + } + + if (isIP(hostname)) { + if (isLinkLocalAddress(hostname)) { + throw new OutboundUrlError("Requests to link local addresses are not allowed"); + } + return url; + } + + const addresses = await resolver(hostname); + if (addresses.some(isLinkLocalAddress)) { + throw new OutboundUrlError( + `${hostname} resolves to a link local address, which is not allowed` + ); + } + + return url; +} + +export interface OutboundTarget { + /** The URL as given, parsed and checked. */ + url: URL; + /** The URL to actually request: pinned to the checked address for plain http. */ + requestUrl: string; + /** Headers the request must carry (the original Host when pinned). */ + headers: Record; +} + +/** + * Check a user supplied URL and decide where the request goes. Plain http + * to a host name is pinned to the first address the check saw; everything + * else is requested as given. + */ +export async function resolveOutboundTarget( + rawUrl: string, + resolver: AddressResolver = defaultResolver +): Promise { + const url = await assertSafeOutboundUrl(rawUrl, resolver); + const hostname = stripBrackets(url.hostname); + + if (url.protocol !== "http:" || isIP(hostname)) { + return { url, requestUrl: url.toString(), headers: {} }; + } + + const [address] = await resolver(hostname.toLowerCase()); + if (!address || isLinkLocalAddress(address)) { + // Unresolvable names are left to fetch, which fails on them anyway. + return { url, requestUrl: url.toString(), headers: {} }; + } + + const pinned = new URL(url.toString()); + pinned.hostname = isIP(address) === 6 ? `[${address}]` : address; + return { url, requestUrl: pinned.toString(), headers: { Host: url.host } }; +} + +/** + * fetch for user supplied URLs: runs the guard first, pins plain http to + * the checked address and never follows a redirect, so a permitted host + * cannot bounce the request to a blocked one. + */ +export async function safeFetch( + rawUrl: string, + init: RequestInit = {}, + resolver?: AddressResolver +): Promise { + const target = await resolveOutboundTarget(rawUrl, resolver); + let headers: HeadersInit; + if (init.headers instanceof Headers || Array.isArray(init.headers)) { + const merged = new Headers(init.headers); + for (const [name, value] of Object.entries(target.headers)) merged.set(name, value); + headers = merged; + } else { + headers = { ...(init.headers ?? {}), ...target.headers }; + } + return fetch(target.requestUrl, { ...init, headers, redirect: "manual" }); +} diff --git a/src/pages/api/gitea/test-connection.ts b/src/pages/api/gitea/test-connection.ts index 9ede4ea..9d84a0b 100644 --- a/src/pages/api/gitea/test-connection.ts +++ b/src/pages/api/gitea/test-connection.ts @@ -1,6 +1,8 @@ import type { APIRoute } from 'astro'; import { httpGet, HttpError } from '@/lib/http-client'; import { createSecureErrorResponse } from '@/lib/utils'; +import { requireAuthenticatedUserId } from '@/lib/auth-guards'; +import { assertSafeOutboundUrl, resolveOutboundTarget, OutboundUrlError } from '@/lib/utils/outbound-url'; import { DESTINATION_PROVIDER_LABELS, isPushDestinationKind, @@ -81,13 +83,27 @@ async function testPushTarget( } } -export const POST: APIRoute = async ({ request }) => { +export const POST: APIRoute = async ({ request, locals }) => { + // The route makes a server side request to a URL from the body, so it is + // for signed in users only, like every other API route that reaches out. + const authResult = await requireAuthenticatedUserId({ request, locals }); + if ('response' in authResult) return authResult.response; + try { const body = await request.json(); const { url, token, username } = body; const provider = normalizeDestinationProviderKind(body.provider); if (isPushDestinationKind(provider)) { + const pushUrl = normalizeDestinationBaseUrl(url, provider); + try { + await assertSafeOutboundUrl(pushUrl); + } catch (error) { + if (error instanceof OutboundUrlError) { + return json({ success: false, message: error.message }, 400); + } + throw error; + } return testPushTarget(provider, { url, token, username }); } @@ -96,10 +112,24 @@ export const POST: APIRoute = async ({ request }) => { } // Normalize the URL (remove trailing slash if present) - const baseUrl = url.endsWith('/') ? url.slice(0, -1) : url; + const givenUrl = String(url).endsWith('/') ? String(url).slice(0, -1) : String(url); + + // Checked and, for plain http, pinned to the checked address so the + // two requests below go where the check looked. + let target: Awaited>; + try { + target = await resolveOutboundTarget(givenUrl); + } catch (error) { + if (error instanceof OutboundUrlError) { + return json({ success: false, message: error.message }, 400); + } + throw error; + } + const baseUrl = target.requestUrl.replace(/\/$/, ''); // Test the connection by fetching the authenticated user const response = await httpGet(`${baseUrl}/api/v1/user`, { + ...target.headers, 'Authorization': `token ${token}`, 'Accept': 'application/json', }); @@ -114,6 +144,7 @@ export const POST: APIRoute = async ({ request }) => { let serverInfo: ReturnType | undefined; try { const versionResp = await httpGet(`${baseUrl}/api/v1/version`, { + ...target.headers, 'Accept': 'application/json', }); if (typeof versionResp.data?.version === 'string') { diff --git a/src/pages/api/github/test-connection.test.ts b/src/pages/api/github/test-connection.test.ts index 17fb13a..169f1ec 100644 --- a/src/pages/api/github/test-connection.test.ts +++ b/src/pages/api/github/test-connection.test.ts @@ -55,7 +55,7 @@ describe("GitHub Test Connection API", () => { body: JSON.stringify({}) }); - const response = await POST({ request } as any); + const response = await POST({ request, locals: { user: { id: "user-1" } } } as any); expect(response.status).toBe(400); @@ -75,7 +75,7 @@ describe("GitHub Test Connection API", () => { }) }); - const response = await POST({ request } as any); + const response = await POST({ request, locals: { user: { id: "user-1" } } } as any); expect(response.status).toBe(200); @@ -101,7 +101,7 @@ describe("GitHub Test Connection API", () => { }) }); - const response = await POST({ request } as any); + const response = await POST({ request, locals: { user: { id: "user-1" } } } as any); expect(response.status).toBe(400); @@ -124,7 +124,7 @@ describe("GitHub Test Connection API", () => { }) }); - const response = await POST({ request } as any); + const response = await POST({ request, locals: { user: { id: "user-1" } } } as any); expect(response.status).toBe(500); diff --git a/src/pages/api/github/test-connection.ts b/src/pages/api/github/test-connection.ts index b9da9e3..ac47a3f 100644 --- a/src/pages/api/github/test-connection.ts +++ b/src/pages/api/github/test-connection.ts @@ -1,5 +1,7 @@ import type { APIRoute } from "astro"; import { createSecureErrorResponse } from "@/lib/utils"; +import { requireAuthenticatedUserId } from "@/lib/auth-guards"; +import { assertSafeOutboundUrl, OutboundUrlError } from "@/lib/utils/outbound-url"; import { SOURCE_PROVIDER_LABELS, createSourceProvider, @@ -20,7 +22,12 @@ function json(body: unknown, status: number): Response { * compatibility, but serves every source provider: the body carries the * provider and, for GitLab and Gitea, the instance URL. */ -export const POST: APIRoute = async ({ request }) => { +export const POST: APIRoute = async ({ request, locals }) => { + // The route makes a server side request to a URL from the body, so it is + // for signed in users only, like every other API route that reaches out. + const authResult = await requireAuthenticatedUserId({ request, locals }); + if ("response" in authResult) return authResult.response; + const body = await request.json().catch(() => ({})); const { token, username, provider: rawProvider, url } = body ?? {}; const provider = normalizeSourceProviderKind(rawProvider); @@ -31,10 +38,20 @@ export const POST: APIRoute = async ({ request }) => { return json({ success: false, message: `${label} token is required` }, 400); } + const instanceUrl = normalizeSourceUrl(url, provider); + try { + await assertSafeOutboundUrl(instanceUrl); + } catch (error) { + if (error instanceof OutboundUrlError) { + return json({ success: false, message: error.message }, 400); + } + throw error; + } + // GitHub honors GH_API_URL / GITHUB_API_URL for GHES / GHEC inside the adapter. const sourceProvider = createSourceProvider({ provider, - url: normalizeSourceUrl(url, provider), + url: instanceUrl, username: typeof username === "string" ? username : "", token, }); diff --git a/src/pages/api/sso/discover.ts b/src/pages/api/sso/discover.ts index 8eb7d64..394baca 100644 --- a/src/pages/api/sso/discover.ts +++ b/src/pages/api/sso/discover.ts @@ -1,6 +1,7 @@ import type { APIContext } from "astro"; import { createSecureErrorResponse } from "@/lib/utils"; import { requireAuth } from "@/lib/utils/auth-helpers"; +import { safeFetch, OutboundUrlError } from "@/lib/utils/outbound-url"; // POST /api/sso/discover - Discover OIDC configuration from issuer URL export async function POST(context: APIContext) { @@ -45,13 +46,16 @@ export async function POST(context: APIContext) { let response: Response; try { - response = await fetch(discoveryUrl, { + response = await safeFetch(discoveryUrl, { signal: controller.signal, headers: { 'Accept': 'application/json', } }); } catch (fetchError) { + if (fetchError instanceof OutboundUrlError) { + throw fetchError; + } if (fetchError instanceof Error && fetchError.name === 'AbortError') { throw new Error(`Request timeout: The OIDC provider at ${trimmedIssuer} did not respond within 10 seconds`); } diff --git a/src/pages/api/sso/providers.test.ts b/src/pages/api/sso/providers.test.ts new file mode 100644 index 0000000..d700b8a --- /dev/null +++ b/src/pages/api/sso/providers.test.ts @@ -0,0 +1,162 @@ +/** + * SSO provider ownership (GHSA-2hpx-83vg-gm45). + * + * The routes authenticated the caller but read, updated and deleted + * providers by id across every user, and the list returned the OIDC client + * secret. Every query is now scoped to the caller and the secret stays on + * the server. + * + * Runs in an isolated child so the @/lib/db and auth mocks cannot leak into + * other files (same harness as ../organizations/[id]/status.test.ts). + */ + +import { describe, test, expect, mock } from "bun:test"; +import { Database } from "bun:sqlite"; +import { drizzle } from "drizzle-orm/bun-sqlite"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import * as schema from "@/lib/db/schema"; + +const CHILD_FLAG = "GM_SSO_PROVIDERS_ROUTE_ISOLATED"; +const isChild = !!process.env[CHILD_FLAG]; + +if (!isChild) { + test("sso providers route (ownership) - isolated child suite", () => { + const res = Bun.spawnSync({ + cmd: [process.execPath, "test", import.meta.path], + env: { ...process.env, [CHILD_FLAG]: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + if (res.exitCode !== 0) { + console.error(res.stdout.toString()); + console.error(res.stderr.toString()); + } + expect(res.exitCode).toBe(0); + }, 60_000); +} + +const sqlite = new Database(":memory:"); +const db = drizzle({ client: sqlite }); + +function applyMigrations(): void { + const folder = join(process.cwd(), "drizzle"); + const journal = JSON.parse(readFileSync(join(folder, "meta", "_journal.json"), "utf8")) as { + entries: { tag: string }[]; + }; + for (const entry of journal.entries) { + const sql = readFileSync(join(folder, `${entry.tag}.sql`), "utf8"); + for (const statement of sql.split("--> statement-breakpoint")) { + if (statement.trim()) sqlite.run(statement); + } + } +} + +let currentUserId = "alice"; + +if (isChild) { + applyMigrations(); + sqlite.run("PRAGMA foreign_keys = OFF"); + + mock.module("@/lib/utils/auth-helpers", () => ({ + requireAuth: mock(async () => ({ user: { id: currentUserId }, response: null })), + })); + + mock.module("@/lib/db", () => ({ ...schema, db })); +} + +const routes = isChild + ? await import("./providers") + : ({} as typeof import("./providers")); + +function seed(id: string, userId: string, providerId: string) { + sqlite.run( + `INSERT INTO sso_providers (id, issuer, domain, oidc_config, user_id, provider_id, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + [ + id, + "https://idp.example.com", + "example.com", + JSON.stringify({ + clientId: `client-${id}`, + clientSecret: `secret-${id}`, + authorizationEndpoint: "https://idp.example.com/auth", + tokenEndpoint: "https://idp.example.com/token", + jwksEndpoint: "https://idp.example.com/jwks", + userInfoEndpoint: "https://idp.example.com/userinfo", + scopes: ["openid"], + }), + userId, + providerId, + Date.now(), + Date.now(), + ] + ); +} + +function ctx(method: string, query = "", body?: unknown) { + return { + request: new Request(`http://localhost/api/sso/providers${query}`, { + method, + headers: { "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }), + } as any; +} + +describe.skipIf(!isChild)("SSO providers ownership", () => { + test("GET lists only the caller's providers and never the client secret", async () => { + seed("p-alice", "alice", "alice-idp"); + seed("p-bob", "bob", "bob-idp"); + + currentUserId = "alice"; + const response = await routes.GET(ctx("GET")); + expect(response.status).toBe(200); + const list = (await response.json()) as any[]; + expect(list.map((p) => p.id)).toEqual(["p-alice"]); + expect(JSON.stringify(list)).not.toContain("secret-"); + expect(list[0].oidcConfig.hasClientSecret).toBe(true); + expect(list[0].oidcConfig.clientId).toBe("client-p-alice"); + }); + + test("PUT on another user's provider answers 404 and changes nothing", async () => { + currentUserId = "alice"; + const response = await routes.PUT(ctx("PUT", "?id=p-bob", { domain: "evil.example" })); + expect(response.status).toBe(404); + + const row = sqlite.query("SELECT domain, oidc_config FROM sso_providers WHERE id = 'p-bob'").get() as any; + expect(row.domain).toBe("example.com"); + expect(JSON.parse(row.oidc_config).clientSecret).toBe("secret-p-bob"); + }); + + test("PUT with an empty secret keeps the stored one", async () => { + currentUserId = "alice"; + const response = await routes.PUT( + ctx("PUT", "?id=p-alice", { domain: "alice.example", clientSecret: "" }) + ); + const body = await response.json(); + expect([response.status, body]).toEqual([200, expect.anything()]); + expect(body.oidcConfig.clientSecret).toBeUndefined(); + expect(body.oidcConfig.hasClientSecret).toBe(true); + + const row = sqlite.query("SELECT domain, oidc_config FROM sso_providers WHERE id = 'p-alice'").get() as any; + expect(row.domain).toBe("alice.example"); + expect(JSON.parse(row.oidc_config).clientSecret).toBe("secret-p-alice"); + }); + + test("DELETE on another user's provider answers 404 and keeps the row", async () => { + currentUserId = "alice"; + const response = await routes.DELETE(ctx("DELETE", "?id=p-bob")); + expect(response.status).toBe(404); + const count = sqlite.query("SELECT count(*) AS n FROM sso_providers WHERE id = 'p-bob'").get() as any; + expect(count.n).toBe(1); + }); + + test("DELETE on the caller's own provider works", async () => { + currentUserId = "bob"; + const response = await routes.DELETE(ctx("DELETE", "?id=p-bob")); + expect(response.status).toBe(200); + const count = sqlite.query("SELECT count(*) AS n FROM sso_providers WHERE id = 'p-bob'").get() as any; + expect(count.n).toBe(0); + }); +}); diff --git a/src/pages/api/sso/providers.ts b/src/pages/api/sso/providers.ts index ea63b3c..6cab2d6 100644 --- a/src/pages/api/sso/providers.ts +++ b/src/pages/api/sso/providers.ts @@ -3,23 +3,47 @@ import { createSecureErrorResponse } from "@/lib/utils"; import { requireAuth } from "@/lib/utils/auth-helpers"; import { db, ssoProviders } from "@/lib/db"; import { nanoid } from "nanoid"; -import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import { normalizeOidcProviderConfig, OidcConfigError, type RawOidcConfig } from "@/lib/sso/oidc-config"; +/** + * Providers are owned by the user who created them. Every read and write + * here is scoped to the caller, and the client secret never leaves the + * server: the API reports whether one is stored and the form keeps it by + * sending an empty value. + */ +export function formatProviderForClient(provider: { + oidcConfig: string | null; + [key: string]: unknown; +}) { + const oidcConfig = provider.oidcConfig ? JSON.parse(provider.oidcConfig) : undefined; + if (oidcConfig && typeof oidcConfig === "object") { + const { clientSecret, ...rest } = oidcConfig as { clientSecret?: unknown }; + return { + ...provider, + oidcConfig: { ...rest, hasClientSecret: typeof clientSecret === "string" && clientSecret.length > 0 }, + samlConfig: (provider as any).samlConfig ? JSON.parse((provider as any).samlConfig) : undefined, + }; + } + return { + ...provider, + oidcConfig, + samlConfig: (provider as any).samlConfig ? JSON.parse((provider as any).samlConfig) : undefined, + }; +} + // GET /api/sso/providers - List all SSO providers export async function GET(context: APIContext) { try { const { user, response } = await requireAuth(context); if (response) return response; - const providers = await db.select().from(ssoProviders); + const providers = await db + .select() + .from(ssoProviders) + .where(eq(ssoProviders.userId, user.id)); - // Parse JSON fields before sending - const formattedProviders = providers.map(provider => ({ - ...provider, - oidcConfig: provider.oidcConfig ? JSON.parse(provider.oidcConfig) : undefined, - samlConfig: (provider as any).samlConfig ? JSON.parse((provider as any).samlConfig) : undefined, - })); + const formattedProviders = providers.map(formatProviderForClient); return new Response(JSON.stringify(formattedProviders), { status: 200, @@ -142,12 +166,7 @@ export async function POST(context: APIContext) { }) .returning(); - // Parse JSON fields before sending - const formattedProvider = { - ...newProvider, - oidcConfig: newProvider.oidcConfig ? JSON.parse(newProvider.oidcConfig) : undefined, - samlConfig: (newProvider as any).samlConfig ? JSON.parse((newProvider as any).samlConfig) : undefined, - }; + const formattedProvider = formatProviderForClient(newProvider); return new Response(JSON.stringify(formattedProvider), { status: 201, @@ -193,11 +212,11 @@ export async function PUT(context: APIContext) { organizationId, } = body; - // Get existing provider + // Get existing provider; another user's provider reads as not found const [existingProvider] = await db .select() .from(ssoProviders) - .where(eq(ssoProviders.id, providerId)) + .where(and(eq(ssoProviders.id, providerId), eq(ssoProviders.userId, user.id))) .limit(1); if (!existingProvider) { @@ -228,7 +247,9 @@ export async function PUT(context: APIContext) { const mergedConfig: RawOidcConfig = { clientId: clientId ?? existingConfig.clientId, - clientSecret: clientSecret ?? existingConfig.clientSecret, + // The list endpoint never returns the secret, so the form sends an + // empty value to keep the stored one. + clientSecret: clientSecret ? clientSecret : existingConfig.clientSecret, authorizationEndpoint: authorizationEndpoint ?? existingConfig.authorizationEndpoint, tokenEndpoint: tokenEndpoint ?? existingConfig.tokenEndpoint, jwksEndpoint: jwksEndpoint ?? existingConfig.jwksEndpoint, @@ -269,15 +290,10 @@ export async function PUT(context: APIContext) { organizationId: organizationId !== undefined ? organizationId : existingProvider.organizationId, updatedAt: new Date(), }) - .where(eq(ssoProviders.id, providerId)) + .where(and(eq(ssoProviders.id, providerId), eq(ssoProviders.userId, user.id))) .returning(); - // Parse JSON fields before sending - const formattedProvider = { - ...updatedProvider, - oidcConfig: JSON.parse(updatedProvider.oidcConfig), - samlConfig: (updatedProvider as any).samlConfig ? JSON.parse((updatedProvider as any).samlConfig) : undefined, - }; + const formattedProvider = formatProviderForClient(updatedProvider); return new Response(JSON.stringify(formattedProvider), { status: 200, @@ -309,7 +325,7 @@ export async function DELETE(context: APIContext) { const deleted = await db .delete(ssoProviders) - .where(eq(ssoProviders.id, providerId)) + .where(and(eq(ssoProviders.id, providerId), eq(ssoProviders.userId, user.id))) .returning(); if (deleted.length === 0) { diff --git a/src/pages/api/test-connection-guard.test.ts b/src/pages/api/test-connection-guard.test.ts new file mode 100644 index 0000000..a43c44c --- /dev/null +++ b/src/pages/api/test-connection-guard.test.ts @@ -0,0 +1,108 @@ +/** + * The two connection test routes make a server side request to a URL from + * the request body. They were the only non-public API routes without an + * auth guard (GHSA-9m33-xfrc-5jxw, GHSA-6m23-28hh-gjh2). Both now require a + * signed in user and refuse link local and metadata addresses. + * + * Runs in an isolated child: the auth module is replaced here and bun's + * mock.module is process wide. + */ + +import { describe, test, expect, mock } from "bun:test"; + +const CHILD_FLAG = "GM_TEST_CONNECTION_GUARD_ISOLATED"; +const isChild = !!process.env[CHILD_FLAG]; + +if (!isChild) { + test("connection test routes (auth guard) - isolated child suite", () => { + const res = Bun.spawnSync({ + cmd: [process.execPath, "test", import.meta.path], + env: { ...process.env, [CHILD_FLAG]: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + if (res.exitCode !== 0) { + console.error(res.stdout.toString()); + console.error(res.stderr.toString()); + } + expect(res.exitCode).toBe(0); + }, 60_000); +} + +if (isChild) { + // No session anywhere: the guard must answer 401 on its own. + mock.module("@/lib/auth", () => ({ + auth: { api: { getSession: async () => null } }, + })); +} + +const gitea = isChild ? await import("./gitea/test-connection") : ({} as any); +const github = isChild ? await import("./github/test-connection") : ({} as any); + +function post(path: string, body: unknown, locals?: unknown) { + return { + request: new Request(`http://localhost/api/${path}`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }), + locals: locals ?? {}, + } as any; +} + +describe.skipIf(!isChild)("connection test routes", () => { + const fetchCalls: string[] = []; + const realFetch = globalThis.fetch; + globalThis.fetch = (async (input: any) => { + fetchCalls.push(String(input)); + return new Response(JSON.stringify({ login: "someone" }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + }) as typeof fetch; + + test("both routes answer 401 without a session and never fetch", async () => { + const giteaResponse = await gitea.POST( + post("gitea/test-connection", { url: "http://127.0.0.1:9999", token: "x" }) + ); + expect(giteaResponse.status).toBe(401); + + const githubResponse = await github.POST( + post("github/test-connection", { provider: "gitea", url: "http://127.0.0.1:9999", token: "x" }) + ); + expect(githubResponse.status).toBe(401); + expect(fetchCalls).toEqual([]); + }); + + test("a signed in user cannot point either route at the metadata service", async () => { + const locals = { user: { id: "user-1" } }; + + const giteaResponse = await gitea.POST( + post("gitea/test-connection", { url: "http://169.254.169.254", token: "x" }, locals) + ); + expect(giteaResponse.status).toBe(400); + expect((await giteaResponse.json()).message).toMatch(/link local/); + + const githubResponse = await github.POST( + post( + "github/test-connection", + { provider: "gitea", url: "http://169.254.169.254", token: "x" }, + locals + ) + ); + expect(githubResponse.status).toBe(400); + expect((await githubResponse.json()).message).toMatch(/link local/); + expect(fetchCalls).toEqual([]); + }); + + test("a signed in user can still test a LAN Gitea", async () => { + const response = await gitea.POST( + post("gitea/test-connection", { url: "http://192.168.1.10:3000/", token: "x" }, { + user: { id: "user-1" }, + }) + ); + expect(response.status).toBe(200); + expect(fetchCalls[0]).toBe("http://192.168.1.10:3000/api/v1/user"); + globalThis.fetch = realFetch; + }); +}); diff --git a/www/src/pages/docs/environment-variables.mdx b/www/src/pages/docs/environment-variables.mdx index 5cc693d..89278dc 100644 --- a/www/src/pages/docs/environment-variables.mdx +++ b/www/src/pages/docs/environment-variables.mdx @@ -43,6 +43,7 @@ Variables fall into two groups. | `BETTER_AUTH_LOG_LEVEL` | `warn` | Better Auth logger verbosity: `debug`, `info`, `warn`, or `error`. Use `debug` to trace SSO and OIDC sign-in problems. Better Auth does not read the `DEBUG` variable. | | `ENCRYPTION_SECRET` | none | Key for AES-256-GCM encryption of stored tokens. Generate with `openssl rand -base64 48`. | | `AUTH_DEFAULT_METHOD` | `email` | Tab the login page opens on when both email and SSO are available: `email` or `sso`. The method last used in that browser wins over this default. | +| `AUTH_ALLOW_SIGNUP` | `false` | Keep email and password sign-up open after the first account exists. Off, the sign-up endpoint answers 403 once a user exists. | In Docker, `BETTER_AUTH_SECRET` and `ENCRYPTION_SECRET` are generated on first boot if unset and saved to `/app/data/.better_auth_secret` and `/app/data/.encryption_secret` so they persist across restarts. Outside Docker there is no fallback: a missing `BETTER_AUTH_SECRET` logs a security warning in production and uses a known insecure development value.