Pass the Keycloak client ID to next build (WEB-031)

.env.example

@@ -1 +1,5 @@
-DNS_DOMAIN=localhost
+DNS_DOMAIN=localhost
+# Keycloak client for the /appeal login. Must match the client TripBot redeems
+# the code with (KEYCLOAK_BAN_APPEALS_CLIENT_ID). Read when `next dev` starts or
+# `next build` runs, not by a running production container.
+NEXT_PUBLIC_KEYCLOAK_CLIENT_ID=

Dockerfile

@@ -54,6 +54,18 @@ COPY --chown=node:node --from=development /usr/src/app/node_modules ./node_modul
 # Copy over the existing source code
 COPY --chown=node:node . .
 
+# Next.js writes `process.env.NEXT_PUBLIC_*` into the browser bundle as literal
+# values during `next build`, so they must be set here, at build time. Setting
+# them on the running container has no effect on client code. Pass them with
+# `docker build --build-arg` or `build.args` in compose. An unset arg becomes an
+# empty string, and the code then uses its fallback (see src/utils/keycloak.ts).
+ARG NEXT_PUBLIC_DNS_DOMAIN
+ARG NEXT_PUBLIC_KEYCLOAK_URL
+ARG NEXT_PUBLIC_KEYCLOAK_CLIENT_ID
+ENV NEXT_PUBLIC_DNS_DOMAIN=$NEXT_PUBLIC_DNS_DOMAIN \
+    NEXT_PUBLIC_KEYCLOAK_URL=$NEXT_PUBLIC_KEYCLOAK_URL \
+    NEXT_PUBLIC_KEYCLOAK_CLIENT_ID=$NEXT_PUBLIC_KEYCLOAK_CLIENT_ID
+
 RUN npx next build
 
 ###################

REVIEW-ISSUES.md

@@ -59,6 +59,7 @@ Priorities: **P0** exploitable or critical, fix now · **P1** real bug or exposu
 | WEB-028 | P2  | deploy         | The shared deployer gives up after ~60 s when another deploy is running                    |
 | WEB-029 | P3  | bug            | Factsheets pager shows the full count ("1-10 of 555") after filtering                      |
 | WEB-030 | P2  | perf           | Expanding a factsheet row keeps the browser busy for tens of seconds                       |
+| WEB-031 | P1  | auth, build    | Ban-appeal login fails: the Keycloak client ID never reaches `next build`                  |
 
 ### Overlap groups (same files; take together or in sequence)
 
@@ -67,11 +68,11 @@ Priorities: **P0** exploitable or critical, fix now · **P1** real bug or exposu
 - **Template scripts:** WEB-008, WEB-009, WEB-022 (`_document.tsx`, `_app.tsx`, `public/assets/vendor`).
 - **Drug data:** WEB-007, WEB-010, WEB-029, WEB-030 (`_app.tsx`, `Factsheets.tsx`,
   `factsheets/[drug].tsx`, `DrugInfo.tsx`).
-- **Auth:** WEB-005, WEB-006, WEB-025 (`keycloak.ts`, `ProfileButton.tsx`, `appeal.tsx`).
+- **Auth:** WEB-005, WEB-006, WEB-025, WEB-031 (`keycloak.ts`, `ProfileButton.tsx`, `appeal.tsx`).
 - **Homepage:** WEB-003, WEB-020, WEB-021 (`pages/index.tsx`, `Counter.tsx`, `Ghost.tsx`).
 - **Workflows:** WEB-012, WEB-013, WEB-015, WEB-017 (`.github/workflows/`).
-- **Docker/Drone:** WEB-014, WEB-016, WEB-018, WEB-027, WEB-028 (`.drone.yml`, `Dockerfile`,
-  `.dockerignore`, Drone and deployer settings on the servers).
+- **Docker/Drone:** WEB-014, WEB-016, WEB-018, WEB-027, WEB-028, WEB-031 (`.drone.yml`,
+  `Dockerfile`, `.dockerignore`, Drone and deployer settings on the servers).
 
 ---
 
@@ -568,3 +569,25 @@ drug, and the table is wrapped in several providers and animations.
 
 **Fix.** Profile an expansion in Chrome DevTools (Performance panel) to find what keeps the
 main thread or compositor busy, then fix that.
+
+### WEB-031 — Ban-appeal login fails: the Keycloak client ID never reaches `next build`
+
+- Priority: P1 · Area: auth, build
+- Status: fixed — fix/WEB-031-keycloak-build-args (#266)
+- Notes: Also needs `build.args` for the three `NEXT_PUBLIC_*` variables in the compose files
+  that build the image: `docker-compose.website-prod.yml` on the UAT server (WEB-014) and
+  `docker-compose.website.yml` in the production server's repo. Production builds from `main`.
+- Files: `Dockerfile`, `src/utils/keycloak.ts`
+
+**Problem.** Logging in at `/appeal` fails after the user signs in. TripBot logs "Keycloak
+token error" and `POST /api/v2/keycloak/token` returns 500. Keycloak binds an auth code to
+the client that requested it, and TripBot redeems it as the ban-appeals client
+(`KEYCLOAK_BAN_APPEALS_CLIENT_ID`). The browser asked as `website`. Next.js writes
+`NEXT_PUBLIC_*` values into the browser bundle during `next build`, but the Dockerfile never
+declared them, so the build saw none. The live bundle still has
+`process.env.NEXT_PUBLIC_KEYCLOAK_CLIENT_ID||"website"` and always uses the fallback.
+
+**Fix.** Declare the three variables as `ARG`/`ENV` in the Dockerfile's build stage and pass
+them as compose `build.args`. `keycloak.ts` no longer falls back to `website`: a missing
+client ID throws outside production and is logged in production, where Keycloak then rejects
+the request before the user signs in.

src/utils/__tests__/keycloak.test.ts

@@ -0,0 +1,66 @@
+import { getLoginUrl } from "../keycloak";
+
+// NODE_ENV is typed read-only; tests need to switch it.
+const env = process.env as Record<string, string | undefined>;
+const saved = {
+  NODE_ENV: env.NODE_ENV,
+  NEXT_PUBLIC_KEYCLOAK_URL: env.NEXT_PUBLIC_KEYCLOAK_URL,
+  NEXT_PUBLIC_KEYCLOAK_CLIENT_ID: env.NEXT_PUBLIC_KEYCLOAK_CLIENT_ID,
+};
+
+function restore(name: keyof typeof saved) {
+  if (saved[name] === undefined) delete env[name];
+  else env[name] = saved[name];
+}
+
+beforeEach(() => {
+  delete env.NEXT_PUBLIC_KEYCLOAK_URL;
+  delete env.NEXT_PUBLIC_KEYCLOAK_CLIENT_ID;
+});
+
+afterEach(() => {
+  restore("NODE_ENV");
+  restore("NEXT_PUBLIC_KEYCLOAK_URL");
+  restore("NEXT_PUBLIC_KEYCLOAK_CLIENT_ID");
+  jest.restoreAllMocks();
+});
+
+describe("getLoginUrl", () => {
+  it("asks Keycloak for a code as the configured client", () => {
+    env.NEXT_PUBLIC_KEYCLOAK_CLIENT_ID = "ban-appeals";
+    const url = new URL(getLoginUrl());
+
+    expect(url.origin + url.pathname).toBe(
+      "https://auth.tripsit.me/realms/TripSit/protocol/openid-connect/auth",
+    );
+    expect(url.searchParams.get("client_id")).toBe("ban-appeals");
+    expect(url.searchParams.get("redirect_uri")).toBe(
+      `${window.location.origin}/appeal`,
+    );
+    expect(url.searchParams.get("response_type")).toBe("code");
+  });
+
+  it("uses NEXT_PUBLIC_KEYCLOAK_URL when it is set", () => {
+    env.NEXT_PUBLIC_KEYCLOAK_CLIENT_ID = "ban-appeals";
+    env.NEXT_PUBLIC_KEYCLOAK_URL = "https://auth.example.test";
+
+    expect(getLoginUrl()).toMatch(
+      /^https:\/\/auth\.example\.test\/realms\/TripSit\//,
+    );
+  });
+
+  it("throws outside production when the client ID is missing", () => {
+    expect(() => getLoginUrl()).toThrow(/NEXT_PUBLIC_KEYCLOAK_CLIENT_ID/);
+  });
+
+  it("never falls back to the old 'website' client in production", () => {
+    env.NODE_ENV = "production";
+    const error = jest.spyOn(console, "error").mockImplementation(() => {});
+    const url = new URL(getLoginUrl());
+
+    expect(url.searchParams.get("client_id")).toBe("");
+    expect(error).toHaveBeenCalledWith(
+      expect.stringContaining("NEXT_PUBLIC_KEYCLOAK_CLIENT_ID"),
+    );
+  });
+});

src/utils/keycloak.ts

@@ -1,22 +1,47 @@
 // src/utils/keycloak.ts
-// Simple Keycloak helper functions (placeholder values)
+// Builds the Keycloak login and logout URLs for the ban appeal flow.
+//
+// Next.js replaces each `process.env.NEXT_PUBLIC_*` expression with its value
+// when `next build` runs, so in the browser these are fixed at build time.
+// Changing them on a running container does nothing; they must be passed as
+// build args (see the Dockerfile). Next.js only replaces the full
+// `process.env.NAME` expression, so don't destructure or index process.env here.
+
+const CLIENT_ID_MISSING =
+  "NEXT_PUBLIC_KEYCLOAK_CLIENT_ID was not set when this bundle was built. " +
+  "Set it to the ban-appeals Keycloak client that TripBot redeems the code " +
+  "with (KEYCLOAK_BAN_APPEALS_CLIENT_ID); otherwise the token exchange fails.";
 
-// Helper function to get config at runtime instead of build time
 function getKeycloakConfig() {
   return {
     realm: "TripSit",
     url: process.env.NEXT_PUBLIC_KEYCLOAK_URL || "https://auth.tripsit.me",
-    clientId: process.env.NEXT_PUBLIC_KEYCLOAK_CLIENT_ID || "website",
-    redirectUri:
-      typeof window !== "undefined" ? `${window.location.origin}/appeal` : "",
   };
 }
 
+// Keycloak binds an auth code to the client that asked for it, and TripBot's
+// /api/v2/keycloak/token redeems it as the ban-appeals client. Any other client
+// ID here makes every login fail after the user signs in, so there is no
+// default. Outside production a missing ID throws; in production it is logged,
+// and Keycloak rejects the login request straight away.
+function getClientId(): string {
+  const clientId = process.env.NEXT_PUBLIC_KEYCLOAK_CLIENT_ID;
+  if (!clientId) {
+    if (process.env.NODE_ENV !== "production") {
+      throw new Error(CLIENT_ID_MISSING);
+    }
+    console.error(CLIENT_ID_MISSING);
+    return "";
+  }
+  return clientId;
+}
+
 export function getLoginUrl(): string {
   const config = getKeycloakConfig();
   const params = new URLSearchParams({
-    client_id: config.clientId,
-    redirect_uri: config.redirectUri,
+    client_id: getClientId(),
+    redirect_uri:
+      typeof window !== "undefined" ? `${window.location.origin}/appeal` : "",
     response_type: "code",
     scope: "openid profile email",
   });