From 950279dcb8cc3f0191809eb4971820fce0a9bca7 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:56:48 +0000 Subject: [PATCH 1/5] feat(auth): Replace Slack sign-in with an allowed domain list Remove the experimental slack-sign-in pre-auth flow. One optional, recommended domain list (dashboard.allowedGoogleDomains) now limits both surfaces: - Slack: mentions and DMs start a Turn only when the author's Slack-verified profile email is on an allowed domain. - Google: dashboard sign-in accepts only Workspace accounts whose verified hd claim is on an allowed domain. Junior sends hd= for one domain and hd=* for more. Drop the email allowlist (allowedEmails, JUNIOR_DASHBOARD_ALLOWED_EMAILS). Co-Authored-By: Sergiy Deceive-skiy <3760543+sergical@users.noreply.github.com> Co-Authored-By: David Cramer --- .../src/content/docs/operate/dashboard.md | 3 +- .../content/docs/reference/config-and-env.md | 45 +++-- packages/junior-dashboard/README.md | 6 +- packages/junior-dashboard/e2e/harness.ts | 2 +- packages/junior-dashboard/src/api/schema.ts | 1 - packages/junior-dashboard/src/app.ts | 24 +-- packages/junior-dashboard/src/auth.ts | 11 +- .../junior-dashboard/tests/client-api.test.ts | 2 +- .../tests/dashboard-routes.test.ts | 33 +--- .../tests/dashboard-shell-routes.test.ts | 3 +- .../tests/dashboard-test-helpers.ts | 1 - .../tests/telemetry-components.test.tsx | 1 - packages/junior/src/app.ts | 69 ++----- packages/junior/src/chat/experimental.ts | 1 - packages/junior/src/chat/identities/README.md | 32 ++- packages/junior/src/chat/identities/sql.ts | 59 +----- .../src/chat/ingress/slack-allowed-domain.ts | 51 +++++ .../junior/src/chat/ingress/slack-sign-in.ts | 77 -------- .../junior/src/chat/ingress/slack-webhook.ts | 25 ++- packages/junior/src/chat/sign-in/allowlist.ts | 54 +++-- packages/junior/src/chat/sign-in/auth.ts | 82 +++++--- packages/junior/src/chat/sign-in/slack.ts | 163 --------------- packages/junior/src/dashboard-routes.ts | 9 +- .../tests/integration/google-sign-in.test.ts | 107 ++++++++++ .../slack/webhook-auth-boundary.test.ts | 185 +++--------------- .../tests/unit/chat/sign-in-auth.test.ts | 52 +---- 26 files changed, 365 insertions(+), 733 deletions(-) create mode 100644 packages/junior/src/chat/ingress/slack-allowed-domain.ts delete mode 100644 packages/junior/src/chat/ingress/slack-sign-in.ts delete mode 100644 packages/junior/src/chat/sign-in/slack.ts create mode 100644 packages/junior/tests/integration/google-sign-in.test.ts diff --git a/packages/docs/src/content/docs/operate/dashboard.md b/packages/docs/src/content/docs/operate/dashboard.md index 0cbd13be79..9f78fdfe98 100644 --- a/packages/docs/src/content/docs/operate/dashboard.md +++ b/packages/docs/src/content/docs/operate/dashboard.md @@ -13,7 +13,7 @@ related: - /start-here/verify-and-troubleshoot/ --- -Use `@sentry/junior-dashboard` when you want browser access to Junior runtime diagnostics without exposing plugin, skill, or filesystem discovery publicly. The dashboard mounts into the same Nitro deployment as Junior. Junior core owns its Better Auth session. With the experimental `slack-sign-in` feature, Slack uses the same session, so a dashboard sign-in also counts as a recent Slack sign-in. +Use `@sentry/junior-dashboard` when you want browser access to Junior runtime diagnostics without exposing plugin, skill, or filesystem discovery publicly. The dashboard mounts into the same Nitro deployment as Junior. Junior core owns its Better Auth session. The allowed Google domains also limit who can use Junior in Slack. See [Allowed domains](/reference/config-and-env/#allowed-domains). ## Install @@ -69,7 +69,6 @@ You can also provide the same authorization policy through deployment environmen | Variable | Purpose | | ---------------------------------- | ------------------------------------------------------------- | | `JUNIOR_DASHBOARD_GOOGLE_DOMAINS` | Comma-separated or JSON array of allowed Google domains. | -| `JUNIOR_DASHBOARD_ALLOWED_EMAILS` | Comma-separated or JSON array of explicit email allowlist. | | `JUNIOR_DASHBOARD_TRUSTED_ORIGINS` | Comma-separated or JSON array of Better Auth trusted origins. | | `JUNIOR_DASHBOARD_AUTH_REQUIRED` | Set to `false` only for explicit local dashboard auth bypass. | diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index 4f2971ae30..cafb73f602 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -92,12 +92,11 @@ If you mount `@sentry/junior-dashboard`, set these browser-auth variables: Configure allowed Google Workspace domains in `createApp({ dashboard })` and `juniorNitro({ dashboard })` for normal deployments. Set these optional policy variables when you prefer environment-managed dashboard authorization: -| Variable | Required | Purpose | -| ------------------------------------- | -------- | --------------------------------------------------------------------------------------- | -| `JUNIOR_DASHBOARD_GOOGLE_DOMAINS` | No | Comma-separated or JSON array of allowed Google domains. | -| `JUNIOR_DASHBOARD_ALLOWED_EMAILS` | No | Comma-separated or JSON array of explicit email allowlist. | -| `JUNIOR_DASHBOARD_TRUSTED_ORIGINS` | No | Comma-separated or JSON array of Better Auth trusted origins. | -| `JUNIOR_DASHBOARD_MOCK_CONVERSATIONS` | No | Set to `true` to replace conversation API responses with local/demo visual-QA fixtures. | +| Variable | Required | Purpose | +| ------------------------------------- | -------- | ------------------------------------------------------------------------------------------ | +| `JUNIOR_DASHBOARD_GOOGLE_DOMAINS` | No | Comma-separated or JSON array of allowed domains. See [Allowed domains](#allowed-domains). | +| `JUNIOR_DASHBOARD_TRUSTED_ORIGINS` | No | Comma-separated or JSON array of Better Auth trusted origins. | +| `JUNIOR_DASHBOARD_MOCK_CONVERSATIONS` | No | Set to `true` to replace conversation API responses with local/demo visual-QA fixtures. | For local/demo dashboard visual QA, set `JUNIOR_DASHBOARD_MOCK_CONVERSATIONS=true` to replace conversation API responses with sample fixtures. @@ -168,9 +167,6 @@ const app = await createApp({ // database directly and skip Conversation privacy checks. Enable them only // on isolated deployments, such as Previews behind deployment protection. "operator-tools": true, - // Ask each Slack user for a recent Google sign-in before Junior - // replies to their mentions and DMs. - "slack-sign-in": true, }, }); ``` @@ -214,20 +210,23 @@ pre-stable surface. `passive-routing` turns on replies to non-mention messages in threads Junior already joined. Leave it unset in production unless you are testing that path. -`slack-sign-in` makes each Slack user sign in with Google before Junior starts -work from their mentions or DMs. Until then, Junior drops the message and sends -the user a private sign-in link. A sign-in counts for 7 days. Set -`slack.signInMaxAgeDays` to change this number. A dashboard sign-in counts too: -Slack and the dashboard use one Better Auth session, so a person who signed in -to one of them does not sign in again for the other. - -The dashboard is not required. Sign-in uses the same Google OAuth client, -allowlist, and callback as the dashboard: `GOOGLE_CLIENT_ID`, -`GOOGLE_CLIENT_SECRET`, `JUNIOR_DASHBOARD_GOOGLE_DOMAINS` or -`JUNIOR_DASHBOARD_ALLOWED_EMAILS`, and `/api/auth/callback/google` as -an authorized redirect URI. The Google email must match the Slack profile -email. Passive thread replies, slash commands, and Slack actions are not checked -yet. +## Allowed domains + +`dashboard.allowedGoogleDomains` (or `JUNIOR_DASHBOARD_GOOGLE_DOMAINS`) is one +list of domains for Slack and the dashboard. It is optional, but we recommend +it. Junior logs a warning at startup when it is empty. + +- **Slack:** Junior starts work from a mention or DM only when the email of the + author's Slack profile is on an allowed domain. Slack verifies that email. + Junior drops other messages and does not reply. Passive thread replies, slash + commands, and Slack actions are not checked yet. +- **Dashboard:** Google sign-in accepts only Google Workspace accounts on an + allowed domain. Junior checks the verified `hd` claim of the Google ID token, + not the email. With one domain, Junior also sends it to Google as the `hd` + hint. With more domains, it sends `hd=*`, so Google shows only Workspace + accounts. Dashboard auth requires at least one domain. + +When the list is empty, every member of the Slack workspace can use Junior. `operator-tools` adds `runOperatorSql` to non-public Conversations. The tool runs one SQL statement against the app database and returns up to 200 rows. diff --git a/packages/junior-dashboard/README.md b/packages/junior-dashboard/README.md index 760ba020df..4f04ea94ac 100644 --- a/packages/junior-dashboard/README.md +++ b/packages/junior-dashboard/README.md @@ -9,9 +9,9 @@ name, through dashboard settings. - `createDashboardApp` mounts the dashboard routes and receives host configuration through `JuniorDashboardOptions`. -- Core owns the Better Auth session and passes it in as `auth`. Slack sign-in - uses the same session. Dashboard routes fail closed when identity or required - configuration is missing. +- Core owns the Better Auth session and passes it in as `auth`. Core accepts + only Google Workspace accounts on the allowed domains. Dashboard routes fail + closed when identity or required configuration is missing. - The installable shell is static only: a public web app manifest, theme color, and install icon. There is no service worker and no offline app shell cache. - API schemas under `src/api/` define the client/server boundary. diff --git a/packages/junior-dashboard/e2e/harness.ts b/packages/junior-dashboard/e2e/harness.ts index b146752bb9..9e8f4aba3a 100644 --- a/packages/junior-dashboard/e2e/harness.ts +++ b/packages/junior-dashboard/e2e/harness.ts @@ -69,7 +69,7 @@ export async function startDashboardE2eServer( process.env.DATABASE_URL ??= "postgres://localhost/junior-dashboard-e2e"; const { createDashboardApp } = await import("../dist/app.js"); const app = createDashboardApp({ - allowedEmails: ["dev@example.com"], + allowedGoogleDomains: ["example.com"], auth: { async getSession() { return { diff --git a/packages/junior-dashboard/src/api/schema.ts b/packages/junior-dashboard/src/api/schema.ts index 2fa56ef326..147edb4226 100644 --- a/packages/junior-dashboard/src/api/schema.ts +++ b/packages/junior-dashboard/src/api/schema.ts @@ -33,7 +33,6 @@ export const dashboardProfileUpdateSchema = z export const dashboardConfigSchema = z .object({ - allowedEmailCount: z.number(), allowedGoogleDomainCount: z.number(), authRequired: z.boolean(), authPath: z.string(), diff --git a/packages/junior-dashboard/src/app.ts b/packages/junior-dashboard/src/app.ts index 6f107abbbb..627b5153cc 100644 --- a/packages/junior-dashboard/src/app.ts +++ b/packages/junior-dashboard/src/app.ts @@ -79,7 +79,6 @@ export interface JuniorDashboardOptions { authPath?: string; authRequired?: boolean; allowedGoogleDomains?: string[]; - allowedEmails?: string[]; auth?: DashboardAuth; componentGallery?: boolean; mockConversations?: boolean; @@ -163,8 +162,6 @@ function resolveDashboardOptions( allowedGoogleDomains: options.allowedGoogleDomains ?? readEnvList("JUNIOR_DASHBOARD_GOOGLE_DOMAINS"), - allowedEmails: - options.allowedEmails ?? readEnvList("JUNIOR_DASHBOARD_ALLOWED_EMAILS"), componentGallery: options.componentGallery ?? readEnvFlag("JUNIOR_DASHBOARD_COMPONENT_GALLERY"), @@ -405,7 +402,6 @@ export function createDashboardApp( DEFAULT_AUTH_PATH, ); const allowedDomains = normalizeValues(options.allowedGoogleDomains); - const allowedEmails = normalizeValues(options.allowedEmails); const agentName = options.agentName?.trim() || "Junior"; const authRequired = options.authRequired !== false; @@ -415,17 +411,11 @@ export function createDashboardApp( canonicalBaseURL = resolveDashboardBaseURL({ baseURL: configuredBaseURL }); } - if ( - authRequired && - allowedDomains.length === 0 && - allowedEmails.length === 0 - ) { - throw new Error( - "Junior dashboard auth requires allowedGoogleDomains or allowedEmails", - ); + if (authRequired && allowedDomains.length === 0) { + throw new Error("Junior dashboard auth requires allowedGoogleDomains"); } - // Core owns the Google sign-in session and shares it with Slack. + // Core owns the Google sign-in session. const auth = options.auth; if (authRequired && !auth) { throw new Error("Junior dashboard auth requires the core sign-in session"); @@ -448,10 +438,7 @@ export function createDashboardApp( return Response.redirect(returnUrl, 302); } const session = await auth.getSession(c.req.raw); - if ( - session && - dashboardSessionIsAuthorized(session, allowedDomains, allowedEmails) - ) { + if (session && dashboardSessionIsAuthorized(session, allowedDomains)) { return Response.redirect(returnUrl, 302); } return auth.signInWithGoogle(c.req.raw, returnUrl); @@ -540,7 +527,7 @@ export function createDashboardApp( componentGallery: options.componentGallery, }); } - if (!dashboardSessionIsAuthorized(session, allowedDomains, allowedEmails)) { + if (!dashboardSessionIsAuthorized(session, allowedDomains)) { return forbidden(c.req.raw, agentName); } const sanitizedSession = sanitizeDashboardSession(session); @@ -621,7 +608,6 @@ export function createDashboardApp( ); app.get("/api/config", () => { return jsonResponse(dashboardConfigSchema, { - allowedEmailCount: allowedEmails.length, allowedGoogleDomainCount: allowedDomains.length, authRequired, authPath, diff --git a/packages/junior-dashboard/src/auth.ts b/packages/junior-dashboard/src/auth.ts index a8cc683c72..520c24b7e7 100644 --- a/packages/junior-dashboard/src/auth.ts +++ b/packages/junior-dashboard/src/auth.ts @@ -25,19 +25,22 @@ export function dashboardBearerSession(email: string): DashboardSession { return { user: { email, emailVerified: true } }; } -/** Check the dashboard allowlist against one verified session. */ +/** + * Check the allowed domains against one verified session. Core accepts a + * Google sign-in only for a Workspace account on an allowed domain. This + * check also applies a changed list to existing sessions and personal tokens. + */ export function dashboardSessionIsAuthorized( session: DashboardSession, allowedDomains: string[], - allowedEmails: string[], ): boolean { const email = session.user.email.toLowerCase(); const separator = email.lastIndexOf("@"); const domain = separator > 0 ? email.slice(separator + 1) : undefined; return Boolean( session.user.emailVerified && - (allowedEmails.includes(email) || - (domain !== undefined && allowedDomains.includes(domain))), + domain !== undefined && + allowedDomains.includes(domain), ); } diff --git a/packages/junior-dashboard/tests/client-api.test.ts b/packages/junior-dashboard/tests/client-api.test.ts index d0ed4a5494..2f9505c965 100644 --- a/packages/junior-dashboard/tests/client-api.test.ts +++ b/packages/junior-dashboard/tests/client-api.test.ts @@ -117,7 +117,7 @@ describe("dashboard client API", () => { user: { email: "dev@example.com", emailVerified: true }, }); const app = createDashboardApp({ - allowedEmails: ["dev@example.com"], + allowedGoogleDomains: ["example.com"], mockConversations: true, auth: { ...sessionAuth, diff --git a/packages/junior-dashboard/tests/dashboard-routes.test.ts b/packages/junior-dashboard/tests/dashboard-routes.test.ts index fce8815c89..1087d7a7fa 100644 --- a/packages/junior-dashboard/tests/dashboard-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-routes.test.ts @@ -27,7 +27,6 @@ vi.mock("@sentry/junior/api", async (importOriginal) => ({ function dashboard(session: DashboardSession | null) { return createDashboardApp({ allowedGoogleDomains: ["sentry.io"], - allowedEmails: ["admin@example.com"], auth: auth(session), }); } @@ -260,7 +259,7 @@ describe("dashboard routes", () => { it("starts sign-in when the auth prefix overlaps the login route", async () => { const app = createDashboardApp({ - allowedEmails: ["admin@example.com"], + allowedGoogleDomains: ["sentry.io"], auth: auth(null), authPath: "/auth", }); @@ -561,7 +560,7 @@ describe("dashboard routes", () => { it("preserves the enabled component gallery path through login", async () => { let callbackURL: string | undefined; const app = createDashboardApp({ - allowedEmails: ["admin@example.com"], + allowedGoogleDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -598,7 +597,6 @@ describe("dashboard routes", () => { expect(response.status).toBe(200); expect(response.headers.get(DASHBOARD_VERSION_HEADER)).toBe(JUNIOR_VERSION); expect(await response.json()).toEqual({ - allowedEmailCount: 1, allowedGoogleDomainCount: 1, authRequired: true, authPath: "/api/auth", @@ -625,25 +623,10 @@ describe("dashboard routes", () => { expect(await response.json()).toEqual({ error: "forbidden" }); }); - it("allows explicitly configured email exceptions", async () => { - const app = dashboard({ - user: { - email: "admin@example.com", - emailVerified: true, - }, - }); - - const response = await app.fetch( - new Request("http://localhost/api/runtime"), - ); - - expect(response.status).toBe(200); - }); - - it("requires verified email for explicitly configured email exceptions", async () => { + it("requires a verified email on an allowed domain", async () => { const app = dashboard({ user: { - email: "admin@example.com", + email: "person@sentry.io", emailVerified: false, }, }); @@ -657,9 +640,6 @@ describe("dashboard routes", () => { it("resolves auth policy from env when dashboard options omit allowlists", async () => { process.env.JUNIOR_DASHBOARD_GOOGLE_DOMAINS = "sentry.io, example.com"; - process.env.JUNIOR_DASHBOARD_ALLOWED_EMAILS = JSON.stringify([ - "admin@example.com", - ]); process.env.JUNIOR_DASHBOARD_TRUSTED_ORIGINS = "https://junior.example.com"; process.env.JUNIOR_DASHBOARD_MOCK_CONVERSATIONS = "true"; @@ -678,19 +658,18 @@ describe("dashboard routes", () => { expect(response.status).toBe(200); expect(await response.json()).toMatchObject({ - allowedEmailCount: 1, allowedGoogleDomainCount: 2, authRequired: true, }); }); it("fails clearly when list env JSON is malformed", async () => { - process.env.JUNIOR_DASHBOARD_ALLOWED_EMAILS = '["admin@example.com"'; + process.env.JUNIOR_DASHBOARD_GOOGLE_DOMAINS = '["sentry.io"'; expect(() => createDashboardApp({ authRequired: false, }), - ).toThrow("JUNIOR_DASHBOARD_ALLOWED_EMAILS must be a JSON string array"); + ).toThrow("JUNIOR_DASHBOARD_GOOGLE_DOMAINS must be a JSON string array"); }); }); diff --git a/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts b/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts index 0c53066507..971efecc77 100644 --- a/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts @@ -18,7 +18,6 @@ vi.mock("@sentry/junior/api", async (importOriginal) => ({ function dashboard(session: DashboardSession | null) { return createDashboardApp({ allowedGoogleDomains: ["sentry.io"], - allowedEmails: ["admin@example.com"], auth: auth(session), }); } @@ -225,7 +224,7 @@ describe("dashboard shell routes", () => { it("serves the installable shell manifest without auth", async () => { const app = createDashboardApp({ agentName: "Marky", - allowedEmails: ["admin@example.com"], + allowedGoogleDomains: ["sentry.io"], auth: auth(null), basePath: "/ops", }); diff --git a/packages/junior-dashboard/tests/dashboard-test-helpers.ts b/packages/junior-dashboard/tests/dashboard-test-helpers.ts index e2ca64b9e9..1166be8205 100644 --- a/packages/junior-dashboard/tests/dashboard-test-helpers.ts +++ b/packages/junior-dashboard/tests/dashboard-test-helpers.ts @@ -12,7 +12,6 @@ export const dashboardEnvNames = [ "JUNIOR_DASHBOARD_AUTH_REQUIRED", "JUNIOR_DASHBOARD_COMPONENT_GALLERY", "JUNIOR_DASHBOARD_GOOGLE_DOMAINS", - "JUNIOR_DASHBOARD_ALLOWED_EMAILS", "JUNIOR_DASHBOARD_TRUSTED_ORIGINS", "JUNIOR_DASHBOARD_MOCK_CONVERSATIONS", "SENTRY_DSN", diff --git a/packages/junior-dashboard/tests/telemetry-components.test.tsx b/packages/junior-dashboard/tests/telemetry-components.test.tsx index 23e5b95fae..94a8db857a 100644 --- a/packages/junior-dashboard/tests/telemetry-components.test.tsx +++ b/packages/junior-dashboard/tests/telemetry-components.test.tsx @@ -91,7 +91,6 @@ function renderTranscript(detail: ConversationTranscript): string { function systemData(): SystemData { return { config: { - allowedEmailCount: 0, allowedGoogleDomainCount: 0, authPath: "/api/auth", authRequired: false, diff --git a/packages/junior/src/app.ts b/packages/junior/src/app.ts index 08d9c17bc0..511ef1756e 100644 --- a/packages/junior/src/app.ts +++ b/packages/junior/src/app.ts @@ -26,7 +26,6 @@ import { executeAgentRun } from "@/chat/agent"; import { normalizeSandboxEgressTracePropagationDomains } from "@/chat/sandbox/egress/tracing"; import { getExperimentalFeatures, - isExperimentalFeatureEnabled, setExperimentalFeatures, type ExperimentalFeaturesConfig, } from "@/chat/experimental"; @@ -122,12 +121,7 @@ import { type SignInAuth, type SignInAuthConfig, } from "@/chat/sign-in/auth"; -import { - DEFAULT_SLACK_SIGN_IN_MAX_AGE_DAYS, - handleSlackSignIn, - SLACK_SIGN_IN_PATH, - type SlackSignIn, -} from "@/chat/sign-in/slack"; +import { resolveAllowedDomains } from "@/chat/sign-in/allowlist"; import { JUNIOR_VERSION } from "./version"; import { createDashboardRouteRegistrations, @@ -183,12 +177,6 @@ export interface JuniorAppOptions extends BotModelConfig { * `JUNIOR_CROSS_ACTOR_MID_RUN_MODE` or `follow_up`. */ crossActorMidRunMode?: CrossActorMidRunMode; - /** - * With `experimental["slack-sign-in"]`, how many days a Google sign-in, - * in Slack or the dashboard, lets a person use Junior in Slack. - * Defaults to 7. - */ - signInMaxAgeDays?: number; }; /** Install-wide provider defaults. Unregistered `provider.key` entries warn at startup. */ configDefaults?: Record; @@ -455,7 +443,7 @@ export async function createApp(options?: JuniorAppOptions): Promise { }); let sandboxEgressTracePropagationDomains: string[] = []; let signInConfig: SignInAuthConfig | undefined; - let slackSignIn: SlackSignIn | undefined; + let allowedDomains: string[] = []; try { sandboxEgressTracePropagationDomains = normalizeSandboxEgressTracePropagationDomains( @@ -477,26 +465,16 @@ export async function createApp(options?: JuniorAppOptions): Promise { if (options?.slack?.crossActorMidRunMode) { setCrossActorMidRunMode(options.slack.crossActorMidRunMode); } - const dashboardMounted = Boolean(dashboard && !dashboard.disabled); - if (isExperimentalFeatureEnabled("slack-sign-in") || dashboardMounted) { - signInConfig = resolveSignInAuthConfig(botConfig.userName, dashboard); + // One domain list for Slack authors and dashboard sign-ins. + allowedDomains = resolveAllowedDomains(dashboard); + if (allowedDomains.length === 0) { + logWarn("app.allowed_domains.unset", { + "app.reason": + "dashboard.allowedGoogleDomains is empty, so any Slack workspace member can use Junior", + }); } - if (signInConfig && isExperimentalFeatureEnabled("slack-sign-in")) { - const { allowlist } = signInConfig; - if (!allowlist.domains.length && !allowlist.emails.length) { - throw new Error( - "experimental.slack-sign-in requires allowed Google domains or emails", - ); - } - const maxAgeDays = - options?.slack?.signInMaxAgeDays ?? DEFAULT_SLACK_SIGN_IN_MAX_AGE_DAYS; - if (!(maxAgeDays > 0)) { - throw new Error("slack.signInMaxAgeDays must be greater than 0"); - } - slackSignIn = { - baseURL: signInConfig.baseURL, - maxAgeMs: maxAgeDays * 24 * 60 * 60 * 1000, - }; + if (dashboard && !dashboard.disabled) { + signInConfig = resolveSignInAuthConfig(botConfig.userName, dashboard); } if (shouldValidatePluginCatalog) { pluginCatalogRuntime.getSignature(); @@ -612,7 +590,7 @@ export async function createApp(options?: JuniorAppOptions): Promise { restoreRuntimeConfig(); throw error; } - // One Google sign-in session for the dashboard and Slack. + // The Google sign-in session of the dashboard. const signInAuth: SignInAuth | undefined = signInConfig ? createSignInAuth(signInConfig) : undefined; @@ -621,7 +599,7 @@ export async function createApp(options?: JuniorAppOptions): Promise { queue: conversationWorkQueue, services: runtimeServiceOverrides, }), - ...(slackSignIn ? { signIn: slackSignIn } : undefined), + allowedDomains, }; const app = new Hono(); @@ -682,27 +660,6 @@ export async function createApp(options?: JuniorAppOptions): Promise { }); }); - if (signInAuth && signInConfig && slackSignIn) { - const auth = signInAuth; - // The dashboard serves the sign-in routes when it is mounted. - if (!dashboard || dashboard.disabled) { - app.on(["GET", "POST"], `${signInConfig.authPath}/*`, (c) => - auth.handler(c.req.raw), - ); - } - const { allowlist } = signInConfig; - const signIn = slackSignIn; - app.get(SLACK_SIGN_IN_PATH, (c) => - handleSlackSignIn({ - allowlist, - auth, - signIn, - request: c.req.raw, - state: getStateAdapter(), - }), - ); - } - app.get("/api/internal/sandbox-egress/signals", (c) => { return sandboxEgressSignalsGET(c.req.raw); }); diff --git a/packages/junior/src/chat/experimental.ts b/packages/junior/src/chat/experimental.ts index 81cc8a73c8..dc7d1ef166 100644 --- a/packages/junior/src/chat/experimental.ts +++ b/packages/junior/src/chat/experimental.ts @@ -6,7 +6,6 @@ export const EXPERIMENTAL_FEATURES = [ "operator-tools", "passive-routing", - "slack-sign-in", "subagents", ] as const; diff --git a/packages/junior/src/chat/identities/README.md b/packages/junior/src/chat/identities/README.md index 78c8703abf..b2b3ca58bb 100644 --- a/packages/junior/src/chat/identities/README.md +++ b/packages/junior/src/chat/identities/README.md @@ -25,8 +25,8 @@ these identities, so only core links them. `isNativeIdentityProvider()` in - A Slack identity has the Slack team ID as its tenant and the Slack user ID as its subject. - A Google identity has an empty tenant and the Google OIDC `sub` as its - subject. Each allowed Google sign-in, from Slack or the dashboard, saves it - with the verified Google email and sets `signed_in_at`. One Google account is + subject. Each dashboard Google sign-in saves it with the verified Google + email and sets `signed_in_at`. One Google account is one row, so every feature that reads the `google` identity sees the same account. - The Google Workspace plugin is not a Google identity. It stores Junior's own @@ -57,23 +57,15 @@ Provider handles and subject IDs always remain identity-scoped. Display names are presentation data and must never be used to link identities or grant authority. -## Slack Sign-In +## Allowed Domains -With the experimental `slack-sign-in` feature, Slack ingress accepts mentions -and DMs only when the user of the Slack identity has a Google identity with a -recent `signed_in_at`. The default limit is 7 days -(`slack.signInMaxAgeDays`). `chat/sign-in/` owns the sign-in. It does not need -the dashboard. +`chat/sign-in/allowlist.ts` owns one list of allowed domains for Slack and the +dashboard. An empty list allows every domain. -- `chat/sign-in/auth.ts` owns the one Better Auth instance. Slack and the - dashboard use it, so they share one browser session and one Google callback. - Core mounts its routes when the dashboard is not mounted. -- After each Google callback, a Better Auth hook saves the Google identity when - the verified email is on the dashboard allowlist. -- Ingress saves the Slack identity with its Slack profile email. Then it stores - one sign-in state for that Slack identity and sends a private link to - `/api/sign-in/slack`. -- That page reads the shared session. It starts one Google sign-in when there is - no session or no recent sign-in. It succeeds only when the user of the Slack - identity has the session email. A forwarded link cannot give another - person's Slack account access. +- Slack ingress starts a Turn from a mention or DM only when the email of the + author's Slack profile is on an allowed domain + (`chat/ingress/slack-allowed-domain.ts`). +- `chat/sign-in/auth.ts` owns the one Better Auth instance of the dashboard. It + accepts a Google sign-in only when the verified `hd` claim is an allowed + domain. After each Google callback, a Better Auth hook saves the Google + identity. diff --git a/packages/junior/src/chat/identities/sql.ts b/packages/junior/src/chat/identities/sql.ts index a8f36e477d..7e79a830d1 100644 --- a/packages/junior/src/chat/identities/sql.ts +++ b/packages/junior/src/chat/identities/sql.ts @@ -1,6 +1,5 @@ import { randomUUID } from "node:crypto"; -import { and, eq, gte, inArray, sql } from "drizzle-orm"; -import { alias } from "drizzle-orm/pg-core"; +import { and, eq, inArray, sql } from "drizzle-orm"; import type { JuniorDatabase, JuniorSqlDatabase } from "@/db/db"; import { juniorIdentities, juniorUsers } from "@/db/schema"; import { @@ -12,8 +11,6 @@ import { type IdentityRow = typeof juniorIdentities.$inferSelect; -const googleIdentity = alias(juniorIdentities, "google_identity"); - function dateFromMs(ms: number): Date { return new Date(ms); } @@ -319,28 +316,6 @@ export async function deleteProviderIdentityForUser( ); } -/** Read the user linked to one Slack identity. */ -export async function findSlackIdentityUserId( - executor: JuniorSqlDatabase, - slackTeamId: string, - slackUserId: string, -): Promise { - const rows = await executor - .db() - .select({ userId: juniorIdentities.userId }) - .from(juniorIdentities) - .where( - and( - eq(juniorIdentities.kind, "user"), - eq(juniorIdentities.provider, "slack"), - eq(juniorIdentities.providerTenantId, slackTeamId), - eq(juniorIdentities.providerSubjectId, slackUserId), - ), - ) - .limit(1); - return rows[0]?.userId ?? undefined; -} - /** * Save one Google sign-in. Link the Google identity to the user of its * verified email, and set its sign-in time. @@ -369,35 +344,3 @@ export async function recordGoogleSignIn( .where(eq(juniorIdentities.id, stored.id)); return stored; } - -/** - * Return whether the user of one Slack identity signed in with Google at or - * after `signedInSince`. - */ -export async function slackUserHasGoogleSignIn( - executor: JuniorSqlDatabase, - args: { signedInSince: Date; slackTeamId: string; slackUserId: string }, -): Promise { - const rows = await executor - .db() - .select({ id: googleIdentity.id }) - .from(juniorIdentities) - .innerJoin( - googleIdentity, - eq(googleIdentity.userId, juniorIdentities.userId), - ) - .where( - and( - eq(juniorIdentities.kind, "user"), - eq(juniorIdentities.provider, "slack"), - eq(juniorIdentities.providerTenantId, args.slackTeamId), - eq(juniorIdentities.providerSubjectId, args.slackUserId), - eq(googleIdentity.kind, "user"), - eq(googleIdentity.provider, GOOGLE_IDENTITY_PROVIDER), - eq(googleIdentity.emailVerified, true), - gte(googleIdentity.signedInAt, args.signedInSince), - ), - ) - .limit(1); - return rows.length > 0; -} diff --git a/packages/junior/src/chat/ingress/slack-allowed-domain.ts b/packages/junior/src/chat/ingress/slack-allowed-domain.ts new file mode 100644 index 0000000000..fb4064da6f --- /dev/null +++ b/packages/junior/src/chat/ingress/slack-allowed-domain.ts @@ -0,0 +1,51 @@ +/** Limit Slack use of Junior to authors on the allowed domains. */ +import type { StateAdapter } from "chat"; +import { logWarn } from "@/chat/logging"; +import { isAllowedDomain } from "@/chat/sign-in/allowlist"; +import { parseSlackUserId } from "@/chat/slack/ids"; +import { lookupSlackUserProfile } from "@/chat/slack/users"; + +// Like workspace membership: avoid a lookup per message, and let a changed +// profile email apply within five minutes. +const ALLOWED_TTL_MS = 5 * 60 * 1000; +const NOT_ALLOWED_TTL_MS = 30 * 1000; + +/** + * Return whether a Slack author may start a Turn. With allowed domains, the + * email of the author's Slack profile must be on one of them. Slack verifies + * that email, so Junior trusts it. Without allowed domains, every author may. + */ +export async function isSlackAuthorDomainAllowed(args: { + allowedDomains: readonly string[]; + state: StateAdapter; + teamId: string | undefined; + user: unknown; +}): Promise { + if (args.allowedDomains.length === 0) return true; + const userId = parseSlackUserId(args.user); + if (!args.teamId || !userId) return false; + const key = `slack:allowed-domain:${args.teamId}:${userId}`; + const cached = await args.state.get(key); + if (typeof cached === "boolean") return cached; + + const profile = await lookupSlackUserProfile(userId); + const email = profile.email?.toLowerCase(); + const allowed = + profile.id === userId && + isAllowedDomain( + email?.slice(email.lastIndexOf("@") + 1), + args.allowedDomains, + ); + if (!allowed) { + // A Slack app without `users:read.email` sees no email and blocks all. + logWarn("slack.allowed_domain.rejected", { + "app.reason": email ? "domain_not_allowed" : "missing_email", + }); + } + await args.state.set( + key, + allowed, + allowed ? ALLOWED_TTL_MS : NOT_ALLOWED_TTL_MS, + ); + return allowed; +} diff --git a/packages/junior/src/chat/ingress/slack-sign-in.ts b/packages/junior/src/chat/ingress/slack-sign-in.ts deleted file mode 100644 index 10e372d6e7..0000000000 --- a/packages/junior/src/chat/ingress/slack-sign-in.ts +++ /dev/null @@ -1,77 +0,0 @@ -/** Require a recent Google sign-in before a Slack user can start a Turn. */ -import { botConfig } from "@/chat/config"; -import { getSqlExecutor } from "@/chat/db"; -import type { SlackInboundEvent } from "@/chat/ingress/slack-payload"; -import { - slackUserHasGoogleSignIn, - upsertIdentity, -} from "@/chat/identities/sql"; -import type { SlackInstallationContext } from "@/chat/slack/adapter-context"; -import { parseSlackUserId } from "@/chat/slack/ids"; -import { formatSlackLink } from "@/chat/slack/mrkdwn"; -import { lookupSlackUserProfile } from "@/chat/slack/users"; -import { deliverPrivateMessage } from "@/chat/oauth-flow"; -import { createSlackSignInUrl, type SlackSignIn } from "@/chat/sign-in/slack"; -import type { StateAdapter } from "chat"; - -/** - * Return whether this Slack user may start work. - * - * The user of the Slack identity must have signed in with Google, in Slack or - * in the dashboard, within `signIn.maxAgeMs`. When it has not, save the Slack - * identity with the email from the Slack profile, so the sign-in page can match - * it, and check again. Then send the user a private sign-in link and drop the - * message. - */ -export async function hasSlackSignIn(args: { - event: SlackInboundEvent; - installation: SlackInstallationContext; - signIn: SlackSignIn | undefined; - state: StateAdapter; -}): Promise { - if (!args.signIn) return true; - const teamId = args.installation.teamId; - const channelId = args.event.channel; - const userId = parseSlackUserId(args.event.user); - if (!teamId || !channelId || !userId) return false; - const executor = getSqlExecutor(); - const signedInSince = new Date(Date.now() - args.signIn.maxAgeMs); - const hasSignIn = () => - slackUserHasGoogleSignIn(executor, { - signedInSince, - slackTeamId: teamId, - slackUserId: userId, - }); - if (await hasSignIn()) return true; - - const profile = await lookupSlackUserProfile(userId); - await upsertIdentity(executor, { - kind: "user", - provider: "slack", - providerTenantId: teamId, - providerSubjectId: userId, - ...(profile.real_name ? { displayName: profile.real_name } : undefined), - ...(profile.name ? { handle: profile.name } : undefined), - ...(profile.email - ? { email: profile.email, emailVerified: true } - : undefined), - metadata: { platform: "slack" }, - }); - // The first Slack message links the Slack identity to its user. That user - // may already have a recent dashboard sign-in. - if (await hasSignIn()) return true; - - const link = await createSlackSignInUrl({ - baseURL: args.signIn.baseURL, - slackTeamId: teamId, - slackUserId: userId, - state: args.state, - }); - await deliverPrivateMessage({ - channelId, - ...(args.event.thread_ts ? { threadTs: args.event.thread_ts } : undefined), - userId, - text: `${formatSlackLink(link, `Sign in to use ${botConfig.userName}`)}. Sign in with your work Google account, then send your message again.`, - }); - return false; -} diff --git a/packages/junior/src/chat/ingress/slack-webhook.ts b/packages/junior/src/chat/ingress/slack-webhook.ts index b7afc5544a..4947d30c3f 100644 --- a/packages/junior/src/chat/ingress/slack-webhook.ts +++ b/packages/junior/src/chat/ingress/slack-webhook.ts @@ -59,8 +59,7 @@ import { withNormalizedThreadId, } from "@/chat/ingress/message-router"; import { isSlackWorkspaceMember } from "@/chat/ingress/workspace-membership"; -import { hasSlackSignIn } from "@/chat/ingress/slack-sign-in"; -import type { SlackSignIn } from "@/chat/sign-in/slack"; +import { isSlackAuthorDomainAllowed } from "@/chat/ingress/slack-allowed-domain"; import { slackMessageAttributes } from "./slack-message-telemetry"; import { runWithWorkspaceTeamId } from "@/chat/slack/workspace-context"; import { parseSlackThreadId } from "@/chat/slack/context"; @@ -136,8 +135,8 @@ export interface SlackWebhookServices { getSlackAdapter: () => SlackAdapter; queue: ConversationWorkQueue; conversationStore?: ConversationStore; - /** Set when Slack use needs a recent Google sign-in. */ - signIn?: SlackSignIn; + /** Email domains Slack authors need to start a Turn. Empty allows all. */ + allowedDomains?: readonly string[]; runtime: Pick< SlackTurnRuntime, | "handleAssistantContextChanged" @@ -449,7 +448,7 @@ async function routeParsedMessage(args: { conversationStore?: ConversationStore; queue: ConversationWorkQueue; receivedAtMs: number; - signIn: SlackSignIn | undefined; + allowedDomains: readonly string[]; state: StateAdapter; }): Promise { if (shouldIgnoreMessage(args.message)) { @@ -534,8 +533,18 @@ async function routeParsedMessage(args: { return; } - // Passive thread messages are not checked for sign-in yet. - if (route === "mention" && !(await hasSlackSignIn(args))) return; + // Passive thread messages are not checked for the allowed domains yet. + if ( + route === "mention" && + !(await isSlackAuthorDomainAllowed({ + allowedDomains: args.allowedDomains, + state: args.state, + teamId: args.installation.teamId, + user: args.event.user, + })) + ) { + return; + } // Keep non-mention thread messages as Conversation history without waking a // worker when passive routing is off. Later explicit mentions still need them. @@ -711,7 +720,7 @@ async function handleSlackEvent(args: { conversationStore: args.services.conversationStore, queue: args.services.queue, receivedAtMs, - signIn: args.services.signIn, + allowedDomains: args.services.allowedDomains ?? [], state, }), { keepAlive: true, waitMs: 10_000 }, diff --git a/packages/junior/src/chat/sign-in/allowlist.ts b/packages/junior/src/chat/sign-in/allowlist.ts index d0089fe6a3..315ad21eac 100644 --- a/packages/junior/src/chat/sign-in/allowlist.ts +++ b/packages/junior/src/chat/sign-in/allowlist.ts @@ -1,12 +1,10 @@ -/** Google accounts that may sign in. The dashboard uses the same list. */ +/** + * Domains that may use Junior. Google sign-ins must come from a Google + * Workspace account on one of these domains, and Slack authors must have a + * Slack profile email on one of them. The dashboard and Slack use one list. + */ import { z } from "zod"; -/** Google accounts that may sign in. Empty lists allow nobody. */ -export interface SignInAllowlist { - domains: string[]; - emails: string[]; -} - /** * Read a list env var the same way as the dashboard: a JSON string array or * comma-separated values. @@ -27,30 +25,30 @@ export function readEnvList(name: string): string[] | undefined { } /** - * Resolve the sign-in allowlist. Use the same values as the dashboard: its - * options when set, else its environment variables. + * Resolve the allowed domains: the dashboard option when set, else its + * environment variable. An empty list allows every domain. */ -export function resolveSignInAllowlist( - options: { allowedEmails?: string[]; allowedGoogleDomains?: string[] } = {}, -): SignInAllowlist { - const list = (values: string[] | undefined, name: string) => - (values ?? readEnvList(name) ?? []) - .map((value) => value.trim().toLowerCase()) - .filter(Boolean); - return { - domains: list( - options.allowedGoogleDomains, - "JUNIOR_DASHBOARD_GOOGLE_DOMAINS", +export function resolveAllowedDomains( + options: { allowedGoogleDomains?: string[] } = {}, +): string[] { + return [ + ...new Set( + ( + options.allowedGoogleDomains ?? + readEnvList("JUNIOR_DASHBOARD_GOOGLE_DOMAINS") ?? + [] + ) + .map((value) => value.trim().toLowerCase()) + .filter(Boolean), ), - emails: list(options.allowedEmails, "JUNIOR_DASHBOARD_ALLOWED_EMAILS"), - }; + ]; } -/** Return whether a normalized Google email is on the allowlist. */ -export function isSignInAllowed( - email: string, - allowlist: SignInAllowlist, +/** Return whether a verified domain may use Junior. An empty list allows all. */ +export function isAllowedDomain( + domain: string | undefined, + allowedDomains: readonly string[], ): boolean { - const domain = email.slice(email.lastIndexOf("@") + 1); - return allowlist.emails.includes(email) || allowlist.domains.includes(domain); + if (allowedDomains.length === 0) return true; + return Boolean(domain && allowedDomains.includes(domain.toLowerCase())); } diff --git a/packages/junior/src/chat/sign-in/auth.ts b/packages/junior/src/chat/sign-in/auth.ts index 18e2bfbaaf..9a3cd7af92 100644 --- a/packages/junior/src/chat/sign-in/auth.ts +++ b/packages/junior/src/chat/sign-in/auth.ts @@ -1,26 +1,40 @@ /** - * Google sign-in session shared by the dashboard and Slack. + * Google sign-in session for the dashboard. * * Better Auth runs the Google OAuth flow and keeps the browser session in an - * encrypted cookie. Each sign-in also saves the native Google identity with its - * sign-in time, so Slack can check a recent sign-in without the browser. + * encrypted cookie. With allowed domains, only Google Workspace accounts on + * those domains can sign in. Each sign-in also saves the native Google + * identity. */ import { createAuthMiddleware } from "better-auth/api"; import { betterAuth } from "better-auth/minimal"; +import { decodeJwt } from "jose"; +import { z } from "zod"; import { getSqlExecutor } from "@/chat/db"; import { normalizeIdentityEmail } from "@/chat/identities/identity"; import { recordGoogleSignIn } from "@/chat/identities/sql"; import { resolveBaseUrl } from "@/chat/oauth-flow"; import { - isSignInAllowed, + isAllowedDomain, readEnvList, - resolveSignInAllowlist, - type SignInAllowlist, + resolveAllowedDomains, } from "./allowlist"; const DEFAULT_AUTH_PATH = "/api/auth"; const DEFAULT_SESSION_MAX_AGE_SECONDS = 60 * 60 * 8; +// Claims Junior reads from the Google ID token. Google sets `hd` only for +// Google Workspace accounts, so it proves the account belongs to the domain. +// The email domain does not: a personal Google account can use any email. +const googleClaimsSchema = z.object({ + sub: z.string().min(1), + email: z.string().optional(), + email_verified: z.boolean().optional(), + hd: z.string().optional(), + name: z.string().optional(), + picture: z.string().optional(), +}); + /** Verified Google account of one browser session. */ export interface SignInSession { user: { email: string; emailVerified: boolean; name?: string }; @@ -40,7 +54,8 @@ export interface SignInAuth { /** Settings for the shared Google sign-in session. */ export interface SignInAuthConfig { agentName: string; - allowlist: SignInAllowlist; + /** Google Workspace domains that may sign in. Empty allows every account. */ + allowedDomains: string[]; authPath: string; baseURL: string; googleClientId?: string; @@ -52,7 +67,6 @@ export interface SignInAuthConfig { /** Dashboard options that also configure the sign-in session. */ export interface SignInAuthOptions { - allowedEmails?: string[]; allowedGoogleDomains?: string[]; authPath?: string; baseURL?: string; @@ -87,7 +101,7 @@ export function resolveSignInAuthConfig( ); return { agentName, - allowlist: resolveSignInAllowlist(options), + allowedDomains: resolveAllowedDomains(options), authPath, baseURL, sessionMaxAgeSeconds: options.sessionMaxAgeSeconds, @@ -114,11 +128,16 @@ function createBetterAuth(config: SignInAuthConfig) { ); const sessionMaxAgeSeconds = config.sessionMaxAgeSeconds ?? DEFAULT_SESSION_MAX_AGE_SECONDS; - // Google rejects other accounts when `hd` is set, so set it only when the - // allowlist is one domain. - const { domains, emails } = config.allowlist; + // `hd` limits the Google account chooser: to the one allowed domain, or to + // Workspace accounts when more than one domain is allowed. It is only a + // hint, so `getUserInfo` checks the `hd` claim. + const { allowedDomains } = config; const hostedDomain = - domains.length === 1 && emails.length === 0 ? domains[0] : undefined; + allowedDomains.length === 0 + ? undefined + : allowedDomains.length === 1 + ? allowedDomains[0] + : "*"; return betterAuth({ appName: config.agentName, baseURL: config.baseURL, @@ -137,12 +156,24 @@ function createBetterAuth(config: SignInAuthConfig) { ), hd: hostedDomain, prompt: "select_account", - mapProfileToUser(profile) { + // Replaces Better Auth's default, which can check only one `hd`. + // The ID token comes from Google's token endpoint over TLS, or was + // verified by `verifyIdToken` first. + async getUserInfo(token) { + if (!token.idToken) return null; + const parsed = googleClaimsSchema.safeParse(decodeJwt(token.idToken)); + if (!parsed.success) return null; + const claims = parsed.data; + if (!isAllowedDomain(claims.hd, allowedDomains)) return null; return { - email: profile.email, - emailVerified: profile.email_verified, - image: profile.picture, - name: profile.name, + user: { + id: claims.sub, + email: claims.email, + emailVerified: claims.email_verified === true, + image: claims.picture, + name: claims.name, + }, + data: claims, }; }, }, @@ -163,18 +194,13 @@ function createBetterAuth(config: SignInAuthConfig) { }, }, hooks: { - // Save each allowed Google sign-in, so Slack can check it later. + // Save the native Google identity of each sign-in. `getUserInfo` + // already rejected accounts outside the allowed domains. after: createAuthMiddleware(async (ctx) => { const created = ctx.context.newSession; if (!created || !ctx.path.startsWith("/callback/")) return; const email = normalizeIdentityEmail(created.user.email); - if ( - !created.user.emailVerified || - !email || - !isSignInAllowed(email, config.allowlist) - ) { - return; - } + if (!created.user.emailVerified || !email) return; const accounts = await ctx.context.internalAdapter.findAccounts( created.user.id, ); @@ -195,8 +221,8 @@ function createBetterAuth(config: SignInAuthConfig) { } /** - * Create the shared Google sign-in session. Better Auth starts on first use, - * so apps that never sign in need no Google credentials. + * Create the Google sign-in session. Better Auth starts on first use, so apps + * that never sign in need no Google credentials. */ export function createSignInAuth(config: SignInAuthConfig): SignInAuth { let auth: ReturnType | undefined; diff --git a/packages/junior/src/chat/sign-in/slack.ts b/packages/junior/src/chat/sign-in/slack.ts deleted file mode 100644 index 6de94e44a3..0000000000 --- a/packages/junior/src/chat/sign-in/slack.ts +++ /dev/null @@ -1,163 +0,0 @@ -/** - * Slack sign-in through the shared Google sign-in session. - * - * Slack sends a person here when they have no recent Google sign-in. The page - * uses the browser session that the dashboard also uses. When the person has - * no session, or an old one, it starts a Google sign-in and comes back here. - * The sign-in counts only when its Google email belongs to the user of the - * Slack identity that got the link. - */ -import { randomBytes } from "node:crypto"; -import type { StateAdapter } from "chat"; -import { z } from "zod"; -import { botConfig } from "@/chat/config"; -import { getSqlExecutor } from "@/chat/db"; -import { normalizeIdentityEmail } from "@/chat/identities/identity"; -import { - findSlackIdentityUserId, - slackUserHasGoogleSignIn, -} from "@/chat/identities/sql"; -import { findUserByEmailFromSql } from "@/chat/plugins/viewer"; -import { escapeXml } from "@/chat/xml"; -import { htmlCallbackResponse } from "@/handlers/oauth-html"; -import { isSignInAllowed, type SignInAllowlist } from "./allowlist"; -import type { SignInAuth } from "./auth"; - -export const SLACK_SIGN_IN_PATH = "/api/sign-in/slack"; - -/** Default time a Google sign-in lets a Slack user use Junior. */ -export const DEFAULT_SLACK_SIGN_IN_MAX_AGE_DAYS = 7; - -const SIGN_IN_STATE_TTL_MS = 10 * 60 * 1000; - -/** What Slack ingress needs to check and request sign-in. */ -export interface SlackSignIn { - baseURL: string; - maxAgeMs: number; -} - -const signInStateSchema = z.object({ - // Set after one Google sign-in, so a failed match cannot loop. - redirected: z.boolean(), - slackTeamId: z.string().min(1), - slackUserId: z.string().min(1), -}); - -function stateKey(state: string): string { - return `sign-in:v2:${state}`; -} - -function page(title: string, message: string, status: number): Response { - return htmlCallbackResponse(escapeXml(title), escapeXml(message), status); -} - -/** Store one Slack sign-in request and return its link. */ -export async function createSlackSignInUrl(args: { - baseURL: string; - slackTeamId: string; - slackUserId: string; - state: StateAdapter; -}): Promise { - const state = randomBytes(24).toString("base64url"); - await args.state.set( - stateKey(state), - { - redirected: false, - slackTeamId: args.slackTeamId, - slackUserId: args.slackUserId, - } satisfies z.input, - SIGN_IN_STATE_TTL_MS, - ); - const url = new URL(SLACK_SIGN_IN_PATH, args.baseURL); - url.searchParams.set("state", state); - return url.toString(); -} - -/** - * Finish a Slack sign-in when the browser has a recent Google sign-in for the - * user of the Slack identity. Otherwise start one Google sign-in. - */ -export async function handleSlackSignIn(args: { - allowlist: SignInAllowlist; - auth: SignInAuth; - request: Request; - signIn: SlackSignIn; - state: StateAdapter; -}): Promise { - const token = new URL(args.request.url).searchParams.get("state"); - const parsed = token - ? signInStateSchema.safeParse(await args.state.get(stateKey(token))) - : undefined; - if (!token || !parsed?.success) { - return page( - "Sign-in link expired", - `Mention ${botConfig.userName} in Slack again to get a new link.`, - 400, - ); - } - const stored = parsed.data; - const session = await args.auth.getSession(args.request); - const email = session?.user.emailVerified - ? normalizeIdentityEmail(session.user.email) - : undefined; - let failure: Response | undefined; - if (email && !isSignInAllowed(email, args.allowlist)) { - failure = page( - "Account not allowed", - `${email} cannot use ${botConfig.userName}. Sign in with your work account.`, - 403, - ); - } else if (email) { - const executor = getSqlExecutor(); - const userId = await findSlackIdentityUserId( - executor, - stored.slackTeamId, - stored.slackUserId, - ); - const user = await findUserByEmailFromSql(executor.db(), email); - if (!userId || user?.id !== userId) { - failure = page( - "Slack account does not match", - `Your Slack email is not ${email}. Sign in with the account that matches your Slack email.`, - 403, - ); - } else if ( - await slackUserHasGoogleSignIn(executor, { - signedInSince: new Date(Date.now() - args.signIn.maxAgeMs), - slackTeamId: stored.slackTeamId, - slackUserId: stored.slackUserId, - }) - ) { - await args.state.delete(stateKey(token)); - return page( - "You are signed in", - "Return to Slack and send your message again.", - 200, - ); - } - } - if (stored.redirected) { - return ( - failure ?? - page( - "Sign-in failed", - "Google sign-in did not finish. Sign in with your work account.", - 400, - ) - ); - } - // No session, another account, or an old sign-in: sign in with Google once. - await args.state.set( - stateKey(token), - { ...stored, redirected: true } satisfies z.input, - SIGN_IN_STATE_TTL_MS, - ); - // Come back to the public URL, not the URL this server received. - const returnUrl = new URL(SLACK_SIGN_IN_PATH, args.signIn.baseURL); - returnUrl.searchParams.set("state", token); - return await args.auth.signInWithGoogle( - args.request, - returnUrl.toString(), - returnUrl.toString(), - ); -} diff --git a/packages/junior/src/dashboard-routes.ts b/packages/junior/src/dashboard-routes.ts index 3b003b326b..87de4d1fce 100644 --- a/packages/junior/src/dashboard-routes.ts +++ b/packages/junior/src/dashboard-routes.ts @@ -16,9 +16,12 @@ export interface JuniorDashboardOptions { authPath?: string; /** Require a dashboard browser session before serving dashboard pages and APIs. */ authRequired?: boolean; - /** Exact Google account emails allowed to open the dashboard. */ - allowedEmails?: string[]; - /** Google Workspace domains allowed to open the dashboard. */ + /** + * Domains that may use Junior: Google Workspace domains for dashboard + * sign-in and Slack profile email domains for Slack. Recommended. The + * dashboard needs it when `authRequired` is on. Empty lets every Slack + * workspace member use Junior. + */ allowedGoogleDomains?: string[]; /** Browser route prefix for the dashboard shell. */ basePath?: string; diff --git a/packages/junior/tests/integration/google-sign-in.test.ts b/packages/junior/tests/integration/google-sign-in.test.ts new file mode 100644 index 0000000000..caee122828 --- /dev/null +++ b/packages/junior/tests/integration/google-sign-in.test.ts @@ -0,0 +1,107 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { http, HttpResponse } from "msw"; +import { closeDb } from "@/chat/db"; +import { createSignInAuth } from "@/chat/sign-in/auth"; +import { mswServer } from "../msw/server"; + +const BASE_URL = "https://junior.example.com"; + +function createAuth(allowedDomains: string[]) { + return createSignInAuth({ + agentName: "junior", + allowedDomains, + authPath: "/api/auth", + baseURL: BASE_URL, + googleClientId: "google-client", + googleClientSecret: "google-secret", + secret: "0123456789abcdef0123456789abcdef", + trustedOrigins: [], + }); +} + +// Fake Google: the token endpoint returns an ID token with these claims. +function useGoogleAccount(claims: { email: string; hd?: string }) { + mswServer.use( + http.post("https://oauth2.googleapis.com/token", () => { + const payload = Buffer.from( + JSON.stringify({ + aud: "google-client", + email_verified: true, + iss: "https://accounts.google.com", + sub: `google-${claims.email}`, + ...claims, + }), + ).toString("base64url"); + return HttpResponse.json({ id_token: `e30.${payload}.sig` }); + }), + ); +} + +function cookieHeader(response: Response): string { + return response.headers + .getSetCookie() + .map((cookie) => cookie.split(";")[0]) + .join("; "); +} + +async function startSignIn(auth: ReturnType) { + const started = await auth.signInWithGoogle( + new Request(`${BASE_URL}/auth/login`), + `${BASE_URL}/`, + ); + return { google: new URL(started.headers.get("location")!), started }; +} + +/** Start a Google sign-in and come back with a code, like a browser. */ +async function signIn(auth: ReturnType) { + const { google, started } = await startSignIn(auth); + const callback = new URL(`${BASE_URL}/api/auth/callback/google`); + callback.searchParams.set("state", google.searchParams.get("state")!); + callback.searchParams.set("code", "google-code"); + const response = await auth.handler( + new Request(callback, { headers: { cookie: cookieHeader(started) } }), + ); + return { + location: response.headers.get("location") ?? "", + session: new Request(`${BASE_URL}/`, { + headers: { cookie: cookieHeader(response) }, + }), + }; +} + +describe("Google sign-in", () => { + afterEach(async () => { + await closeDb(); + }); + + it("accepts only Google Workspace accounts on the allowed domains", async () => { + // One domain limits the account chooser to it. More domains limit it to + // Workspace accounts. + expect( + (await startSignIn(createAuth(["example.com"]))).google.searchParams.get( + "hd", + ), + ).toBe("example.com"); + const auth = createAuth(["example.com", "example.org"]); + expect((await startSignIn(auth)).google.searchParams.get("hd")).toBe("*"); + + // `hd` decides, not the email: a personal Google account can use any + // email address, and only Workspace accounts have `hd`. + for (const claims of [ + { email: "person@example.com" }, + { email: "person@example.com", hd: "contractor.dev" }, + ]) { + useGoogleAccount(claims); + const rejected = await signIn(auth); + expect(rejected.location).toContain("error=unable_to_get_user_info"); + expect(await auth.getSession(rejected.session)).toBeNull(); + } + + useGoogleAccount({ email: "person@example.org", hd: "example.org" }); + const accepted = await signIn(auth); + expect(accepted.location).toBe(`${BASE_URL}/`); + expect(await auth.getSession(accepted.session)).toMatchObject({ + user: { email: "person@example.org", emailVerified: true }, + }); + }); +}); diff --git a/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts b/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts index d0ddc863e5..7e16321384 100644 --- a/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts +++ b/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts @@ -21,7 +21,7 @@ import { createSlackWebhookTestClient } from "../../fixtures/slack/webhook-clien import { SUITE_EXPERIMENTAL } from "../../fixtures/experimental-setup"; import { createTestMessage } from "../../fixtures/slack-harness"; import { slackApiOutbox } from "../../fixtures/slack-api-outbox"; -import { usersInfoOk } from "../../fixtures/slack/factories/api"; +import { authTestOk, usersInfoOk } from "../../fixtures/slack/factories/api"; import { slackEventsApiEnvelope } from "../../fixtures/slack/factories/events"; import { getCapturedSlackApiCalls, @@ -261,194 +261,57 @@ describe("Slack webhook auth boundary", () => { await state.disconnect(); }); - it("asks Slack members for a recent Google sign-in shared with the dashboard", async () => { - vi.stubEnv("GOOGLE_CLIENT_ID", "google-client"); - vi.stubEnv("GOOGLE_CLIENT_SECRET", "google-secret"); - const baseURL = "https://junior.example.com"; + it("lets only Slack authors on the allowed domains start a Turn", async () => { const queue = createConversationWorkQueueTestAdapter(); - // The production Slack adapter uses SLACK_SIGNING_SECRET from test setup. const slack = createSlackWebhookTestClient({ signingSecret: SIGNING_SECRET, }); const waitUntil = slack.waitUntil(); const app = await createApp({ conversationWorkQueue: () => queue, - // No dashboard app, so core serves the Better Auth routes itself. dashboard: { - // Two domains, so Google does not limit accounts with `hd`. allowedGoogleDomains: ["example.com", "example.org"], - baseURL, disabled: true, }, - experimental: { ...SUITE_EXPERIMENTAL, "slack-sign-in": true }, + experimental: SUITE_EXPERIMENTAL, waitUntil: waitUntil.fn, }); await getStateAdapter().connect(); - // Fake Google. Each visit signs in as `googleEmail`, or the person - // cancels when it is empty. - let googleEmail = ""; - let googleVisits = 0; + // Membership and the domain check can both look up the author. + const emails: Record = { + U123: "testuser@example.com", + UCONTRACTOR: "someone@contractor.dev", + }; mswServer.use( - http.post("https://oauth2.googleapis.com/token", () => { - const claims = { - aud: "google-client", - email: googleEmail, - email_verified: true, - hd: googleEmail.split("@")[1], - iss: "https://accounts.google.com", - sub: `google-${googleEmail}`, - }; - const payload = Buffer.from(JSON.stringify(claims)).toString( - "base64url", + http.post("https://slack.com/api/users.info", async ({ request }) => { + const user = new URLSearchParams(await request.text()).get("user")!; + return HttpResponse.json( + usersInfoOk({ userId: user, email: emails[user] }), ); - return HttpResponse.json({ id_token: `e30.${payload}.sig` }); }), ); let sequence = 1; - const send = async (fields: Record = {}) => { + const mention = async (user: string) => { const envelope = slackEventsApiEnvelope({ - eventType: "message", - channel: "D123", - user: "U123", + eventType: "app_mention", + channel: "C123", + user, ts: `1712345.${String(sequence++).padStart(6, "0")}`, - threadTs: "1712345.000001", - text: "hello", + text: `<@${authTestOk().user_id}> hello`, }); - Object.assign(envelope.event, fields); - const response = await app.fetch(slack.event(envelope)); + expect((await app.fetch(slack.event(envelope))).status).toBe(200); await waitUntil.flush(); - return response; - }; - const sendBlocked = async () => { - const prompts = getCapturedSlackApiCalls("chat.postMessage").length; - expect((await send()).status).toBe(200); - const calls = getCapturedSlackApiCalls("chat.postMessage"); - expect(calls).toHaveLength(prompts + 1); - expect(calls.at(-1)?.params).toMatchObject({ channel: "D123" }); - return /<([^|>]+)\|/ - .exec(String(calls.at(-1)?.params.text))![1]! - .replaceAll("&", "&"); - }; - // A browser with one cookie jar. It follows redirects through the app - // and the fake Google consent page. - const createBrowser = () => { - const cookies = new Map(); - const fetchApp = async (url: URL, init: RequestInit = {}) => { - if (url.origin === "https://accounts.google.com") { - googleVisits += 1; - const back = new URL(url.searchParams.get("redirect_uri")!); - back.searchParams.set("state", url.searchParams.get("state")!); - if (googleEmail) back.searchParams.set("code", "google-code"); - else back.searchParams.set("error", "access_denied"); - return Response.redirect(back, 302); - } - const headers = new Headers(init.headers); - headers.set( - "cookie", - [...cookies].map(([k, v]) => `${k}=${v}`).join("; "), - ); - const response = await app.fetch( - new Request(url, { ...init, headers }), - ); - for (const cookie of response.headers.getSetCookie()) { - const [pair] = cookie.split(";"); - const name = pair!.slice(0, pair!.indexOf("=")); - const value = pair!.slice(pair!.indexOf("=") + 1); - if (value && !/max-age=0/i.test(cookie)) cookies.set(name, value); - else cookies.delete(name); - } - return response; - }; - const follow = async (response: Response, from: URL) => { - for (;;) { - const location = response.headers.get("location"); - if (!location) return response; - from = new URL(location, from); - response = await fetchApp(from); - } - }; - const visit = async (start: string, email: string) => { - googleEmail = email; - return await follow(await fetchApp(new URL(start)), new URL(start)); - }; - // Like the dashboard client: start Google sign-in with Better Auth. - const signInFromDashboard = async (email: string) => { - googleEmail = email; - const url = new URL("/api/auth/sign-in/social", baseURL); - const started = await fetchApp(url, { - body: JSON.stringify({ - callbackURL: `${baseURL}/`, - provider: "google", - }), - headers: { "content-type": "application/json", origin: baseURL }, - method: "POST", - }); - const { url: google } = (await started.json()) as { url: string }; - return await follow(await fetchApp(new URL(google)), new URL(google)); - }; - const session = async () => - await ( - await fetchApp(new URL("/api/auth/get-session", baseURL)) - ).json(); - return { cookies, session, signInFromDashboard, visit }; }; - // Slack Connect authors never get a sign-in link or a Google redirect. - // Membership is checked first, so Slack is not even asked for a profile. - expect( - (await send({ user: "UEXTERNAL", user_team: "TEXTERNAL" })).status, - ).toBe(200); - expect(queue.queuedMessages()).toEqual([]); - expect(getCapturedSlackApiCalls("users.info")).toEqual([]); - expect(slackApiOutbox.messages()).toEqual([]); - expect(getCapturedSlackApiCalls("chat.postEphemeral")).toEqual([]); - expect(googleVisits).toBe(0); - - // A dashboard sign-in before the first Slack message counts for Slack. - const browser = createBrowser(); - await browser.signInFromDashboard("testuser@example.com"); - expect(await browser.session()).toMatchObject({ - user: { email: "testuser@example.com" }, - }); - expect(googleVisits).toBe(1); - // The session cookie never carries the Google account or its tokens. - expect([...browser.cookies.keys()].join()).not.toContain("account"); - expect((await send()).status).toBe(200); + // Slack verifies the profile email, so its domain decides. + await mention("U123"); expect(queue.queuedMessages()).toHaveLength(1); - expect(getCapturedSlackApiCalls("chat.postMessage")).toEqual([]); - - // After the max age, Slack asks again. - vi.useFakeTimers({ toFake: ["Date"] }); - vi.setSystemTime(Date.now() + 8 * 24 * 60 * 60 * 1000); - // Accounts outside the allowlist, another person's account, or a - // cancelled Google sign-in fail. - for (const [email, status] of [ - ["testuser@contractor.dev", 403], - ["someone@example.com", 403], - ["", 400], - ] as const) { - expect( - (await createBrowser().visit(await sendBlocked(), email)).status, - ).toBe(status); - } + // Junior drops the message and does not reply. + await mention("UCONTRACTOR"); expect(queue.queuedMessages()).toHaveLength(1); - - // The Slack page signs in the matching account. - const link = await sendBlocked(); - const visits = googleVisits; - expect((await browser.visit(link, "testuser@example.com")).status).toBe( - 200, - ); - expect(googleVisits).toBe(visits + 1); - expect((await send()).status).toBe(200); - expect(queue.queuedMessages()).toHaveLength(2); - - // The dashboard uses the session from the Slack page. - expect(await browser.session()).toMatchObject({ - user: { email: "testuser@example.com" }, - }); - expect(googleVisits).toBe(visits + 1); + expect(getCapturedSlackApiCalls("chat.postEphemeral")).toEqual([]); + expect(slackApiOutbox.messages()).toEqual([]); }); it.each(["message", "factory"] as const)( diff --git a/packages/junior/tests/unit/chat/sign-in-auth.test.ts b/packages/junior/tests/unit/chat/sign-in-auth.test.ts index 0da9d70991..81d12f93aa 100644 --- a/packages/junior/tests/unit/chat/sign-in-auth.test.ts +++ b/packages/junior/tests/unit/chat/sign-in-auth.test.ts @@ -1,65 +1,27 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { resolveSignInAllowlist } from "@/chat/sign-in/allowlist"; -import { createSignInAuth } from "@/chat/sign-in/auth"; +import { resolveAllowedDomains } from "@/chat/sign-in/allowlist"; afterEach(() => { vi.unstubAllEnvs(); }); -describe("sign-in allowlist", () => { - it("reads dashboard env lists as JSON arrays or comma separated values", () => { +describe("allowed domains", () => { + it("reads the dashboard env list as a JSON array or comma separated values", () => { vi.stubEnv( "JUNIOR_DASHBOARD_GOOGLE_DOMAINS", '["Sentry.io", "example.com"]', ); - vi.stubEnv( - "JUNIOR_DASHBOARD_ALLOWED_EMAILS", - "a@example.net, b@example.net", - ); + expect(resolveAllowedDomains()).toEqual(["sentry.io", "example.com"]); - expect(resolveSignInAllowlist()).toEqual({ - domains: ["sentry.io", "example.com"], - emails: ["a@example.net", "b@example.net"], - }); + vi.stubEnv("JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "sentry.io, example.com"); + expect(resolveAllowedDomains()).toEqual(["sentry.io", "example.com"]); }); it("rejects an env list that is not a JSON string array", () => { vi.stubEnv("JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "[1]"); - expect(() => resolveSignInAllowlist()).toThrow( + expect(() => resolveAllowedDomains()).toThrow( "JUNIOR_DASHBOARD_GOOGLE_DOMAINS must be a JSON string array", ); }); }); - -describe("sign-in session", () => { - it("limits the Google account chooser only when the allowlist is one domain", async () => { - const googleUrl = async (domains: string[], emails: string[]) => { - const auth = createSignInAuth({ - agentName: "junior", - allowlist: { domains, emails }, - authPath: "/api/auth", - baseURL: "https://junior.example.com", - googleClientId: "google-client", - googleClientSecret: "google-secret", - secret: "0123456789abcdef0123456789abcdef", - trustedOrigins: [], - }); - const response = await auth.signInWithGoogle( - new Request("https://junior.example.com/auth/login"), - "https://junior.example.com/", - ); - expect(response.headers.get("set-cookie")).toContain("oauth_state"); - return new URL(response.headers.get("location")!); - }; - - expect((await googleUrl(["example.com"], [])).searchParams.get("hd")).toBe( - "example.com", - ); - expect( - ( - await googleUrl(["example.com"], ["person@partner.dev"]) - ).searchParams.has("hd"), - ).toBe(false); - }); -}); From 003cafb4592c3d59a13efda0333fc88f2ceca91c Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:03:58 +0000 Subject: [PATCH 2/5] ref(auth): Make allowedDomains a top-level app setting Domain allowlisting gates Slack and the dashboard, so it no longer lives under dashboard or carries a Google-specific name. Configure it with createApp({ allowedDomains }) or JUNIOR_ALLOWED_DOMAINS. Startup fails on the removed dashboard.allowedGoogleDomains and JUNIOR_DASHBOARD_GOOGLE_DOMAINS so a stale config cannot silently allow every Slack workspace member. Co-Authored-By: David Cramer --- apps/example/nitro.config.ts | 1 - apps/example/server.ts | 2 +- .../src/content/docs/operate/dashboard.md | 10 +-- .../content/docs/reference/config-and-env.md | 29 ++++++--- packages/junior-dashboard/e2e/harness.ts | 2 +- packages/junior-dashboard/src/api/schema.ts | 2 +- packages/junior-dashboard/src/app.ts | 43 ++---------- .../junior-dashboard/tests/client-api.test.ts | 2 +- .../tests/dashboard-mock-routes.test.ts | 6 +- .../tests/dashboard-output.test.ts | 4 +- .../tests/dashboard-plugin-routes.test.ts | 14 ++-- .../tests/dashboard-routes.test.ts | 45 +++++-------- .../tests/dashboard-shell-routes.test.ts | 4 +- .../tests/dashboard-test-helpers.ts | 1 - .../tests/telemetry-components.test.tsx | 2 +- packages/junior/src/app.ts | 25 +++++-- packages/junior/src/chat/sign-in/allowlist.ts | 40 ++++++++---- packages/junior/src/chat/sign-in/auth.ts | 10 +-- packages/junior/src/dashboard-routes.ts | 13 ++-- .../slack/webhook-auth-boundary.test.ts | 5 +- .../tests/unit/app-allowed-domains.test.ts | 65 +++++++++++++++++++ packages/junior/tests/unit/app-config.test.ts | 3 - .../unit/build/nitro-plugin-module.test.ts | 2 +- .../tests/unit/build/virtual-config.test.ts | 6 +- .../tests/unit/chat/sign-in-auth.test.ts | 34 +++++++--- .../junior/tests/unit/cli/init-cli.test.ts | 1 + 26 files changed, 218 insertions(+), 153 deletions(-) create mode 100644 packages/junior/tests/unit/app-allowed-domains.test.ts diff --git a/apps/example/nitro.config.ts b/apps/example/nitro.config.ts index 0be4225755..a5e8bc44db 100644 --- a/apps/example/nitro.config.ts +++ b/apps/example/nitro.config.ts @@ -13,7 +13,6 @@ const config = defineConfig({ juniorNitro({ dashboard: { authRequired: exampleDashboardAuthRequired(), - allowedGoogleDomains: ["sentry.io"], componentGallery: exampleDashboardComponentGallery(), mockConversations: exampleDashboardMockConversations(), }, diff --git a/apps/example/server.ts b/apps/example/server.ts index a733d1f533..f7badd31eb 100644 --- a/apps/example/server.ts +++ b/apps/example/server.ts @@ -17,10 +17,10 @@ const [ ]); const app = await createApp({ + allowedDomains: ["sentry.io"], briefs: { enabled: true }, dashboard: { authRequired: exampleDashboardAuthRequired(), - allowedGoogleDomains: ["sentry.io"], componentGallery: exampleDashboardComponentGallery(), mockConversations: exampleDashboardMockConversations(), }, diff --git a/packages/docs/src/content/docs/operate/dashboard.md b/packages/docs/src/content/docs/operate/dashboard.md index 9f78fdfe98..5942c15def 100644 --- a/packages/docs/src/content/docs/operate/dashboard.md +++ b/packages/docs/src/content/docs/operate/dashboard.md @@ -25,16 +25,17 @@ pnpm add @sentry/junior-dashboard ## Configure the dashboard -Pass `dashboard` to `createApp()`. Configure the Google Workspace domain that -should be allowed to view the dashboard: +Pass `dashboard` to `createApp()`. Dashboard sign-in requires at least one +[allowed domain](/reference/config-and-env/#allowed-domains). The same list +also gates who can use Junior in Slack: ```ts title="server.ts" import { createApp } from "@sentry/junior"; import { plugins } from "./plugins"; export default await createApp({ + allowedDomains: ["sentry.io"], dashboard: { - allowedGoogleDomains: ["sentry.io"], trustedOrigins: ["https://"], }, plugins, @@ -52,7 +53,6 @@ export default defineConfig({ modules: [ juniorNitro({ dashboard: { - allowedGoogleDomains: ["sentry.io"], trustedOrigins: ["https://"], }, plugins: "./plugins", @@ -68,7 +68,7 @@ You can also provide the same authorization policy through deployment environmen | Variable | Purpose | | ---------------------------------- | ------------------------------------------------------------- | -| `JUNIOR_DASHBOARD_GOOGLE_DOMAINS` | Comma-separated or JSON array of allowed Google domains. | +| `JUNIOR_ALLOWED_DOMAINS` | Comma-separated or JSON array of allowed domains. | | `JUNIOR_DASHBOARD_TRUSTED_ORIGINS` | Comma-separated or JSON array of Better Auth trusted origins. | | `JUNIOR_DASHBOARD_AUTH_REQUIRED` | Set to `false` only for explicit local dashboard auth bypass. | diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index cafb73f602..28cee9a61e 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -90,13 +90,12 @@ If you mount `@sentry/junior-dashboard`, set these browser-auth variables: | `GOOGLE_CLIENT_SECRET` | Yes | Google OAuth client secret. | | `BETTER_AUTH_SECRET` | No | Optional override for dashboard cookies. Defaults to `JUNIOR_SECRET`. | -Configure allowed Google Workspace domains in `createApp({ dashboard })` and `juniorNitro({ dashboard })` for normal deployments. Set these optional policy variables when you prefer environment-managed dashboard authorization: +Dashboard auth requires at least one [allowed domain](#allowed-domains). Set these optional policy variables when you prefer environment-managed dashboard authorization: -| Variable | Required | Purpose | -| ------------------------------------- | -------- | ------------------------------------------------------------------------------------------ | -| `JUNIOR_DASHBOARD_GOOGLE_DOMAINS` | No | Comma-separated or JSON array of allowed domains. See [Allowed domains](#allowed-domains). | -| `JUNIOR_DASHBOARD_TRUSTED_ORIGINS` | No | Comma-separated or JSON array of Better Auth trusted origins. | -| `JUNIOR_DASHBOARD_MOCK_CONVERSATIONS` | No | Set to `true` to replace conversation API responses with local/demo visual-QA fixtures. | +| Variable | Required | Purpose | +| ------------------------------------- | -------- | --------------------------------------------------------------------------------------- | +| `JUNIOR_DASHBOARD_TRUSTED_ORIGINS` | No | Comma-separated or JSON array of Better Auth trusted origins. | +| `JUNIOR_DASHBOARD_MOCK_CONVERSATIONS` | No | Set to `true` to replace conversation API responses with local/demo visual-QA fixtures. | For local/demo dashboard visual QA, set `JUNIOR_DASHBOARD_MOCK_CONVERSATIONS=true` to replace conversation API responses with sample fixtures. @@ -212,9 +211,17 @@ already joined. Leave it unset in production unless you are testing that path. ## Allowed domains -`dashboard.allowedGoogleDomains` (or `JUNIOR_DASHBOARD_GOOGLE_DOMAINS`) is one -list of domains for Slack and the dashboard. It is optional, but we recommend -it. Junior logs a warning at startup when it is empty. +`createApp({ allowedDomains })` is one list of domains for Slack and the +dashboard. When the option is unset, Junior reads `JUNIOR_ALLOWED_DOMAINS`, a +comma-separated or JSON array of domains. It is optional, but we recommend it. +Junior logs a warning at startup when it is empty. + +```ts title="server.ts" +export default await createApp({ + allowedDomains: ["sentry.io"], + plugins, +}); +``` - **Slack:** Junior starts work from a mention or DM only when the email of the author's Slack profile is on an allowed domain. Slack verifies that email. @@ -228,6 +235,10 @@ it. Junior logs a warning at startup when it is empty. When the list is empty, every member of the Slack workspace can use Junior. +The old `dashboard.allowedGoogleDomains` option and +`JUNIOR_DASHBOARD_GOOGLE_DOMAINS` variable were removed. Junior fails at +startup when either is set, so a stale config cannot silently allow everyone. + `operator-tools` adds `runOperatorSql` to non-public Conversations. The tool runs one SQL statement against the app database and returns up to 200 rows. Each call uses its own database connection, which is closed afterwards, so diff --git a/packages/junior-dashboard/e2e/harness.ts b/packages/junior-dashboard/e2e/harness.ts index 9e8f4aba3a..ae0e16b5db 100644 --- a/packages/junior-dashboard/e2e/harness.ts +++ b/packages/junior-dashboard/e2e/harness.ts @@ -69,7 +69,7 @@ export async function startDashboardE2eServer( process.env.DATABASE_URL ??= "postgres://localhost/junior-dashboard-e2e"; const { createDashboardApp } = await import("../dist/app.js"); const app = createDashboardApp({ - allowedGoogleDomains: ["example.com"], + allowedDomains: ["example.com"], auth: { async getSession() { return { diff --git a/packages/junior-dashboard/src/api/schema.ts b/packages/junior-dashboard/src/api/schema.ts index 147edb4226..6d913cc76f 100644 --- a/packages/junior-dashboard/src/api/schema.ts +++ b/packages/junior-dashboard/src/api/schema.ts @@ -33,7 +33,7 @@ export const dashboardProfileUpdateSchema = z export const dashboardConfigSchema = z .object({ - allowedGoogleDomainCount: z.number(), + allowedDomainCount: z.number(), authRequired: z.boolean(), authPath: z.string(), basePath: z.string(), diff --git a/packages/junior-dashboard/src/app.ts b/packages/junior-dashboard/src/app.ts index 627b5153cc..1f6a6061ab 100644 --- a/packages/junior-dashboard/src/app.ts +++ b/packages/junior-dashboard/src/app.ts @@ -78,13 +78,14 @@ export interface JuniorDashboardOptions { baseURL?: string; authPath?: string; authRequired?: boolean; - allowedGoogleDomains?: string[]; auth?: DashboardAuth; componentGallery?: boolean; mockConversations?: boolean; } interface DashboardRuntimeOptions extends JuniorDashboardOptions { + /** The app's `allowedDomains`, from core. Auth requires at least one. */ + allowedDomains?: string[]; authenticatedRoutes?: readonly AuthenticatedRoute[]; /** The queue `createApp()` uses, so web input reaches the same worker. */ conversationWorkQueue?: ConversationWorkQueue; @@ -112,37 +113,6 @@ function normalizeValues(values: string[] | undefined): string[] { ]; } -/** Read dashboard list env vars as comma-separated strings or JSON arrays. */ -function readEnvList(name: string): string[] | undefined { - const value = process.env[name]; - if (!value?.trim()) { - return undefined; - } - - if (value.trim().startsWith("[")) { - let parsed: unknown; - try { - parsed = JSON.parse(value); - } catch (error) { - throw new Error(`${name} must be a JSON string array`, { - cause: error, - }); - } - if ( - !Array.isArray(parsed) || - parsed.some((item) => typeof item !== "string") - ) { - throw new Error(`${name} must be a JSON string array`); - } - return parsed; - } - - return value - .split(",") - .map((item) => item.trim()) - .filter(Boolean); -} - /** Read dashboard boolean env vars; only explicit true/false values apply. */ function readEnvFlag(name: string): boolean | undefined { const value = process.env[name]?.trim(); @@ -159,9 +129,6 @@ function resolveDashboardOptions( ...options, authRequired: options.authRequired ?? readEnvFlag("JUNIOR_DASHBOARD_AUTH_REQUIRED"), - allowedGoogleDomains: - options.allowedGoogleDomains ?? - readEnvList("JUNIOR_DASHBOARD_GOOGLE_DOMAINS"), componentGallery: options.componentGallery ?? readEnvFlag("JUNIOR_DASHBOARD_COMPONENT_GALLERY"), @@ -401,7 +368,7 @@ export function createDashboardApp( options.authPath ?? DEFAULT_AUTH_PATH, DEFAULT_AUTH_PATH, ); - const allowedDomains = normalizeValues(options.allowedGoogleDomains); + const allowedDomains = normalizeValues(options.allowedDomains); const agentName = options.agentName?.trim() || "Junior"; const authRequired = options.authRequired !== false; @@ -412,7 +379,7 @@ export function createDashboardApp( } if (authRequired && allowedDomains.length === 0) { - throw new Error("Junior dashboard auth requires allowedGoogleDomains"); + throw new Error("Junior dashboard auth requires allowedDomains"); } // Core owns the Google sign-in session. @@ -608,7 +575,7 @@ export function createDashboardApp( ); app.get("/api/config", () => { return jsonResponse(dashboardConfigSchema, { - allowedGoogleDomainCount: allowedDomains.length, + allowedDomainCount: allowedDomains.length, authRequired, authPath, basePath, diff --git a/packages/junior-dashboard/tests/client-api.test.ts b/packages/junior-dashboard/tests/client-api.test.ts index 2f9505c965..890750b0b5 100644 --- a/packages/junior-dashboard/tests/client-api.test.ts +++ b/packages/junior-dashboard/tests/client-api.test.ts @@ -117,7 +117,7 @@ describe("dashboard client API", () => { user: { email: "dev@example.com", emailVerified: true }, }); const app = createDashboardApp({ - allowedGoogleDomains: ["example.com"], + allowedDomains: ["example.com"], mockConversations: true, auth: { ...sessionAuth, diff --git a/packages/junior-dashboard/tests/dashboard-mock-routes.test.ts b/packages/junior-dashboard/tests/dashboard-mock-routes.test.ts index 61c50df834..7846f437d0 100644 --- a/packages/junior-dashboard/tests/dashboard-mock-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-mock-routes.test.ts @@ -72,7 +72,7 @@ describe("dashboard canonical-event mock routes", () => { it("serves canonical detail, directory, and aggregate reports", async () => { const app = createDashboardApp({ authRequired: false, - allowedGoogleDomains: [], + allowedDomains: [], mockConversations: true, }); @@ -257,7 +257,7 @@ describe("dashboard canonical-event mock routes", () => { it("serves direct canonical event fixtures for every dashboard state", async () => { const app = createDashboardApp({ authRequired: false, - allowedGoogleDomains: [], + allowedDomains: [], mockConversations: true, }); @@ -409,7 +409,7 @@ describe("dashboard canonical-event mock routes", () => { it("loads canonical child conversations through the ordinary detail route", async () => { const app = createDashboardApp({ authRequired: false, - allowedGoogleDomains: [], + allowedDomains: [], mockConversations: true, }); const parentResponse = await app.fetch( diff --git a/packages/junior-dashboard/tests/dashboard-output.test.ts b/packages/junior-dashboard/tests/dashboard-output.test.ts index 153b791853..35e57c8487 100644 --- a/packages/junior-dashboard/tests/dashboard-output.test.ts +++ b/packages/junior-dashboard/tests/dashboard-output.test.ts @@ -39,7 +39,7 @@ export default defineConfig({ juniorNitro({ dashboard: { authRequired: false, - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], }, }), ], @@ -56,7 +56,7 @@ export default defineConfig({ export default await createApp({ dashboard: { authRequired: false, - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], }, plugins: defineJuniorPlugins([]), }); diff --git a/packages/junior-dashboard/tests/dashboard-plugin-routes.test.ts b/packages/junior-dashboard/tests/dashboard-plugin-routes.test.ts index 401e0effe3..c43bcadaeb 100644 --- a/packages/junior-dashboard/tests/dashboard-plugin-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-plugin-routes.test.ts @@ -51,9 +51,9 @@ describe("dashboard plugin routes", () => { it("mounts dashboard routes through core app config", async () => { mockDashboardVirtualConfig(); const app = await createApp({ + allowedDomains: ["sentry.io"], dashboard: { authRequired: false, - allowedGoogleDomains: ["sentry.io"], }, plugins: defineJuniorPlugins([]), }); @@ -89,9 +89,9 @@ describe("dashboard plugin routes", () => { mockDashboardVirtualConfig(); const pluginApp = memoryRoutes(); const app = await createApp({ + allowedDomains: ["sentry.io"], dashboard: { authRequired: false, - allowedGoogleDomains: ["sentry.io"], }, plugins: defineJuniorPlugins([ defineJuniorPlugin({ @@ -125,7 +125,7 @@ describe("dashboard plugin routes", () => { it("protects plugin API route apps with dashboard auth", async () => { const pluginApp = memoryRoutes(); const unauthenticated = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null), pluginRoutes: [{ app: pluginApp, pluginName: "memory" }], }); @@ -138,7 +138,7 @@ describe("dashboard plugin routes", () => { }); const authenticated = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth({ user: { email: "person@sentry.io", @@ -166,7 +166,7 @@ describe("dashboard plugin routes", () => { return c.body(null, 204); }); const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null), pluginRoutes: [{ app: pluginApp, pluginName: "memory" }], }); @@ -185,7 +185,7 @@ describe("dashboard plugin routes", () => { it("passes sanitized auth context to plugin API route apps", async () => { let pluginContext: unknown; const authenticated = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth({ user: { email: "person@sentry.io", @@ -246,7 +246,7 @@ describe("dashboard plugin routes", () => { ); const app = createDashboardApp({ authRequired: false, - allowedGoogleDomains: [], + allowedDomains: [], pluginRoutes: [{ app: pluginApp, pluginName: "viewer" }], }); const response = await app.fetch( diff --git a/packages/junior-dashboard/tests/dashboard-routes.test.ts b/packages/junior-dashboard/tests/dashboard-routes.test.ts index 1087d7a7fa..81eb9d988c 100644 --- a/packages/junior-dashboard/tests/dashboard-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-routes.test.ts @@ -26,7 +26,7 @@ vi.mock("@sentry/junior/api", async (importOriginal) => ({ function dashboard(session: DashboardSession | null) { return createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(session), }); } @@ -91,7 +91,7 @@ describe("dashboard routes", () => { it("uses the requested dashboard path as the Google sign-in callback", async () => { let callbackURL: string | undefined; const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -123,7 +123,7 @@ describe("dashboard routes", () => { ]; const unauthenticatedApp = createDashboardApp({ authenticatedRoutes, - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -142,7 +142,7 @@ describe("dashboard routes", () => { const authenticatedApp = createDashboardApp({ authenticatedRoutes, - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth({ user: { email: "person@sentry.io", @@ -168,7 +168,7 @@ describe("dashboard routes", () => { path: "/_junior/acp/auth/:transactionId", }, ], - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth({ user: { email: "person@example.com", @@ -190,7 +190,7 @@ describe("dashboard routes", () => { process.env.JUNIOR_BASE_URL = "https://junior.example.com"; let callbackURL: string | undefined; const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -233,7 +233,7 @@ describe("dashboard routes", () => { it("preserves non-root dashboard base paths through Google sign-in", async () => { let callbackURL: string | undefined; const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -259,7 +259,7 @@ describe("dashboard routes", () => { it("starts sign-in when the auth prefix overlaps the login route", async () => { const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null), authPath: "/auth", }); @@ -277,7 +277,7 @@ describe("dashboard routes", () => { it("falls back to the dashboard root for unsafe login return paths", async () => { let callbackURL: string | undefined; const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -296,7 +296,7 @@ describe("dashboard routes", () => { it("does not restart Google sign-in for an already authorized session", async () => { let startedGoogleSignIn = false; const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth( { user: { @@ -326,7 +326,7 @@ describe("dashboard routes", () => { it("can explicitly disable dashboard auth for local development", async () => { const app = createDashboardApp({ authRequired: false, - allowedGoogleDomains: [], + allowedDomains: [], }); const page = await app.fetch(new Request("http://localhost/")); @@ -492,7 +492,7 @@ describe("dashboard routes", () => { it("keeps mock profile edits process-local", async () => { const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth({ user: { email: "person@sentry.io", @@ -560,7 +560,7 @@ describe("dashboard routes", () => { it("preserves the enabled component gallery path through login", async () => { let callbackURL: string | undefined; const app = createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -597,7 +597,7 @@ describe("dashboard routes", () => { expect(response.status).toBe(200); expect(response.headers.get(DASHBOARD_VERSION_HEADER)).toBe(JUNIOR_VERSION); expect(await response.json()).toEqual({ - allowedGoogleDomainCount: 1, + allowedDomainCount: 1, authRequired: true, authPath: "/api/auth", basePath: "/", @@ -638,12 +638,11 @@ describe("dashboard routes", () => { expect(response.status).toBe(403); }); - it("resolves auth policy from env when dashboard options omit allowlists", async () => { - process.env.JUNIOR_DASHBOARD_GOOGLE_DOMAINS = "sentry.io, example.com"; - process.env.JUNIOR_DASHBOARD_TRUSTED_ORIGINS = "https://junior.example.com"; + it("reports the allowed domains from core", async () => { process.env.JUNIOR_DASHBOARD_MOCK_CONVERSATIONS = "true"; const app = createDashboardApp({ + allowedDomains: ["sentry.io", "example.com"], auth: auth({ user: { email: "person@sentry.io", @@ -658,18 +657,8 @@ describe("dashboard routes", () => { expect(response.status).toBe(200); expect(await response.json()).toMatchObject({ - allowedGoogleDomainCount: 2, + allowedDomainCount: 2, authRequired: true, }); }); - - it("fails clearly when list env JSON is malformed", async () => { - process.env.JUNIOR_DASHBOARD_GOOGLE_DOMAINS = '["sentry.io"'; - - expect(() => - createDashboardApp({ - authRequired: false, - }), - ).toThrow("JUNIOR_DASHBOARD_GOOGLE_DOMAINS must be a JSON string array"); - }); }); diff --git a/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts b/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts index 971efecc77..83dd33e1b8 100644 --- a/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts @@ -17,7 +17,7 @@ vi.mock("@sentry/junior/api", async (importOriginal) => ({ function dashboard(session: DashboardSession | null) { return createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(session), }); } @@ -224,7 +224,7 @@ describe("dashboard shell routes", () => { it("serves the installable shell manifest without auth", async () => { const app = createDashboardApp({ agentName: "Marky", - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null), basePath: "/ops", }); diff --git a/packages/junior-dashboard/tests/dashboard-test-helpers.ts b/packages/junior-dashboard/tests/dashboard-test-helpers.ts index 1166be8205..ed608a16a6 100644 --- a/packages/junior-dashboard/tests/dashboard-test-helpers.ts +++ b/packages/junior-dashboard/tests/dashboard-test-helpers.ts @@ -11,7 +11,6 @@ export const dashboardEnvNames = [ "VERCEL_URL", "JUNIOR_DASHBOARD_AUTH_REQUIRED", "JUNIOR_DASHBOARD_COMPONENT_GALLERY", - "JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "JUNIOR_DASHBOARD_TRUSTED_ORIGINS", "JUNIOR_DASHBOARD_MOCK_CONVERSATIONS", "SENTRY_DSN", diff --git a/packages/junior-dashboard/tests/telemetry-components.test.tsx b/packages/junior-dashboard/tests/telemetry-components.test.tsx index 94a8db857a..d4c44d4022 100644 --- a/packages/junior-dashboard/tests/telemetry-components.test.tsx +++ b/packages/junior-dashboard/tests/telemetry-components.test.tsx @@ -91,7 +91,7 @@ function renderTranscript(detail: ConversationTranscript): string { function systemData(): SystemData { return { config: { - allowedGoogleDomainCount: 0, + allowedDomainCount: 0, authPath: "/api/auth", authRequired: false, basePath: "/", diff --git a/packages/junior/src/app.ts b/packages/junior/src/app.ts index 511ef1756e..600fa8a886 100644 --- a/packages/junior/src/app.ts +++ b/packages/junior/src/app.ts @@ -121,7 +121,10 @@ import { type SignInAuth, type SignInAuthConfig, } from "@/chat/sign-in/auth"; -import { resolveAllowedDomains } from "@/chat/sign-in/allowlist"; +import { + assertNoLegacyAllowedDomains, + resolveAllowedDomains, +} from "@/chat/sign-in/allowlist"; import { JUNIOR_VERSION } from "./version"; import { createDashboardRouteRegistrations, @@ -147,6 +150,14 @@ export interface JuniorAppOptions extends BotModelConfig { * default-model call per completed Turn. Disabled by default. */ briefs?: { enabled?: boolean }; + /** + * Domains that may use Junior. Slack checks the email domain of the + * author's Slack profile. Dashboard Google sign-in checks the `hd` claim of + * the Google Workspace account. Defaults to `JUNIOR_ALLOWED_DOMAINS`. + * Recommended. Dashboard auth requires at least one. Empty lets every + * Slack workspace member use Junior. + */ + allowedDomains?: string[]; /** Authenticated dashboard mounted by core when configured. */ dashboard?: JuniorDashboardOptions; /** @@ -466,15 +477,20 @@ export async function createApp(options?: JuniorAppOptions): Promise { setCrossActorMidRunMode(options.slack.crossActorMidRunMode); } // One domain list for Slack authors and dashboard sign-ins. - allowedDomains = resolveAllowedDomains(dashboard); + assertNoLegacyAllowedDomains(Object.keys(dashboard ?? {})); + allowedDomains = resolveAllowedDomains(options?.allowedDomains); if (allowedDomains.length === 0) { logWarn("app.allowed_domains.unset", { "app.reason": - "dashboard.allowedGoogleDomains is empty, so any Slack workspace member can use Junior", + "allowedDomains is empty, so any Slack workspace member can use Junior", }); } if (dashboard && !dashboard.disabled) { - signInConfig = resolveSignInAuthConfig(botConfig.userName, dashboard); + signInConfig = resolveSignInAuthConfig( + botConfig.userName, + allowedDomains, + dashboard, + ); } if (shouldValidatePluginCatalog) { pluginCatalogRuntime.getSignature(); @@ -624,6 +640,7 @@ export async function createApp(options?: JuniorAppOptions): Promise { mountRoutes( app, await createDashboardRouteRegistrations({ + allowedDomains, auth: signInAuth, authenticatedRoutes, conversationWorkQueue, diff --git a/packages/junior/src/chat/sign-in/allowlist.ts b/packages/junior/src/chat/sign-in/allowlist.ts index 315ad21eac..ee10c36302 100644 --- a/packages/junior/src/chat/sign-in/allowlist.ts +++ b/packages/junior/src/chat/sign-in/allowlist.ts @@ -1,7 +1,8 @@ /** - * Domains that may use Junior. Google sign-ins must come from a Google - * Workspace account on one of these domains, and Slack authors must have a - * Slack profile email on one of them. The dashboard and Slack use one list. + * Domains that may use Junior. Slack authors must have a Slack profile email + * on one of them, and dashboard Google sign-ins must come from a Google + * Workspace account whose `hd` claim is one of them. Slack and the dashboard + * share one list. */ import { z } from "zod"; @@ -25,25 +26,38 @@ export function readEnvList(name: string): string[] | undefined { } /** - * Resolve the allowed domains: the dashboard option when set, else its - * environment variable. An empty list allows every domain. + * Resolve the allowed domains: the `createApp` option when set, else + * `JUNIOR_ALLOWED_DOMAINS`. An empty list allows every domain. */ -export function resolveAllowedDomains( - options: { allowedGoogleDomains?: string[] } = {}, -): string[] { +export function resolveAllowedDomains(allowedDomains?: string[]): string[] { return [ ...new Set( - ( - options.allowedGoogleDomains ?? - readEnvList("JUNIOR_DASHBOARD_GOOGLE_DOMAINS") ?? - [] - ) + (allowedDomains ?? readEnvList("JUNIOR_ALLOWED_DOMAINS") ?? []) .map((value) => value.trim().toLowerCase()) .filter(Boolean), ), ]; } +/** + * Fail startup on the removed Google-specific names. Ignoring them would + * leave the list empty, which lets every Slack workspace member in. + */ +export function assertNoLegacyAllowedDomains( + dashboardOptionKeys: readonly string[], +) { + if (dashboardOptionKeys.includes("allowedGoogleDomains")) { + throw new Error( + "dashboard.allowedGoogleDomains was removed; set createApp({ allowedDomains }) instead", + ); + } + if (process.env.JUNIOR_DASHBOARD_GOOGLE_DOMAINS?.trim()) { + throw new Error( + "JUNIOR_DASHBOARD_GOOGLE_DOMAINS was removed; set JUNIOR_ALLOWED_DOMAINS instead", + ); + } +} + /** Return whether a verified domain may use Junior. An empty list allows all. */ export function isAllowedDomain( domain: string | undefined, diff --git a/packages/junior/src/chat/sign-in/auth.ts b/packages/junior/src/chat/sign-in/auth.ts index 9a3cd7af92..948ccb5e01 100644 --- a/packages/junior/src/chat/sign-in/auth.ts +++ b/packages/junior/src/chat/sign-in/auth.ts @@ -14,11 +14,7 @@ import { getSqlExecutor } from "@/chat/db"; import { normalizeIdentityEmail } from "@/chat/identities/identity"; import { recordGoogleSignIn } from "@/chat/identities/sql"; import { resolveBaseUrl } from "@/chat/oauth-flow"; -import { - isAllowedDomain, - readEnvList, - resolveAllowedDomains, -} from "./allowlist"; +import { isAllowedDomain, readEnvList } from "./allowlist"; const DEFAULT_AUTH_PATH = "/api/auth"; const DEFAULT_SESSION_MAX_AGE_SECONDS = 60 * 60 * 8; @@ -67,7 +63,6 @@ export interface SignInAuthConfig { /** Dashboard options that also configure the sign-in session. */ export interface SignInAuthOptions { - allowedGoogleDomains?: string[]; authPath?: string; baseURL?: string; sessionMaxAgeSeconds?: number; @@ -90,6 +85,7 @@ function withHttps(host: string): string { /** Resolve sign-in settings from dashboard options and environment. */ export function resolveSignInAuthConfig( agentName: string, + allowedDomains: string[], options: SignInAuthOptions = {}, ): SignInAuthConfig { const authPath = `/${trimSlashes(options.authPath?.trim() || DEFAULT_AUTH_PATH, true)}`; @@ -101,7 +97,7 @@ export function resolveSignInAuthConfig( ); return { agentName, - allowedDomains: resolveAllowedDomains(options), + allowedDomains, authPath, baseURL, sessionMaxAgeSeconds: options.sessionMaxAgeSeconds, diff --git a/packages/junior/src/dashboard-routes.ts b/packages/junior/src/dashboard-routes.ts index 87de4d1fce..fefaa7da11 100644 --- a/packages/junior/src/dashboard-routes.ts +++ b/packages/junior/src/dashboard-routes.ts @@ -16,13 +16,6 @@ export interface JuniorDashboardOptions { authPath?: string; /** Require a dashboard browser session before serving dashboard pages and APIs. */ authRequired?: boolean; - /** - * Domains that may use Junior: Google Workspace domains for dashboard - * sign-in and Slack profile email domains for Slack. Recommended. The - * dashboard needs it when `authRequired` is on. Empty lets every Slack - * workspace member use Junior. - */ - allowedGoogleDomains?: string[]; /** Browser route prefix for the dashboard shell. */ basePath?: string; /** Public deployment origin used for auth callbacks and external links. */ @@ -41,6 +34,8 @@ export interface JuniorDashboardOptions { export interface JuniorDashboardRuntimeOptions extends JuniorDashboardOptions { agentName?: string; + /** The app's `allowedDomains`. Dashboard auth requires at least one. */ + allowedDomains: string[]; /** Google sign-in session that the dashboard shares with Slack. */ auth?: SignInAuth; authenticatedRoutes?: readonly AuthenticatedRoute[]; @@ -72,6 +67,7 @@ const DASHBOARD_PACKAGE_NAME = "@sentry/junior-dashboard"; /** Mount the dashboard app on the dashboard-owned host paths. */ export async function createDashboardRouteRegistrations(args: { + allowedDomains: string[]; auth: SignInAuth | undefined; authenticatedRoutes: readonly AuthenticatedRoute[]; conversationWorkQueue: ConversationWorkQueue; @@ -86,6 +82,7 @@ export async function createDashboardRouteRegistrations(args: { const createDashboardApp = args.createDashboardApp ?? (await loadDashboardAppFactory()); return dashboardRouteRegistrations({ + allowedDomains: args.allowedDomains, auth: args.auth, authenticatedRoutes: args.authenticatedRoutes, conversationWorkQueue: args.conversationWorkQueue, @@ -313,6 +310,7 @@ function dashboardOwnedRoutePath( } function dashboardRouteRegistrations(args: { + allowedDomains: string[]; auth: SignInAuth | undefined; authenticatedRoutes: readonly AuthenticatedRoute[]; conversationWorkQueue: ConversationWorkQueue; @@ -325,6 +323,7 @@ function dashboardRouteRegistrations(args: { const dashboardOptions: JuniorDashboardRuntimeOptions = { ...args.dashboard, agentName: botConfig.userName, + allowedDomains: args.allowedDomains, ...(args.auth ? { auth: args.auth } : undefined), authenticatedRoutes: args.authenticatedRoutes, conversationWorkQueue: args.conversationWorkQueue, diff --git a/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts b/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts index 7e16321384..aaf9a68407 100644 --- a/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts +++ b/packages/junior/tests/integration/slack/webhook-auth-boundary.test.ts @@ -268,11 +268,8 @@ describe("Slack webhook auth boundary", () => { }); const waitUntil = slack.waitUntil(); const app = await createApp({ + allowedDomains: ["example.com", "example.org"], conversationWorkQueue: () => queue, - dashboard: { - allowedGoogleDomains: ["example.com", "example.org"], - disabled: true, - }, experimental: SUITE_EXPERIMENTAL, waitUntil: waitUntil.fn, }); diff --git a/packages/junior/tests/unit/app-allowed-domains.test.ts b/packages/junior/tests/unit/app-allowed-domains.test.ts new file mode 100644 index 0000000000..bfa10ae3ba --- /dev/null +++ b/packages/junior/tests/unit/app-allowed-domains.test.ts @@ -0,0 +1,65 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createApp, defineJuniorPlugins } from "@/app"; +import { setDashboardConversationLinkOptions } from "@/chat/dashboard-link"; + +afterEach(() => { + setDashboardConversationLinkOptions(undefined); + vi.doUnmock("#junior/config"); + vi.unstubAllEnvs(); +}); + +describe("createApp allowed domains", () => { + it("passes top-level allowedDomains to the dashboard", async () => { + const createDashboardApp = vi.fn((_options: unknown) => ({ + fetch: () => new Response("dashboard"), + })); + vi.doMock("#junior/config", () => ({ + createDashboardApp, + functionMaxDurationSeconds: undefined, + dashboard: undefined, + pluginSet: defineJuniorPlugins([]), + plugins: undefined, + pluginRuntimeRegistrations: [], + })); + + const app = await createApp({ + allowedDomains: [" Sentry.io "], + dashboard: { authRequired: false }, + plugins: defineJuniorPlugins([]), + }); + await app.fetch(new Request("http://localhost/")); + + expect(createDashboardApp).toHaveBeenCalledWith( + expect.objectContaining({ allowedDomains: ["sentry.io"] }), + ); + }); + + it("rejects the removed dashboard.allowedGoogleDomains option", async () => { + vi.doMock("#junior/config", () => ({ + createDashboardApp: vi.fn(), + functionMaxDurationSeconds: undefined, + dashboard: undefined, + pluginSet: defineJuniorPlugins([]), + plugins: undefined, + pluginRuntimeRegistrations: [], + })); + + await expect( + createApp({ + dashboard: { + authRequired: false, + allowedGoogleDomains: ["sentry.io"], + } as never, + plugins: defineJuniorPlugins([]), + }), + ).rejects.toThrow("dashboard.allowedGoogleDomains was removed"); + }); + + it("rejects the removed JUNIOR_DASHBOARD_GOOGLE_DOMAINS variable", async () => { + vi.stubEnv("JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "sentry.io"); + + await expect( + createApp({ plugins: defineJuniorPlugins([]) }), + ).rejects.toThrow("JUNIOR_DASHBOARD_GOOGLE_DOMAINS was removed"); + }); +}); diff --git a/packages/junior/tests/unit/app-config.test.ts b/packages/junior/tests/unit/app-config.test.ts index 2b2c3a3442..7e29e88e01 100644 --- a/packages/junior/tests/unit/app-config.test.ts +++ b/packages/junior/tests/unit/app-config.test.ts @@ -806,7 +806,6 @@ describe("createApp plugin config", () => { functionMaxDurationSeconds: undefined, dashboard: { authRequired: false, - allowedGoogleDomains: ["sentry.io"], componentGallery: true, }, pluginSet: defineJuniorPlugins([ @@ -915,7 +914,6 @@ describe("createApp plugin config", () => { createApp({ dashboard: { authRequired: false, - allowedGoogleDomains: ["sentry.io"], }, plugins: defineJuniorPlugins([ defineJuniorPlugin({ @@ -959,7 +957,6 @@ describe("createApp plugin config", () => { await createApp({ dashboard: { - allowedGoogleDomains: ["sentry.io"], authRequired: false, basePath: "/ops", baseURL: "https://junior.example.com", diff --git a/packages/junior/tests/unit/build/nitro-plugin-module.test.ts b/packages/junior/tests/unit/build/nitro-plugin-module.test.ts index f21752b8ab..224ddf4b59 100644 --- a/packages/junior/tests/unit/build/nitro-plugin-module.test.ts +++ b/packages/junior/tests/unit/build/nitro-plugin-module.test.ts @@ -449,7 +449,7 @@ describe("juniorNitro plugin modules", () => { }; juniorNitro({ - dashboard: { allowedGoogleDomains: ["sentry.io"] }, + dashboard: { basePath: "/ops" }, }).nitro.setup(nitro); expect(rollupBeforeHooks).toHaveLength(1); diff --git a/packages/junior/tests/unit/build/virtual-config.test.ts b/packages/junior/tests/unit/build/virtual-config.test.ts index 6679a8912c..e6b5deee2d 100644 --- a/packages/junior/tests/unit/build/virtual-config.test.ts +++ b/packages/junior/tests/unit/build/virtual-config.test.ts @@ -42,16 +42,14 @@ describe("renderVirtualConfig", () => { it("imports the dashboard app factory when dashboard config is present", () => { const code = renderVirtualConfig({ dashboard: { - allowedGoogleDomains: ["sentry.io"], + basePath: "/ops", }, }); expect(code).toContain( 'import { createDashboardApp as juniorCreateDashboardApp } from "@sentry/junior-dashboard";', ); - expect(code).toContain( - 'export const dashboard = {"allowedGoogleDomains":["sentry.io"]};', - ); + expect(code).toContain('export const dashboard = {"basePath":"/ops"};'); }); it("does not import the dashboard route factory when dashboard config is disabled", () => { diff --git a/packages/junior/tests/unit/chat/sign-in-auth.test.ts b/packages/junior/tests/unit/chat/sign-in-auth.test.ts index 81d12f93aa..44307a12c1 100644 --- a/packages/junior/tests/unit/chat/sign-in-auth.test.ts +++ b/packages/junior/tests/unit/chat/sign-in-auth.test.ts @@ -1,27 +1,43 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { resolveAllowedDomains } from "@/chat/sign-in/allowlist"; +import { + assertNoLegacyAllowedDomains, + resolveAllowedDomains, +} from "@/chat/sign-in/allowlist"; afterEach(() => { vi.unstubAllEnvs(); }); describe("allowed domains", () => { - it("reads the dashboard env list as a JSON array or comma separated values", () => { - vi.stubEnv( - "JUNIOR_DASHBOARD_GOOGLE_DOMAINS", - '["Sentry.io", "example.com"]', - ); + it("reads JUNIOR_ALLOWED_DOMAINS as a JSON array or comma separated values", () => { + vi.stubEnv("JUNIOR_ALLOWED_DOMAINS", '["Sentry.io", "example.com"]'); expect(resolveAllowedDomains()).toEqual(["sentry.io", "example.com"]); - vi.stubEnv("JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "sentry.io, example.com"); + vi.stubEnv("JUNIOR_ALLOWED_DOMAINS", " sentry.io, example.com,SENTRY.io "); expect(resolveAllowedDomains()).toEqual(["sentry.io", "example.com"]); }); + it("prefers the createApp option over the env list", () => { + vi.stubEnv("JUNIOR_ALLOWED_DOMAINS", "example.com"); + expect(resolveAllowedDomains(["Sentry.io"])).toEqual(["sentry.io"]); + }); + it("rejects an env list that is not a JSON string array", () => { - vi.stubEnv("JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "[1]"); + vi.stubEnv("JUNIOR_ALLOWED_DOMAINS", "[1]"); expect(() => resolveAllowedDomains()).toThrow( - "JUNIOR_DASHBOARD_GOOGLE_DOMAINS must be a JSON string array", + "JUNIOR_ALLOWED_DOMAINS must be a JSON string array", + ); + }); + + it("fails on the removed Google-specific names instead of allowing everyone", () => { + expect(() => + assertNoLegacyAllowedDomains(["allowedGoogleDomains"]), + ).toThrow("dashboard.allowedGoogleDomains was removed"); + + vi.stubEnv("JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "sentry.io"); + expect(() => assertNoLegacyAllowedDomains([])).toThrow( + "JUNIOR_DASHBOARD_GOOGLE_DOMAINS was removed", ); }); }); diff --git a/packages/junior/tests/unit/cli/init-cli.test.ts b/packages/junior/tests/unit/cli/init-cli.test.ts index 89d263ce25..2e72d3a624 100644 --- a/packages/junior/tests/unit/cli/init-cli.test.ts +++ b/packages/junior/tests/unit/cli/init-cli.test.ts @@ -36,6 +36,7 @@ function removeExampleOnlyServerConfig(source: string): string { "", ) .replace(' import("./dashboard.ts"),\n', "") + .replace(' allowedDomains: ["sentry.io"],\n', "") .replace(/ dashboard: \{[\s\S]*? \},\n plugins,/, " plugins,") .replace(/ configDefaults: \{[\s\S]*? \},\n/, ""), ); From 8900e9e5446165d41045892491c92df200ed381f Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:06:56 +0000 Subject: [PATCH 3/5] fix(auth): Treat a malformed Google ID token as a sign-in error decodeJwt throws on a malformed compact JWT before the claims schema runs, so a bad id_token still returned a 500 from the callback. Catch it and return null so Better Auth redirects with unable_to_get_user_info. --- packages/junior/src/chat/sign-in/auth.ts | 22 +++++++++-- .../tests/integration/google-sign-in.test.ts | 37 ++++++++++++------- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/packages/junior/src/chat/sign-in/auth.ts b/packages/junior/src/chat/sign-in/auth.ts index 948ccb5e01..ae3b12c52e 100644 --- a/packages/junior/src/chat/sign-in/auth.ts +++ b/packages/junior/src/chat/sign-in/auth.ts @@ -31,6 +31,23 @@ const googleClaimsSchema = z.object({ picture: z.string().optional(), }); +/** + * Read Google ID token claims. Returns undefined for a malformed token, so + * Better Auth redirects with `unable_to_get_user_info` instead of a 500. + */ +function readGoogleClaims( + idToken: string, +): z.infer | undefined { + let payload: unknown; + try { + payload = decodeJwt(idToken); + } catch { + return undefined; + } + const parsed = googleClaimsSchema.safeParse(payload); + return parsed.success ? parsed.data : undefined; +} + /** Verified Google account of one browser session. */ export interface SignInSession { user: { email: string; emailVerified: boolean; name?: string }; @@ -157,9 +174,8 @@ function createBetterAuth(config: SignInAuthConfig) { // verified by `verifyIdToken` first. async getUserInfo(token) { if (!token.idToken) return null; - const parsed = googleClaimsSchema.safeParse(decodeJwt(token.idToken)); - if (!parsed.success) return null; - const claims = parsed.data; + const claims = readGoogleClaims(token.idToken); + if (!claims) return null; if (!isAllowedDomain(claims.hd, allowedDomains)) return null; return { user: { diff --git a/packages/junior/tests/integration/google-sign-in.test.ts b/packages/junior/tests/integration/google-sign-in.test.ts index caee122828..853755f11b 100644 --- a/packages/junior/tests/integration/google-sign-in.test.ts +++ b/packages/junior/tests/integration/google-sign-in.test.ts @@ -19,24 +19,28 @@ function createAuth(allowedDomains: string[]) { }); } -// Fake Google: the token endpoint returns an ID token with these claims. -function useGoogleAccount(claims: { email: string; hd?: string }) { +// Fake Google: the token endpoint returns this ID token. +function useGoogleIdToken(idToken: string) { mswServer.use( - http.post("https://oauth2.googleapis.com/token", () => { - const payload = Buffer.from( - JSON.stringify({ - aud: "google-client", - email_verified: true, - iss: "https://accounts.google.com", - sub: `google-${claims.email}`, - ...claims, - }), - ).toString("base64url"); - return HttpResponse.json({ id_token: `e30.${payload}.sig` }); - }), + http.post("https://oauth2.googleapis.com/token", () => + HttpResponse.json({ id_token: idToken }), + ), ); } +function useGoogleAccount(claims: { email: string; hd?: string }) { + const payload = Buffer.from( + JSON.stringify({ + aud: "google-client", + email_verified: true, + iss: "https://accounts.google.com", + sub: `google-${claims.email}`, + ...claims, + }), + ).toString("base64url"); + useGoogleIdToken(`e30.${payload}.sig`); +} + function cookieHeader(response: Response): string { return response.headers .getSetCookie() @@ -97,6 +101,11 @@ describe("Google sign-in", () => { expect(await auth.getSession(rejected.session)).toBeNull(); } + // A malformed ID token is a normal sign-in error, not a server error. + useGoogleIdToken("not-a-jwt"); + const malformed = await signIn(auth); + expect(malformed.location).toContain("error=unable_to_get_user_info"); + useGoogleAccount({ email: "person@example.org", hd: "example.org" }); const accepted = await signIn(auth); expect(accepted.location).toBe(`${BASE_URL}/`); From c8b050f832a586ec55a438dd658e2fee4d7e2ee2 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:09:59 +0000 Subject: [PATCH 4/5] ref(auth): Clean up allowed-domain diff Drop stale comments that still describe a Google session shared with Slack, read the Slack profile email domain only when the email has an @, and remove a unit test that duplicated the createApp legacy-name coverage. Co-Authored-By: David Cramer --- packages/junior-dashboard/src/auth.ts | 2 +- .../src/chat/ingress/slack-allowed-domain.ts | 8 +++----- packages/junior/src/chat/sign-in/allowlist.ts | 5 +---- packages/junior/src/chat/sign-in/auth.ts | 2 +- packages/junior/src/dashboard-routes.ts | 2 +- .../junior/tests/unit/chat/sign-in-auth.test.ts | 16 +--------------- 6 files changed, 8 insertions(+), 27 deletions(-) diff --git a/packages/junior-dashboard/src/auth.ts b/packages/junior-dashboard/src/auth.ts index 520c24b7e7..a354c7d79c 100644 --- a/packages/junior-dashboard/src/auth.ts +++ b/packages/junior-dashboard/src/auth.ts @@ -4,7 +4,7 @@ import type { DashboardSession } from "./api/schema"; export type DashboardUser = DashboardSession["user"]; export type { DashboardSession }; -/** Google sign-in session that core shares with Slack. */ +/** Google sign-in session that core owns. */ export interface DashboardAuth { handler(request: Request): Promise; getSession(request: Request): Promise; diff --git a/packages/junior/src/chat/ingress/slack-allowed-domain.ts b/packages/junior/src/chat/ingress/slack-allowed-domain.ts index fb4064da6f..8843ec93d0 100644 --- a/packages/junior/src/chat/ingress/slack-allowed-domain.ts +++ b/packages/junior/src/chat/ingress/slack-allowed-domain.ts @@ -30,12 +30,10 @@ export async function isSlackAuthorDomainAllowed(args: { const profile = await lookupSlackUserProfile(userId); const email = profile.email?.toLowerCase(); + const separator = email?.lastIndexOf("@") ?? -1; + const domain = separator > 0 ? email?.slice(separator + 1) : undefined; const allowed = - profile.id === userId && - isAllowedDomain( - email?.slice(email.lastIndexOf("@") + 1), - args.allowedDomains, - ); + profile.id === userId && isAllowedDomain(domain, args.allowedDomains); if (!allowed) { // A Slack app without `users:read.email` sees no email and blocks all. logWarn("slack.allowed_domain.rejected", { diff --git a/packages/junior/src/chat/sign-in/allowlist.ts b/packages/junior/src/chat/sign-in/allowlist.ts index ee10c36302..152131f2c2 100644 --- a/packages/junior/src/chat/sign-in/allowlist.ts +++ b/packages/junior/src/chat/sign-in/allowlist.ts @@ -6,10 +6,7 @@ */ import { z } from "zod"; -/** - * Read a list env var the same way as the dashboard: a JSON string array or - * comma-separated values. - */ +/** Read a list env var: a JSON string array or comma-separated values. */ export function readEnvList(name: string): string[] | undefined { const value = process.env[name]?.trim(); if (!value) return undefined; diff --git a/packages/junior/src/chat/sign-in/auth.ts b/packages/junior/src/chat/sign-in/auth.ts index ae3b12c52e..1fc4f4b00f 100644 --- a/packages/junior/src/chat/sign-in/auth.ts +++ b/packages/junior/src/chat/sign-in/auth.ts @@ -64,7 +64,7 @@ export interface SignInAuth { ): Promise; } -/** Settings for the shared Google sign-in session. */ +/** Settings for the dashboard Google sign-in session. */ export interface SignInAuthConfig { agentName: string; /** Google Workspace domains that may sign in. Empty allows every account. */ diff --git a/packages/junior/src/dashboard-routes.ts b/packages/junior/src/dashboard-routes.ts index fefaa7da11..93b141dc65 100644 --- a/packages/junior/src/dashboard-routes.ts +++ b/packages/junior/src/dashboard-routes.ts @@ -36,7 +36,7 @@ export interface JuniorDashboardRuntimeOptions extends JuniorDashboardOptions { agentName?: string; /** The app's `allowedDomains`. Dashboard auth requires at least one. */ allowedDomains: string[]; - /** Google sign-in session that the dashboard shares with Slack. */ + /** Google sign-in session of the dashboard. */ auth?: SignInAuth; authenticatedRoutes?: readonly AuthenticatedRoute[]; conversationWorkQueue?: ConversationWorkQueue; diff --git a/packages/junior/tests/unit/chat/sign-in-auth.test.ts b/packages/junior/tests/unit/chat/sign-in-auth.test.ts index 44307a12c1..391255c7f5 100644 --- a/packages/junior/tests/unit/chat/sign-in-auth.test.ts +++ b/packages/junior/tests/unit/chat/sign-in-auth.test.ts @@ -1,8 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { - assertNoLegacyAllowedDomains, - resolveAllowedDomains, -} from "@/chat/sign-in/allowlist"; +import { resolveAllowedDomains } from "@/chat/sign-in/allowlist"; afterEach(() => { vi.unstubAllEnvs(); @@ -29,15 +26,4 @@ describe("allowed domains", () => { "JUNIOR_ALLOWED_DOMAINS must be a JSON string array", ); }); - - it("fails on the removed Google-specific names instead of allowing everyone", () => { - expect(() => - assertNoLegacyAllowedDomains(["allowedGoogleDomains"]), - ).toThrow("dashboard.allowedGoogleDomains was removed"); - - vi.stubEnv("JUNIOR_DASHBOARD_GOOGLE_DOMAINS", "sentry.io"); - expect(() => assertNoLegacyAllowedDomains([])).toThrow( - "JUNIOR_DASHBOARD_GOOGLE_DOMAINS was removed", - ); - }); }); From 4574f19d79230ade2bd45cb95f36f9183e5675d1 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:12:22 +0000 Subject: [PATCH 5/5] feat(cli): Scaffold allowedDomains with an example.com placeholder New apps from junior init set allowedDomains to example.com so the recommended domain gate is visible from the start. Docs snippets use the same placeholder. Co-Authored-By: David Cramer --- packages/docs/src/content/docs/operate/dashboard.md | 2 +- packages/docs/src/content/docs/reference/config-and-env.md | 2 +- packages/docs/src/content/docs/start-here/existing-app.md | 5 +++++ packages/docs/src/content/docs/start-here/quickstart.mdx | 5 +++++ packages/junior/src/cli/init.ts | 1 + packages/junior/tests/unit/cli/init-cli.test.ts | 6 +++++- 6 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/docs/src/content/docs/operate/dashboard.md b/packages/docs/src/content/docs/operate/dashboard.md index 5942c15def..79b331682e 100644 --- a/packages/docs/src/content/docs/operate/dashboard.md +++ b/packages/docs/src/content/docs/operate/dashboard.md @@ -34,7 +34,7 @@ import { createApp } from "@sentry/junior"; import { plugins } from "./plugins"; export default await createApp({ - allowedDomains: ["sentry.io"], + allowedDomains: ["example.com"], dashboard: { trustedOrigins: ["https://"], }, diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index 28cee9a61e..caec624bc6 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -218,7 +218,7 @@ Junior logs a warning at startup when it is empty. ```ts title="server.ts" export default await createApp({ - allowedDomains: ["sentry.io"], + allowedDomains: ["example.com"], plugins, }); ``` diff --git a/packages/docs/src/content/docs/start-here/existing-app.md b/packages/docs/src/content/docs/start-here/existing-app.md index 38b06fed51..17b956b4e5 100644 --- a/packages/docs/src/content/docs/start-here/existing-app.md +++ b/packages/docs/src/content/docs/start-here/existing-app.md @@ -27,12 +27,17 @@ import { createApp } from "@sentry/junior"; import { plugins } from "./plugins.ts"; const app = await createApp({ + allowedDomains: ["example.com"], plugins, }); export default app; ``` +Replace `example.com` with your company's email domain. Only Slack users and +dashboard sign-ins on [allowed domains](/reference/config-and-env/#allowed-domains) +can use Junior. + `createApp()` mounts the supported Junior routes listed in [Route & Handler Surface](/reference/handler-surface/). ## Add Nitro wiring diff --git a/packages/docs/src/content/docs/start-here/quickstart.mdx b/packages/docs/src/content/docs/start-here/quickstart.mdx index 50f089444e..bf25b83659 100644 --- a/packages/docs/src/content/docs/start-here/quickstart.mdx +++ b/packages/docs/src/content/docs/start-here/quickstart.mdx @@ -179,12 +179,17 @@ import { createApp } from "@sentry/junior"; import { plugins } from "./plugins.ts"; const app = await createApp({ + allowedDomains: ["example.com"], plugins, }); export default app; ``` +Replace `example.com` with your company's email domain. Only Slack users and +dashboard sign-ins on [allowed domains](/reference/config-and-env/#allowed-domains) +can use Junior. + Run the app check after changing plugins or skills: ```bash diff --git a/packages/junior/src/cli/init.ts b/packages/junior/src/cli/init.ts index da6ada8174..f25a871fbd 100644 --- a/packages/junior/src/cli/init.ts +++ b/packages/junior/src/cli/init.ts @@ -18,6 +18,7 @@ const [ ]); const app = await createApp({ + allowedDomains: ["example.com"], briefs: { enabled: true }, plugins, }); diff --git a/packages/junior/tests/unit/cli/init-cli.test.ts b/packages/junior/tests/unit/cli/init-cli.test.ts index 2e72d3a624..a76c9a14d6 100644 --- a/packages/junior/tests/unit/cli/init-cli.test.ts +++ b/packages/junior/tests/unit/cli/init-cli.test.ts @@ -36,7 +36,11 @@ function removeExampleOnlyServerConfig(source: string): string { "", ) .replace(' import("./dashboard.ts"),\n', "") - .replace(' allowedDomains: ["sentry.io"],\n', "") + // The example deployment allows Sentry; the scaffold uses a placeholder. + .replace( + 'allowedDomains: ["sentry.io"]', + 'allowedDomains: ["example.com"]', + ) .replace(/ dashboard: \{[\s\S]*? \},\n plugins,/, " plugins,") .replace(/ configDefaults: \{[\s\S]*? \},\n/, ""), );