From bf3467b1c583b70f6594d9957be56cacb825dc2a Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Wed, 20 May 2026 17:09:05 -0400 Subject: [PATCH 1/9] build: add GitHub org access configuration --- terraform/environments/production/checks.tf | 5 +++-- .../environments/production/terraform.tfvars.example | 6 +++++- terraform/environments/production/variables.tf | 12 +++++++++--- terraform/environments/production/web-cloudflare.tf | 1 + terraform/environments/production/web-vercel.tf | 6 ++++++ 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/terraform/environments/production/checks.tf b/terraform/environments/production/checks.tf index dfbd31faa..4faeb2d1f 100644 --- a/terraform/environments/production/checks.tf +++ b/terraform/environments/production/checks.tf @@ -20,9 +20,10 @@ resource "terraform_data" "access_control_gate" { condition = ( var.unsafe_allow_all_users || length([for item in split(",", var.allowed_users) : trimspace(item) if trimspace(item) != ""]) > 0 || - length([for item in split(",", var.allowed_email_domains) : trimspace(item) if trimspace(item) != ""]) > 0 + length([for item in split(",", var.allowed_email_domains) : trimspace(item) if trimspace(item) != ""]) > 0 || + length([for item in split(",", var.allowed_github_orgs) : trimspace(item) if trimspace(item) != ""]) > 0 ) - error_message = "At least one access control allowlist must be configured. Set allowed_users or allowed_email_domains, or set unsafe_allow_all_users = true to explicitly allow all authenticated GitHub users." + error_message = "At least one access control allowlist must be configured. Set allowed_users, allowed_email_domains, or allowed_github_orgs, or set unsafe_allow_all_users = true to explicitly allow all authenticated GitHub users." } } } diff --git a/terraform/environments/production/terraform.tfvars.example b/terraform/environments/production/terraform.tfvars.example index 563c82201..0b34d5d8c 100644 --- a/terraform/environments/production/terraform.tfvars.example +++ b/terraform/environments/production/terraform.tfvars.example @@ -280,6 +280,10 @@ allowed_users = "" # Example: "example.com,corp.io" allowed_email_domains = "" +# Comma-separated list of GitHub organizations whose active members can sign in +# Requires GitHub App Organization permissions: Members read-only. +allowed_github_orgs = "" + # Explicitly bypass the Terraform access-control safety check and allow any -# authenticated GitHub user to sign in when both allowlists are empty. +# authenticated GitHub user to sign in when all allowlists are empty. unsafe_allow_all_users = false diff --git a/terraform/environments/production/variables.tf b/terraform/environments/production/variables.tf index 95a55aa51..3cde5039f 100644 --- a/terraform/environments/production/variables.tf +++ b/terraform/environments/production/variables.tf @@ -406,19 +406,25 @@ variable "r2_media_bucket_name" { # ============================================================================= variable "allowed_users" { - description = "Comma-separated list of GitHub usernames allowed to sign in. Leave empty only when allowed_email_domains is set or unsafe_allow_all_users is true." + description = "Comma-separated list of GitHub usernames allowed to sign in. Leave empty only when another allowlist is set or unsafe_allow_all_users is true." type = string default = "" } variable "allowed_email_domains" { - description = "Comma-separated list of email domains allowed to sign in (e.g., 'example.com,corp.io'). Leave empty only when allowed_users is set or unsafe_allow_all_users is true." + description = "Comma-separated list of email domains allowed to sign in (e.g., 'example.com,corp.io'). Leave empty only when another allowlist is set or unsafe_allow_all_users is true." + type = string + default = "" +} + +variable "allowed_github_orgs" { + description = "Comma-separated list of GitHub organization logins whose active members are allowed to sign in. Leave empty only when another allowlist is set or unsafe_allow_all_users is true." type = string default = "" } variable "unsafe_allow_all_users" { - description = "Bypass Terraform's access-control safety check and allow any authenticated GitHub user to sign in when both allowlists are empty. Set to true only for intentionally open deployments." + description = "Bypass Terraform's access-control safety check and allow any authenticated GitHub user to sign in when all allowlists are empty. Set to true only for intentionally open deployments." type = bool default = false } diff --git a/terraform/environments/production/web-cloudflare.tf b/terraform/environments/production/web-cloudflare.tf index e5717a74d..3aad59b71 100644 --- a/terraform/environments/production/web-cloudflare.tf +++ b/terraform/environments/production/web-cloudflare.tf @@ -77,6 +77,7 @@ resource "local_file" "web_app_wrangler_production" { NEXT_PUBLIC_APP_ICON_URL = "${var.app_icon_url}" ALLOWED_USERS = "${var.allowed_users}" ALLOWED_EMAIL_DOMAINS = "${var.allowed_email_domains}" + ALLOWED_GITHUB_ORGS = "${var.allowed_github_orgs}" UNSAFE_ALLOW_ALL_USERS = "${tostring(var.unsafe_allow_all_users)}" [assets] diff --git a/terraform/environments/production/web-vercel.tf b/terraform/environments/production/web-vercel.tf index 98a8f305e..5c796d4e0 100644 --- a/terraform/environments/production/web-vercel.tf +++ b/terraform/environments/production/web-vercel.tf @@ -99,6 +99,12 @@ module "web_app" { targets = ["production", "preview"] sensitive = false }, + { + key = "ALLOWED_GITHUB_ORGS" + value = var.allowed_github_orgs + targets = ["production", "preview"] + sensitive = false + }, { key = "UNSAFE_ALLOW_ALL_USERS" value = tostring(var.unsafe_allow_all_users) From b77b8f81bd8c7cc3e06604169a138b3f992fb27d Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Wed, 20 May 2026 17:09:11 -0400 Subject: [PATCH 2/9] feat: allow GitHub organization members to sign in --- packages/web/src/lib/access-control.test.ts | 135 +++++++++++++++++++- packages/web/src/lib/access-control.ts | 70 +++++++++- packages/web/src/lib/auth.ts | 28 +++- 3 files changed, 221 insertions(+), 12 deletions(-) diff --git a/packages/web/src/lib/access-control.test.ts b/packages/web/src/lib/access-control.test.ts index a4bfb61fd..69c086220 100644 --- a/packages/web/src/lib/access-control.test.ts +++ b/packages/web/src/lib/access-control.test.ts @@ -1,5 +1,10 @@ -import { describe, it, expect } from "vitest"; -import { parseAllowlist, parseBooleanEnv, checkAccessAllowed } from "./access-control"; +import { describe, it, expect, vi } from "vitest"; +import { + parseAllowlist, + parseBooleanEnv, + checkAccessAllowed, + checkGitHubOrganizationAccess, +} from "./access-control"; describe("parseAllowlist", () => { it("returns empty array for undefined", () => { @@ -65,6 +70,28 @@ describe("checkAccessAllowed", () => { }); }); + describe("when allowedOrganizations is set", () => { + const config = { + allowedDomains: [], + allowedUsers: [], + allowedOrganizations: ["Acme"], + unsafeAllowAllUsers: false, + }; + + it("allows users with active org membership", () => { + expect(checkAccessAllowed(config, { activeOrganizations: ["acme"] })).toBe(true); + }); + + it("allows users with different org case", () => { + expect(checkAccessAllowed(config, { activeOrganizations: ["Acme"] })).toBe(true); + }); + + it("denies users without matching active org membership", () => { + expect(checkAccessAllowed(config, { activeOrganizations: ["other"] })).toBe(false); + expect(checkAccessAllowed(config, {})).toBe(false); + }); + }); + describe("when allowedUsers is set", () => { const config = { allowedDomains: [], @@ -157,6 +184,29 @@ describe("checkAccessAllowed", () => { }); }); + describe("when allowedUsers, allowedDomains, and allowedOrganizations are set (OR logic)", () => { + const config = { + allowedDomains: ["company.com"], + allowedUsers: ["specialuser"], + allowedOrganizations: ["acme"], + unsafeAllowAllUsers: false, + }; + + it("allows users matching org membership", () => { + expect(checkAccessAllowed(config, { activeOrganizations: ["acme"] })).toBe(true); + }); + + it("denies users matching none of the configured policies", () => { + expect( + checkAccessAllowed(config, { + githubUsername: "randomuser", + email: "user@other.com", + activeOrganizations: ["other"], + }) + ).toBe(false); + }); + }); + describe("when unsafeAllowAllUsers is true with populated allowlists", () => { const config = { allowedDomains: ["company.com"], @@ -193,3 +243,84 @@ describe("checkAccessAllowed", () => { }); }); }); + +describe("checkGitHubOrganizationAccess", () => { + it("returns true when any configured organization membership is active", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(new Response(JSON.stringify({ state: "pending" }))) + .mockResolvedValueOnce( + new Response(JSON.stringify({ state: "active" })) + ) as unknown as typeof fetch; + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["pending-org", "active-org"], + fetchImpl, + userAgent: "Test App", + }) + ).resolves.toBe(true); + + expect(fetchImpl).toHaveBeenCalledWith( + "https://api.github.com/user/memberships/orgs/active-org", + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: "Bearer token", + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "Test App", + }) as HeadersInit, + }) + ); + }); + + it("returns false for pending membership", async () => { + const fetchImpl = vi.fn(async () => new Response(JSON.stringify({ state: "pending" }))); + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }) + ).resolves.toBe(false); + }); + + it("returns false for denied GitHub responses", async () => { + const fetchImpl = vi.fn(async () => new Response("Not Found", { status: 404 })); + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }) + ).resolves.toBe(false); + }); + + it("returns false without an access token or org allowlist", async () => { + await expect( + checkGitHubOrganizationAccess({ accessToken: undefined, allowedOrganizations: ["acme"] }) + ).resolves.toBe(false); + + await expect( + checkGitHubOrganizationAccess({ accessToken: "token", allowedOrganizations: [] }) + ).resolves.toBe(false); + }); + + it("URL-encodes organization names", async () => { + const fetchImpl = vi.fn(async () => new Response(JSON.stringify({ state: "active" }))); + + await checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme labs"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(fetchImpl).toHaveBeenCalledWith( + "https://api.github.com/user/memberships/orgs/acme%20labs", + expect.any(Object) + ); + }); +}); diff --git a/packages/web/src/lib/access-control.ts b/packages/web/src/lib/access-control.ts index 7e5e1c0f6..6bf2103ce 100644 --- a/packages/web/src/lib/access-control.ts +++ b/packages/web/src/lib/access-control.ts @@ -1,12 +1,21 @@ export interface AccessControlConfig { allowedDomains: string[]; allowedUsers: string[]; + allowedOrganizations?: string[]; unsafeAllowAllUsers: boolean; } export interface AccessCheckParams { githubUsername?: string; email?: string; + activeOrganizations?: string[]; +} + +export interface GitHubOrganizationAccessParams { + accessToken?: string; + allowedOrganizations: string[]; + fetchImpl?: typeof fetch; + userAgent?: string; } /** @@ -28,9 +37,10 @@ export function parseBooleanEnv(value: string | undefined): boolean { * Check if a user is allowed to sign in based on access control configuration. * * Returns true if: - * - Both allowlists are empty and unsafeAllowAllUsers is true + * - All allowlists are empty and unsafeAllowAllUsers is true * - User's GitHub username is in allowedUsers * - User's email domain is in allowedDomains + * - User has active membership in an allowed GitHub organization * * Logic is OR-based: matching either list grants access. */ @@ -39,10 +49,15 @@ export function checkAccessAllowed( params: AccessCheckParams ): boolean { const { allowedDomains, allowedUsers, unsafeAllowAllUsers } = config; - const { githubUsername, email } = params; + const allowedOrganizations = (config.allowedOrganizations ?? []).map((org) => org.toLowerCase()); + const { githubUsername, email, activeOrganizations } = params; // Empty allowlists only permit sign-in when explicitly enabled. - if (allowedDomains.length === 0 && allowedUsers.length === 0) { + if ( + allowedDomains.length === 0 && + allowedUsers.length === 0 && + allowedOrganizations.length === 0 + ) { return unsafeAllowAllUsers; } @@ -59,5 +74,54 @@ export function checkAccessAllowed( } } + // Check GitHub organization membership allowlist + if (activeOrganizations) { + const normalizedActiveOrganizations = activeOrganizations.map((org) => org.toLowerCase()); + if (allowedOrganizations.some((org) => normalizedActiveOrganizations.includes(org))) { + return true; + } + } + return false; } + +/** + * Check if a GitHub user access token belongs to at least one allowed organization. + */ +export async function checkGitHubOrganizationAccess({ + accessToken, + allowedOrganizations, + fetchImpl = fetch, + userAgent = "Open-Inspect", +}: GitHubOrganizationAccessParams): Promise { + if (!accessToken || allowedOrganizations.length === 0) { + return false; + } + + const checks = allowedOrganizations.map(async (org) => { + try { + const response = await fetchImpl( + `https://api.github.com/user/memberships/orgs/${encodeURIComponent(org)}`, + { + headers: { + Authorization: `Bearer ${accessToken}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": userAgent, + }, + } + ); + + if (!response.ok) { + return false; + } + + const membership = (await response.json()) as { state?: string }; + return membership.state === "active"; + } catch { + return false; + } + }); + + return (await Promise.all(checks)).some(Boolean); +} diff --git a/packages/web/src/lib/auth.ts b/packages/web/src/lib/auth.ts index c7441b127..338d3a87f 100644 --- a/packages/web/src/lib/auth.ts +++ b/packages/web/src/lib/auth.ts @@ -1,6 +1,12 @@ import type { NextAuthOptions } from "next-auth"; import GitHubProvider from "next-auth/providers/github"; -import { checkAccessAllowed, parseAllowlist, parseBooleanEnv } from "./access-control"; +import { DEFAULT_APP_NAME } from "@open-inspect/shared"; +import { + checkAccessAllowed, + checkGitHubOrganizationAccess, + parseAllowlist, + parseBooleanEnv, +} from "./access-control"; // Extend NextAuth types to include GitHub-specific user info declare module "next-auth" { @@ -33,29 +39,37 @@ export const authOptions: NextAuthOptions = { clientSecret: process.env.GITHUB_CLIENT_SECRET!, authorization: { params: { - scope: "read:user user:email repo", + scope: "read:user user:email repo read:org", }, }, }), ], callbacks: { - async signIn({ profile, user }) { + async signIn({ account, profile, user }) { const config = { allowedDomains: parseAllowlist(process.env.ALLOWED_EMAIL_DOMAINS), allowedUsers: parseAllowlist(process.env.ALLOWED_USERS), + allowedOrganizations: parseAllowlist(process.env.ALLOWED_GITHUB_ORGS), unsafeAllowAllUsers: parseBooleanEnv(process.env.UNSAFE_ALLOW_ALL_USERS), }; const githubProfile = profile as { login?: string }; - const isAllowed = checkAccessAllowed(config, { + const isAllowedByStaticPolicy = checkAccessAllowed(config, { githubUsername: githubProfile.login, email: user.email ?? undefined, }); - if (!isAllowed) { - return false; + if (isAllowedByStaticPolicy) { + return true; } - return true; + + const isAllowedByOrgMembership = await checkGitHubOrganizationAccess({ + accessToken: account?.access_token, + allowedOrganizations: config.allowedOrganizations, + userAgent: process.env.NEXT_PUBLIC_APP_NAME?.trim() || DEFAULT_APP_NAME, + }); + + return isAllowedByOrgMembership; }, async jwt({ token, account, profile }) { if (account) { From 7b814610c9967db08e3f47188a5d9e052860bafd Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Wed, 20 May 2026 17:09:18 -0400 Subject: [PATCH 3/9] docs: document GitHub organization access control --- README.md | 4 +++- docs/GETTING_STARTED.md | 31 ++++++++++++++++++------------- docs/SETUP_GUIDE.md | 5 ++++- packages/web/.env.example | 4 +++- packages/web/README.md | 12 +++++++----- 5 files changed, 35 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index cdf5e7826..e18007a53 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,9 @@ built for internal use where all employees are trusted and have access to compan web interface 2. **Install GitHub App only on intended repositories** - The App's installation scope defines what the system can access -3. **Use GitHub's repository selection** - When installing the App, select specific repositories +3. **Restrict sign-in** - Configure allowed GitHub users, email domains, or active GitHub + organization membership (`ALLOWED_GITHUB_ORGS`) +4. **Use GitHub's repository selection** - When installing the App, select specific repositories rather than "All repositories" ## Architecture diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index 7d3890716..8e9becab3 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -215,21 +215,23 @@ access. - Issues: **Read & Write** _(required if enabling GitHub bot)_ - Pull requests: **Read & Write** - Metadata: **Read-only** -6. Click **"Create GitHub App"** -7. Note the **App ID** and **Client ID** (top of page) -8. Under **"Client secrets"**, click **"Generate a new client secret"** and note the **Client +6. Set **Organization permissions**: + - Members: **Read-only** _(required when using `ALLOWED_GITHUB_ORGS`)_ +7. Click **"Create GitHub App"** +8. Note the **App ID** and **Client ID** (top of page) +9. Under **"Client secrets"**, click **"Generate a new client secret"** and note the **Client Secret** -9. Scroll down to **"Private keys"** and click **"Generate a private key"** (downloads a .pem file) -10. **Convert the key to PKCS#8 format** (required for Cloudflare Workers): +10. Scroll down to **"Private keys"** and click **"Generate a private key"** (downloads a .pem file) +11. **Convert the key to PKCS#8 format** (required for Cloudflare Workers): ```bash openssl pkcs8 -topk8 -inform PEM -outform PEM -nocrypt \ -in ~/Downloads/your-app-name.*.private-key.pem \ -out private-key-pkcs8.pem ``` -11. **Install the app** on your account/organization: +12. **Install the app** on your account/organization: - Click "Install App" in the sidebar - Select the repositories you want Open-Inspect to access -12. Note the **Installation ID** from the URL after installing: +13. Note the **Installation ID** from the URL after installing: ``` https://github.com/settings/installations/INSTALLATION_ID ``` @@ -416,15 +418,17 @@ enable_service_bindings = false # Access Control (set at least one allowlist for production) allowed_users = "your-github-username" # Comma-separated GitHub usernames, or empty allowed_email_domains = "" # Comma-separated domains (e.g., "example.com,corp.io") +allowed_github_orgs = "" # Comma-separated orgs whose active members can sign in # Explicitly opt into open access only if you want any authenticated GitHub user -# to be able to sign in when both allowlists are empty. +# to be able to sign in when all allowlists are empty. unsafe_allow_all_users = false ``` -> **Note**: Review `allowed_users` and `allowed_email_domains` carefully - these control who can -> sign in. Terraform now fails if both are empty unless you explicitly set -> `unsafe_allow_all_users = true`. +> **Note**: Review `allowed_users`, `allowed_email_domains`, and `allowed_github_orgs` carefully - +> these control who can sign in. Terraform now fails if all are empty unless you explicitly set +> `unsafe_allow_all_users = true`. `allowed_github_orgs` checks active GitHub organization +> membership and requires GitHub App Organization permissions: Members read-only. --- @@ -683,8 +687,9 @@ Go to your fork's Settings → Secrets and variables → Actions, and add: | `INTERNAL_CALLBACK_SECRET` | Generated callback secret | | `MODAL_API_SECRET` | Generated Modal API secret | | `NEXTAUTH_SECRET` | Generated NextAuth secret | -| `ALLOWED_USERS` | Comma-separated GitHub usernames (or empty for all users) | -| `ALLOWED_EMAIL_DOMAINS` | Comma-separated email domains (or empty for all domains) | +| `ALLOWED_USERS` | Comma-separated GitHub usernames allowed to sign in | +| `ALLOWED_EMAIL_DOMAINS` | Comma-separated email domains allowed to sign in | +| `ALLOWED_GITHUB_ORGS` | Comma-separated GitHub orgs whose active members can sign in | | `ENABLE_GITHUB_BOT` | `true` to deploy GitHub bot worker (or empty to skip) | | `GH_WEBHOOK_SECRET` | GitHub webhook secret (required if GitHub bot enabled) | | `GH_BOT_USERNAME` | GitHub App bot username, e.g., `my-app[bot]` (required if GitHub bot enabled) | diff --git a/docs/SETUP_GUIDE.md b/docs/SETUP_GUIDE.md index b64a98efe..b953b116b 100644 --- a/docs/SETUP_GUIDE.md +++ b/docs/SETUP_GUIDE.md @@ -92,6 +92,8 @@ INTERNAL_CALLBACK_SECRET=your_shared_secret # Optional access control ALLOWED_USERS= ALLOWED_EMAIL_DOMAINS= +ALLOWED_GITHUB_ORGS= +UNSAFE_ALLOW_ALL_USERS=false # Optional whitelabel branding (defaults shown). NEXT_PUBLIC_* vars are # inlined into the client bundle at build time — restart `npm run dev` @@ -212,7 +214,8 @@ Your GitHub callback URL does not exactly match the running app URL. ### Access denied after sign-in -Check `ALLOWED_USERS` and `ALLOWED_EMAIL_DOMAINS` in `packages/web/.env.local`. +Check `ALLOWED_USERS`, `ALLOWED_EMAIL_DOMAINS`, and `ALLOWED_GITHUB_ORGS` in +`packages/web/.env.local`. ### Web can load, but session APIs return 401 diff --git a/packages/web/.env.example b/packages/web/.env.example index 5846fe893..d41343b00 100644 --- a/packages/web/.env.example +++ b/packages/web/.env.example @@ -14,6 +14,8 @@ NEXT_PUBLIC_WS_URL=wss://open-inspect-control-plane.YOUR-ACCOUNT.workers.dev # Generate with: openssl rand -base64 32 INTERNAL_CALLBACK_SECRET= -# Access Control (comma-separated, leave empty to allow all GitHub users) +# Access Control (comma-separated; configure at least one allowlist unless UNSAFE_ALLOW_ALL_USERS=true) ALLOWED_EMAIL_DOMAINS= ALLOWED_USERS= +ALLOWED_GITHUB_ORGS= +UNSAFE_ALLOW_ALL_USERS=false diff --git a/packages/web/README.md b/packages/web/README.md index 8ccfccbc1..c73507fb5 100644 --- a/packages/web/README.md +++ b/packages/web/README.md @@ -84,17 +84,19 @@ NEXTAUTH_SECRET=your_random_secret # Generate: openssl rand -base64 32 # Access Control ALLOWED_USERS=username1,username2 # Comma-separated GitHub usernames ALLOWED_EMAIL_DOMAINS=example.com,corp.io # Comma-separated email domains -UNSAFE_ALLOW_ALL_USERS=false # Set true to explicitly allow all users when both lists are empty +ALLOWED_GITHUB_ORGS=acme,umbrella # Comma-separated GitHub orgs with active members allowed +UNSAFE_ALLOW_ALL_USERS=false # Set true to explicitly allow all users when all lists are empty # Control Plane CONTROL_PLANE_URL=http://localhost:8787 NEXT_PUBLIC_WS_URL=ws://localhost:8787 ``` -> **Access Control**: If both `ALLOWED_USERS` and `ALLOWED_EMAIL_DOMAINS` are empty, sign-in is -> denied unless `UNSAFE_ALLOW_ALL_USERS=true`. For Terraform-managed production deploys, Terraform -> also fails validation unless you set at least one allowlist or explicitly opt in with -> `unsafe_allow_all_users = true`. +> **Access Control**: If `ALLOWED_USERS`, `ALLOWED_EMAIL_DOMAINS`, and `ALLOWED_GITHUB_ORGS` are all +> empty, sign-in is denied unless `UNSAFE_ALLOW_ALL_USERS=true`. For Terraform-managed production +> deploys, Terraform also fails validation unless you set at least one allowlist or explicitly opt +> in with `unsafe_allow_all_users = true`. `ALLOWED_GITHUB_ORGS` requires GitHub App Organization +> permissions: Members read-only. ### Development From c31d15dee8a9eccbe50796196f24a73b8943d0db Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Fri, 22 May 2026 09:44:11 -0400 Subject: [PATCH 4/9] fix: request read:org only for org allowlists --- packages/web/src/lib/auth.test.ts | 43 +++++++++++++++++++++++++++++++ packages/web/src/lib/auth.ts | 12 ++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 packages/web/src/lib/auth.test.ts diff --git a/packages/web/src/lib/auth.test.ts b/packages/web/src/lib/auth.test.ts new file mode 100644 index 000000000..92e65a0b6 --- /dev/null +++ b/packages/web/src/lib/auth.test.ts @@ -0,0 +1,43 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@open-inspect/shared", () => ({ + DEFAULT_APP_NAME: "Open-Inspect", +})); + +vi.mock("next-auth/providers/github", () => ({ + default: (config: unknown) => ({ + id: "github", + type: "oauth", + options: config, + }), +})); + +import { BASE_GITHUB_OAUTH_SCOPE, buildGitHubOAuthScope } from "./auth"; + +const ORIGINAL_ENV = { ...process.env }; + +afterEach(() => { + vi.restoreAllMocks(); + for (const key of [ + "ALLOWED_EMAIL_DOMAINS", + "ALLOWED_USERS", + "ALLOWED_GITHUB_ORGS", + "UNSAFE_ALLOW_ALL_USERS", + ]) { + if (ORIGINAL_ENV[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = ORIGINAL_ENV[key]; + } + } +}); + +describe("buildGitHubOAuthScope", () => { + it("requests base scopes when organization access is disabled", () => { + expect(buildGitHubOAuthScope([])).toBe(BASE_GITHUB_OAUTH_SCOPE); + }); + + it("requests read:org only when organization access is configured", () => { + expect(buildGitHubOAuthScope(["acme"])).toBe(`${BASE_GITHUB_OAUTH_SCOPE} read:org`); + }); +}); diff --git a/packages/web/src/lib/auth.ts b/packages/web/src/lib/auth.ts index 338d3a87f..74bcd352b 100644 --- a/packages/web/src/lib/auth.ts +++ b/packages/web/src/lib/auth.ts @@ -31,6 +31,16 @@ declare module "next-auth/jwt" { } } +export const BASE_GITHUB_OAUTH_SCOPE = "read:user user:email repo"; + +export function buildGitHubOAuthScope( + allowedOrganizations = parseAllowlist(process.env.ALLOWED_GITHUB_ORGS) +): string { + return allowedOrganizations.length > 0 + ? `${BASE_GITHUB_OAUTH_SCOPE} read:org` + : BASE_GITHUB_OAUTH_SCOPE; +} + export const authOptions: NextAuthOptions = { debug: process.env.NODE_ENV === "development" || process.env.NEXTAUTH_DEBUG === "true", providers: [ @@ -39,7 +49,7 @@ export const authOptions: NextAuthOptions = { clientSecret: process.env.GITHUB_CLIENT_SECRET!, authorization: { params: { - scope: "read:user user:email repo read:org", + scope: buildGitHubOAuthScope(), }, }, }), From f28f20b79d0fc3f7128f8e7944c255797945607d Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Fri, 22 May 2026 09:44:25 -0400 Subject: [PATCH 5/9] fix: add GitHub org membership diagnostics --- packages/web/src/lib/access-control.test.ts | 137 +++++++++++++++++++- packages/web/src/lib/access-control.ts | 99 ++++++++++++-- packages/web/src/lib/auth.test.ts | 28 +++- packages/web/src/lib/auth.ts | 51 ++++++-- 4 files changed, 288 insertions(+), 27 deletions(-) diff --git a/packages/web/src/lib/access-control.test.ts b/packages/web/src/lib/access-control.test.ts index 69c086220..0aa50ea64 100644 --- a/packages/web/src/lib/access-control.test.ts +++ b/packages/web/src/lib/access-control.test.ts @@ -1,11 +1,17 @@ -import { describe, it, expect, vi } from "vitest"; +import { afterEach, describe, it, expect, vi } from "vitest"; import { parseAllowlist, parseBooleanEnv, checkAccessAllowed, checkGitHubOrganizationAccess, + getAccessAllowReason, } from "./access-control"; +afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); +}); + describe("parseAllowlist", () => { it("returns empty array for undefined", () => { expect(parseAllowlist(undefined)).toEqual([]); @@ -244,8 +250,39 @@ describe("checkAccessAllowed", () => { }); }); +describe("getAccessAllowReason", () => { + it("returns the matching allow reason", () => { + expect( + getAccessAllowReason( + { allowedDomains: [], allowedUsers: ["alice"], unsafeAllowAllUsers: false }, + { githubUsername: "Alice" } + ) + ).toBe("username_allowlist"); + + expect( + getAccessAllowReason( + { allowedDomains: ["company.com"], allowedUsers: [], unsafeAllowAllUsers: false }, + { email: "user@company.com" } + ) + ).toBe("email_domain_allowlist"); + + expect( + getAccessAllowReason( + { + allowedDomains: [], + allowedUsers: [], + allowedOrganizations: ["acme"], + unsafeAllowAllUsers: false, + }, + { activeOrganizations: ["Acme"] } + ) + ).toBe("org_membership"); + }); +}); + describe("checkGitHubOrganizationAccess", () => { it("returns true when any configured organization membership is active", async () => { + vi.spyOn(console, "info").mockImplementation(() => {}); const fetchImpl = vi .fn() .mockResolvedValueOnce(new Response(JSON.stringify({ state: "pending" }))) @@ -275,7 +312,22 @@ describe("checkGitHubOrganizationAccess", () => { ); }); + it("returns early after the first active membership", async () => { + const fetchImpl = vi.fn(async () => new Response(JSON.stringify({ state: "active" }))); + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["active-org", "other-org"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }) + ).resolves.toBe(true); + + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + it("returns false for pending membership", async () => { + const info = vi.spyOn(console, "info").mockImplementation(() => {}); const fetchImpl = vi.fn(async () => new Response(JSON.stringify({ state: "pending" }))); await expect( @@ -285,9 +337,15 @@ describe("checkGitHubOrganizationAccess", () => { fetchImpl: fetchImpl as unknown as typeof fetch, }) ).resolves.toBe(false); + + expect(info).toHaveBeenCalledWith("[github-org-access] membership not active", { + org: "acme", + state: "pending", + }); }); it("returns false for denied GitHub responses", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); const fetchImpl = vi.fn(async () => new Response("Not Found", { status: 404 })); await expect( @@ -297,9 +355,17 @@ describe("checkGitHubOrganizationAccess", () => { fetchImpl: fetchImpl as unknown as typeof fetch, }) ).resolves.toBe(false); + + expect(warn).toHaveBeenCalledWith("[github-org-access] membership request failed", { + org: "acme", + status: 404, + hint: expect.any(String), + }); }); it("returns false without an access token or org allowlist", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + await expect( checkGitHubOrganizationAccess({ accessToken: undefined, allowedOrganizations: ["acme"] }) ).resolves.toBe(false); @@ -307,6 +373,11 @@ describe("checkGitHubOrganizationAccess", () => { await expect( checkGitHubOrganizationAccess({ accessToken: "token", allowedOrganizations: [] }) ).resolves.toBe(false); + + expect(warn).toHaveBeenCalledWith("[github-org-access] membership check skipped", { + reason: "missing_access_token", + organizationCount: 1, + }); }); it("URL-encodes organization names", async () => { @@ -323,4 +394,68 @@ describe("checkGitHubOrganizationAccess", () => { expect.any(Object) ); }); + + it("logs missing membership state", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => new Response(JSON.stringify({ state: null }))); + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }) + ).resolves.toBe(false); + + expect(warn).toHaveBeenCalledWith("[github-org-access] membership response missing state", { + org: "acme", + state: null, + }); + }); + + it("logs unexpected membership state", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => new Response(JSON.stringify({ state: "unknown" }))); + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }) + ).resolves.toBe(false); + + expect(warn).toHaveBeenCalledWith("[github-org-access] membership response unexpected state", { + org: "acme", + state: "unknown", + }); + }); + + it("aborts timed out membership requests", async () => { + vi.useFakeTimers(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn( + (_url, init) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => { + reject(new DOMException("Aborted", "AbortError")); + }); + }) + ) as unknown as typeof fetch; + + const result = checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme"], + fetchImpl, + timeoutMs: 50, + }); + + await vi.advanceTimersByTimeAsync(50); + await expect(result).resolves.toBe(false); + expect(warn).toHaveBeenCalledWith("[github-org-access] membership request error", { + org: "acme", + error: "AbortError", + message: "Aborted", + }); + }); }); diff --git a/packages/web/src/lib/access-control.ts b/packages/web/src/lib/access-control.ts index 6bf2103ce..1f9694fe2 100644 --- a/packages/web/src/lib/access-control.ts +++ b/packages/web/src/lib/access-control.ts @@ -16,8 +16,17 @@ export interface GitHubOrganizationAccessParams { allowedOrganizations: string[]; fetchImpl?: typeof fetch; userAgent?: string; + timeoutMs?: number; } +export const GITHUB_MEMBERSHIP_CHECK_TIMEOUT_MS = 10_000; + +export type AccessAllowReason = + | "unsafe_allow_all" + | "username_allowlist" + | "email_domain_allowlist" + | "org_membership"; + /** * Parse comma-separated environment variable into a lowercase, trimmed array */ @@ -48,6 +57,13 @@ export function checkAccessAllowed( config: AccessControlConfig, params: AccessCheckParams ): boolean { + return getAccessAllowReason(config, params) !== null; +} + +export function getAccessAllowReason( + config: AccessControlConfig, + params: AccessCheckParams +): AccessAllowReason | null { const { allowedDomains, allowedUsers, unsafeAllowAllUsers } = config; const allowedOrganizations = (config.allowedOrganizations ?? []).map((org) => org.toLowerCase()); const { githubUsername, email, activeOrganizations } = params; @@ -58,19 +74,19 @@ export function checkAccessAllowed( allowedUsers.length === 0 && allowedOrganizations.length === 0 ) { - return unsafeAllowAllUsers; + return unsafeAllowAllUsers ? "unsafe_allow_all" : null; } // Check explicit user allowlist (GitHub username) if (githubUsername && allowedUsers.includes(githubUsername.toLowerCase())) { - return true; + return "username_allowlist"; } // Check email domain allowlist if (email) { const domain = email.toLowerCase().split("@")[1]; if (domain && allowedDomains.includes(domain)) { - return true; + return "email_domain_allowlist"; } } @@ -78,11 +94,11 @@ export function checkAccessAllowed( if (activeOrganizations) { const normalizedActiveOrganizations = activeOrganizations.map((org) => org.toLowerCase()); if (allowedOrganizations.some((org) => normalizedActiveOrganizations.includes(org))) { - return true; + return "org_membership"; } } - return false; + return null; } /** @@ -93,12 +109,24 @@ export async function checkGitHubOrganizationAccess({ allowedOrganizations, fetchImpl = fetch, userAgent = "Open-Inspect", + timeoutMs = GITHUB_MEMBERSHIP_CHECK_TIMEOUT_MS, }: GitHubOrganizationAccessParams): Promise { - if (!accessToken || allowedOrganizations.length === 0) { + if (allowedOrganizations.length === 0) { return false; } - const checks = allowedOrganizations.map(async (org) => { + if (!accessToken) { + console.warn("[github-org-access] membership check skipped", { + reason: "missing_access_token", + organizationCount: allowedOrganizations.length, + }); + return false; + } + + for (const org of allowedOrganizations) { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), timeoutMs); + try { const response = await fetchImpl( `https://api.github.com/user/memberships/orgs/${encodeURIComponent(org)}`, @@ -109,19 +137,62 @@ export async function checkGitHubOrganizationAccess({ "X-GitHub-Api-Version": "2022-11-28", "User-Agent": userAgent, }, + signal: controller.signal, } ); if (!response.ok) { - return false; + console.warn("[github-org-access] membership request failed", { + org, + status: response.status, + hint: getGitHubMembershipFailureHint(response.status), + }); + continue; } - const membership = (await response.json()) as { state?: string }; - return membership.state === "active"; - } catch { - return false; + const membership = (await response.json()) as { state?: string | null }; + if (membership.state === "active") { + return true; + } + + if (membership.state == null) { + console.warn("[github-org-access] membership response missing state", { + org, + state: membership.state ?? null, + }); + } else if (membership.state === "pending") { + console.info("[github-org-access] membership not active", { + org, + state: membership.state, + }); + } else { + console.warn("[github-org-access] membership response unexpected state", { + org, + state: membership.state, + }); + } + } catch (error) { + console.warn("[github-org-access] membership request error", { + org, + error: error instanceof Error ? error.name : "unknown", + message: error instanceof Error ? error.message : String(error), + }); + } finally { + clearTimeout(timeout); } - }); + } + + return false; +} + +function getGitHubMembershipFailureHint(status: number): string | undefined { + if (status === 403) { + return "Verify the GitHub OAuth token has read:org access and any organization SAML requirements are satisfied. If this deployment also uses a GitHub App, make sure membership read permission changes were republished and approved."; + } + + if (status === 404) { + return "GitHub returns 404 when the user is not an organization member or the token cannot read that membership."; + } - return (await Promise.all(checks)).some(Boolean); + return undefined; } diff --git a/packages/web/src/lib/auth.test.ts b/packages/web/src/lib/auth.test.ts index 92e65a0b6..a3bfad0b2 100644 --- a/packages/web/src/lib/auth.test.ts +++ b/packages/web/src/lib/auth.test.ts @@ -12,7 +12,7 @@ vi.mock("next-auth/providers/github", () => ({ }), })); -import { BASE_GITHUB_OAUTH_SCOPE, buildGitHubOAuthScope } from "./auth"; +import { authOptions, BASE_GITHUB_OAUTH_SCOPE, buildGitHubOAuthScope } from "./auth"; const ORIGINAL_ENV = { ...process.env }; @@ -41,3 +41,29 @@ describe("buildGitHubOAuthScope", () => { expect(buildGitHubOAuthScope(["acme"])).toBe(`${BASE_GITHUB_OAUTH_SCOPE} read:org`); }); }); + +describe("authOptions signIn", () => { + it("logs static allow decisions without sensitive token data", async () => { + process.env.ALLOWED_EMAIL_DOMAINS = ""; + process.env.ALLOWED_USERS = "alice"; + process.env.ALLOWED_GITHUB_ORGS = ""; + process.env.UNSAFE_ALLOW_ALL_USERS = ""; + const info = vi.spyOn(console, "info").mockImplementation(() => {}); + const signIn = authOptions.callbacks?.signIn; + + await expect( + signIn?.({ + account: { access_token: "secret-token" }, + profile: { login: "Alice" }, + user: { email: "alice@example.com" }, + } as never) + ).resolves.toBe(true); + + expect(info).toHaveBeenCalledWith("[auth] sign-in decision", { + login: "Alice", + decision: "allow", + reason: "username_allowlist", + }); + expect(JSON.stringify(info.mock.calls)).not.toContain("secret-token"); + }); +}); diff --git a/packages/web/src/lib/auth.ts b/packages/web/src/lib/auth.ts index 74bcd352b..39f7d77f5 100644 --- a/packages/web/src/lib/auth.ts +++ b/packages/web/src/lib/auth.ts @@ -2,8 +2,9 @@ import type { NextAuthOptions } from "next-auth"; import GitHubProvider from "next-auth/providers/github"; import { DEFAULT_APP_NAME } from "@open-inspect/shared"; import { - checkAccessAllowed, + type AccessControlConfig, checkGitHubOrganizationAccess, + getAccessAllowReason, parseAllowlist, parseBooleanEnv, } from "./access-control"; @@ -41,6 +42,27 @@ export function buildGitHubOAuthScope( : BASE_GITHUB_OAUTH_SCOPE; } +function getAccessControlConfig(): AccessControlConfig { + return { + allowedDomains: parseAllowlist(process.env.ALLOWED_EMAIL_DOMAINS), + allowedUsers: parseAllowlist(process.env.ALLOWED_USERS), + allowedOrganizations: parseAllowlist(process.env.ALLOWED_GITHUB_ORGS), + unsafeAllowAllUsers: parseBooleanEnv(process.env.UNSAFE_ALLOW_ALL_USERS), + }; +} + +function logSignInDecision( + login: string | undefined, + decision: "allow" | "deny", + reason: string +): void { + console.info("[auth] sign-in decision", { + login: login ?? null, + decision, + reason, + }); +} + export const authOptions: NextAuthOptions = { debug: process.env.NODE_ENV === "development" || process.env.NEXTAUTH_DEBUG === "true", providers: [ @@ -56,29 +78,36 @@ export const authOptions: NextAuthOptions = { ], callbacks: { async signIn({ account, profile, user }) { - const config = { - allowedDomains: parseAllowlist(process.env.ALLOWED_EMAIL_DOMAINS), - allowedUsers: parseAllowlist(process.env.ALLOWED_USERS), - allowedOrganizations: parseAllowlist(process.env.ALLOWED_GITHUB_ORGS), - unsafeAllowAllUsers: parseBooleanEnv(process.env.UNSAFE_ALLOW_ALL_USERS), - }; - + const config = getAccessControlConfig(); + const allowedOrganizations = config.allowedOrganizations ?? []; const githubProfile = profile as { login?: string }; - const isAllowedByStaticPolicy = checkAccessAllowed(config, { + const staticAllowReason = getAccessAllowReason(config, { githubUsername: githubProfile.login, email: user.email ?? undefined, }); - if (isAllowedByStaticPolicy) { + if (staticAllowReason) { + logSignInDecision(githubProfile.login, "allow", staticAllowReason); return true; } + if (allowedOrganizations.length === 0) { + logSignInDecision(githubProfile.login, "deny", "no_matching_policy"); + return false; + } + const isAllowedByOrgMembership = await checkGitHubOrganizationAccess({ accessToken: account?.access_token, - allowedOrganizations: config.allowedOrganizations, + allowedOrganizations, userAgent: process.env.NEXT_PUBLIC_APP_NAME?.trim() || DEFAULT_APP_NAME, }); + logSignInDecision( + githubProfile.login, + isAllowedByOrgMembership ? "allow" : "deny", + isAllowedByOrgMembership ? "org_membership" : "org_membership_denied" + ); + return isAllowedByOrgMembership; }, async jwt({ token, account, profile }) { From 64a4471d8ec5cf8abad3b4976ada8fa875fe80da Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Fri, 22 May 2026 09:44:31 -0400 Subject: [PATCH 6/9] ci: pass GitHub org allowlist to Terraform --- .github/workflows/terraform.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/terraform.yml b/.github/workflows/terraform.yml index 1fd2b432b..44e4c7537 100644 --- a/.github/workflows/terraform.yml +++ b/.github/workflows/terraform.yml @@ -204,6 +204,7 @@ jobs: TF_VAR_modal_api_secret: ${{ secrets.MODAL_API_SECRET }} TF_VAR_allowed_users: ${{ secrets.ALLOWED_USERS }} TF_VAR_allowed_email_domains: ${{ secrets.ALLOWED_EMAIL_DOMAINS }} + TF_VAR_allowed_github_orgs: ${{ secrets.ALLOWED_GITHUB_ORGS }} TF_VAR_deployment_name: ${{ secrets.DEPLOYMENT_NAME }} TF_VAR_enable_github_bot: "${{ secrets.ENABLE_GITHUB_BOT || 'false' }}" TF_VAR_github_webhook_secret: ${{ secrets.GH_WEBHOOK_SECRET }} @@ -335,6 +336,7 @@ jobs: TF_VAR_modal_api_secret: ${{ secrets.MODAL_API_SECRET }} TF_VAR_allowed_users: ${{ secrets.ALLOWED_USERS }} TF_VAR_allowed_email_domains: ${{ secrets.ALLOWED_EMAIL_DOMAINS }} + TF_VAR_allowed_github_orgs: ${{ secrets.ALLOWED_GITHUB_ORGS }} TF_VAR_deployment_name: ${{ secrets.DEPLOYMENT_NAME }} TF_VAR_enable_github_bot: "${{ secrets.ENABLE_GITHUB_BOT || 'false' }}" TF_VAR_github_webhook_secret: ${{ secrets.GH_WEBHOOK_SECRET }} From 07a834bb01b9065e9292de8de774f8008d350b26 Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Fri, 22 May 2026 09:44:35 -0400 Subject: [PATCH 7/9] docs: clarify GitHub org access rollout --- docs/GETTING_STARTED.md | 13 +++++++++---- docs/SETUP_GUIDE.md | 8 +++++++- packages/web/.env.example | 4 ++++ packages/web/README.md | 8 ++++++-- .../production/terraform.tfvars.example | 7 ++++++- 5 files changed, 32 insertions(+), 8 deletions(-) diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index 8e9becab3..8ef011bd8 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -215,8 +215,10 @@ access. - Issues: **Read & Write** _(required if enabling GitHub bot)_ - Pull requests: **Read & Write** - Metadata: **Read-only** -6. Set **Organization permissions**: - - Members: **Read-only** _(required when using `ALLOWED_GITHUB_ORGS`)_ +6. If using `ALLOWED_GITHUB_ORGS`/`allowed_github_orgs`, set **Organization permissions**: + - Members: **Read-only** + - For existing GitHub Apps, republish the permission change and request/approve installation + updates before testing org membership sign-in. 7. Click **"Create GitHub App"** 8. Note the **App ID** and **Client ID** (top of page) 9. Under **"Client secrets"**, click **"Generate a new client secret"** and note the **Client @@ -427,8 +429,11 @@ unsafe_allow_all_users = false > **Note**: Review `allowed_users`, `allowed_email_domains`, and `allowed_github_orgs` carefully - > these control who can sign in. Terraform now fails if all are empty unless you explicitly set -> `unsafe_allow_all_users = true`. `allowed_github_orgs` checks active GitHub organization -> membership and requires GitHub App Organization permissions: Members read-only. +> `unsafe_allow_all_users = true`. **Allowlists use OR semantics**: matching any configured +> username, email domain, or active GitHub org membership grants access. `allowed_github_orgs` +> checks membership at sign-in only with the signing-in user's OAuth token; existing sessions last +> until session expiry. The `read:org` OAuth scope is requested only when org access is configured. +> GitHub Apps using org access need Organization permissions: Members read-only. --- diff --git a/docs/SETUP_GUIDE.md b/docs/SETUP_GUIDE.md index b953b116b..5c50772bc 100644 --- a/docs/SETUP_GUIDE.md +++ b/docs/SETUP_GUIDE.md @@ -90,6 +90,10 @@ NEXT_PUBLIC_WS_URL=wss://open-inspect-control-plane-..workers.d INTERNAL_CALLBACK_SECRET=your_shared_secret # Optional access control +# Allowlists are OR-based: matching any configured user, email domain, or GitHub org grants access. +# ALLOWED_GITHUB_ORGS requests read:org only when set, then checks active org membership +# with the signing-in user's OAuth token. Existing sessions last until session expiry. +# Requires GitHub App Organization permissions: Members read-only. ALLOWED_USERS= ALLOWED_EMAIL_DOMAINS= ALLOWED_GITHUB_ORGS= @@ -215,7 +219,9 @@ Your GitHub callback URL does not exactly match the running app URL. ### Access denied after sign-in Check `ALLOWED_USERS`, `ALLOWED_EMAIL_DOMAINS`, and `ALLOWED_GITHUB_ORGS` in -`packages/web/.env.local`. +`packages/web/.env.local`. If `ALLOWED_GITHUB_ORGS` is set, make sure your GitHub App has +Organization permissions: Members read-only and that the updated permission was republished and +approved for the installation. ### Web can load, but session APIs return 401 diff --git a/packages/web/.env.example b/packages/web/.env.example index d41343b00..fbe749d6f 100644 --- a/packages/web/.env.example +++ b/packages/web/.env.example @@ -15,6 +15,10 @@ NEXT_PUBLIC_WS_URL=wss://open-inspect-control-plane.YOUR-ACCOUNT.workers.dev INTERNAL_CALLBACK_SECRET= # Access Control (comma-separated; configure at least one allowlist unless UNSAFE_ALLOW_ALL_USERS=true) +# **OR-based**: matching any configured user, email domain, or GitHub org grants access. +# ALLOWED_GITHUB_ORGS requests read:org only when set, then checks active org membership +# with the signing-in user's OAuth token. Existing sessions last until session expiry. +# Requires GitHub App Organization permissions: Members read-only. ALLOWED_EMAIL_DOMAINS= ALLOWED_USERS= ALLOWED_GITHUB_ORGS= diff --git a/packages/web/README.md b/packages/web/README.md index c73507fb5..3224bcea0 100644 --- a/packages/web/README.md +++ b/packages/web/README.md @@ -95,8 +95,12 @@ NEXT_PUBLIC_WS_URL=ws://localhost:8787 > **Access Control**: If `ALLOWED_USERS`, `ALLOWED_EMAIL_DOMAINS`, and `ALLOWED_GITHUB_ORGS` are all > empty, sign-in is denied unless `UNSAFE_ALLOW_ALL_USERS=true`. For Terraform-managed production > deploys, Terraform also fails validation unless you set at least one allowlist or explicitly opt -> in with `unsafe_allow_all_users = true`. `ALLOWED_GITHUB_ORGS` requires GitHub App Organization -> permissions: Members read-only. +> in with `unsafe_allow_all_users = true`. **Allowlists use OR semantics**: matching any configured +> username, email domain, or active GitHub org membership grants access. `ALLOWED_GITHUB_ORGS` is +> checked at sign-in with the signing-in user's OAuth token; existing sessions last until session +> expiry. The `read:org` OAuth scope is requested only when `ALLOWED_GITHUB_ORGS` is configured. +> GitHub Apps using org access need Organization permissions: Members read-only; existing GitHub +> Apps must republish/request approval after that permission changes. ### Development diff --git a/terraform/environments/production/terraform.tfvars.example b/terraform/environments/production/terraform.tfvars.example index 0b34d5d8c..c1d153d40 100644 --- a/terraform/environments/production/terraform.tfvars.example +++ b/terraform/environments/production/terraform.tfvars.example @@ -271,6 +271,8 @@ enable_service_bindings = false # ============================================================================= # Access Control # ============================================================================= +# Allowlists are OR-based: matching any configured user, email domain, or GitHub +# org grants access. # Comma-separated list of GitHub usernames allowed to sign in # Set at least one access-control allowlist unless you intentionally opt into # open access with unsafe_allow_all_users = true. @@ -281,7 +283,10 @@ allowed_users = "" allowed_email_domains = "" # Comma-separated list of GitHub organizations whose active members can sign in -# Requires GitHub App Organization permissions: Members read-only. +# Requests read:org only when set, then checks membership at sign-in with the +# signing-in user's OAuth token. Existing sessions last until session expiry. +# Requires GitHub App Organization permissions: Members read-only; existing apps +# must republish/request approval after this permission changes. allowed_github_orgs = "" # Explicitly bypass the Terraform access-control safety check and allow any From ac396b60761dd55b3721af20d9c3efddbbe5a9ab Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Fri, 22 May 2026 10:09:05 -0400 Subject: [PATCH 8/9] test: cover GitHub org auth boundary --- packages/web/src/lib/access-control.test.ts | 13 ++ packages/web/src/lib/auth.test.ts | 203 +++++++++++++++++--- 2 files changed, 194 insertions(+), 22 deletions(-) diff --git a/packages/web/src/lib/access-control.test.ts b/packages/web/src/lib/access-control.test.ts index 0aa50ea64..819d5310f 100644 --- a/packages/web/src/lib/access-control.test.ts +++ b/packages/web/src/lib/access-control.test.ts @@ -229,6 +229,19 @@ describe("checkAccessAllowed", () => { expect(checkAccessAllowed(config, { githubUsername: "randomuser" })).toBe(false); expect(checkAccessAllowed(config, { email: "user@other.com" })).toBe(false); }); + + it("does not bypass populated organization allowlists", () => { + const orgConfig = { + allowedDomains: [], + allowedUsers: [], + allowedOrganizations: ["acme"], + unsafeAllowAllUsers: true, + }; + + expect(checkAccessAllowed(orgConfig, {})).toBe(false); + expect(checkAccessAllowed(orgConfig, { activeOrganizations: ["other"] })).toBe(false); + expect(checkAccessAllowed(orgConfig, { activeOrganizations: ["acme"] })).toBe(true); + }); }); describe("multiple values in allowlists", () => { diff --git a/packages/web/src/lib/auth.test.ts b/packages/web/src/lib/auth.test.ts index a3bfad0b2..887195e20 100644 --- a/packages/web/src/lib/auth.test.ts +++ b/packages/web/src/lib/auth.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; +import type { NextAuthOptions } from "next-auth"; vi.mock("@open-inspect/shared", () => ({ DEFAULT_APP_NAME: "Open-Inspect", @@ -12,47 +13,55 @@ vi.mock("next-auth/providers/github", () => ({ }), })); -import { authOptions, BASE_GITHUB_OAUTH_SCOPE, buildGitHubOAuthScope } from "./auth"; - const ORIGINAL_ENV = { ...process.env }; afterEach(() => { vi.restoreAllMocks(); - for (const key of [ - "ALLOWED_EMAIL_DOMAINS", - "ALLOWED_USERS", - "ALLOWED_GITHUB_ORGS", - "UNSAFE_ALLOW_ALL_USERS", - ]) { - if (ORIGINAL_ENV[key] === undefined) { - delete process.env[key]; - } else { - process.env[key] = ORIGINAL_ENV[key]; - } - } + vi.unstubAllGlobals(); + resetAuthEnv(); }); describe("buildGitHubOAuthScope", () => { - it("requests base scopes when organization access is disabled", () => { + it("requests base scopes when organization access is disabled", async () => { + const { BASE_GITHUB_OAUTH_SCOPE, buildGitHubOAuthScope } = await importAuthModule(); + expect(buildGitHubOAuthScope([])).toBe(BASE_GITHUB_OAUTH_SCOPE); }); - it("requests read:org only when organization access is configured", () => { + it("requests read:org only when organization access is configured", async () => { + const { BASE_GITHUB_OAUTH_SCOPE, buildGitHubOAuthScope } = await importAuthModule(); + expect(buildGitHubOAuthScope(["acme"])).toBe(`${BASE_GITHUB_OAUTH_SCOPE} read:org`); }); }); +describe("GitHub provider scope", () => { + it("omits read:org when organization access is disabled", async () => { + const { authOptions, BASE_GITHUB_OAUTH_SCOPE } = await importAuthModule({ + ALLOWED_GITHUB_ORGS: "", + }); + + expect(getGitHubProviderScope(authOptions)).toBe(BASE_GITHUB_OAUTH_SCOPE); + }); + + it("includes read:org when organization access is configured", async () => { + const { authOptions, BASE_GITHUB_OAUTH_SCOPE } = await importAuthModule({ + ALLOWED_GITHUB_ORGS: "acme", + }); + + expect(getGitHubProviderScope(authOptions)).toBe(`${BASE_GITHUB_OAUTH_SCOPE} read:org`); + }); +}); + describe("authOptions signIn", () => { it("logs static allow decisions without sensitive token data", async () => { - process.env.ALLOWED_EMAIL_DOMAINS = ""; - process.env.ALLOWED_USERS = "alice"; - process.env.ALLOWED_GITHUB_ORGS = ""; - process.env.UNSAFE_ALLOW_ALL_USERS = ""; + const { authOptions } = await importAuthModule({ + ALLOWED_USERS: "alice", + }); const info = vi.spyOn(console, "info").mockImplementation(() => {}); - const signIn = authOptions.callbacks?.signIn; await expect( - signIn?.({ + getSignIn(authOptions)({ account: { access_token: "secret-token" }, profile: { login: "Alice" }, user: { email: "alice@example.com" }, @@ -66,4 +75,154 @@ describe("authOptions signIn", () => { }); expect(JSON.stringify(info.mock.calls)).not.toContain("secret-token"); }); + + it("checks configured organization membership with the OAuth access token", async () => { + const { authOptions } = await importAuthModule({ + ALLOWED_GITHUB_ORGS: "acme", + NEXT_PUBLIC_APP_NAME: "Test App", + }); + const info = vi.spyOn(console, "info").mockImplementation(() => {}); + const fetchImpl = vi.fn( + async () => new Response(JSON.stringify({ state: "active" })) + ) as unknown as typeof fetch; + vi.stubGlobal("fetch", fetchImpl); + + await expect( + getSignIn(authOptions)({ + account: { access_token: "oauth-token" }, + profile: { login: "member" }, + user: { email: "member@example.com" }, + } as never) + ).resolves.toBe(true); + + expect(fetchImpl).toHaveBeenCalledWith( + "https://api.github.com/user/memberships/orgs/acme", + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: "Bearer oauth-token", + "User-Agent": "Test App", + }) as HeadersInit, + }) + ); + expect(info).toHaveBeenCalledWith("[auth] sign-in decision", { + login: "member", + decision: "allow", + reason: "org_membership", + }); + }); + + it("denies organization access when the OAuth access token is missing", async () => { + const { authOptions } = await importAuthModule({ + ALLOWED_GITHUB_ORGS: "acme", + }); + const info = vi.spyOn(console, "info").mockImplementation(() => {}); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn() as unknown as typeof fetch; + vi.stubGlobal("fetch", fetchImpl); + + await expect( + getSignIn(authOptions)({ + account: {}, + profile: { login: "member" }, + user: { email: "member@example.com" }, + } as never) + ).resolves.toBe(false); + + expect(fetchImpl).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith("[github-org-access] membership check skipped", { + reason: "missing_access_token", + organizationCount: 1, + }); + expect(info).toHaveBeenCalledWith("[auth] sign-in decision", { + login: "member", + decision: "deny", + reason: "org_membership_denied", + }); + }); + + it.each([ + ["404 response", () => new Response("Not Found", { status: 404 })], + ["pending membership", () => new Response(JSON.stringify({ state: "pending" }))], + ])("denies organization access for %s", async (_label, responseFactory) => { + const { authOptions } = await importAuthModule({ + ALLOWED_GITHUB_ORGS: "acme", + }); + vi.spyOn(console, "info").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => responseFactory()) as unknown as typeof fetch; + vi.stubGlobal("fetch", fetchImpl); + + await expect( + getSignIn(authOptions)({ + account: { access_token: "oauth-token" }, + profile: { login: "member" }, + user: { email: "member@example.com" }, + } as never) + ).resolves.toBe(false); + }); + + it("does not let unsafe open access bypass configured org allowlists", async () => { + const { authOptions } = await importAuthModule({ + ALLOWED_GITHUB_ORGS: "acme", + UNSAFE_ALLOW_ALL_USERS: "true", + }); + vi.spyOn(console, "info").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => new Response("Not Found", { status: 404 })); + vi.stubGlobal("fetch", fetchImpl); + + await expect( + getSignIn(authOptions)({ + account: { access_token: "oauth-token" }, + profile: { login: "outsider" }, + user: { email: "outsider@example.com" }, + } as never) + ).resolves.toBe(false); + }); }); + +async function importAuthModule(env: Record = {}) { + vi.resetModules(); + resetAuthEnv(); + for (const [key, value] of Object.entries(env)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + + return import("./auth"); +} + +function resetAuthEnv(): void { + for (const key of [ + "ALLOWED_EMAIL_DOMAINS", + "ALLOWED_USERS", + "ALLOWED_GITHUB_ORGS", + "UNSAFE_ALLOW_ALL_USERS", + "NEXT_PUBLIC_APP_NAME", + ]) { + if (ORIGINAL_ENV[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = ORIGINAL_ENV[key]; + } + } +} + +function getGitHubProviderScope(authOptions: NextAuthOptions): string { + const provider = authOptions.providers[0] as { + options: { authorization: { params: { scope: string } } }; + }; + return provider.options.authorization.params.scope; +} + +function getSignIn(authOptions: NextAuthOptions) { + const signIn = authOptions.callbacks?.signIn; + if (!signIn) { + throw new Error("signIn callback is not configured"); + } + + return signIn; +} From 8f01c4a73cdf870da50c73656796d9cabcab5a25 Mon Sep 17 00:00:00 2001 From: Scott Watermasysk Date: Fri, 22 May 2026 10:17:05 -0400 Subject: [PATCH 9/9] fix: distinguish unavailable GitHub org checks --- packages/web/src/lib/access-control.test.ts | 159 ++++++++++++++++---- packages/web/src/lib/access-control.ts | 66 +++++++- packages/web/src/lib/auth.test.ts | 36 ++++- packages/web/src/lib/auth.ts | 20 ++- 4 files changed, 238 insertions(+), 43 deletions(-) diff --git a/packages/web/src/lib/access-control.test.ts b/packages/web/src/lib/access-control.test.ts index 819d5310f..ee872810e 100644 --- a/packages/web/src/lib/access-control.test.ts +++ b/packages/web/src/lib/access-control.test.ts @@ -310,7 +310,7 @@ describe("checkGitHubOrganizationAccess", () => { fetchImpl, userAgent: "Test App", }) - ).resolves.toBe(true); + ).resolves.toEqual({ allowed: true, reason: "active_membership", organization: "active-org" }); expect(fetchImpl).toHaveBeenCalledWith( "https://api.github.com/user/memberships/orgs/active-org", @@ -334,7 +334,7 @@ describe("checkGitHubOrganizationAccess", () => { allowedOrganizations: ["active-org", "other-org"], fetchImpl: fetchImpl as unknown as typeof fetch, }) - ).resolves.toBe(true); + ).resolves.toEqual({ allowed: true, reason: "active_membership", organization: "active-org" }); expect(fetchImpl).toHaveBeenCalledTimes(1); }); @@ -349,17 +349,32 @@ describe("checkGitHubOrganizationAccess", () => { allowedOrganizations: ["acme"], fetchImpl: fetchImpl as unknown as typeof fetch, }) - ).resolves.toBe(false); + ).resolves.toEqual({ allowed: false, reason: "not_member" }); - expect(info).toHaveBeenCalledWith("[github-org-access] membership not active", { - org: "acme", - state: "pending", - }); + expect(info).toHaveBeenCalledWith( + "[github-org-access] membership not active", + expect.objectContaining({ + org: "acme", + state: "pending", + elapsedMs: expect.any(Number), + }) + ); }); - it("returns false for denied GitHub responses", async () => { + it("returns not_member for denied GitHub responses", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - const fetchImpl = vi.fn(async () => new Response("Not Found", { status: 404 })); + const fetchImpl = vi.fn( + async () => + new Response("Not Found", { + status: 404, + headers: { + "x-github-request-id": "github-request-id", + "x-ratelimit-limit": "60", + "x-ratelimit-remaining": "59", + "x-ratelimit-reset": "1710000000", + }, + }) + ); await expect( checkGitHubOrganizationAccess({ @@ -367,13 +382,56 @@ describe("checkGitHubOrganizationAccess", () => { allowedOrganizations: ["acme"], fetchImpl: fetchImpl as unknown as typeof fetch, }) - ).resolves.toBe(false); + ).resolves.toEqual({ allowed: false, reason: "not_member" }); - expect(warn).toHaveBeenCalledWith("[github-org-access] membership request failed", { - org: "acme", - status: 404, - hint: expect.any(String), - }); + expect(warn).toHaveBeenCalledWith( + "[github-org-access] membership request failed", + expect.objectContaining({ + org: "acme", + status: 404, + requestId: "github-request-id", + rateLimitLimit: "60", + rateLimitRemaining: "59", + rateLimitReset: "1710000000", + elapsedMs: expect.any(Number), + hint: expect.any(String), + }) + ); + }); + + it("returns unavailable for operational GitHub responses", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn( + async () => + new Response("rate limited", { + status: 429, + headers: { + "x-github-request-id": "github-request-id", + "x-ratelimit-remaining": "0", + "retry-after": "30", + }, + }) + ); + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }) + ).resolves.toEqual({ allowed: false, reason: "unavailable" }); + + expect(warn).toHaveBeenCalledWith( + "[github-org-access] membership request failed", + expect.objectContaining({ + org: "acme", + status: 429, + requestId: "github-request-id", + rateLimitRemaining: "0", + retryAfter: "30", + elapsedMs: expect.any(Number), + }) + ); }); it("returns false without an access token or org allowlist", async () => { @@ -381,11 +439,11 @@ describe("checkGitHubOrganizationAccess", () => { await expect( checkGitHubOrganizationAccess({ accessToken: undefined, allowedOrganizations: ["acme"] }) - ).resolves.toBe(false); + ).resolves.toEqual({ allowed: false, reason: "unavailable" }); await expect( checkGitHubOrganizationAccess({ accessToken: "token", allowedOrganizations: [] }) - ).resolves.toBe(false); + ).resolves.toEqual({ allowed: false, reason: "not_member" }); expect(warn).toHaveBeenCalledWith("[github-org-access] membership check skipped", { reason: "missing_access_token", @@ -418,12 +476,16 @@ describe("checkGitHubOrganizationAccess", () => { allowedOrganizations: ["acme"], fetchImpl: fetchImpl as unknown as typeof fetch, }) - ).resolves.toBe(false); + ).resolves.toEqual({ allowed: false, reason: "unavailable" }); - expect(warn).toHaveBeenCalledWith("[github-org-access] membership response missing state", { - org: "acme", - state: null, - }); + expect(warn).toHaveBeenCalledWith( + "[github-org-access] membership response missing state", + expect.objectContaining({ + org: "acme", + state: null, + elapsedMs: expect.any(Number), + }) + ); }); it("logs unexpected membership state", async () => { @@ -436,12 +498,39 @@ describe("checkGitHubOrganizationAccess", () => { allowedOrganizations: ["acme"], fetchImpl: fetchImpl as unknown as typeof fetch, }) - ).resolves.toBe(false); + ).resolves.toEqual({ allowed: false, reason: "unavailable" }); - expect(warn).toHaveBeenCalledWith("[github-org-access] membership response unexpected state", { - org: "acme", - state: "unknown", - }); + expect(warn).toHaveBeenCalledWith( + "[github-org-access] membership response unexpected state", + expect.objectContaining({ + org: "acme", + state: "unknown", + elapsedMs: expect.any(Number), + }) + ); + }); + + it("returns unavailable for malformed membership responses", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => new Response("not-json")); + + await expect( + checkGitHubOrganizationAccess({ + accessToken: "token", + allowedOrganizations: ["acme"], + fetchImpl: fetchImpl as unknown as typeof fetch, + }) + ).resolves.toEqual({ allowed: false, reason: "unavailable" }); + + expect(warn).toHaveBeenCalledWith( + "[github-org-access] membership request error", + expect.objectContaining({ + org: "acme", + error: expect.any(String), + message: expect.any(String), + elapsedMs: expect.any(Number), + }) + ); }); it("aborts timed out membership requests", async () => { @@ -464,11 +553,15 @@ describe("checkGitHubOrganizationAccess", () => { }); await vi.advanceTimersByTimeAsync(50); - await expect(result).resolves.toBe(false); - expect(warn).toHaveBeenCalledWith("[github-org-access] membership request error", { - org: "acme", - error: "AbortError", - message: "Aborted", - }); + await expect(result).resolves.toEqual({ allowed: false, reason: "unavailable" }); + expect(warn).toHaveBeenCalledWith( + "[github-org-access] membership request error", + expect.objectContaining({ + org: "acme", + error: "AbortError", + message: "Aborted", + elapsedMs: expect.any(Number), + }) + ); }); }); diff --git a/packages/web/src/lib/access-control.ts b/packages/web/src/lib/access-control.ts index 1f9694fe2..128fab299 100644 --- a/packages/web/src/lib/access-control.ts +++ b/packages/web/src/lib/access-control.ts @@ -21,6 +21,17 @@ export interface GitHubOrganizationAccessParams { export const GITHUB_MEMBERSHIP_CHECK_TIMEOUT_MS = 10_000; +export type GitHubOrganizationAccessResult = + | { + allowed: true; + reason: "active_membership"; + organization: string; + } + | { + allowed: false; + reason: "not_member" | "unavailable"; + }; + export type AccessAllowReason = | "unsafe_allow_all" | "username_allowlist" @@ -110,9 +121,9 @@ export async function checkGitHubOrganizationAccess({ fetchImpl = fetch, userAgent = "Open-Inspect", timeoutMs = GITHUB_MEMBERSHIP_CHECK_TIMEOUT_MS, -}: GitHubOrganizationAccessParams): Promise { +}: GitHubOrganizationAccessParams): Promise { if (allowedOrganizations.length === 0) { - return false; + return { allowed: false, reason: "not_member" }; } if (!accessToken) { @@ -120,12 +131,15 @@ export async function checkGitHubOrganizationAccess({ reason: "missing_access_token", organizationCount: allowedOrganizations.length, }); - return false; + return { allowed: false, reason: "unavailable" }; } + let isUnavailable = false; + for (const org of allowedOrganizations) { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); + const startedAt = performance.now(); try { const response = await fetchImpl( @@ -145,54 +159,96 @@ export async function checkGitHubOrganizationAccess({ console.warn("[github-org-access] membership request failed", { org, status: response.status, + ...getGitHubResponseDiagnostics(response, startedAt), hint: getGitHubMembershipFailureHint(response.status), }); + if (isGitHubMembershipUnavailableStatus(response.status)) { + isUnavailable = true; + } continue; } const membership = (await response.json()) as { state?: string | null }; if (membership.state === "active") { - return true; + return { allowed: true, reason: "active_membership", organization: org }; } if (membership.state == null) { + isUnavailable = true; console.warn("[github-org-access] membership response missing state", { org, state: membership.state ?? null, + ...getGitHubResponseDiagnostics(response, startedAt), }); } else if (membership.state === "pending") { console.info("[github-org-access] membership not active", { org, state: membership.state, + ...getGitHubResponseDiagnostics(response, startedAt), }); } else { + isUnavailable = true; console.warn("[github-org-access] membership response unexpected state", { org, state: membership.state, + ...getGitHubResponseDiagnostics(response, startedAt), }); } } catch (error) { + isUnavailable = true; console.warn("[github-org-access] membership request error", { org, error: error instanceof Error ? error.name : "unknown", message: error instanceof Error ? error.message : String(error), + elapsedMs: getElapsedMs(startedAt), }); } finally { clearTimeout(timeout); } } - return false; + return { allowed: false, reason: isUnavailable ? "unavailable" : "not_member" }; } function getGitHubMembershipFailureHint(status: number): string | undefined { + if (status === 401) { + return "GitHub rejected the OAuth token while checking organization membership."; + } + if (status === 403) { return "Verify the GitHub OAuth token has read:org access and any organization SAML requirements are satisfied. If this deployment also uses a GitHub App, make sure membership read permission changes were republished and approved."; } + if (status === 429) { + return "GitHub rate limited the organization membership check."; + } + if (status === 404) { return "GitHub returns 404 when the user is not an organization member or the token cannot read that membership."; } + if (status >= 500) { + return "GitHub returned a server error while checking organization membership."; + } + return undefined; } + +function isGitHubMembershipUnavailableStatus(status: number): boolean { + return status !== 404; +} + +function getGitHubResponseDiagnostics(response: Response, startedAt: number) { + return { + requestId: response.headers.get("x-github-request-id"), + rateLimitLimit: response.headers.get("x-ratelimit-limit"), + rateLimitRemaining: response.headers.get("x-ratelimit-remaining"), + rateLimitReset: response.headers.get("x-ratelimit-reset"), + retryAfter: response.headers.get("retry-after"), + elapsedMs: getElapsedMs(startedAt), + }; +} + +function getElapsedMs(startedAt: number): number { + return Math.round(performance.now() - startedAt); +} diff --git a/packages/web/src/lib/auth.test.ts b/packages/web/src/lib/auth.test.ts index 887195e20..71b39148a 100644 --- a/packages/web/src/lib/auth.test.ts +++ b/packages/web/src/lib/auth.test.ts @@ -136,7 +136,7 @@ describe("authOptions signIn", () => { expect(info).toHaveBeenCalledWith("[auth] sign-in decision", { login: "member", decision: "deny", - reason: "org_membership_denied", + reason: "org_membership_unavailable", }); }); @@ -161,6 +161,40 @@ describe("authOptions signIn", () => { ).resolves.toBe(false); }); + it.each([ + ["429 response", () => new Response("Rate Limited", { status: 429 })], + ["server error", () => new Response("Server Error", { status: 500 })], + [ + "network error", + () => { + throw new TypeError("fetch failed"); + }, + ], + ["malformed JSON", () => new Response("not-json")], + ])("reports organization verification unavailable for %s", async (_label, responseFactory) => { + const { authOptions } = await importAuthModule({ + ALLOWED_GITHUB_ORGS: "acme", + }); + const info = vi.spyOn(console, "info").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => responseFactory()) as unknown as typeof fetch; + vi.stubGlobal("fetch", fetchImpl); + + await expect( + getSignIn(authOptions)({ + account: { access_token: "oauth-token" }, + profile: { login: "member" }, + user: { email: "member@example.com" }, + } as never) + ).resolves.toBe(false); + + expect(info).toHaveBeenCalledWith("[auth] sign-in decision", { + login: "member", + decision: "deny", + reason: "org_membership_unavailable", + }); + }); + it("does not let unsafe open access bypass configured org allowlists", async () => { const { authOptions } = await importAuthModule({ ALLOWED_GITHUB_ORGS: "acme", diff --git a/packages/web/src/lib/auth.ts b/packages/web/src/lib/auth.ts index 39f7d77f5..bd765240c 100644 --- a/packages/web/src/lib/auth.ts +++ b/packages/web/src/lib/auth.ts @@ -96,7 +96,7 @@ export const authOptions: NextAuthOptions = { return false; } - const isAllowedByOrgMembership = await checkGitHubOrganizationAccess({ + const orgMembership = await checkGitHubOrganizationAccess({ accessToken: account?.access_token, allowedOrganizations, userAgent: process.env.NEXT_PUBLIC_APP_NAME?.trim() || DEFAULT_APP_NAME, @@ -104,11 +104,11 @@ export const authOptions: NextAuthOptions = { logSignInDecision( githubProfile.login, - isAllowedByOrgMembership ? "allow" : "deny", - isAllowedByOrgMembership ? "org_membership" : "org_membership_denied" + orgMembership.allowed ? "allow" : "deny", + getOrgMembershipDecisionReason(orgMembership) ); - return isAllowedByOrgMembership; + return orgMembership.allowed; }, async jwt({ token, account, profile }) { if (account) { @@ -141,3 +141,15 @@ export const authOptions: NextAuthOptions = { error: "/access-denied", }, }; + +function getOrgMembershipDecisionReason( + orgMembership: Awaited> +): string { + if (orgMembership.allowed) { + return "org_membership"; + } + + return orgMembership.reason === "unavailable" + ? "org_membership_unavailable" + : "org_membership_denied"; +}