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 0cbd13be79..79b331682e 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 @@ -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: ["example.com"], 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,8 +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_DASHBOARD_ALLOWED_EMAILS` | Comma-separated or JSON array of explicit email allowlist. | +| `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 4f2971ae30..caec624bc6 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -90,12 +90,10 @@ 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 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. | @@ -168,9 +166,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 +209,35 @@ 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 + +`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: ["example.com"], + 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. + 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. + +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. 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-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..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({ - allowedEmails: ["dev@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 2fa56ef326..6d913cc76f 100644 --- a/packages/junior-dashboard/src/api/schema.ts +++ b/packages/junior-dashboard/src/api/schema.ts @@ -33,8 +33,7 @@ export const dashboardProfileUpdateSchema = z export const dashboardConfigSchema = z .object({ - allowedEmailCount: z.number(), - 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 3ec51108e2..143fdde964 100644 --- a/packages/junior-dashboard/src/app.ts +++ b/packages/junior-dashboard/src/app.ts @@ -78,14 +78,14 @@ export interface JuniorDashboardOptions { baseURL?: string; authPath?: string; authRequired?: boolean; - allowedGoogleDomains?: string[]; - allowedEmails?: 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; @@ -113,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(); @@ -160,11 +129,6 @@ function resolveDashboardOptions( ...options, authRequired: options.authRequired ?? readEnvFlag("JUNIOR_DASHBOARD_AUTH_REQUIRED"), - allowedGoogleDomains: - options.allowedGoogleDomains ?? - readEnvList("JUNIOR_DASHBOARD_GOOGLE_DOMAINS"), - allowedEmails: - options.allowedEmails ?? readEnvList("JUNIOR_DASHBOARD_ALLOWED_EMAILS"), componentGallery: options.componentGallery ?? readEnvFlag("JUNIOR_DASHBOARD_COMPONENT_GALLERY"), @@ -404,8 +368,7 @@ export function createDashboardApp( options.authPath ?? DEFAULT_AUTH_PATH, DEFAULT_AUTH_PATH, ); - const allowedDomains = normalizeValues(options.allowedGoogleDomains); - const allowedEmails = normalizeValues(options.allowedEmails); + const allowedDomains = normalizeValues(options.allowedDomains); const agentName = options.agentName?.trim() || "Junior"; const authRequired = options.authRequired !== false; @@ -415,17 +378,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 allowedDomains"); } - // 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 +405,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 +494,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); @@ -622,8 +576,7 @@ export function createDashboardApp( ); app.get("/api/config", () => { return jsonResponse(dashboardConfigSchema, { - allowedEmailCount: allowedEmails.length, - allowedGoogleDomainCount: allowedDomains.length, + allowedDomainCount: allowedDomains.length, authRequired, authPath, basePath, diff --git a/packages/junior-dashboard/src/auth.ts b/packages/junior-dashboard/src/auth.ts index a8cc683c72..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; @@ -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 f2bbd9aaed..99f9bc8a26 100644 --- a/packages/junior-dashboard/tests/client-api.test.ts +++ b/packages/junior-dashboard/tests/client-api.test.ts @@ -116,7 +116,7 @@ describe("dashboard client API", () => { user: { email: "dev@example.com", emailVerified: true }, }); const app = createDashboardApp({ - allowedEmails: ["dev@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 fce8815c89..81eb9d988c 100644 --- a/packages/junior-dashboard/tests/dashboard-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-routes.test.ts @@ -26,8 +26,7 @@ vi.mock("@sentry/junior/api", async (importOriginal) => ({ function dashboard(session: DashboardSession | null) { return createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], - allowedEmails: ["admin@example.com"], + allowedDomains: ["sentry.io"], auth: auth(session), }); } @@ -92,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; }), @@ -124,7 +123,7 @@ describe("dashboard routes", () => { ]; const unauthenticatedApp = createDashboardApp({ authenticatedRoutes, - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -143,7 +142,7 @@ describe("dashboard routes", () => { const authenticatedApp = createDashboardApp({ authenticatedRoutes, - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth({ user: { email: "person@sentry.io", @@ -169,7 +168,7 @@ describe("dashboard routes", () => { path: "/_junior/acp/auth/:transactionId", }, ], - allowedGoogleDomains: ["sentry.io"], + allowedDomains: ["sentry.io"], auth: auth({ user: { email: "person@example.com", @@ -191,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; }), @@ -234,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; }), @@ -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"], + allowedDomains: ["sentry.io"], auth: auth(null), authPath: "/auth", }); @@ -278,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; }), @@ -297,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: { @@ -327,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/")); @@ -493,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", @@ -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"], + allowedDomains: ["sentry.io"], auth: auth(null, (value) => { callbackURL = value; }), @@ -598,8 +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({ - allowedEmailCount: 1, - allowedGoogleDomainCount: 1, + allowedDomainCount: 1, authRequired: true, authPath: "/api/auth", basePath: "/", @@ -625,25 +623,10 @@ describe("dashboard routes", () => { expect(await response.json()).toEqual({ error: "forbidden" }); }); - it("allows explicitly configured email exceptions", async () => { + it("requires a verified email on an allowed domain", 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 () => { - const app = dashboard({ - user: { - email: "admin@example.com", + email: "person@sentry.io", emailVerified: false, }, }); @@ -655,15 +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_ALLOWED_EMAILS = JSON.stringify([ - "admin@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", @@ -678,19 +657,8 @@ describe("dashboard routes", () => { expect(response.status).toBe(200); expect(await response.json()).toMatchObject({ - allowedEmailCount: 1, - allowedGoogleDomainCount: 2, + allowedDomainCount: 2, authRequired: true, }); }); - - it("fails clearly when list env JSON is malformed", async () => { - process.env.JUNIOR_DASHBOARD_ALLOWED_EMAILS = '["admin@example.com"'; - - expect(() => - createDashboardApp({ - authRequired: false, - }), - ).toThrow("JUNIOR_DASHBOARD_ALLOWED_EMAILS 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..83dd33e1b8 100644 --- a/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts +++ b/packages/junior-dashboard/tests/dashboard-shell-routes.test.ts @@ -17,8 +17,7 @@ vi.mock("@sentry/junior/api", async (importOriginal) => ({ function dashboard(session: DashboardSession | null) { return createDashboardApp({ - allowedGoogleDomains: ["sentry.io"], - allowedEmails: ["admin@example.com"], + allowedDomains: ["sentry.io"], 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"], + 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 e2ca64b9e9..ed608a16a6 100644 --- a/packages/junior-dashboard/tests/dashboard-test-helpers.ts +++ b/packages/junior-dashboard/tests/dashboard-test-helpers.ts @@ -11,8 +11,6 @@ export const dashboardEnvNames = [ "VERCEL_URL", "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 06f54a6b3e..812ee51304 100644 --- a/packages/junior-dashboard/tests/telemetry-components.test.tsx +++ b/packages/junior-dashboard/tests/telemetry-components.test.tsx @@ -91,8 +91,7 @@ function renderTranscript(detail: ConversationTranscript): string { function systemData(): SystemData { return { config: { - allowedEmailCount: 0, - 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 08d9c17bc0..600fa8a886 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"; @@ -123,11 +122,9 @@ import { 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"; + assertNoLegacyAllowedDomains, + resolveAllowedDomains, +} from "@/chat/sign-in/allowlist"; import { JUNIOR_VERSION } from "./version"; import { createDashboardRouteRegistrations, @@ -153,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; /** @@ -183,12 +188,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 +454,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 +476,21 @@ 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. + assertNoLegacyAllowedDomains(Object.keys(dashboard ?? {})); + allowedDomains = resolveAllowedDomains(options?.allowedDomains); + if (allowedDomains.length === 0) { + logWarn("app.allowed_domains.unset", { + "app.reason": + "allowedDomains 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, + allowedDomains, + dashboard, + ); } if (shouldValidatePluginCatalog) { pluginCatalogRuntime.getSignature(); @@ -612,7 +606,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 +615,7 @@ export async function createApp(options?: JuniorAppOptions): Promise { queue: conversationWorkQueue, services: runtimeServiceOverrides, }), - ...(slackSignIn ? { signIn: slackSignIn } : undefined), + allowedDomains, }; const app = new Hono(); @@ -646,6 +640,7 @@ export async function createApp(options?: JuniorAppOptions): Promise { mountRoutes( app, await createDashboardRouteRegistrations({ + allowedDomains, auth: signInAuth, authenticatedRoutes, conversationWorkQueue, @@ -682,27 +677,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..8843ec93d0 --- /dev/null +++ b/packages/junior/src/chat/ingress/slack-allowed-domain.ts @@ -0,0 +1,49 @@ +/** 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 separator = email?.lastIndexOf("@") ?? -1; + const domain = separator > 0 ? email?.slice(separator + 1) : undefined; + const allowed = + 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", { + "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 225dfd94d0..c4973d3f1a 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..152131f2c2 100644 --- a/packages/junior/src/chat/sign-in/allowlist.ts +++ b/packages/junior/src/chat/sign-in/allowlist.ts @@ -1,16 +1,12 @@ -/** Google accounts that may sign in. The dashboard uses the same 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. + * 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"; + +/** 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; @@ -27,30 +23,43 @@ 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 `createApp` option when set, else + * `JUNIOR_ALLOWED_DOMAINS`. 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(allowedDomains?: string[]): string[] { + return [ + ...new Set( + (allowedDomains ?? readEnvList("JUNIOR_ALLOWED_DOMAINS") ?? []) + .map((value) => value.trim().toLowerCase()) + .filter(Boolean), ), - emails: list(options.allowedEmails, "JUNIOR_DASHBOARD_ALLOWED_EMAILS"), - }; + ]; +} + +/** + * 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 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..1fc4f4b00f 100644 --- a/packages/junior/src/chat/sign-in/auth.ts +++ b/packages/junior/src/chat/sign-in/auth.ts @@ -1,26 +1,53 @@ /** - * 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, - readEnvList, - resolveSignInAllowlist, - type SignInAllowlist, -} from "./allowlist"; +import { isAllowedDomain, readEnvList } 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(), +}); + +/** + * 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 }; @@ -37,10 +64,11 @@ 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; - allowlist: SignInAllowlist; + /** Google Workspace domains that may sign in. Empty allows every account. */ + allowedDomains: string[]; authPath: string; baseURL: string; googleClientId?: string; @@ -52,8 +80,6 @@ export interface SignInAuthConfig { /** Dashboard options that also configure the sign-in session. */ export interface SignInAuthOptions { - allowedEmails?: string[]; - allowedGoogleDomains?: string[]; authPath?: string; baseURL?: string; sessionMaxAgeSeconds?: number; @@ -76,6 +102,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)}`; @@ -87,7 +114,7 @@ export function resolveSignInAuthConfig( ); return { agentName, - allowlist: resolveSignInAllowlist(options), + allowedDomains, authPath, baseURL, sessionMaxAgeSeconds: options.sessionMaxAgeSeconds, @@ -114,11 +141,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 +169,23 @@ 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 claims = readGoogleClaims(token.idToken); + if (!claims) return null; + 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 +206,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 +233,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/cli/init.ts b/packages/junior/src/cli/init.ts index 770ecb00e7..66ec76dcda 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/src/dashboard-routes.ts b/packages/junior/src/dashboard-routes.ts index 9f8cbf50d7..cadbd423be 100644 --- a/packages/junior/src/dashboard-routes.ts +++ b/packages/junior/src/dashboard-routes.ts @@ -16,10 +16,6 @@ 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. */ - allowedGoogleDomains?: string[]; /** Browser route prefix for the dashboard shell. */ basePath?: string; /** Public deployment origin used for auth callbacks and external links. */ @@ -38,7 +34,9 @@ export interface JuniorDashboardOptions { export interface JuniorDashboardRuntimeOptions extends JuniorDashboardOptions { agentName?: string; - /** Google sign-in session that the dashboard shares with Slack. */ + /** The app's `allowedDomains`. Dashboard auth requires at least one. */ + allowedDomains: string[]; + /** Google sign-in session of the dashboard. */ auth?: SignInAuth; authenticatedRoutes?: readonly AuthenticatedRoute[]; conversationWorkQueue?: ConversationWorkQueue; @@ -69,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; @@ -83,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, @@ -310,6 +310,7 @@ function dashboardOwnedRoutePath( } function dashboardRouteRegistrations(args: { + allowedDomains: string[]; auth: SignInAuth | undefined; authenticatedRoutes: readonly AuthenticatedRoute[]; conversationWorkQueue: ConversationWorkQueue; @@ -322,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/google-sign-in.test.ts b/packages/junior/tests/integration/google-sign-in.test.ts new file mode 100644 index 0000000000..853755f11b --- /dev/null +++ b/packages/junior/tests/integration/google-sign-in.test.ts @@ -0,0 +1,116 @@ +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 this ID token. +function useGoogleIdToken(idToken: string) { + mswServer.use( + 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() + .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(); + } + + // 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}/`); + 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..aaf9a68407 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,54 @@ 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({ + allowedDomains: ["example.com", "example.org"], 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/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 86541a232b..1d978daf29 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 0da9d70991..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,65 +1,29 @@ 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", () => { - vi.stubEnv( - "JUNIOR_DASHBOARD_GOOGLE_DOMAINS", - '["Sentry.io", "example.com"]', - ); - vi.stubEnv( - "JUNIOR_DASHBOARD_ALLOWED_EMAILS", - "a@example.net, b@example.net", - ); +describe("allowed domains", () => { + 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"]); - expect(resolveSignInAllowlist()).toEqual({ - domains: ["sentry.io", "example.com"], - emails: ["a@example.net", "b@example.net"], - }); + vi.stubEnv("JUNIOR_ALLOWED_DOMAINS", " sentry.io, example.com,SENTRY.io "); + 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( - "JUNIOR_DASHBOARD_GOOGLE_DOMAINS must be a JSON string array", - ); + it("prefers the createApp option over the env list", () => { + vi.stubEnv("JUNIOR_ALLOWED_DOMAINS", "example.com"); + expect(resolveAllowedDomains(["Sentry.io"])).toEqual(["sentry.io"]); }); -}); -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")!); - }; + it("rejects an env list that is not a JSON string array", () => { + vi.stubEnv("JUNIOR_ALLOWED_DOMAINS", "[1]"); - expect((await googleUrl(["example.com"], [])).searchParams.get("hd")).toBe( - "example.com", + expect(() => resolveAllowedDomains()).toThrow( + "JUNIOR_ALLOWED_DOMAINS must be a JSON string array", ); - expect( - ( - await googleUrl(["example.com"], ["person@partner.dev"]) - ).searchParams.has("hd"), - ).toBe(false); }); }); diff --git a/packages/junior/tests/unit/cli/init-cli.test.ts b/packages/junior/tests/unit/cli/init-cli.test.ts index 89d263ce25..a76c9a14d6 100644 --- a/packages/junior/tests/unit/cli/init-cli.test.ts +++ b/packages/junior/tests/unit/cli/init-cli.test.ts @@ -36,6 +36,11 @@ function removeExampleOnlyServerConfig(source: string): string { "", ) .replace(' import("./dashboard.ts"),\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/, ""), );