mirror of
https://github.com/RayLabsHQ/gitea-mirror.git
synced 2026-10-19 01:31:01 -05:00
fix(security): guard connection tests, close sign-up and scope SSO providers (#447)
* fix(security): guard connection tests, close sign-up and scope SSO providers Fixes the four privately reported issues GHSA-9m33-xfrc-5jxw, GHSA-6m23-28hh-gjh2, GHSA-2hpx-83vg-gm45 and GHSA-5pp8-r7f5-6q8p. - The Gitea and GitHub test-connection routes 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. - Email sign-up is refused server side once an account exists unless AUTH_ALLOW_SIGNUP=true. Only the signup page redirected before; the endpoint accepted new accounts from anyone reaching the instance. - SSO providers are scoped to their owner for list, update and delete, and the client secret is never returned. An empty secret on update keeps the stored one, and the form says so. - HttpError messages carry the status only; the upstream body stays on the response field for logs and never reaches API clients. - A narrow outbound guard for user supplied URLs (connection tests, OIDC discovery, ntfy, Gotify, Apprise, webhook): link local and cloud metadata addresses are refused, host names are resolved and checked, and redirects are not followed. Private networks stay allowed since mirroring to a LAN Gitea is the normal deployment. * fix(security): pin plain http requests to the checked address The guard resolved a host name and checked its addresses, but fetch resolved the name a second time, so a name that changes its answer between the two lookups could still reach a link local address. Plain http requests now go to the address the check saw, with the original name in the Host header. https keeps the name: the certificate is validated against it and the metadata services speak no TLS.
This commit is contained in:
1 parent
610bd6040d
commit
c6157cdaad
23 files changed
+880
-58
No files matched your search
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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`.
|
||||
|
||||
@@ -454,6 +454,7 @@ export function SSOSettings() {
|
||||
<Input
|
||||
id="clientSecret"
|
||||
type="password"
|
||||
placeholder={editingProvider ? "Leave blank to keep the current secret" : ""}
|
||||
value={providerForm.clientSecret}
|
||||
onChange={e => setProviderForm(prev => ({ ...prev, clientSecret: e.target.value }))}
|
||||
/>
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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<number> }) =>
|
||||
({
|
||||
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;
|
||||
@@ -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<number>`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 /
|
||||
|
||||
+8
-16
@@ -41,25 +41,17 @@ export async function httpRequest<T = any>(
|
||||
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<T = any>(
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
@@ -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<void> {
|
||||
@@ -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}`);
|
||||
}
|
||||
@@ -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<void> {
|
||||
@@ -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}`);
|
||||
}
|
||||
@@ -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}`);
|
||||
}
|
||||
@@ -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}`);
|
||||
}
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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<string[]>;
|
||||
|
||||
async function defaultResolver(hostname: string): Promise<string[]> {
|
||||
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<URL> {
|
||||
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<string, string>;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<OutboundTarget> {
|
||||
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<Response> {
|
||||
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" });
|
||||
}
|
||||
@@ -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<ReturnType<typeof resolveOutboundTarget>>;
|
||||
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<typeof parseServerInfo> | undefined;
|
||||
try {
|
||||
const versionResp = await httpGet(`${baseUrl}/api/v1/version`, {
|
||||
...target.headers,
|
||||
'Accept': 'application/json',
|
||||
});
|
||||
if (typeof versionResp.data?.version === 'string') {
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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`);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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) {
|
||||
|
||||
@@ -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;
|
||||
});
|
||||
});
|
||||
@@ -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.
|
||||
|
||||
|
||||
Reference in new issue
Block a user