diff --git a/TODO.md b/TODO.md index 62a3f42ca..8f3ed4678 100644 --- a/TODO.md +++ b/TODO.md @@ -112,15 +112,25 @@ What's already there: The prerequisite nobody expects: -- [ ] **The button can't just be added.** Social login is hidden on self-hosted - outright today — `{!selfHosted && }` at - `apps/dashboard/src/app/(auth)/login/page.tsx:246` and - `register/page.tsx:154` — because an operator with no `GITHUB_CLIENT_ID` - would get buttons that fail, and **nothing tells the dashboard which - providers are configured**. `OAuthButtons` hardcodes github+google. SSO - needs a server-advertised provider list (public, read-only, alongside the - `authMode`/`selfHosted` values `useAuthContext` already serves). That - endpoint doesn't exist yet and is the real first task. +- [x] **The button can't just be added** — DONE for the *primitive*, not for + SSO. `GET /health/env` now advertises `authProviders`, the social logins + the server actually has credentials for + (`apps/api/src/lib/auth-providers.ts` owns the predicate; `lib/auth.ts` + reads the SAME resolver to decide registration, so "a button is shown" and + "the provider is registered" cannot drift). Public, read-only, IDs only — + no client id, no secret, asserted by test. The dashboard carries it + through the auth layout's context, and `{!selfHosted && }` + is gone from both `login/page.tsx` and `register/page.tsx`: + `OAuthButtons` maps the advertised list and renders nothing (not even the + divider) when it is empty. Net effect — cloud unchanged, self-hosted + WITHOUT creds unchanged (nothing shown), self-hosted WITH creds now shows + buttons that work. `authMode === "none"` is untouched: the login page + returns its zero-auth redirect before the form renders. + What SSO still has to add here: an entry with a different `kind` (the + discriminator already exists and non-`"social"` entries are deliberately + NOT drawn as branded buttons), an operator-named label instead of a + hardcoded one, and the env gating for the issuer. Everything below is + still open. Decisions to settle before coding: @@ -211,9 +221,17 @@ What actually hardcodes GitHub — each is a decision, not a rename: case; GitLab releases would be a third mode. - [ ] **Dashboard speaks GitHub throughout**: `ServerGitHubConnect`, `GithubPermissionModal`, `DeployCredentialModal`, the deploy wizard's - import step, `ResourcePicker`. Needs a server-advertised provider list — - the SAME missing primitive as the SSO item above (`OAuthButtons` hardcodes - github+google). Build that endpoint once and both features use it. + import step, `ResourcePicker`. Still all GitHub — this item is NOT done. + What IS done is the shared primitive it was blocked on: the public + read-only surface exists and has its first list on it — `GET /health/env` + → `authProviders`, derived in `apps/api/src/lib/auth-providers.ts` (see + the Auth section above). That covers LOGIN providers only. Git providers + are a different capability record (can it list repos? push a deploy key? + serve a tarball?) and join the same endpoint as their own field rather + than being squeezed into `authProviders` — a git remote is not a login + button. The pattern to copy: one module owning the "is it configured?" + predicate, read by both the feature and the endpoint, IDs/capabilities + only and never credentials. - [ ] **`gh` CLI as an ambient identity** (`sources/gh-cli-source.ts`, `github.local-auth.ts:360` parses `oauth_token` under `github.com:` in hosts.yml) has no equivalent worth matching. `glab` exists; decide diff --git a/apps/api/src/lib/auth-providers.ts b/apps/api/src/lib/auth-providers.ts new file mode 100644 index 000000000..54ff1730c --- /dev/null +++ b/apps/api/src/lib/auth-providers.ts @@ -0,0 +1,89 @@ +/** + * THE list of social/OAuth login providers this instance actually has + * credentials for — and the single place the condition "is + * configured?" is written. + * + * Why this exists: a provider is registered with Better Auth only when its + * client id AND secret are both present (`socialProviders` in lib/auth.ts), but + * nothing told the dashboard which ones made it in. So the dashboard guessed — + * `{!selfHosted && }` — and the guess was wrong in both + * directions: a self-hosted operator who HAD set GITHUB_CLIENT_ID/SECRET got no + * button at all, and any cloud deploy missing a pair would have rendered a + * button that dead-ends at Better Auth's "provider not found". + * + * The rule is written once, here, and read by two callers: + * - lib/auth.ts → whether to register the provider (needs the creds) + * - GET /health/env → whether to ADVERTISE it (never the creds) + * Keeping the predicate in one place is the same discipline as lib/auth-mode.ts: + * a second copy of a security-shaped condition drifts, and the drift is only + * visible as a login screen that disagrees with what the API enforces. + * + * SECRET DISCIPLINE: `configuredAuthProviders()` is served UNAUTHENTICATED. It + * returns provider IDs and nothing else — never a client id, never a secret, not + * even a redacted one. `socialProviderCredentials()` is the only export that + * touches credential values, it is server-internal, and its return value must + * never reach a response body. + */ + +import { env } from "../config/env"; + +/** Every social provider the server knows how to register. */ +export const SOCIAL_AUTH_PROVIDERS = ["github", "google"] as const; +export type SocialAuthProviderId = (typeof SOCIAL_AUTH_PROVIDERS)[number]; + +/** + * One advertised provider. Deliberately a RECORD, not a bare string: this is the + * public shape and it has to grow without breaking older dashboards. + * + * `kind` is the discriminator that growth will need. Today the only value is + * "social" (an env-configured OAuth provider whose button the dashboard already + * knows how to draw). A later SSO/OIDC provider is a different thing to render — + * one operator-named issuer rather than a branded button — and gets its own + * kind then. Nothing here implements SSO; the field only means a client can + * filter on it instead of assuming every entry is a github-shaped button. + * + * (The git-provider capability record the GitLab work needs is a SEPARATE + * surface — these are login providers. See TODO.md "Git providers".) + */ +export interface AdvertisedAuthProvider { + id: SocialAuthProviderId; + kind: "social"; +} + +/** + * The credential pair for a provider, or null when the operator hasn't + * configured it. Both halves are required: a client id without a secret cannot + * complete a redirect flow, so half-configured counts as not configured. + * + * SERVER-INTERNAL. Callers pass the result straight into Better Auth's provider + * config; it must not be serialised into any response. + */ +export function socialProviderCredentials( + id: SocialAuthProviderId, +): { clientId: string; clientSecret: string } | null { + const pair = + id === "github" + ? { clientId: env.GITHUB_CLIENT_ID, clientSecret: env.GITHUB_CLIENT_SECRET } + : { clientId: env.GOOGLE_CLIENT_ID, clientSecret: env.GOOGLE_CLIENT_SECRET }; + + if (!pair.clientId || !pair.clientSecret) return null; + return { clientId: pair.clientId, clientSecret: pair.clientSecret }; +} + +/** True when this provider is registered with Better Auth on this instance. */ +export function isSocialProviderConfigured(id: SocialAuthProviderId): boolean { + return socialProviderCredentials(id) !== null; +} + +/** + * The public, read-only answer to "which login providers can I actually use?", + * in a stable order. Empty array = password login only, which is the correct + * answer for a default self-hosted instance and the reason the dashboard can + * render this list unconditionally instead of gating on `selfHosted`. + */ +export function configuredAuthProviders(): AdvertisedAuthProvider[] { + return SOCIAL_AUTH_PROVIDERS.filter(isSocialProviderConfigured).map((id) => ({ + id, + kind: "social" as const, + })); +} diff --git a/apps/api/src/lib/auth.ts b/apps/api/src/lib/auth.ts index 75ccf44b7..3ec80fdc8 100644 --- a/apps/api/src/lib/auth.ts +++ b/apps/api/src/lib/auth.ts @@ -21,6 +21,7 @@ import { teardownBillingForOrg, } from "../modules/billing/billing-org-cleanup"; import { provisionUser } from "./provision-user"; +import { socialProviderCredentials } from "./auth-providers"; import { safeErrorMessage } from "@repo/core"; /** @@ -115,6 +116,12 @@ function getSharedCookieDomain() { const sharedCookieDomain = getSharedCookieDomain(); const useSessionCookieCache = getDriver() !== "pglite"; +// Credential pairs, or null when the operator configured neither/half of one. +// Same resolver GET /health/env uses to advertise the provider list, so "a +// button is shown" and "the provider is registered" cannot disagree. +const githubOAuth = socialProviderCredentials("github"); +const googleOAuth = socialProviderCredentials("google"); + export const auth = betterAuth({ basePath: "/api/auth", // Dynamic when served on a public URL — every absolute OAuth/auth URL is built @@ -190,13 +197,17 @@ export const auth = betterAuth({ : undefined, }, - /* ---------- OAuth Providers ---------- */ + /* ---------- OAuth Providers ---------- + A provider is registered only when BOTH halves of its credential pair are + set. That condition now lives in lib/auth-providers.ts, because + GET /health/env advertises the same list to the dashboard so it can draw + exactly the buttons that will work — inlining the env check here again is + how the two would drift. */ socialProviders: { - ...(env.GITHUB_CLIENT_ID && env.GITHUB_CLIENT_SECRET + ...(githubOAuth ? { github: { - clientId: env.GITHUB_CLIENT_ID, - clientSecret: env.GITHUB_CLIENT_SECRET, + ...githubOAuth, scope: ["read:user", "user:email"], mapProfileToUser: (profile: any) => ({ name: profile.name || profile.login, @@ -206,14 +217,7 @@ export const auth = betterAuth({ }, } : {}), - ...(env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET - ? { - google: { - clientId: env.GOOGLE_CLIENT_ID, - clientSecret: env.GOOGLE_CLIENT_SECRET, - }, - } - : {}), + ...(googleOAuth ? { google: googleOAuth } : {}), }, /* ---------- Account Linking ---------- */ diff --git a/apps/api/src/modules/health/health.routes.ts b/apps/api/src/modules/health/health.routes.ts index 748360d04..cd58ee371 100644 --- a/apps/api/src/modules/health/health.routes.ts +++ b/apps/api/src/modules/health/health.routes.ts @@ -9,6 +9,7 @@ import { APP_VERSION } from "../../lib/app-version"; import { getAuthMode } from "../../lib/auth-mode"; import { resolveProductMode } from "../../lib/product-mode"; import { resolveHostControlEnabled } from "../../lib/host-control"; +import { configuredAuthProviders } from "../../lib/auth-providers"; /** Running server version (apps/api/package.json, via lib/app-version — the same * value sent to the cloud on every call). Lets the dashboard tell a self-hosted @@ -117,6 +118,17 @@ healthRoutes.get("/env", rateLimiterFor("default-anon"), async (c) => { version: APP_VERSION, authMode, productMode, + // Which social logins this instance actually has credentials for, derived + // from the same predicate that decides whether Better Auth registers them + // (lib/auth-providers.ts). The dashboard renders exactly these buttons. + // Before this, it inferred "cloud ⇒ github+google, self-hosted ⇒ none", + // which hid working buttons from any operator who had configured a + // provider. IDs ONLY — this route is unauthenticated, so no client id and + // no secret may ever be added to these records. (LOGIN providers only. The + // git-provider capability record the GitLab work needs — #75, TODO.md "Git + // providers" — belongs on this same public surface as its own field, not + // folded in here: a git remote is not a login button.) + authProviders: configuredAuthProviders(), teamMode, migrationTargetUrl, migrationInProgress, diff --git a/apps/api/test/lib/auth-providers.test.ts b/apps/api/test/lib/auth-providers.test.ts new file mode 100644 index 000000000..ce84db70e --- /dev/null +++ b/apps/api/test/lib/auth-providers.test.ts @@ -0,0 +1,105 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +/** + * Which social logins does this instance actually have credentials for? + * + * The dashboard used to answer that question itself, with `!selfHosted` — a + * proxy that is wrong in both directions (a self-hosted operator WITH GitHub + * creds got no button; a cloud deploy missing a pair would have got a button + * that dead-ends). The answer now comes from the server, derived from the same + * predicate that decides whether Better Auth registers the provider at all. + * + * These tests pin the two halves that matter: + * 1. configured → advertised, not configured → absent (and half-configured + * counts as NOT configured — a client id with no secret cannot complete a + * redirect flow); + * 2. the advertised payload carries IDs and nothing else. This list is served + * unauthenticated, so a client id leaking into it is a real disclosure, and + * it would leak by the most ordinary of edits: returning the config object + * the registration path already builds. + */ + +const env: Record = {}; + +vi.mock("../../src/config/env", () => ({ + get env() { + return env; + }, +})); + +async function load() { + vi.resetModules(); + return import("../../src/lib/auth-providers"); +} + +afterEach(() => { + for (const key of Object.keys(env)) delete env[key]; +}); + +describe("configuredAuthProviders", () => { + it("advertises nothing on a default self-hosted instance", async () => { + const { configuredAuthProviders } = await load(); + // No creds in env — password login only. The dashboard renders no buttons, + // which is the same thing an operator sees today, just for a true reason. + expect(configuredAuthProviders()).toEqual([]); + }); + + it("advertises a provider once both halves of its credential pair are set", async () => { + env.GITHUB_CLIENT_ID = "Iv1.self-hosted-app"; + env.GITHUB_CLIENT_SECRET = "shhh-github"; + + const { configuredAuthProviders } = await load(); + // The improvement this whole change exists for: a self-hosted operator who + // DID configure GitHub now gets the button, where before it was hidden. + expect(configuredAuthProviders()).toEqual([{ id: "github", kind: "social" }]); + }); + + it("omits a half-configured provider — an id without a secret cannot sign anyone in", async () => { + env.GITHUB_CLIENT_ID = "Iv1.half-done"; + env.GOOGLE_CLIENT_SECRET = "secret-without-an-id"; + + const { configuredAuthProviders } = await load(); + expect(configuredAuthProviders()).toEqual([]); + }); + + it("advertises both, in a stable order, when both are configured (the cloud case)", async () => { + env.GITHUB_CLIENT_ID = "gh-id"; + env.GITHUB_CLIENT_SECRET = "gh-secret"; + env.GOOGLE_CLIENT_ID = "goog-id"; + env.GOOGLE_CLIENT_SECRET = "goog-secret"; + + const { configuredAuthProviders } = await load(); + expect(configuredAuthProviders().map((p) => p.id)).toEqual(["github", "google"]); + }); + + it("never puts a client id or secret in the advertised payload", async () => { + env.GITHUB_CLIENT_ID = "Iv1.SECRET-CLIENT-ID"; + env.GITHUB_CLIENT_SECRET = "SECRET-CLIENT-SECRET"; + env.GOOGLE_CLIENT_ID = "google-SECRET-CLIENT-ID"; + env.GOOGLE_CLIENT_SECRET = "google-SECRET-CLIENT-SECRET"; + + const { configuredAuthProviders } = await load(); + const serialized = JSON.stringify(configuredAuthProviders()); + + for (const value of Object.values(env)) { + expect(serialized).not.toContain(value); + } + // Belt and braces: the only keys are the two documented ones, so a future + // field can't be added without a test author looking at this assertion. + expect(Object.keys(configuredAuthProviders()[0]!).sort()).toEqual(["id", "kind"]); + }); +}); + +describe("socialProviderCredentials", () => { + it("hands the registration path both halves when configured", async () => { + env.GOOGLE_CLIENT_ID = "goog-id"; + env.GOOGLE_CLIENT_SECRET = "goog-secret"; + + const { socialProviderCredentials } = await load(); + expect(socialProviderCredentials("google")).toEqual({ + clientId: "goog-id", + clientSecret: "goog-secret", + }); + expect(socialProviderCredentials("github")).toBeNull(); + }); +}); diff --git a/apps/api/test/modules/health-env-auth-providers.test.ts b/apps/api/test/modules/health-env-auth-providers.test.ts new file mode 100644 index 000000000..c97085b0d --- /dev/null +++ b/apps/api/test/modules/health-env-auth-providers.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it, vi } from "vitest"; + +/** + * GET /health/env is the public, read-only surface the dashboard bootstraps + * from (`authMode`, `selfHosted`, …). `authProviders` joins it: the list of + * social logins this instance actually has credentials for, so the login page + * can render the buttons that will work instead of guessing from `selfHosted`. + * + * This file owns the CONFIGURED case, and it has to set the credentials in + * `process.env` before `config/env` parses them — env is validated once at + * import time, so `vi.hoisted` (which runs before the hoisted imports) is the + * only place a test can influence it. The not-configured case lives in + * health-env-authmode.test.ts, which runs with the suite's bare env. + * + * No auth is applied to this route on purpose, which is exactly why the last + * assertion here is about what is NOT in the body: the credential values must + * never appear, in any field, redacted or otherwise. + */ + +const CREDS = vi.hoisted(() => { + const values = { + GITHUB_CLIENT_ID: "Iv1.health-env-github-id", + GITHUB_CLIENT_SECRET: "health-env-github-secret", + GOOGLE_CLIENT_ID: "health-env-google-id.apps.googleusercontent.com", + GOOGLE_CLIENT_SECRET: "health-env-google-secret", + }; + Object.assign(process.env, values); + return values; +}); + +vi.mock("@repo/db", () => ({ + repos: { instanceSettings: { get: async () => ({}) } }, +})); + +async function getEnv() { + const { Hono } = await import("hono"); + const { healthRoutes } = await import("../../src/modules/health/health.routes"); + // Mirror clientIpMiddleware (app.ts) — the route is rate-limited per IP and + // 400s without a resolvable subject when mounted in isolation. + const app = new Hono<{ Variables: { clientIp: string } }>(); + app.use("*", async (c, next) => { + c.set("clientIp", "127.0.0.1"); + await next(); + }); + app.route("/", healthRoutes); + const res = await app.request("/env"); + return { res, body: (await res.json()) as Record }; +} + +describe("GET /health/env authProviders — credentials configured", () => { + it("advertises every provider whose credentials are set", async () => { + const { res, body } = await getEnv(); + expect(res.status).toBe(200); + expect(body.authProviders).toEqual([ + { id: "github", kind: "social" }, + { id: "google", kind: "social" }, + ]); + }); + + it("leaks no client id and no client secret on this unauthenticated route", async () => { + const { body } = await getEnv(); + const serialized = JSON.stringify(body); + for (const value of Object.values(CREDS)) { + expect(serialized).not.toContain(value); + } + }); +}); diff --git a/apps/api/test/modules/health-env-authmode.test.ts b/apps/api/test/modules/health-env-authmode.test.ts index 3913cb4b7..37486b965 100644 --- a/apps/api/test/modules/health-env-authmode.test.ts +++ b/apps/api/test/modules/health-env-authmode.test.ts @@ -96,6 +96,18 @@ describe("GET /health/env authMode", () => { expect((await getEnv()).body.authMode).toBe("local"); }); + it("advertises no social providers when no OAuth credentials are configured", async () => { + // The suite runs with a bare env (no GITHUB_/GOOGLE_ client creds), which is + // the default self-hosted instance: password login only. The field must be + // PRESENT and empty rather than absent — the dashboard renders whatever it + // is handed, and a missing key would be indistinguishable from an older + // server on the client side. The configured case lives in + // health-env-auth-providers.test.ts (it has to stuff process.env before + // config/env parses). + const { body } = await getEnv(); + expect(body.authProviders).toEqual([]); + }); + it("still reports the other instanceSettings fields", async () => { settings.authMode = "none"; settings.teamMode = "multi_user"; diff --git a/apps/dashboard/src/app/(auth)/layout.tsx b/apps/dashboard/src/app/(auth)/layout.tsx index 8a07bd346..d277cd6b2 100644 --- a/apps/dashboard/src/app/(auth)/layout.tsx +++ b/apps/dashboard/src/app/(auth)/layout.tsx @@ -100,7 +100,12 @@ export default async function AuthLayout({ if (!deploymentInfo) return ; return ( - +
{children}
 ); diff --git a/apps/dashboard/src/app/(auth)/login/page.tsx b/apps/dashboard/src/app/(auth)/login/page.tsx index 1c4b5691a..b943fa50e 100644 --- a/apps/dashboard/src/app/(auth)/login/page.tsx +++ b/apps/dashboard/src/app/(auth)/login/page.tsx @@ -48,7 +48,7 @@ function LoginPageInner() { const searchParams = useSearchParams(); const { toast } = useToast(); const { t } = useI18n(); - const { authMode, cloudAuthUrl, selfHosted } = useAuthContext(); + const { authMode, cloudAuthUrl, selfHosted, authProviders } = useAuthContext(); const isDesktop = typeof window !== "undefined" && !!window.desktop?.isDesktop; const handleBack = isDesktop ? () => { void window.desktop?.reset?.(); } : undefined; @@ -308,8 +308,13 @@ function LoginPageInner() { - {/* OAuth only for SaaS (cloud-hosted) - hidden on self-hosted */} - {!selfHosted && } + {/* Whatever the SERVER says it has credentials for. It used to be + `!selfHosted &&` — a stand-in for "are any providers configured?" that + hid working buttons from every self-hosted operator who had set + GITHUB_CLIENT_ID/SECRET. OAuthButtons renders nothing (not even the + divider) when the list is empty, which is the default self-hosted + instance, so this is safe to mount unconditionally. */} + {/* Public sign-up is a SaaS-only front door. On a self-hosted instance the only account is the CLI-created admin; everyone else joins via an diff --git a/apps/dashboard/src/app/(auth)/providers.tsx b/apps/dashboard/src/app/(auth)/providers.tsx index 9825fcd00..89eea156d 100644 --- a/apps/dashboard/src/app/(auth)/providers.tsx +++ b/apps/dashboard/src/app/(auth)/providers.tsx @@ -2,17 +2,24 @@ import React, { createContext, useContext } from "react"; import { CLOUD_DASHBOARD_URL } from "@repo/core"; +import type { AdvertisedAuthProvider } from "@/lib/auth-providers"; interface AuthContextValue { authMode: "cloud" | "local" | "none"; cloudAuthUrl: string; selfHosted: boolean; + /** Social logins the API advertises as configured (GET /health/env). The + * login/register pages render exactly these; empty means password only. */ + authProviders: AdvertisedAuthProvider[]; } const AuthContext = createContext({ authMode: "local", cloudAuthUrl: CLOUD_DASHBOARD_URL, selfHosted: true, + // Default to "none advertised": showing a button that isn't configured is a + // dead end, showing none is merely password login. + authProviders: [], }); export function useAuthContext() { @@ -24,11 +31,18 @@ interface AuthProvidersProps { authMode: "cloud" | "local" | "none"; cloudAuthUrl: string; selfHosted: boolean; + authProviders?: AdvertisedAuthProvider[]; } -export function AuthProviders({ children, authMode, cloudAuthUrl, selfHosted }: AuthProvidersProps) { +export function AuthProviders({ + children, + authMode, + cloudAuthUrl, + selfHosted, + authProviders = [], +}: AuthProvidersProps) { return ( - + {children} ); diff --git a/apps/dashboard/src/app/(auth)/register/page.tsx b/apps/dashboard/src/app/(auth)/register/page.tsx index fc89ac967..ba0847815 100644 --- a/apps/dashboard/src/app/(auth)/register/page.tsx +++ b/apps/dashboard/src/app/(auth)/register/page.tsx @@ -33,7 +33,7 @@ function RegisterPageInner() { const searchParams = useSearchParams(); const { toast } = useToast(); const { t } = useI18n(); - const { selfHosted } = useAuthContext(); + const { authProviders } = useAuthContext(); const [name, setName] = useState(""); const [email, setEmail] = useState(""); @@ -148,10 +148,11 @@ function RegisterPageInner() { - {/* Social login is cloud-only — a self-hosted operator rarely sets - GOOGLE_/GITHUB_ client creds, so the buttons would just fail. Mirror - the login page, which hides them on self-hosted. */} - {!selfHosted && } + {/* Mirrors the login page: render the providers the SERVER advertises as + configured, not a `!selfHosted` guess. A self-hosted operator who HAS + set GITHUB_/GOOGLE_ client creds gets working buttons; one who hasn't + gets nothing rendered, exactly as before. */} +

{t.auth.register.hasAccount}{" "} diff --git a/apps/dashboard/src/components/oauth-buttons.render.test.tsx b/apps/dashboard/src/components/oauth-buttons.render.test.tsx new file mode 100644 index 000000000..d9cd0792f --- /dev/null +++ b/apps/dashboard/src/components/oauth-buttons.render.test.tsx @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { I18nProvider } from "@/components/i18n-provider"; +import { OAuthButtons } from "./oauth-buttons"; +import type { AdvertisedAuthProvider } from "@/lib/auth-providers"; + +/** + * The mapping is unit-tested in lib/auth-providers.test.ts; this asserts the + * component actually DRAWS it — the buttons the server advertised, and the + * divider only when there is something to divide. + * + * Server-rendered to markup, same approach as MonitoringView.test.tsx: no jsdom, + * no testing-library. Click behaviour isn't reachable this way, which is fine — + * the regression this guards is a page that renders no social login on an + * instance that has one configured (the bug being fixed), or one that renders a + * button for a provider the server never advertised. + */ + +const render = (providers: AdvertisedAuthProvider[] | undefined) => + renderToStaticMarkup( + + + , + ); + +describe("OAuthButtons renders the server's list", () => { + it("renders nothing at all — not even the divider — for an empty list", () => { + // A default self-hosted instance. The page mounts this unconditionally now, + // so an empty list has to produce zero markup or the login form grows a + // stray "or" separator with nothing under it. + expect(render([])).toBe(""); + expect(render(undefined)).toBe(""); + }); + + it("renders the one configured provider on a self-hosted instance", () => { + // The improvement: GITHUB_CLIENT_ID/SECRET set on a self-hosted box now + // yields a real button, where the `!selfHosted` gate rendered none. + const html = render([{ id: "github", kind: "social" }]); + expect(html).toContain("Continue with GitHub"); + expect(html).not.toContain("Continue with Google"); + expect(html).toContain("or"); // divider is back once there IS a button + }); + + it("renders both when both are configured (the cloud case, unchanged)", () => { + const html = render([ + { id: "github", kind: "social" }, + { id: "google", kind: "social" }, + ]); + expect(html).toContain("Continue with GitHub"); + expect(html).toContain("Continue with Google"); + }); + + it("renders no button for a provider the server did not advertise", () => { + const html = render([{ id: "google", kind: "social" }]); + expect(html).toContain("Continue with Google"); + expect(html).not.toContain("Continue with GitHub"); + }); +}); diff --git a/apps/dashboard/src/components/oauth-buttons.tsx b/apps/dashboard/src/components/oauth-buttons.tsx index c544a42a2..4dd8901a1 100644 --- a/apps/dashboard/src/components/oauth-buttons.tsx +++ b/apps/dashboard/src/components/oauth-buttons.tsx @@ -7,6 +7,11 @@ import { useToast } from "@/components/toast"; import { useI18n } from "@/components/i18n-provider"; import { Button } from "@/components/ui/button"; import { Github, Loader2 } from "lucide-react"; +import { + renderableOAuthProviders, + type AdvertisedAuthProvider, + type RenderableOAuthProviderId, +} from "@/lib/auth-providers"; function GoogleIcon() { return ( @@ -19,17 +24,36 @@ function GoogleIcon() { ); } +/** Icon per renderable provider. The label comes from i18n (`t.auth.oauth[id]`), + * so adding a provider means one entry here and one key there. */ +const PROVIDER_ICONS: Record React.ReactElement> = { + github: () => , + google: () => , +}; + /** - * Shared OAuth buttons - GitHub + Google. - * Includes the divider above them. + * Shared OAuth buttons, rendered from the provider list the SERVER advertises + * (GET /health/env → `authProviders`, plumbed through the auth layout's + * context). Renders nothing at all — no divider either — when the list is + * empty, so callers can mount this unconditionally instead of guessing from + * `selfHosted` which providers exist. + * * Pass callbackURL to override the default post-OAuth redirect. */ -export function OAuthButtons({ callbackURL = "/" }: { callbackURL?: string }) { +export function OAuthButtons({ + providers, + callbackURL = "/", +}: { + providers: readonly AdvertisedAuthProvider[] | undefined; + callbackURL?: string; +}) { const { toast } = useToast(); const { t } = useI18n(); - const [loading, setLoading] = useState<"github" | "google" | null>(null); + const [loading, setLoading] = useState(null); + + const visible = renderableOAuthProviders(providers); - async function handleOAuth(provider: "github" | "google") { + async function handleOAuth(provider: RenderableOAuthProviderId) { setLoading(provider); try { // Resolve callbackURL against the DASHBOARD origin. Better Auth resolves @@ -64,6 +88,10 @@ export function OAuthButtons({ callbackURL = "/" }: { callbackURL?: string }) { } } + // Nothing configured server-side → no divider, no buttons. This is what makes + // the callers' unconditional `` safe. + if (visible.length === 0) return null; + return ( <> {/* Divider */} @@ -75,25 +103,18 @@ export function OAuthButtons({ callbackURL = "/" }: { callbackURL?: string }) { {/* OAuth buttons */}

- - - + {visible.map((provider) => ( + + ))}
); diff --git a/apps/dashboard/src/lib/auth-providers.test.ts b/apps/dashboard/src/lib/auth-providers.test.ts new file mode 100644 index 000000000..95967550a --- /dev/null +++ b/apps/dashboard/src/lib/auth-providers.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; +import { renderableOAuthProviders } from "./auth-providers"; + +/** + * The login page used to decide which social buttons exist with `!selfHosted`, + * which is not a fact about providers at all. It cost a self-hosted operator who + * had configured GITHUB_CLIENT_ID/SECRET their working GitHub button, and it + * would have drawn a dead button on any cloud deploy missing a pair. + * + * The server now advertises the configured providers (GET /health/env → + * `authProviders`) and this mapping decides what to draw from it. Two failure + * modes are worth pinning: + * - drawing MORE than was advertised (the old hardcoded github+google pair) — + * every extra button is a dead end at Better Auth's "provider not found"; + * - drawing something the build has no label or icon for, which is how a + * future SSO entry would arrive at an older dashboard. + */ + +describe("renderableOAuthProviders", () => { + it("renders nothing when the server advertises nothing", () => { + // The default self-hosted instance. Callers use this to skip the divider + // too, so "empty" has to stay genuinely empty. + expect(renderableOAuthProviders([])).toEqual([]); + }); + + it("renders nothing when the server is too old to advertise a list", () => { + expect(renderableOAuthProviders(undefined)).toEqual([]); + }); + + it("renders only what was advertised — one configured provider, one button", () => { + // THE improvement: self-hosted with GitHub creds configured. Before this + // change the page rendered no buttons; now it renders GitHub, and still not + // Google, which has no credentials on that instance. + expect(renderableOAuthProviders([{ id: "github", kind: "social" }])).toEqual(["github"]); + expect(renderableOAuthProviders([{ id: "google", kind: "social" }])).toEqual(["google"]); + }); + + it("renders both for a cloud deploy that configures both", () => { + expect( + renderableOAuthProviders([ + { id: "google", kind: "social" }, + { id: "github", kind: "social" }, + ]), + // Order is the dashboard's, not the server's, so the page looks the same + // on every instance. + ).toEqual(["github", "google"]); + }); + + it("drops an advertised provider this build cannot label or draw", () => { + // A newer server (the SSO work) advertising something this dashboard has no + // icon/label for. Skipping beats an unlabeled mystery button. + expect( + renderableOAuthProviders([ + { id: "github", kind: "social" }, + { id: "okta-oidc", kind: "social" }, + ]), + ).toEqual(["github"]); + }); + + it("does not render a non-social entry as a social button", () => { + // `kind` is the forward-compatibility seam: an SSO/OIDC entry is a + // different thing to render, so it must not fall through to the branded + // "Continue with …" treatment just because its id happens to match. + expect(renderableOAuthProviders([{ id: "github", kind: "sso" }])).toEqual([]); + }); +}); diff --git a/apps/dashboard/src/lib/auth-providers.ts b/apps/dashboard/src/lib/auth-providers.ts new file mode 100644 index 000000000..608e5457d --- /dev/null +++ b/apps/dashboard/src/lib/auth-providers.ts @@ -0,0 +1,59 @@ +/** + * Which social-login buttons should the auth pages draw? + * + * The server answers the first half — GET /health/env advertises the providers + * it actually has credentials for (`apps/api/src/lib/auth-providers.ts`), so the + * dashboard no longer infers it from `selfHosted`. That inference was wrong in + * both directions: it hid working buttons from a self-hosted operator who HAD + * configured GitHub, and it would have drawn dead buttons on any cloud deploy + * missing a credential pair. + * + * This module answers the second half — of the advertised providers, which ones + * can this build actually RENDER? A button needs an icon and a translated label, + * both of which live in the client. So an advertised id the dashboard doesn't + * recognise is dropped rather than rendered as an unlabeled mystery button; the + * server-advertised list is the authority on availability, not on how the UI + * looks. Adding a provider is a one-line edit to RENDERABLE_OAUTH_PROVIDERS plus + * its icon/label in components/oauth-buttons.tsx. + * + * Pure and DOM-free on purpose, so the mapping is testable without a browser. + */ + +/** One entry of the server's advertised list. Loosely typed on purpose: it + * arrives over the wire from a server that may be newer than this build. */ +export type AdvertisedAuthProvider = { + id: string; + /** "social" today. A future SSO/OIDC entry will carry a different kind and is + * deliberately NOT rendered as a social button by this build. */ + kind?: string; +}; + +/** + * The providers this build can draw, in the order they should appear. Order + * lives here rather than following the server's array so the login page looks + * the same on every instance. + */ +export const RENDERABLE_OAUTH_PROVIDERS = ["github", "google"] as const; +export type RenderableOAuthProviderId = (typeof RENDERABLE_OAUTH_PROVIDERS)[number]; + +/** + * Advertised list → the buttons to render. Empty in, empty out: a default + * self-hosted instance advertises nothing and gets no divider and no buttons, + * which is what it renders today — just for a true reason instead of the + * `!selfHosted` guess. + */ +export function renderableOAuthProviders( + advertised: readonly AdvertisedAuthProvider[] | undefined, +): RenderableOAuthProviderId[] { + if (!advertised?.length) return []; + + const offered = new Set( + advertised + // An entry with an explicit non-social kind is something else (SSO/OIDC); + // rendering it as a "Continue with …" button would misrepresent it. + .filter((p) => p.kind === undefined || p.kind === "social") + .map((p) => p.id), + ); + + return RENDERABLE_OAUTH_PROVIDERS.filter((id) => offered.has(id)); +} diff --git a/apps/dashboard/src/lib/server/session.ts b/apps/dashboard/src/lib/server/session.ts index 13f639964..ddcf676aa 100644 --- a/apps/dashboard/src/lib/server/session.ts +++ b/apps/dashboard/src/lib/server/session.ts @@ -1,6 +1,7 @@ import "server-only"; import { cache } from "react"; import { serverApi, ServerApiError } from "./api"; +import type { AdvertisedAuthProvider } from "@/lib/auth-providers"; /** * Session and user types returned by Better Auth's `/api/auth/get-session`. @@ -125,6 +126,14 @@ export type DeploymentInfo = { * send it; `resolveProductView` treats a missing value as "platform". */ productMode?: "platform" | "mail"; + /** + * Social-login providers the API actually has credentials for, advertised by + * GET /health/env. The auth pages render exactly these (see + * lib/auth-providers.ts) instead of assuming "cloud ⇒ github+google". Absent + * on an older server — treat that as "none advertised", which reproduces the + * old self-hosted behaviour. + */ + authProviders?: AdvertisedAuthProvider[]; cloudAuthUrl: string; cloudApiUrl: string; machineName?: string;