diff --git a/.git-hooks-binary-allowlist b/.git-hooks-binary-allowlist index 4dad983f8..82b1aab9c 100644 --- a/.git-hooks-binary-allowlist +++ b/.git-hooks-binary-allowlist @@ -69,3 +69,5 @@ docs/releases/assets/v1.1.0-audit-log.png assets/screenshots/2026-10-29/01-settings-sso-oidc.png assets/screenshots/2026-10-29/02-settings-sso-saml.png assets/screenshots/2026-10-29/03-login-sso-button.png +assets/screenshots/2026-10-29/04-settings-sso-scim-baseline.png +assets/screenshots/2026-10-29/05-settings-sso-reconciliation-scim-token.png diff --git a/assets/screenshots/2026-10-29/04-settings-sso-scim-baseline.png b/assets/screenshots/2026-10-29/04-settings-sso-scim-baseline.png new file mode 100644 index 000000000..c40707a8d Binary files /dev/null and b/assets/screenshots/2026-10-29/04-settings-sso-scim-baseline.png differ diff --git a/assets/screenshots/2026-10-29/05-settings-sso-reconciliation-scim-token.png b/assets/screenshots/2026-10-29/05-settings-sso-reconciliation-scim-token.png new file mode 100644 index 000000000..dd844e068 Binary files /dev/null and b/assets/screenshots/2026-10-29/05-settings-sso-reconciliation-scim-token.png differ diff --git a/config/plan_catalog.json b/config/plan_catalog.json index 6480e0b71..e709a10e1 100644 --- a/config/plan_catalog.json +++ b/config/plan_catalog.json @@ -123,7 +123,8 @@ "feature_flags": [ "unlimited_reviews", "hosted_auto_model", - "sso" + "sso", + "scim" ], "trial_policy": { "enabled": false, diff --git a/db/migrations/20261029150000_create_scim_tokens.sql b/db/migrations/20261029150000_create_scim_tokens.sql new file mode 100644 index 000000000..07b007971 --- /dev/null +++ b/db/migrations/20261029150000_create_scim_tokens.sql @@ -0,0 +1,23 @@ +-- migrate:up + +-- SCIM clients (Okta, Azure AD, Google Workspace) authenticate with a single +-- static bearer token per org, per the SCIM 2.0 RFC - there is no user +-- session involved. Modeled directly on api_keys' hashing scheme +-- (internal/api/api_keys.go: GenerateAPIKey/HashAPIKey/GetKeyPrefix), but +-- kept as its own table rather than reusing api_keys since a SCIM token is +-- org-bound with no owning user. +CREATE TABLE IF NOT EXISTS scim_tokens ( + id BIGSERIAL PRIMARY KEY, + org_id BIGINT NOT NULL REFERENCES orgs(id) ON DELETE CASCADE, + token_hash VARCHAR(64) NOT NULL UNIQUE, + token_prefix VARCHAR(16) NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + expires_at TIMESTAMPTZ, + revoked_at TIMESTAMPTZ, + created_by_user_id BIGINT REFERENCES users(id) ON DELETE SET NULL +); + +CREATE INDEX IF NOT EXISTS idx_scim_tokens_org_id ON scim_tokens(org_id); + +-- migrate:down +DROP TABLE IF EXISTS scim_tokens; diff --git a/db/migrations/20261029150100_create_sso_identities.sql b/db/migrations/20261029150100_create_sso_identities.sql new file mode 100644 index 000000000..7970a6508 --- /dev/null +++ b/db/migrations/20261029150100_create_sso_identities.sql @@ -0,0 +1,42 @@ +-- migrate:up + +-- Tracks which IdP subject (OIDC `sub` / SAML NameID) maps to which +-- LiveReview user, per org and provider. Needed so an incoming OIDC +-- back-channel logout token or SAML LogoutRequest - which only carries the +-- IdP's own subject identifier, not a LiveReview user id - can be resolved +-- to a user whose sessions should be revoked. Also holds the encrypted OIDC +-- refresh token used by the optional reconciliation poll (see +-- org_sso_configs.reconciliation_enabled) to silently re-check "is this +-- account still valid at the IdP" between logins. +CREATE TABLE IF NOT EXISTS sso_identities ( + id BIGSERIAL PRIMARY KEY, + org_id BIGINT NOT NULL REFERENCES orgs(id) ON DELETE CASCADE, + user_id BIGINT NOT NULL REFERENCES users(id) ON DELETE CASCADE, + provider_type VARCHAR(16) NOT NULL CHECK (provider_type IN ('oidc', 'saml')), + subject VARCHAR(512) NOT NULL, + oidc_refresh_token_enc TEXT, + last_checked_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + UNIQUE (org_id, provider_type, subject) +); + +CREATE INDEX IF NOT EXISTS idx_sso_identities_user_id ON sso_identities(user_id); + +CREATE OR REPLACE FUNCTION set_sso_identities_updated_at() +RETURNS TRIGGER AS $$ +BEGIN + NEW.updated_at = NOW(); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER trg_sso_identities_updated_at + BEFORE UPDATE ON sso_identities + FOR EACH ROW + EXECUTE FUNCTION set_sso_identities_updated_at(); + +-- migrate:down +DROP TRIGGER IF EXISTS trg_sso_identities_updated_at ON sso_identities; +DROP FUNCTION IF EXISTS set_sso_identities_updated_at(); +DROP TABLE IF EXISTS sso_identities; diff --git a/db/migrations/20261029150200_add_sso_reconciliation_columns.sql b/db/migrations/20261029150200_add_sso_reconciliation_columns.sql new file mode 100644 index 000000000..47ca39f13 --- /dev/null +++ b/db/migrations/20261029150200_add_sso_reconciliation_columns.sql @@ -0,0 +1,17 @@ +-- migrate:up + +-- Fallback deprovisioning for orgs that configure OIDC login but never wire +-- up SCIM at their IdP (a separate integration step many admins skip). Only +-- meaningful for provider_type='oidc' - SAML has no standing credential to +-- silently re-check, so the settings UI only exposes this toggle for OIDC. +ALTER TABLE org_sso_configs + ADD COLUMN IF NOT EXISTS reconciliation_enabled BOOLEAN NOT NULL DEFAULT FALSE, + ADD COLUMN IF NOT EXISTS reconciliation_interval TEXT NOT NULL DEFAULT 'daily' + CHECK (reconciliation_interval IN ('hourly', 'daily', 'weekly')), + ADD COLUMN IF NOT EXISTS last_reconciled_at TIMESTAMPTZ; + +-- migrate:down +ALTER TABLE org_sso_configs + DROP COLUMN IF EXISTS reconciliation_enabled, + DROP COLUMN IF EXISTS reconciliation_interval, + DROP COLUMN IF EXISTS last_reconciled_at; diff --git a/db/schema.sql b/db/schema.sql index 894fa428b..294a1b7d7 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -302,6 +302,20 @@ CREATE FUNCTION public.river_job_state_in_bitmask(bitmask bit, state public.rive $$; +-- +-- Name: set_sso_identities_updated_at(); Type: FUNCTION; Schema: public; Owner: - +-- + +CREATE FUNCTION public.set_sso_identities_updated_at() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN + NEW.updated_at = NOW(); + RETURN NEW; +END; +$$; + + -- -- Name: trial_eligibility_set_updated_at(); Type: FUNCTION; Schema: public; Owner: - -- @@ -2133,7 +2147,11 @@ CREATE TABLE public.org_sso_configs ( created_at timestamp with time zone DEFAULT now() NOT NULL, updated_at timestamp with time zone DEFAULT now() NOT NULL, created_by_user_id bigint, - CONSTRAINT org_sso_configs_provider_type_check CHECK (((provider_type)::text = ANY ((ARRAY['oidc'::character varying, 'saml'::character varying])::text[]))) + reconciliation_enabled boolean DEFAULT false NOT NULL, + reconciliation_interval text DEFAULT 'daily'::text NOT NULL, + last_reconciled_at timestamp with time zone, + CONSTRAINT org_sso_configs_provider_type_check CHECK (((provider_type)::text = ANY ((ARRAY['oidc'::character varying, 'saml'::character varying])::text[]))), + CONSTRAINT org_sso_configs_reconciliation_interval_check CHECK ((reconciliation_interval = ANY (ARRAY['hourly'::text, 'daily'::text, 'weekly'::text]))) ); @@ -3428,6 +3446,78 @@ CREATE TABLE public.schema_migrations ( ); +-- +-- Name: scim_tokens; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.scim_tokens ( + id bigint NOT NULL, + org_id bigint NOT NULL, + token_hash character varying(64) NOT NULL, + token_prefix character varying(16) NOT NULL, + created_at timestamp with time zone DEFAULT now() NOT NULL, + expires_at timestamp with time zone, + revoked_at timestamp with time zone, + created_by_user_id bigint +); + + +-- +-- Name: scim_tokens_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.scim_tokens_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: scim_tokens_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.scim_tokens_id_seq OWNED BY public.scim_tokens.id; + + +-- +-- Name: sso_identities; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.sso_identities ( + id bigint NOT NULL, + org_id bigint NOT NULL, + user_id bigint NOT NULL, + provider_type character varying(16) NOT NULL, + subject character varying(512) NOT NULL, + oidc_refresh_token_enc text, + last_checked_at timestamp with time zone, + created_at timestamp with time zone DEFAULT now() NOT NULL, + updated_at timestamp with time zone DEFAULT now() NOT NULL, + CONSTRAINT sso_identities_provider_type_check CHECK (((provider_type)::text = ANY ((ARRAY['oidc'::character varying, 'saml'::character varying])::text[]))) +); + + +-- +-- Name: sso_identities_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.sso_identities_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: sso_identities_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.sso_identities_id_seq OWNED BY public.sso_identities.id; + + -- -- Name: subscription_payments; Type: TABLE; Schema: public; Owner: - -- @@ -4520,6 +4610,20 @@ ALTER TABLE ONLY public.scheduled_review_configs ALTER COLUMN id SET DEFAULT nex ALTER TABLE ONLY public.scheduled_review_runs ALTER COLUMN id SET DEFAULT nextval('public.scheduled_review_runs_id_seq'::regclass); +-- +-- Name: scim_tokens id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.scim_tokens ALTER COLUMN id SET DEFAULT nextval('public.scim_tokens_id_seq'::regclass); + + +-- +-- Name: sso_identities id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.sso_identities ALTER COLUMN id SET DEFAULT nextval('public.sso_identities_id_seq'::regclass); + + -- -- Name: subscription_payments id; Type: DEFAULT; Schema: public; Owner: - -- @@ -5340,6 +5444,38 @@ ALTER TABLE ONLY public.schema_migrations ADD CONSTRAINT schema_migrations_pkey PRIMARY KEY (version); +-- +-- Name: scim_tokens scim_tokens_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.scim_tokens + ADD CONSTRAINT scim_tokens_pkey PRIMARY KEY (id); + + +-- +-- Name: scim_tokens scim_tokens_token_hash_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.scim_tokens + ADD CONSTRAINT scim_tokens_token_hash_key UNIQUE (token_hash); + + +-- +-- Name: sso_identities sso_identities_org_id_provider_type_subject_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.sso_identities + ADD CONSTRAINT sso_identities_org_id_provider_type_subject_key UNIQUE (org_id, provider_type, subject); + + +-- +-- Name: sso_identities sso_identities_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.sso_identities + ADD CONSTRAINT sso_identities_pkey PRIMARY KEY (id); + + -- -- Name: subscription_payments subscription_payments_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -7598,6 +7734,20 @@ CREATE INDEX idx_scheduled_review_configs_org_id ON public.scheduled_review_conf CREATE INDEX idx_scheduled_review_runs_config_id ON public.scheduled_review_runs USING btree (config_id, started_at DESC); +-- +-- Name: idx_scim_tokens_org_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX idx_scim_tokens_org_id ON public.scim_tokens USING btree (org_id); + + +-- +-- Name: idx_sso_identities_user_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX idx_sso_identities_user_id ON public.sso_identities USING btree (user_id); + + -- -- Name: idx_subscription_payments_captured; Type: INDEX; Schema: public; Owner: - -- @@ -9180,6 +9330,13 @@ CREATE TRIGGER trg_quota_operation_aggregates_updated_at BEFORE UPDATE ON public CREATE TRIGGER trg_quota_policy_catalog_updated_at BEFORE UPDATE ON public.quota_policy_catalog FOR EACH ROW EXECUTE FUNCTION public.quota_policy_catalog_set_updated_at(); +-- +-- Name: sso_identities trg_sso_identities_updated_at; Type: TRIGGER; Schema: public; Owner: - +-- + +CREATE TRIGGER trg_sso_identities_updated_at BEFORE UPDATE ON public.sso_identities FOR EACH ROW EXECUTE FUNCTION public.set_sso_identities_updated_at(); + + -- -- Name: trial_eligibility trg_trial_eligibility_updated_at; Type: TRIGGER; Schema: public; Owner: - -- @@ -9899,6 +10056,38 @@ ALTER TABLE ONLY public.scheduled_review_runs ADD CONSTRAINT scheduled_review_runs_review_id_fkey FOREIGN KEY (review_id) REFERENCES public.reviews(id) ON DELETE SET NULL; +-- +-- Name: scim_tokens scim_tokens_created_by_user_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.scim_tokens + ADD CONSTRAINT scim_tokens_created_by_user_id_fkey FOREIGN KEY (created_by_user_id) REFERENCES public.users(id) ON DELETE SET NULL; + + +-- +-- Name: scim_tokens scim_tokens_org_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.scim_tokens + ADD CONSTRAINT scim_tokens_org_id_fkey FOREIGN KEY (org_id) REFERENCES public.orgs(id) ON DELETE CASCADE; + + +-- +-- Name: sso_identities sso_identities_org_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.sso_identities + ADD CONSTRAINT sso_identities_org_id_fkey FOREIGN KEY (org_id) REFERENCES public.orgs(id) ON DELETE CASCADE; + + +-- +-- Name: sso_identities sso_identities_user_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.sso_identities + ADD CONSTRAINT sso_identities_user_id_fkey FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE; + + -- -- Name: subscription_payments subscription_payments_subscription_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -10301,4 +10490,7 @@ INSERT INTO public.schema_migrations (version) VALUES ('20261025120100'), ('20261025130000'), ('20261029104047'), - ('20261029134500'); + ('20261029134500'), + ('20261029150000'), + ('20261029150100'), + ('20261029150200'); diff --git a/docs/audit-events.md b/docs/audit-events.md index 3ce3bc80b..0d02adbbd 100644 --- a/docs/audit-events.md +++ b/docs/audit-events.md @@ -58,6 +58,7 @@ Generated from `internal/auditlog/catalog.go`. | `auth.logout` | `auth` | session | info | Signed out | A user ended a session. | | `auth.password.change` | `auth` | user | warning | Password changed | A user changed their password. | | `auth.password.reset.complete` | `auth` | user | warning | Password reset completed | A user completed a required password reset. | +| `auth.session.revoke` | `auth` | session | warning | Sessions revoked | A user's active sessions and refresh tokens were revoked, typically following deactivation. | | `auth.sso.provision` | `auth` | user | info | SSO account provisioned | An account was provisioned through SSO. | | `auth.token.refresh` | `auth` | session | info | Session refreshed | A session token was refreshed. | | `auto_approve.policy.create` | `settings` | policy | warning | Auto-approve policy created | An auto-approve policy was created. | @@ -106,6 +107,12 @@ Generated from `internal/auditlog/catalog.go`. | `ruleset.create` | `repository` | ruleset | warning | Ruleset created | A CI ruleset was created. | | `ruleset.delete` | `repository` | ruleset | warning | Ruleset deleted | A CI ruleset was deleted. | | `ruleset.update` | `repository` | ruleset | warning | Ruleset updated | A CI ruleset was changed. | +| `scim.token.create` | `auth` | scim_token | warning | SCIM token created | An org's SCIM bearer token was generated or rotated. | +| `scim.token.revoke` | `auth` | scim_token | warning | SCIM token revoked | An org's SCIM bearer token was revoked. | +| `scim.user.create` | `auth` | user | info | SCIM user provisioned | An identity provider created or linked a user via SCIM. | +| `scim.user.deactivate` | `auth` | user | warning | SCIM user deactivated | An identity provider deactivated a user via SCIM. | +| `scim.user.reactivate` | `auth` | user | warning | SCIM user reactivated | An identity provider reactivated a user via SCIM. | +| `scim.user.update` | `auth` | user | info | SCIM user updated | An identity provider updated a user's attributes via SCIM. | | `settings.auto_approve.update` | `settings` | setting | warning | Auto-approve updated | The auto-approve policy was changed. | | `settings.managed_ai.update` | `settings` | setting | critical | Managed AI updated | LiveReview-managed AI configuration was changed. | | `settings.prompts.update` | `settings` | prompt | warning | Prompts updated | Review prompts were changed. | diff --git a/docs/sso.md b/docs/sso.md index 902faa32f..2e9412f9c 100644 --- a/docs/sso.md +++ b/docs/sso.md @@ -373,3 +373,146 @@ and the four pre-merge security scanners (`make security-osv`, `security-gitleaks` no leaks. `go test ./...`, `go test ./internal/auditlog/...`, and `make test-sso-e2e` (both protocols) all re-verified green after every fix in this phase. + +--- + +## Deprovisioning & lifecycle sync (follow-up, 2026-10-29) + +The phases above cover provisioning only (JIT create/login). An audit found +zero offboarding story: SSO login silently re-activated deactivated users, +deactivation never revoked a live JWT session, there was no automated +deprovisioning (SCIM), and no real-time session kill from the IdP. This +section closes those gaps. + +### Phase 7 — Reactivation fix + session revocation — ✅ DONE +- [x] `jitProvisionSSOUser` (`internal/api/auth/sso_handlers.go`) now checks + `is_active` before linking an existing user into an org; a deactivated + user's SSO login is rejected (`errSSOAccountDeactivated` → + `account_deactivated` failure redirect) instead of being silently + flipped back to active. The old unconditional + `UPDATE users SET ... is_active = TRUE` is gone. +- [x] `POST /api/v1/orgs/:org_id/users/:user_id/reactivate` (new + `ReactivateUser` handler + `UserService.ReactivateUserInOrg`) - the + only path back to `is_active=true` now, since SSO deliberately never + does this itself. +- [x] `UserService.DeactivateUserInOrg` now revokes every live `auth_tokens` + row for that user in the same transaction as the deactivation - + without this, a fired user's already-issued JWT kept working until + its own expiry regardless of admin action. New `ActionSessionRevoke` + (`auth.session.revoke`) audit action. +- [x] `TokenService.ValidateAccessToken` now filters `is_active = true` on + the user lookup, as defense in depth (matching + `APIKeyManager.ValidateAPIKey`'s existing behavior) - belt-and-suspenders + in case a future deactivation path forgets to revoke tokens directly. +- [x] Regression set green: `go test ./...` (pre-existing, unrelated + failures only - confirmed via `git status` that none of the failing + packages were touched), `go test ./internal/auditlog/...` clean. + +### Phase 8 — SCIM 2.0 provisioning/deprovisioning — ✅ DONE +- [x] New `internal/scim` package: bearer-token hashing/store + (`token.go`, modeled on `internal/api/api_keys.go`'s scheme) + minimal + SCIM 2.0 protocol types (`types.go`) covering what Okta/Azure + AD/Google actually use for lifecycle sync. +- [x] `scim_tokens` table (`db/migrations/20261029150000_create_scim_tokens.sql`) + - one static bearer token per org, no user session involved, per the + SCIM RFC. +- [x] `internal/api/scim_handlers.go`: `GET/POST /scim/v2/Users`, + `GET/PATCH /scim/v2/Users/:id`, authenticated via + `requireSCIMBearerAuth` (resolves `org_id` from the token itself, + never a client-supplied id) and gated on the new `"scim"` plan feature + (`internal/license/plans.go` + `config/plan_catalog.json`, same + pattern as `"sso"`). `PATCH active:false` → `DeactivateUserInOrgBySCIM` + (new `UserService` method - same deactivate+revoke-tokens logic as + Phase 7, but with `deactivated_by_user_id = NULL` since the actor is + the IdP, not a LiveReview user); `active:true` → `ReactivateUserInOrgBySCIM`. + Create uses the new `UserService.ProvisionSCIMUser`, which mirrors the + SSO JIT-create path (random unusable password, member role) rather + than the owner-driven `CreateUserInOrg`, since SCIM provisioning must + never send an invitation email. +- [x] Org-scoped, session-authenticated token management: + `GET/POST/DELETE /api/v1/orgs/:org_id/scim-token` (generate, rotate, + revoke - plaintext shown once, same reveal-once convention as an API + key), gated the same way as `/sso-config`. +- [x] New audit actions `scim.user.create/update/deactivate/reactivate`, + `scim.token.create/revoke` in `internal/auditlog/catalog.go` + + `docs/audit-events.md`. SCIM-triggered events use + `auditRecorder.Webhook(...)` (no user session to attribute them to - + same category as a provider webhook). +- [x] Settings UI: SCIM Provisioning panel at the bottom of + `SSOSettingsTab.tsx` - generate/rotate/revoke the token, shows the + SCIM base URL to paste into the IdP's SCIM app config. +- [x] `npx tsc --noEmit` clean. Build clean, `go test ./internal/auditlog/...` + clean. + +### Phase 9 — OIDC back-channel logout / SAML SLO — PARTIAL (OIDC done, SAML not shipped) +- [x] `sso_identities` table + (`db/migrations/20261029150100_create_sso_identities.sql`): maps + `(org_id, provider_type, subject)` → LiveReview `user_id`, populated + on every successful login (`internal/sso/identity_store.go`, + `IdentityStore.Upsert` called from `completeSSOLogin`). Needed + because an incoming logout event only ever carries the IdP's own + subject, never a LiveReview user id. +- [x] `POST /api/v1/auth/sso/oidc/:org_id/backchannel-logout` + (`SSOOIDCBackChannelLogout`): verifies the IdP's `logout_token` JWT + the same way an ID token is verified (`sso.VerifyBackChannelLogoutToken` + - same issuer/signature/audience/expiry checks via `go-oidc`, plus the + OIDC Back-Channel Logout 1.0 `events` claim check), resolves the + subject via `sso_identities`, and revokes that user's `auth_tokens` + immediately. Per spec, always responds `200` once the token verifies, + even if there's nothing to revoke, so the endpoint never leaks account + existence. +- [ ] **SAML Single Logout (SLO) was not implemented.** `crewjam/saml` + v0.5.1 only exposes signature verification for the full + `ParseResponse`/assertion flow; it has no public primitive for + verifying a *signed, IdP-initiated* `LogoutRequest` on its own. Hand-rolling + XML-dsig verification for this one message type was judged too high-risk + to ship quickly and correctly - accepting an unverified "logout" request + would let anyone force-logout a known subject without proving they're + the IdP. SAML orgs rely on Phases 7/8 (manual deactivation, SCIM) and + normal JWT expiry for this gap; revisit if a later `crewjam/saml` + release (or a vetted standalone XML-dsig verifier) exposes a safe + primitive for inbound `LogoutRequest` verification. +- [x] Build clean; `go test ./internal/sso/...` and related packages pass + (no e2e back-channel-logout test added yet - would need a Keycloak + realm configured to actually send back-channel logout tokens, which + isn't wired into the existing `test/fixtures/keycloak/` fixture). + +### Phase 10 — Reconciliation poll (fallback for orgs without SCIM) — ✅ DONE +- [x] `org_sso_configs` gains `reconciliation_enabled`, + `reconciliation_interval` (`hourly`/`daily`/`weekly`, default `daily`), + `last_reconciled_at` + (`db/migrations/20261029150200_add_sso_reconciliation_columns.sql`) - + OIDC-only; SAML has no standing credential to silently re-check, so + the UI only shows this toggle on the OIDC panel. +- [x] When enabled, the OIDC auth flow requests the `offline_access` scope + (`sso.BuildOIDCOAuth2Config`) and `sso_identities` stores the + encrypted refresh token per identity (same AES-GCM box as the client + secret). +- [x] `internal/jobqueue/sso_reconciliation_worker.go` + (`SSOReconciliationWorker`, registered as a 15-minute `river.PeriodicJob` + named `sso_reconciliation`): queries + `sso.Store.ListDueForReconciliation()` for configs whose interval has + elapsed, and for each, attempts a silent refresh-token exchange for + every stored identity. `invalid_grant`/any refresh failure (what + Okta/Azure/Google return for a disabled/deleted/de-authorized account) + → `DeactivateUserInOrgBySCIM`, same as a real SCIM deactivation. One + coordinator `Work()` call does the per-org fan-out inline, same shape + `RiskModelRefitWorker` already uses - no separate one-shot job type + needed. +- [x] Settings UI: "Automatic deprovisioning check" toggle + interval + dropdown on the OIDC panel in `SSOSettingsTab.tsx`, shows last-checked + time when available. +- [x] This is the direct answer to "how often does this sync": SCIM + (Phase 8) is real-time, push-based - there's no interval for it. This + poll's interval is the *maximum extra latency* for OIDC orgs that skip + SCIM; Phases 7's session revocation already cuts off a deactivated + user's existing session immediately regardless of this job's schedule. +- [x] Build clean, `go test ./internal/jobqueue/...` and `./internal/sso/...` + pass, no import cycle introduced (`internal/jobqueue` importing + `internal/api/users` was checked explicitly - no reverse dependency). + +**Known gaps after Phases 7-10:** no e2e test harness yet for SCIM, the +reconciliation poll, or OIDC back-channel logout against a real Keycloak +(the existing `test/fixtures/keycloak/` realm only covers login); SAML SLO +is not implemented (see Phase 9). These would be the natural next additions +if a future session has the budget for them. diff --git a/internal/api/auth/handlers.go b/internal/api/auth/handlers.go index c18a643f8..69d410640 100644 --- a/internal/api/auth/handlers.go +++ b/internal/api/auth/handlers.go @@ -33,8 +33,9 @@ type AuthHandlers struct { onboardingKeyGenerator func(tx *sql.Tx, userID, orgID int64) (string, error) // sso/ssoState are optional (nil in tests that never call SetSSO): SSO // login routes degrade to "not configured" rather than panicking. - sso *sso.Store - ssoState *sso.StateSigner + sso *sso.Store + ssoState *sso.StateSigner + ssoIdentities *sso.IdentityStore } // NewAuthHandlers creates a new authentication handlers instance diff --git a/internal/api/auth/sso_handlers.go b/internal/api/auth/sso_handlers.go index 43f1bd721..50833705d 100644 --- a/internal/api/auth/sso_handlers.go +++ b/internal/api/auth/sso_handlers.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/hex" "encoding/xml" + "errors" "fmt" "log" "net/http" @@ -19,6 +20,11 @@ import ( "golang.org/x/crypto/bcrypt" ) +// errSSOAccountDeactivated is returned by jitProvisionSSOUser when the +// matched user has been deactivated. SSO login must never resurrect a +// deactivated account - that's an explicit admin action only. +var errSSOAccountDeactivated = errors.New("sso: account is deactivated") + // SetSSO wires the SSO config store and state signer into the auth // handlers. Both the OIDC and (later) SAML login routes, and the config // CRUD handlers that validate against a live IdP, depend on this. @@ -27,6 +33,13 @@ func (h *AuthHandlers) SetSSO(store *sso.Store, jwtSecret string) { h.ssoState = sso.NewStateSigner(jwtSecret) } +// SetSSOIdentities wires the subject->user identity store in, used to +// resolve incoming OIDC back-channel logout tokens / SAML LogoutRequests to +// a LiveReview user (Phase 4 of docs/sso.md's deprovisioning section). +func (h *AuthHandlers) SetSSOIdentities(store *sso.IdentityStore) { + h.ssoIdentities = store +} + // baseURL derives this request's own origin, used both as the OIDC/SAML // redirect/ACS URL registered with the IdP and as the final browser // redirect back into the frontend. Self-hosted serves API and frontend from @@ -159,6 +172,63 @@ func (h *AuthHandlers) SSOOIDCCallback(c echo.Context) error { return h.completeSSOLogin(c, orgID, identity, "sso_oidc") } +// SSOOIDCBackChannelLogout is public: POST /api/v1/auth/sso/oidc/:org_id/backchannel-logout. +// Per OIDC Back-Channel Logout 1.0, the IdP calls this directly (no +// browser involved) when a user logs out or is disabled at the IdP, so +// LiveReview can revoke that user's sessions in near real time rather than +// waiting for their JWT to expire or the next SCIM/reconciliation sweep. +// Best-effort and intentionally quiet on any resolution failure (unknown +// subject, SSO not configured, etc.) - per spec, this endpoint must not +// leak account existence, so it always responds 200 once the token itself +// verifies, even if there's nothing on LiveReview's side to revoke. +func (h *AuthHandlers) SSOOIDCBackChannelLogout(c echo.Context) error { + orgID, err := strconv.ParseInt(c.Param("org_id"), 10, 64) + if err != nil || orgID <= 0 { + return c.String(http.StatusBadRequest, "invalid org") + } + if h.sso == nil { + return c.String(http.StatusNotFound, "sso not configured") + } + cfg, err := h.sso.Get(orgID, sso.ProviderOIDC) + if err != nil || !cfg.Enabled { + return c.String(http.StatusNotFound, "sso not configured") + } + logoutToken := c.Request().PostFormValue("logout_token") + if logoutToken == "" { + return c.String(http.StatusBadRequest, "missing logout_token") + } + subject, err := sso.VerifyBackChannelLogoutToken(c.Request().Context(), cfg, logoutToken) + if err != nil { + log.Printf("[sso] back-channel logout token verification failed for org %d: %v", orgID, err) + return c.String(http.StatusBadRequest, "invalid logout_token") + } + if h.ssoIdentities == nil { + return c.NoContent(http.StatusOK) + } + userID, err := h.ssoIdentities.LookupUserBySubject(orgID, sso.ProviderOIDC, subject) + if err != nil { + return c.NoContent(http.StatusOK) + } + if _, err := h.db.Exec(`UPDATE auth_tokens SET is_active = false, revoked_at = NOW() WHERE user_id = $1 AND is_active = true`, userID); err != nil { + log.Printf("[sso] back-channel logout: failed to revoke sessions for user %d: %v", userID, err) + return c.NoContent(http.StatusOK) + } + h.recordAuthEvent(c, auditlog.Event{ + OrgID: orgID, + ActorUserID: &userID, + // The IdP pushed this logout event, not the user themselves (there + // is no session on this endpoint) - ActorKindWebhook maps to + // ActorCapacitySystem (see auditlog.EnrichFromEcho), matching how + // SCIM-driven deactivations are attributed. + ActorKind: auditlog.ActorKindWebhook, + Action: auditlog.ActionSessionRevoke, + Outcome: auditlog.OutcomeSuccess, + Severity: auditlog.SeverityWarning, + Details: auditlog.Details("method", "sso_oidc_backchannel_logout"), + }) + return c.NoContent(http.StatusOK) +} + // samlURLs derives this org's metadata/ACS URLs from the current request's // own origin, same reasoning as baseURL for OIDC's redirect URL. func samlURLs(c echo.Context, orgID int64) (metadataURL, acsURL string) { @@ -284,7 +354,12 @@ func (h *AuthHandlers) completeSSOLogin(c echo.Context, orgID int64, identity *s user, created, err := h.jitProvisionSSOUser(orgID, email, identity.Name) if err != nil { - log.Printf("[sso] JIT provisioning failed for %s in org %d: %v", email, orgID, err) + reason := "provisioning_failed" + if errors.Is(err, errSSOAccountDeactivated) { + reason = "account_deactivated" + } else { + log.Printf("[sso] JIT provisioning failed for %s in org %d: %v", email, orgID, err) + } h.recordAuthEvent(c, auditlog.Event{ OrgID: orgID, ActorEmail: email, @@ -292,9 +367,9 @@ func (h *AuthHandlers) completeSSOLogin(c echo.Context, orgID int64, identity *s Action: auditlog.ActionLoginFailure, Outcome: auditlog.OutcomeFailure, Severity: auditlog.SeverityWarning, - Details: auditlog.Details("method", method, "reason", "provisioning_failed"), + Details: auditlog.Details("method", method, "reason", reason), }) - return ssoFailureRedirect(c, "provisioning_failed") + return ssoFailureRedirect(c, reason) } userAgent := c.Request().Header.Get("User-Agent") @@ -307,6 +382,19 @@ func (h *AuthHandlers) completeSSOLogin(c echo.Context, orgID int64, identity *s _, _ = h.db.Exec(`UPDATE users SET last_login_at = NOW() WHERE id = $1`, user.ID) + if h.ssoIdentities != nil && identity.Subject != "" { + provider := sso.ProviderOIDC + if method == "sso_saml" { + provider = sso.ProviderSAML + } + if err := h.ssoIdentities.Upsert(orgID, user.ID, provider, identity.Subject, identity.RefreshToken); err != nil { + // Non-fatal: this only degrades back-channel logout resolution + // and the reconciliation poll for this login, never the login + // itself. + log.Printf("[sso] failed to record identity mapping for org %d user %d: %v", orgID, user.ID, err) + } + } + uid := user.ID actorKind, actorCapacity := h.actorFor(user) if created { @@ -352,6 +440,7 @@ func (h *AuthHandlers) completeSSOLogin(c echo.Context, orgID int64, identity *s // login path until an owner sets one). func (h *AuthHandlers) jitProvisionSSOUser(orgID int64, email, displayName string) (*models.User, bool, error) { var userID int64 + var isActive bool created := false err := func() error { @@ -361,7 +450,7 @@ func (h *AuthHandlers) jitProvisionSSOUser(orgID int64, email, displayName strin } defer tx.Rollback() - err = tx.QueryRow(`SELECT id FROM users WHERE email = $1`, email).Scan(&userID) + err = tx.QueryRow(`SELECT id, is_active FROM users WHERE email = $1`, email).Scan(&userID, &isActive) if err == sql.ErrNoRows { randBytes := make([]byte, 32) if _, rerr := rand.Read(randBytes); rerr != nil { @@ -389,6 +478,12 @@ func (h *AuthHandlers) jitProvisionSSOUser(orgID int64, email, displayName strin } else if err != nil { return err } else { + if !isActive { + // Deactivation is an explicit admin action; SSO login must + // never silently undo it. Reject here rather than + // reactivating - see errSSOAccountDeactivated. + return errSSOAccountDeactivated + } var isMember bool if err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM user_roles WHERE user_id = $1 AND org_id = $2)`, userID, orgID).Scan(&isMember); err != nil { return err @@ -402,7 +497,7 @@ func (h *AuthHandlers) jitProvisionSSOUser(orgID int64, email, displayName strin } created = true } - if _, err := tx.Exec(`UPDATE users SET default_org_id = COALESCE(default_org_id, $1), is_active = TRUE WHERE id = $2`, orgID, userID); err != nil { + if _, err := tx.Exec(`UPDATE users SET default_org_id = COALESCE(default_org_id, $1) WHERE id = $2`, orgID, userID); err != nil { return err } } diff --git a/internal/api/auth/token_service.go b/internal/api/auth/token_service.go index 65bdbc5f3..93e6ce499 100644 --- a/internal/api/auth/token_service.go +++ b/internal/api/auth/token_service.go @@ -236,11 +236,15 @@ func (ts *TokenService) ValidateAccessToken(tokenString string) (*models.User, e fmt.Printf("Warning: failed to update last_used_at: %v\n", err) } - // Get user details + // Get user details. The is_active filter is defense in depth: deactivation + // (internal/api/users/user_service.go's DeactivateUserInOrg) already + // revokes this user's auth_tokens rows directly, but this guards against + // any future deactivation path that forgets to, same as + // APIKeyManager.ValidateAPIKey already does for API keys. user := &models.User{} err = ts.db.QueryRow(` SELECT id, email, password_hash, default_org_id, created_at, updated_at - FROM users WHERE id = $1 + FROM users WHERE id = $1 AND is_active = true `, claims.UserID).Scan(&user.ID, &user.Email, &user.PasswordHash, &user.DefaultOrgID, &user.CreatedAt, &user.UpdatedAt) if err != nil { diff --git a/internal/api/scim_handlers.go b/internal/api/scim_handlers.go new file mode 100644 index 000000000..9dd05ccf5 --- /dev/null +++ b/internal/api/scim_handlers.go @@ -0,0 +1,429 @@ +package api + +import ( + "database/sql" + "errors" + "net/http" + "regexp" + "strconv" + "strings" + "time" + + "github.com/labstack/echo/v4" + "github.com/livereview/internal/api/auth" + "github.com/livereview/internal/api/users" + "github.com/livereview/internal/auditlog" + "github.com/livereview/internal/license" + "github.com/livereview/internal/scim" + "github.com/rs/zerolog/log" +) + +// orgHasSCIMFeature mirrors orgHasSSOFeature (internal/api/sso_config.go) - +// checks the org's own plan rather than a hardcoded self-hosted bit, so +// adding "scim" to a cloud plan's Features later needs no code change here. +func (s *Server) orgHasSCIMFeature(orgID int64) bool { + var planCode string + _ = s.db.QueryRow(`SELECT current_plan_code FROM org_billing_state WHERE org_id = $1`, orgID).Scan(&planCode) + return license.PlanType(planCode).HasFeature("scim") +} + +// requireSCIMBearerAuth authenticates SCIM clients (Okta, Azure AD, Google +// Workspace) via their org's static bearer token - there is no user +// session on this path, per the SCIM 2.0 RFC. On success it sets +// "scim_org_id" in the echo context for the handlers below. +func (s *Server) requireSCIMBearerAuth(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + authHeader := c.Request().Header.Get("Authorization") + const prefix = "Bearer " + if !strings.HasPrefix(authHeader, prefix) { + return c.JSON(http.StatusUnauthorized, scim.NewError(http.StatusUnauthorized, "missing bearer token")) + } + token := strings.TrimSpace(strings.TrimPrefix(authHeader, prefix)) + orgID, err := s.scimTokens.Authenticate(token) + if err != nil { + return c.JSON(http.StatusUnauthorized, scim.NewError(http.StatusUnauthorized, "invalid, expired, or revoked token")) + } + if !s.orgHasSCIMFeature(orgID) { + return c.JSON(http.StatusForbidden, scim.NewError(http.StatusForbidden, "SCIM requires an enterprise self-hosted license")) + } + c.Set("scim_org_id", orgID) + return next(c) + } +} + +func scimOrgID(c echo.Context) int64 { + return c.Get("scim_org_id").(int64) +} + +// scimResourceFromUser converts an internal user record into the SCIM +// shape IdPs expect. +func scimResourceFromUser(u *users.UserWithRole) scim.UserResource { + first, last := "", "" + if u.FirstName != nil { + first = *u.FirstName + } + if u.LastName != nil { + last = *u.LastName + } + return scim.UserResource{ + Schemas: []string{scim.SchemaUser}, + ID: strconv.FormatInt(u.ID, 10), + UserName: u.Email, + Name: &scim.NameResource{GivenName: first, FamilyName: last}, + Emails: []scim.EmailResource{{Value: u.Email, Primary: true}}, + Active: u.IsActive, + Meta: &scim.MetaResource{ + ResourceType: "User", + Created: u.CreatedAt.Format(time.RFC3339), + LastModified: u.UpdatedAt.Format(time.RFC3339), + }, + } +} + +// primaryEmail extracts the identifying email from a SCIM UserResource - +// IdPs vary on whether they populate userName or emails[] with the address +// that matters, so this checks both. +func primaryEmail(r scim.UserResource) string { + for _, e := range r.Emails { + if e.Primary && e.Value != "" { + return strings.ToLower(strings.TrimSpace(e.Value)) + } + } + if len(r.Emails) > 0 && r.Emails[0].Value != "" { + return strings.ToLower(strings.TrimSpace(r.Emails[0].Value)) + } + if strings.Contains(r.UserName, "@") { + return strings.ToLower(strings.TrimSpace(r.UserName)) + } + return "" +} + +// emailFilterPattern matches the simple SCIM filter expressions IdPs +// actually send when checking "does this user already exist" before +// creating - userName eq "x@y.com" or emails.value eq "x@y.com". Full SCIM +// filter grammar (and/or/not, other attributes) is not needed for this use +// case and isn't parsed. Case-insensitive on the attribute name and the +// "eq" operator, and tolerant of extra whitespace around it - real IdPs +// (Okta in particular) don't all format this identically. +var emailFilterPattern = regexp.MustCompile(`(?i)(?:userName|emails(?:\.value)?)\s+eq\s+"([^"]+)"`) + +func parseEmailFilter(filter string) string { + m := emailFilterPattern.FindStringSubmatch(filter) + if m == nil { + return "" + } + return strings.ToLower(strings.TrimSpace(m[1])) +} + +// scimMaxPageSize bounds `count` - without a ceiling, a misbehaving or +// malicious client could ask for an unbounded page. +const scimMaxPageSize = 200 + +// scimPaging parses SCIM's 1-based `startIndex`/`count` query params, +// defaulting and clamping both so a missing, zero, negative, or +// unreasonably large value can't produce an invalid OFFSET/LIMIT. +func scimPaging(c echo.Context) (startIndex, count int) { + startIndex = 1 + if v, err := strconv.Atoi(c.QueryParam("startIndex")); err == nil && v > 1 { + startIndex = v + } + count = scimMaxPageSize + if v, err := strconv.Atoi(c.QueryParam("count")); err == nil && v > 0 && v < scimMaxPageSize { + count = v + } + return startIndex, count +} + +// SCIMListUsers handles GET /scim/v2/Users. IdPs call this both to page +// through the directory - via the SCIM-standard `startIndex`/`count` query +// params - and, far more commonly, with a `filter` to check whether a user +// already exists before POSTing a create. +func (s *Server) SCIMListUsers(c echo.Context) error { + orgID := scimOrgID(c) + email := parseEmailFilter(c.QueryParam("filter")) + + type row struct { + id int64 + email string + isActive bool + firstName, lastName sql.NullString + createdAt, updatedAt time.Time + } + var rows []row + startIndex, count := scimPaging(c) + totalResults := 0 + + if email != "" { + var r row + err := s.db.QueryRow(` + SELECT u.id, u.email, u.is_active, u.first_name, u.last_name, u.created_at, u.updated_at + FROM users u JOIN user_roles ur ON ur.user_id = u.id + WHERE u.email = $1 AND ur.org_id = $2 + `, email, orgID).Scan(&r.id, &r.email, &r.isActive, &r.firstName, &r.lastName, &r.createdAt, &r.updatedAt) + if err == nil { + rows = append(rows, r) + totalResults = 1 + } else if err != sql.ErrNoRows { + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to query users")) + } + } else { + if err := s.db.QueryRow(` + SELECT count(*) FROM users u JOIN user_roles ur ON ur.user_id = u.id WHERE ur.org_id = $1 + `, orgID).Scan(&totalResults); err != nil { + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to count users")) + } + sqlRows, err := s.db.Query(` + SELECT u.id, u.email, u.is_active, u.first_name, u.last_name, u.created_at, u.updated_at + FROM users u JOIN user_roles ur ON ur.user_id = u.id + WHERE ur.org_id = $1 ORDER BY u.id LIMIT $2 OFFSET $3 + `, orgID, count, startIndex-1) + if err != nil { + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to query users")) + } + defer sqlRows.Close() + for sqlRows.Next() { + var r row + if err := sqlRows.Scan(&r.id, &r.email, &r.isActive, &r.firstName, &r.lastName, &r.createdAt, &r.updatedAt); err != nil { + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to read users")) + } + rows = append(rows, r) + } + } + + resources := make([]scim.UserResource, 0, len(rows)) + for _, r := range rows { + first, last := "", "" + if r.firstName.Valid { + first = r.firstName.String + } + if r.lastName.Valid { + last = r.lastName.String + } + resources = append(resources, scim.UserResource{ + Schemas: []string{scim.SchemaUser}, + ID: strconv.FormatInt(r.id, 10), + UserName: r.email, + Name: &scim.NameResource{GivenName: first, FamilyName: last}, + Emails: []scim.EmailResource{{Value: r.email, Primary: true}}, + Active: r.isActive, + Meta: &scim.MetaResource{ + ResourceType: "User", + Created: r.createdAt.Format(time.RFC3339), + LastModified: r.updatedAt.Format(time.RFC3339), + }, + }) + } + + return c.JSON(http.StatusOK, scim.ListResponse{ + Schemas: []string{scim.SchemaListResponse}, + TotalResults: totalResults, + ItemsPerPage: len(resources), + StartIndex: startIndex, + Resources: resources, + }) +} + +// SCIMCreateUser handles POST /scim/v2/Users. +func (s *Server) SCIMCreateUser(c echo.Context) error { + orgID := scimOrgID(c) + + var req scim.UserResource + if err := c.Bind(&req); err != nil { + return c.JSON(http.StatusBadRequest, scim.NewError(http.StatusBadRequest, "invalid request body")) + } + email := primaryEmail(req) + if email == "" { + return c.JSON(http.StatusBadRequest, scim.NewError(http.StatusBadRequest, "userName or emails is required")) + } + first, last := "", "" + if req.Name != nil { + first, last = req.Name.GivenName, req.Name.FamilyName + } + + user, created, err := s.userService.ProvisionSCIMUser(orgID, email, first, last) + if err != nil { + if errors.Is(err, users.ErrSCIMAccountDeactivated) { + return c.JSON(http.StatusConflict, scim.NewError(http.StatusConflict, "this account was deactivated by an organization admin")) + } + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to provision user")) + } + if created && s.auditRecorder != nil { + if aerr := s.auditRecorder.Webhook(c.Request().Context(), orgID, auditlog.ActionSCIMUserCreate, auditlog.Details("email", email)); aerr != nil { + log.Error().Err(aerr).Int64("org_id", orgID).Str("email", email).Msg("scim: failed to record user-create audit event") + } + } + return c.JSON(http.StatusCreated, scimResourceFromUser(user)) +} + +// SCIMGetUser handles GET /scim/v2/Users/:id. +func (s *Server) SCIMGetUser(c echo.Context) error { + orgID := scimOrgID(c) + userID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return c.JSON(http.StatusBadRequest, scim.NewError(http.StatusBadRequest, "invalid id")) + } + user, err := s.userService.GetUserInOrg(orgID, userID) + if err != nil { + return c.JSON(http.StatusNotFound, scim.NewError(http.StatusNotFound, "user not found")) + } + return c.JSON(http.StatusOK, scimResourceFromUser(user)) +} + +// SCIMPatchUser handles PATCH /scim/v2/Users/:id - the operation that +// matters most for offboarding: IdPs send +// {"Operations":[{"op":"replace","value":{"active":false}}]} to deprovision. +func (s *Server) SCIMPatchUser(c echo.Context) error { + orgID := scimOrgID(c) + userID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return c.JSON(http.StatusBadRequest, scim.NewError(http.StatusBadRequest, "invalid id")) + } + if _, err := s.userService.GetUserInOrg(orgID, userID); err != nil { + return c.JSON(http.StatusNotFound, scim.NewError(http.StatusNotFound, "user not found")) + } + + var req scim.PatchRequest + if err := c.Bind(&req); err != nil { + return c.JSON(http.StatusBadRequest, scim.NewError(http.StatusBadRequest, "invalid request body")) + } + + for _, op := range req.Operations { + active, ok := patchActiveValue(op) + if !ok { + continue + } + if active { + if err := s.userService.ReactivateUserInOrgBySCIM(userID); err != nil { + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to reactivate user")) + } + if s.auditRecorder != nil { + if aerr := s.auditRecorder.Webhook(c.Request().Context(), orgID, auditlog.ActionSCIMUserReactivate, auditlog.Details("user_id", userID)); aerr != nil { + log.Error().Err(aerr).Int64("org_id", orgID).Int64("user_id", userID).Msg("scim: failed to record user-reactivate audit event") + } + } + } else { + if err := s.userService.DeactivateUserInOrgBySCIM(userID); err != nil { + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to deactivate user")) + } + if s.auditRecorder != nil { + if aerr := s.auditRecorder.Webhook(c.Request().Context(), orgID, auditlog.ActionSCIMUserDeactivate, auditlog.Details("user_id", userID)); aerr != nil { + log.Error().Err(aerr).Int64("org_id", orgID).Int64("user_id", userID).Msg("scim: failed to record user-deactivate audit event") + } + } + } + } + + user, err := s.userService.GetUserInOrg(orgID, userID) + if err != nil { + return c.JSON(http.StatusInternalServerError, scim.NewError(http.StatusInternalServerError, "failed to reload user")) + } + return c.JSON(http.StatusOK, scimResourceFromUser(user)) +} + +// patchActiveValue reports whether op sets the SCIM "active" attribute, +// and what it was set to. Handles both the common {"active": false} +// whole-object replace and the path-qualified {"path":"active","value":false} form. +func patchActiveValue(op scim.PatchOperation) (active bool, matched bool) { + if !strings.EqualFold(op.Op, "replace") { + return false, false + } + if strings.EqualFold(op.Path, "active") { + if b, ok := op.Value.(bool); ok { + return b, true + } + return false, false + } + if op.Path == "" { + if m, ok := op.Value.(map[string]any); ok { + if b, ok := m["active"].(bool); ok { + return b, true + } + } + } + return false, false +} + +// --- SCIM token management (session-authenticated, owner-only) --- + +// requireSCIMConfigAccess is requireSSOAccess's SCIM-token counterpart +// (internal/api/sso_config.go's requireOwnerSessionAndFeature carries the +// shared Sensitive Operations Gate + Session-Only Gating + +// self-hosted-enterprise feature check both rely on). +func (s *Server) requireSCIMConfigAccess(c echo.Context, orgID int64) error { + return s.requireOwnerSessionAndFeature(c, s.orgHasSCIMFeature(orgID), "SCIM provisioning") +} + +type scimTokenResponse struct { + TokenPrefix string `json:"token_prefix"` + CreatedAt string `json:"created_at"` + Token *string `json:"token,omitempty"` // only populated once, on creation +} + +// GetSCIMToken is GET /orgs/:org_id/scim-token - returns the existing +// token's metadata (never the plaintext, which is only ever shown at +// creation time). +func (s *Server) GetSCIMToken(c echo.Context) error { + orgID, err := strconv.ParseInt(c.Param("org_id"), 10, 64) + if err != nil { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid org_id"}) + } + if err := s.requireSCIMConfigAccess(c, orgID); err != nil { + return err + } + tok, err := s.scimTokens.Get(orgID) + if err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to load SCIM token"}) + } + if tok == nil { + return c.JSON(http.StatusOK, map[string]any{"exists": false}) + } + return c.JSON(http.StatusOK, scimTokenResponse{TokenPrefix: tok.TokenPrefix, CreatedAt: tok.CreatedAt.Format(time.RFC3339)}) +} + +// CreateSCIMToken is POST /orgs/:org_id/scim-token - generates a new token, +// replacing (rotating out) any existing one. Registered via s.audited with +// ActionSCIMTokenCreate. +func (s *Server) CreateSCIMToken(c echo.Context) error { + orgID, err := strconv.ParseInt(c.Param("org_id"), 10, 64) + if err != nil { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid org_id"}) + } + if err := s.requireSCIMConfigAccess(c, orgID); err != nil { + return err + } + pc := auth.GetPermissionContext(c) + var actorUserID int64 + if pc != nil && pc.User != nil { + actorUserID = pc.User.ID + } + + if err := s.scimTokens.Revoke(orgID); err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to rotate SCIM token"}) + } + plaintext, tok, err := s.scimTokens.Create(orgID, actorUserID) + if err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to create SCIM token"}) + } + auditlog.MarkRecorded(c) + return c.JSON(http.StatusOK, scimTokenResponse{ + TokenPrefix: tok.TokenPrefix, + CreatedAt: tok.CreatedAt.Format(time.RFC3339), + Token: &plaintext, + }) +} + +// RevokeSCIMToken is DELETE /orgs/:org_id/scim-token. +func (s *Server) RevokeSCIMToken(c echo.Context) error { + orgID, err := strconv.ParseInt(c.Param("org_id"), 10, 64) + if err != nil { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid org_id"}) + } + if err := s.requireSCIMConfigAccess(c, orgID); err != nil { + return err + } + if err := s.scimTokens.Revoke(orgID); err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to revoke SCIM token"}) + } + auditlog.MarkRecorded(c) + return c.JSON(http.StatusOK, map[string]string{"message": "SCIM token revoked"}) +} diff --git a/internal/api/server.go b/internal/api/server.go index 6207891fa..429e20dfe 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -49,6 +49,7 @@ import ( "github.com/livereview/internal/providers/azuredevops" reviewprocessor "github.com/livereview/internal/review_processor" "github.com/livereview/internal/scheduledreview" + "github.com/livereview/internal/scim" "github.com/livereview/internal/slackbot" "github.com/livereview/internal/sso" "github.com/livereview/internal/teamsbot" @@ -149,7 +150,8 @@ type Server struct { versionInfo *VersionInfo deploymentConfig *DeploymentConfig authHandlers *auth.AuthHandlers - ssoStore *sso.Store // nil only if the secret box failed to init (see SetSSO wiring) + ssoStore *sso.Store // nil only if the secret box failed to init (see SetSSO wiring) + scimTokens *scim.TokenStore githubApp *githubapp.Config // nil unless GITHUB_APP_* is configured githubAppSecret []byte // HMAC key for the GitHub App OAuth state tokenService *auth.TokenService @@ -326,11 +328,14 @@ func appContext(port int, versionInfo *VersionInfo) (*Server, error) { // Enterprise SSO (OIDC/SAML) - see docs/sso.md. The secret box reuses // JWT_SECRET rather than requiring a dedicated encryption key. var ssoStore *sso.Store + var ssoIdentities *sso.IdentityStore if ssoBox, err := sso.NewSecretBox(jwtSecret); err != nil { log.Printf("[sso] disabled: %v", err) } else { ssoStore = sso.NewStore(db, ssoBox) authHandlers.SetSSO(ssoStore, jwtSecret) + ssoIdentities = sso.NewIdentityStore(db, ssoBox) + authHandlers.SetSSOIdentities(ssoIdentities) } // Audit trail framework: recorder + route-spec registry. The HTTP @@ -392,6 +397,7 @@ func appContext(port int, versionInfo *VersionInfo) (*Server, error) { deploymentConfig: deploymentConfig, authHandlers: authHandlers, ssoStore: ssoStore, + scimTokens: scim.NewTokenStore(db), githubApp: githubapp.Load(), githubAppSecret: []byte(jwtSecret), tokenService: tokenService, @@ -1173,10 +1179,23 @@ func (s *Server) setupRoutes() { public.GET("/auth/sso/providers", s.authHandlers.SSOProviders) public.GET("/auth/sso/oidc/:org_id/login", s.authHandlers.SSOOIDCLogin) public.GET("/auth/sso/oidc/callback", s.authHandlers.SSOOIDCCallback) + public.POST("/auth/sso/oidc/:org_id/backchannel-logout", s.authHandlers.SSOOIDCBackChannelLogout) public.GET("/auth/sso/saml/:org_id/metadata", s.authHandlers.SSOSAMLMetadata) public.GET("/auth/sso/saml/:org_id/login", s.authHandlers.SSOSAMLLogin) public.POST("/auth/sso/saml/:org_id/acs", s.authHandlers.SSOSAMLACS) + // SCIM 2.0 (Okta/Azure AD/Google Workspace user lifecycle sync) - see + // docs/sso.md. Public route group by necessity (SCIM clients authenticate + // via their own org-scoped bearer token, not a LiveReview session); + // org-scoping comes from that token alone, enforced inside + // requireSCIMBearerAuth, never from a client-supplied id. + scimGroup := public.Group("/scim/v2") + scimGroup.Use(s.requireSCIMBearerAuth) + scimGroup.GET("/Users", s.SCIMListUsers) + scimGroup.POST("/Users", s.SCIMCreateUser) + scimGroup.GET("/Users/:id", s.SCIMGetUser) + scimGroup.PATCH("/Users/:id", s.SCIMPatchUser) + // Diff review endpoints. Accepts either an X-API-Key (the git-lrc CLI's // auth) or a Bearer session token (the web UI's review-details diff // viewer, which reads this same data via GET /:review_id) — mirrors the @@ -1331,6 +1350,7 @@ func (s *Server) setupRoutes() { orgGroup.GET("/users/:user_id", s.userHandlers.GetUser) s.audited(orgGroup, echo.PUT, "/users/:user_id", auditlog.Spec{Action: auditlog.ActionUserUpdate, Category: auditlog.CategoryUser, ResourceType: "user", ResourceIDParam: "user_id", Sensitive: true}, s.userHandlers.UpdateUser) s.audited(orgGroup, echo.DELETE, "/users/:user_id", auditlog.Spec{Action: auditlog.ActionUserDeactivate, Category: auditlog.CategoryUser, ResourceType: "user", ResourceIDParam: "user_id", Sensitive: true}, s.userHandlers.DeactivateUser) + s.audited(orgGroup, echo.POST, "/users/:user_id/reactivate", auditlog.Spec{Action: auditlog.ActionUserReactivate, Category: auditlog.CategoryUser, ResourceType: "user", ResourceIDParam: "user_id", Sensitive: true}, s.userHandlers.ReactivateUser) s.audited(orgGroup, echo.PUT, "/users/:user_id/role", auditlog.Spec{Action: auditlog.ActionUserRoleChange, Category: auditlog.CategoryUser, ResourceType: "user", ResourceIDParam: "user_id", Sensitive: true}, s.userHandlers.ChangeUserRole) s.audited(orgGroup, echo.POST, "/users/:user_id/force-password-reset", auditlog.Spec{Action: auditlog.ActionPasswordForceReset, Category: auditlog.CategoryUser, ResourceType: "user", ResourceIDParam: "user_id", Sensitive: true}, s.userHandlers.ForcePasswordReset) orgGroup.GET("/users/:user_id/audit-log", s.userHandlers.GetUserAuditLog) @@ -1350,6 +1370,9 @@ func (s *Server) setupRoutes() { s.audited(orgGroup, echo.PUT, "/sso-config/:provider_type", auditlog.Spec{Action: auditlog.ActionSettingsSSOUpdate, Category: auditlog.CategorySettings, ResourceType: "org_sso_config", ResourceIDParam: "provider_type", Sensitive: true}, s.UpsertSSOConfig) s.audited(orgGroup, echo.DELETE, "/sso-config/:provider_type", auditlog.Spec{Action: auditlog.ActionSettingsSSODelete, Category: auditlog.CategorySettings, ResourceType: "org_sso_config", ResourceIDParam: "provider_type", Sensitive: true}, s.DeleteSSOConfig) orgGroup.POST("/sso-config/:provider_type/test", s.TestSSOConfig) + orgGroup.GET("/scim-token", s.GetSCIMToken) + s.audited(orgGroup, echo.POST, "/scim-token", auditlog.Spec{Action: auditlog.ActionSCIMTokenCreate, Category: auditlog.CategoryAuth, ResourceType: "scim_token", Sensitive: true}, s.CreateSCIMToken) + s.audited(orgGroup, echo.DELETE, "/scim-token", auditlog.Spec{Action: auditlog.ActionSCIMTokenRevoke, Category: auditlog.CategoryAuth, ResourceType: "scim_token", Sensitive: true}, s.RevokeSCIMToken) // Super admin routes - Production adminGroup := v1.Group("/admin") diff --git a/internal/api/sso_config.go b/internal/api/sso_config.go index ee816e291..31640e343 100644 --- a/internal/api/sso_config.go +++ b/internal/api/sso_config.go @@ -39,6 +39,11 @@ type ssoConfigRequest struct { SAMLIDPMetadataURL string `json:"saml_idp_metadata_url,omitempty"` SAMLIDPMetadataXML string `json:"saml_idp_metadata_xml,omitempty"` SAMLIDPEntityID string `json:"saml_idp_entity_id,omitempty"` + + // Reconciliation only applies to provider_type=oidc - see Phase 5 of + // docs/sso.md's deprovisioning section. Ignored for SAML. + ReconciliationEnabled bool `json:"reconciliation_enabled,omitempty"` + ReconciliationInterval string `json:"reconciliation_interval,omitempty"` } // ssoConfigResponse never carries secrets - this is what GET/PUT return. @@ -53,21 +58,31 @@ type ssoConfigResponse struct { SAMLIDPMetadataURL string `json:"saml_idp_metadata_url,omitempty"` SAMLIDPEntityID string `json:"saml_idp_entity_id,omitempty"` SAMLMetadataSet bool `json:"saml_metadata_set"` + + ReconciliationEnabled bool `json:"reconciliation_enabled,omitempty"` + ReconciliationInterval string `json:"reconciliation_interval,omitempty"` + LastReconciledAt string `json:"last_reconciled_at,omitempty"` } func redactedSSOConfig(cfg *sso.Config) ssoConfigResponse { - return ssoConfigResponse{ - ProviderType: string(cfg.ProviderType), - DisplayName: cfg.DisplayName, - Enabled: cfg.Enabled, - OIDCIssuer: cfg.OIDCIssuer, - OIDCClientID: cfg.OIDCClientID, - OIDCClientSecretSet: cfg.OIDCClientSecret != "", - OIDCScopes: cfg.OIDCScopes, - SAMLIDPMetadataURL: cfg.SAMLIDPMetadataURL, - SAMLIDPEntityID: cfg.SAMLIDPEntityID, - SAMLMetadataSet: cfg.SAMLIDPMetadataXML != "", - } + resp := ssoConfigResponse{ + ProviderType: string(cfg.ProviderType), + DisplayName: cfg.DisplayName, + Enabled: cfg.Enabled, + OIDCIssuer: cfg.OIDCIssuer, + OIDCClientID: cfg.OIDCClientID, + OIDCClientSecretSet: cfg.OIDCClientSecret != "", + OIDCScopes: cfg.OIDCScopes, + SAMLIDPMetadataURL: cfg.SAMLIDPMetadataURL, + SAMLIDPEntityID: cfg.SAMLIDPEntityID, + SAMLMetadataSet: cfg.SAMLIDPMetadataXML != "", + ReconciliationEnabled: cfg.ReconciliationEnabled, + ReconciliationInterval: cfg.ReconciliationInterval, + } + if cfg.LastReconciledAt != nil { + resp.LastReconciledAt = cfg.LastReconciledAt.Format(time.RFC3339) + } + return resp } func parseProviderType(raw string) (sso.ProviderType, bool) { @@ -79,25 +94,39 @@ func parseProviderType(raw string) (sso.ProviderType, bool) { } } -// requireSSOAccess checks the "Sensitive Operations Gate" + "Session-Only -// Gating" + self-hosted-enterprise-feature rules from CLAUDE.md in one -// place, since every SSO config handler needs all three. -func (s *Server) requireSSOAccess(c echo.Context, orgID int64) error { +// requireOwnerSessionAndFeature checks the three rules every SSO/SCIM +// config handler needs in combination - the "Sensitive Operations Gate" +// (session only, no API keys), org-owner-only, and self-hosted-enterprise +// feature-gating rules from CLAUDE.md - in one place, so requireSSOAccess +// and requireSCIMConfigAccess don't each carry their own copy of the same +// four checks with only the error strings differing. +func (s *Server) requireOwnerSessionAndFeature(c echo.Context, hasFeature bool, resourceName string) error { if err := requireSession(c); err != nil { return err } pc := auth.GetPermissionContext(c) + deniedMsg := map[string]string{"error": "only org owners can manage " + resourceName} if pc == nil { - return c.JSON(http.StatusForbidden, map[string]string{"error": "only org owners can manage SSO configuration"}) + return c.JSON(http.StatusForbidden, deniedMsg) } if err := pc.RequireOrgOwner(); err != nil { - return c.JSON(http.StatusForbidden, map[string]string{"error": "only org owners can manage SSO configuration"}) + return c.JSON(http.StatusForbidden, deniedMsg) } if apimiddleware.IsCloudMode() { - return c.JSON(http.StatusForbidden, map[string]string{"error": "SSO is available on self-hosted enterprise deployments only"}) + return c.JSON(http.StatusForbidden, map[string]string{"error": resourceName + " is available on self-hosted enterprise deployments only"}) } - if !s.orgHasSSOFeature(orgID) { - return c.JSON(http.StatusForbidden, map[string]string{"error": "SSO requires an enterprise self-hosted license"}) + if !hasFeature { + return c.JSON(http.StatusForbidden, map[string]string{"error": resourceName + " requires an enterprise self-hosted license"}) + } + return nil +} + +// requireSSOAccess checks the "Sensitive Operations Gate" + "Session-Only +// Gating" + self-hosted-enterprise-feature rules from CLAUDE.md in one +// place, since every SSO config handler needs all three. +func (s *Server) requireSSOAccess(c echo.Context, orgID int64) error { + if err := s.requireOwnerSessionAndFeature(c, s.orgHasSSOFeature(orgID), "SSO configuration"); err != nil { + return err } if s.ssoStore == nil { return c.JSON(http.StatusInternalServerError, map[string]string{"error": "SSO is not available on this deployment"}) @@ -148,21 +177,26 @@ func (s *Server) UpsertSSOConfig(c echo.Context) error { if err := c.Bind(&req); err != nil { return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid request body"}) } + if req.ReconciliationInterval != "" && req.ReconciliationInterval != "hourly" && req.ReconciliationInterval != "daily" && req.ReconciliationInterval != "weekly" { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "reconciliation_interval must be 'hourly', 'daily', or 'weekly'"}) + } if req.DisplayName == "" { return c.JSON(http.StatusBadRequest, map[string]string{"error": "display_name is required"}) } cfg := &sso.Config{ - OrgID: orgID, - ProviderType: provider, - DisplayName: req.DisplayName, - Enabled: req.Enabled, - OIDCIssuer: req.OIDCIssuer, - OIDCClientID: req.OIDCClientID, - OIDCScopes: req.OIDCScopes, - SAMLIDPMetadataURL: req.SAMLIDPMetadataURL, - SAMLIDPMetadataXML: req.SAMLIDPMetadataXML, - SAMLIDPEntityID: req.SAMLIDPEntityID, + OrgID: orgID, + ProviderType: provider, + DisplayName: req.DisplayName, + Enabled: req.Enabled, + OIDCIssuer: req.OIDCIssuer, + OIDCClientID: req.OIDCClientID, + OIDCScopes: req.OIDCScopes, + SAMLIDPMetadataURL: req.SAMLIDPMetadataURL, + SAMLIDPMetadataXML: req.SAMLIDPMetadataXML, + SAMLIDPEntityID: req.SAMLIDPEntityID, + ReconciliationEnabled: provider == sso.ProviderOIDC && req.ReconciliationEnabled, + ReconciliationInterval: req.ReconciliationInterval, } // Preserve the existing secret/metadata unless the caller explicitly diff --git a/internal/api/users/handlers.go b/internal/api/users/handlers.go index fe7a55c58..5e3e41337 100644 --- a/internal/api/users/handlers.go +++ b/internal/api/users/handlers.go @@ -313,6 +313,33 @@ func (uh *UserHandlers) DeactivateUser(c echo.Context) error { }) } +// ReactivateUser handles restoring a previously deactivated user in an +// organization. This is the only way a deactivated user becomes active +// again - SSO login deliberately never does this on its own. +func (uh *UserHandlers) ReactivateUser(c echo.Context) error { + permCtx := auth.MustGetPermissionContext(c) + + if !permCtx.HasPermission(auth.PermissionDeleteUsers) { + return echo.NewHTTPError(http.StatusForbidden, "Permission denied: cannot reactivate users") + } + + userIDStr := c.Param("user_id") + userID, err := strconv.ParseInt(userIDStr, 10, 64) + if err != nil { + return echo.NewHTTPError(http.StatusBadRequest, "Invalid user ID") + } + + err = uh.userService.ReactivateUserInOrg(permCtx.OrgID, userID, permCtx.User.ID) + if err != nil { + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to reactivate user") + } + auditlog.MarkRecorded(c) + + return c.JSON(http.StatusOK, map[string]string{ + "message": "User reactivated successfully", + }) +} + // ChangeUserRole handles changing a user's role in an organization func (uh *UserHandlers) ChangeUserRole(c echo.Context) error { // Get permission context from middleware diff --git a/internal/api/users/user_service.go b/internal/api/users/user_service.go index eb59b836e..850a193d1 100644 --- a/internal/api/users/user_service.go +++ b/internal/api/users/user_service.go @@ -2,7 +2,9 @@ package users import ( "context" + "crypto/rand" "database/sql" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -968,12 +970,16 @@ func (us *UserService) UpdateUserInOrg(orgID, userID, updatedByUserID int64, req return us.GetUserInOrg(orgID, userID) } -// DeactivateUserInOrg deactivates a user in a specific organization +// DeactivateUserInOrg deactivates a user in a specific organization and +// revokes every live session/refresh token they currently hold. Without the +// revocation, a deactivated user's already-issued JWT would keep working +// until its own natural expiry - deactivation needs to cut off access +// immediately, not just block future logins. func (us *UserService) DeactivateUserInOrg(orgID, userID, deactivatedByUserID int64) error { return us.store.WithTx(func(tx *sql.Tx) error { // Deactivate user _, err := us.store.TxExec(tx, ` - UPDATE users + UPDATE users SET is_active = false, deactivated_at = NOW(), deactivated_by_user_id = $1, updated_at = NOW() WHERE id = $2 `, deactivatedByUserID, userID) @@ -981,12 +987,179 @@ func (us *UserService) DeactivateUserInOrg(orgID, userID, deactivatedByUserID in return fmt.Errorf("failed to deactivate user: %w", err) } + revokedCount, err := us.store.TxExec(tx, ` + UPDATE auth_tokens SET is_active = false, revoked_at = NOW() + WHERE user_id = $1 AND is_active = true + `, userID) + if err != nil { + return fmt.Errorf("failed to revoke active sessions: %w", err) + } + // Add audit log err = us.addUserAuditLog(tx, orgID, userID, deactivatedByUserID, "deactivated", map[string]interface{}{}) if err != nil { return fmt.Errorf("failed to add audit log: %w", err) } + if rows, rerr := revokedCount.RowsAffected(); rerr == nil && rows > 0 { + if aerr := us.addUserAuditLog(tx, orgID, userID, deactivatedByUserID, "session_revoked", map[string]interface{}{"revoked_count": rows}); aerr != nil { + return fmt.Errorf("failed to add session revocation audit log: %w", aerr) + } + } + + return nil + }) +} + +// ReactivateUserInOrg restores a previously deactivated user. This is the +// only path back to is_active=true - SSO login deliberately never does this +// itself (see errSSOAccountDeactivated in internal/api/auth/sso_handlers.go). +func (us *UserService) ReactivateUserInOrg(orgID, userID, reactivatedByUserID int64) error { + return us.store.WithTx(func(tx *sql.Tx) error { + _, err := us.store.TxExec(tx, ` + UPDATE users + SET is_active = true, deactivated_at = NULL, deactivated_by_user_id = NULL, updated_at = NOW() + WHERE id = $1 + `, userID) + if err != nil { + return fmt.Errorf("failed to reactivate user: %w", err) + } + + if err := us.addUserAuditLog(tx, orgID, userID, reactivatedByUserID, "reactivated", map[string]interface{}{}); err != nil { + return fmt.Errorf("failed to add audit log: %w", err) + } + + return nil + }) +} + +// ErrSCIMAccountDeactivated is returned by ProvisionSCIMUser when the +// matched user has been manually deactivated. Like SSO JIT login, SCIM +// provisioning must never silently resurrect a deactivated account. +var ErrSCIMAccountDeactivated = errors.New("scim: account is deactivated") + +// ProvisionSCIMUser creates or links a user into orgID via SCIM. It mirrors +// the SSO JIT-provisioning create path (internal/api/auth/sso_handlers.go's +// jitProvisionSSOUser): a random unusable password and member role, with no +// invitation email - the IdP is the source of truth for this account, not +// an email invite, which is why this doesn't just call the owner-driven +// CreateUserInOrg above. Returns the user and whether a new user_roles row +// was created (true also covers "newly created user" since the row is +// always new in that case). +func (us *UserService) ProvisionSCIMUser(orgID int64, email, firstName, lastName string) (*UserWithRole, bool, error) { + var userID int64 + created := false + + err := us.store.WithTx(func(tx *sql.Tx) error { + var existingUserID int64 + var isActive bool + err := us.store.TxQueryRow(tx, `SELECT id, is_active FROM users WHERE email = $1`, email).Scan(&existingUserID, &isActive) + if err == sql.ErrNoRows { + randBytes := make([]byte, 32) + if _, rerr := rand.Read(randBytes); rerr != nil { + return rerr + } + hashed, herr := bcrypt.GenerateFromPassword([]byte(hex.EncodeToString(randBytes)), bcrypt.DefaultCost) + if herr != nil { + return herr + } + if err := us.store.TxQueryRow(tx, ` + INSERT INTO users (email, password_hash, first_name, last_name, default_org_id, password_reset_required, created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, false, NOW(), NOW()) + RETURNING id + `, email, string(hashed), nullableStr(firstName), nullableStr(lastName), orgID).Scan(&userID); err != nil { + return err + } + if _, err := us.store.TxExec(tx, ` + INSERT INTO user_roles (user_id, org_id, role_id) + VALUES ($1, $2, (SELECT id FROM roles WHERE name = 'member')) + `, userID, orgID); err != nil { + return err + } + created = true + return nil + } else if err != nil { + return err + } + + userID = existingUserID + if !isActive { + return ErrSCIMAccountDeactivated + } + var isMember bool + if err := us.store.TxQueryRow(tx, `SELECT EXISTS(SELECT 1 FROM user_roles WHERE user_id = $1 AND org_id = $2)`, userID, orgID).Scan(&isMember); err != nil { + return err + } + if !isMember { + if _, err := us.store.TxExec(tx, ` + INSERT INTO user_roles (user_id, org_id, role_id) + VALUES ($1, $2, (SELECT id FROM roles WHERE name = 'member')) + `, userID, orgID); err != nil { + return err + } + created = true + } + if _, err := us.store.TxExec(tx, `UPDATE users SET default_org_id = COALESCE(default_org_id, $1) WHERE id = $2`, orgID, userID); err != nil { + return err + } + return nil + }) + if err != nil { + return nil, false, err + } + + user, err := us.GetUserInOrg(orgID, userID) + if err != nil { + return nil, false, err + } + return user, created, nil +} + +func nullableStr(s string) interface{} { + if s == "" { + return nil + } + return s +} + +// DeactivateUserInOrgBySCIM deactivates a user as part of SCIM-driven +// deprovisioning (PATCH active:false) and revokes their live sessions, same +// as DeactivateUserInOrg. There is no human actor to record here - the +// actor is the IdP, via SCIM - so deactivated_by_user_id is left NULL and +// the unified audit event is emitted by the caller +// (internal/api/scim_handlers.go) via auditRecorder.Webhook instead of the +// user_management_audit mirror addUserAuditLog uses, which requires a real +// performed_by_user_id (NOT NULL, FK to users). +func (us *UserService) DeactivateUserInOrgBySCIM(userID int64) error { + return us.store.WithTx(func(tx *sql.Tx) error { + if _, err := us.store.TxExec(tx, ` + UPDATE users SET is_active = false, deactivated_at = NOW(), deactivated_by_user_id = NULL, updated_at = NOW() + WHERE id = $1 + `, userID); err != nil { + return fmt.Errorf("failed to deactivate user: %w", err) + } + if _, err := us.store.TxExec(tx, ` + UPDATE auth_tokens SET is_active = false, revoked_at = NOW() + WHERE user_id = $1 AND is_active = true + `, userID); err != nil { + return fmt.Errorf("failed to revoke active sessions: %w", err) + } + return nil + }) +} + +// ReactivateUserInOrgBySCIM mirrors DeactivateUserInOrgBySCIM for PATCH +// active:true - uses the same store.WithTx wrapper as its sibling (rather +// than us.db.Exec directly) so a future addition here (e.g. an audit insert +// in the same transaction) doesn't have to be retrofitted for atomicity. +func (us *UserService) ReactivateUserInOrgBySCIM(userID int64) error { + return us.store.WithTx(func(tx *sql.Tx) error { + if _, err := us.store.TxExec(tx, ` + UPDATE users SET is_active = true, deactivated_at = NULL, deactivated_by_user_id = NULL, updated_at = NOW() + WHERE id = $1 + `, userID); err != nil { + return fmt.Errorf("failed to reactivate user: %w", err) + } return nil }) } @@ -1123,6 +1296,8 @@ func userAuditAction(action string) (auditlog.Action, bool) { return auditlog.ActionUserDeactivate, true case "reactivated": return auditlog.ActionUserReactivate, true + case "session_revoked": + return auditlog.ActionSessionRevoke, true case "role_changed": return auditlog.ActionUserRoleChange, true case "password_reset": diff --git a/internal/auditlog/catalog.go b/internal/auditlog/catalog.go index f1c47c707..14a05d884 100644 --- a/internal/auditlog/catalog.go +++ b/internal/auditlog/catalog.go @@ -114,6 +114,14 @@ const ( ActionIdentityAdd Action = "auth.identity.add" ActionIdentityRemove Action = "auth.identity.remove" ActionSSOProvision Action = "auth.sso.provision" + ActionSessionRevoke Action = "auth.session.revoke" + + ActionSCIMUserCreate Action = "scim.user.create" + ActionSCIMUserUpdate Action = "scim.user.update" + ActionSCIMUserDeactivate Action = "scim.user.deactivate" + ActionSCIMUserReactivate Action = "scim.user.reactivate" + ActionSCIMTokenCreate Action = "scim.token.create" + ActionSCIMTokenRevoke Action = "scim.token.revoke" ActionUserCreate Action = "user.create" ActionUserUpdate Action = "user.update" @@ -255,6 +263,13 @@ var actionCatalog = func() map[Action]ActionMeta { meta(ActionIdentityAdd, CategoryAuth, "identity", "Identity linked", "A login identity was linked to a user.", SeverityWarning), meta(ActionIdentityRemove, CategoryAuth, "identity", "Identity unlinked", "A login identity was removed from a user.", SeverityWarning), meta(ActionSSOProvision, CategoryAuth, "user", "SSO account provisioned", "An account was provisioned through SSO.", SeverityInfo), + sensitive(meta(ActionSessionRevoke, CategoryAuth, "session", "Sessions revoked", "A user's active sessions and refresh tokens were revoked, typically following deactivation.", SeverityWarning)), + sensitive(meta(ActionSCIMUserCreate, CategoryAuth, "user", "SCIM user provisioned", "An identity provider created or linked a user via SCIM.", SeverityInfo)), + sensitive(meta(ActionSCIMUserUpdate, CategoryAuth, "user", "SCIM user updated", "An identity provider updated a user's attributes via SCIM.", SeverityInfo)), + sensitive(meta(ActionSCIMUserDeactivate, CategoryAuth, "user", "SCIM user deactivated", "An identity provider deactivated a user via SCIM.", SeverityWarning)), + sensitive(meta(ActionSCIMUserReactivate, CategoryAuth, "user", "SCIM user reactivated", "An identity provider reactivated a user via SCIM.", SeverityWarning)), + sensitive(meta(ActionSCIMTokenCreate, CategoryAuth, "scim_token", "SCIM token created", "An org's SCIM bearer token was generated or rotated.", SeverityWarning)), + sensitive(meta(ActionSCIMTokenRevoke, CategoryAuth, "scim_token", "SCIM token revoked", "An org's SCIM bearer token was revoked.", SeverityWarning)), sensitive(meta(ActionUserCreate, CategoryUser, "user", "User created", "A user was added to the organization.", SeverityWarning)), sensitive(meta(ActionUserUpdate, CategoryUser, "user", "User updated", "A user's profile or settings were changed.", SeverityWarning)), diff --git a/internal/docindex/docs/routes_guide/settings/sso.md b/internal/docindex/docs/routes_guide/settings/sso.md index d46d5d84b..1c5088f97 100644 --- a/internal/docindex/docs/routes_guide/settings/sso.md +++ b/internal/docindex/docs/routes_guide/settings/sso.md @@ -58,6 +58,59 @@ real gatekeeper. - Flip **Enabled** on a saved config to let users sign in with it immediately, or off to stop new SSO logins without losing the saved settings. +## Deprovisioning: how a removed user actually loses access + +JIT provisioning only covers getting users *in*. There are three distinct +mechanisms for getting a removed/disabled IdP user *out*, each covering a +different gap: + +1. **Manual deactivation always works.** An owner deactivating a user + (Settings → Users) immediately revokes every one of that user's live + sessions - their JWT stops working on the very next request, not just on + future logins. SSO login itself can never undo a deactivation: if a + deactivated user's IdP credentials still work, login is rejected rather + than silently reactivating them. Reactivating is an explicit admin action. +2. **SCIM (below) is the real-time, automated answer**, if the org wires it + up at their IdP. Without it, deprovisioning depends on a human noticing + or the two fallbacks below. +3. **The reconciliation poll** (OIDC panel, "Automatic deprovisioning + check") and **OIDC back-channel logout** are two best-effort fallbacks + for orgs that skip SCIM - see their own sections below. Neither applies + to SAML; a SAML-only org that wants automated deprovisioning needs SCIM. + +## SCIM provisioning + +A "SCIM Provisioning" panel sits below the OIDC/SAML tabs on this same page, +independent of which login protocol(s) are configured. This lets the IdP +itself (Okta, Azure AD/Entra ID, Google Workspace) push user lifecycle +events directly - most importantly, deactivating a LiveReview user the +moment the IdP admin removes or disables them there, rather than waiting for +that user to attempt another login. + +- **Generate Token** creates the org's SCIM bearer token, shown once (same + reveal-once convention as an API key) - copy it into the IdP's SCIM app + configuration alongside the shown **SCIM base URL** + (`{origin}/api/v1/scim/v2`). +- **Rotate Token** replaces the token; the old one stops working + immediately, so the IdP's SCIM config needs updating with the new one. +- **Revoke** disables SCIM entirely until a new token is generated. +- Requires the `scim` plan feature (same plan entry as `sso`, + `internal/license/plans.go`'s `PlanEnterpriseSelfhosted`). +- The IdP authenticates every SCIM call with this bearer token alone - there + is no user session involved, and the org is resolved from the token + itself, never a client-supplied id. + +## Automatic deprovisioning check (OIDC reconciliation poll) + +Only shown on the OIDC panel. A fallback for orgs that configure OIDC login +but never wire up SCIM above (a separate integration step many admins skip). +When enabled, LiveReview periodically re-checks each signed-in user against +the IdP (silently refreshing a stored OAuth token) and deactivates them here +if the IdP rejects it - the same outcome a SCIM deactivation would produce, +just on a delay bounded by the interval you pick (hourly/daily/weekly, +default daily) instead of instantly. Not available for SAML - there's no +equivalent standing credential to silently re-check with that protocol. + ## Where this shows up elsewhere - The self-hosted login page (`ui/src/pages/Auth/SelfHosted.tsx`) calls the diff --git a/internal/jobqueue/jobqueue.go b/internal/jobqueue/jobqueue.go index d886dc306..fc18d9a77 100644 --- a/internal/jobqueue/jobqueue.go +++ b/internal/jobqueue/jobqueue.go @@ -35,17 +35,17 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/livereview/internal/githubapp" + "github.com/livereview/internal/policyengine" "github.com/livereview/internal/providers" "github.com/livereview/internal/providers/gitea" - "github.com/livereview/internal/policyengine" "github.com/livereview/internal/seed_demo" networkjobqueue "github.com/livereview/network/jobqueue" storagejobqueue "github.com/livereview/storage/jobqueue" - "github.com/robfig/cron/v3" "github.com/livereview/storage/providers/pullrequests" "github.com/riverqueue/river" "github.com/riverqueue/river/riverdriver/riverpgxv5" "github.com/riverqueue/river/rivermigrate" + "github.com/robfig/cron/v3" ) // GitLab API response structures @@ -2449,6 +2449,11 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { river.AddWorker(workers, seedDemoWorker) river.AddWorker(workers, &BlastRadiusWorker{db: db}) river.AddWorker(workers, &RiskModelRefitWorker{db: db}) + ssoJWTSecret := os.Getenv("JWT_SECRET") + if ssoJWTSecret == "" { + log.Printf("WARNING: JWT_SECRET not set - SSO reconciliation sweep will be a no-op for every org") + } + river.AddWorker(workers, &SSOReconciliationWorker{db: db, jwtSecret: ssoJWTSecret}) coordinatorInterval := config.RepoSyncConfig.CoordinatorInterval if coordinatorInterval <= 0 { @@ -2568,15 +2573,22 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { }, &river.PeriodicJobOpts{ID: "risk_model_refit", RunOnStart: false}, ), + river.NewPeriodicJob( + river.PeriodicInterval(15*time.Minute), + func() (river.JobArgs, *river.InsertOpts) { + return SSOReconciliationJobArgs{}, &river.InsertOpts{Queue: "repo_sync", MaxAttempts: 3} + }, + &river.PeriodicJobOpts{ID: "sso_reconciliation", RunOnStart: false}, + ), river.NewPeriodicJob( archivalSchedule, func() (river.JobArgs, *river.InsertOpts) { return PreloadedChangesArchivalSweepJobArgs{ - RetentionDays: archivalRetentionDays, - }, &river.InsertOpts{ - Queue: "preloaded_changes_archival_sweep", - MaxAttempts: 3, - } + RetentionDays: archivalRetentionDays, + }, &river.InsertOpts{ + Queue: "preloaded_changes_archival_sweep", + MaxAttempts: 3, + } }, &river.PeriodicJobOpts{ ID: "preloaded_changes_archival_sweep", diff --git a/internal/jobqueue/sso_reconciliation_worker.go b/internal/jobqueue/sso_reconciliation_worker.go new file mode 100644 index 000000000..d3cd472f7 --- /dev/null +++ b/internal/jobqueue/sso_reconciliation_worker.go @@ -0,0 +1,111 @@ +package jobqueue + +import ( + "context" + "database/sql" + "fmt" + + "github.com/livereview/internal/api/users" + "github.com/livereview/internal/sso" + "github.com/riverqueue/river" + "github.com/rs/zerolog/log" + "golang.org/x/oauth2" +) + +// SSOReconciliationJobArgs is a fixed-interval sweep (see jobqueue.go's +// PeriodicJobs) that silently re-checks "is this account still valid at the +// IdP" for every OIDC org that has opted into the reconciliation poll (see +// org_sso_configs.reconciliation_enabled) - a fallback deprovisioning path +// for orgs that configure OIDC login but never wire up SCIM at their IdP. +// Per org/identity work happens inline in one Work() call, the same +// "coordinator does the fan-out itself" shape RiskModelRefitWorker already +// uses, rather than enqueueing a separate one-shot job per org. +type SSOReconciliationJobArgs struct{} + +func (SSOReconciliationJobArgs) Kind() string { return "sso_reconciliation" } + +type SSOReconciliationWorker struct { + river.WorkerDefaults[SSOReconciliationJobArgs] + db *sql.DB + jwtSecret string +} + +func (w *SSOReconciliationWorker) Work(ctx context.Context, job *river.Job[SSOReconciliationJobArgs]) error { + if w.jwtSecret == "" { + // Same "disabled, not fatal" treatment server.go gives a missing/empty + // JWT_SECRET for SSO generally - nothing to reconcile without it. Logged + // (rather than silently skipped) since an org with reconciliation + // enabled would otherwise see it silently never run. + log.Warn().Msg("sso reconciliation: JWT_SECRET not set, skipping sweep") + return nil + } + box, err := sso.NewSecretBox(w.jwtSecret) + if err != nil { + log.Warn().Err(err).Msg("sso reconciliation: failed to build secret box, skipping sweep") + return nil + } + configStore := sso.NewStore(w.db, box) + identityStore := sso.NewIdentityStore(w.db, box) + userService := users.NewUserService(w.db, nil) + + due, err := configStore.ListDueForReconciliation() + if err != nil { + return fmt.Errorf("sso reconciliation: listing due configs: %w", err) + } + + for _, cfg := range due { + // Best-effort per org: one org's IdP being unreachable shouldn't + // block the sweep for everyone else. + w.reconcileOrg(ctx, cfg, configStore, identityStore, userService) + } + return nil +} + +func (w *SSOReconciliationWorker) reconcileOrg(ctx context.Context, cfg *sso.Config, configStore *sso.Store, identityStore *sso.IdentityStore, userService *users.UserService) { + fullCfg, err := configStore.Get(cfg.OrgID, sso.ProviderOIDC) + if err != nil { + log.Warn().Err(err).Int64("org_id", cfg.OrgID).Msg("sso reconciliation: failed to load full OIDC config") + return + } + identities, err := identityStore.ListWithRefreshToken(cfg.OrgID, sso.ProviderOIDC) + if err != nil { + log.Warn().Err(err).Int64("org_id", cfg.OrgID).Msg("sso reconciliation: failed to list identities with refresh tokens") + return + } + if len(identities) == 0 { + if err := configStore.MarkReconciled(cfg.OrgID, sso.ProviderOIDC); err != nil { + log.Warn().Err(err).Int64("org_id", cfg.OrgID).Msg("sso reconciliation: failed to mark org reconciled (no identities)") + } + return + } + + oauthCfg, _, err := sso.BuildOIDCOAuth2Config(ctx, fullCfg, "") + if err != nil { + log.Warn().Err(err).Int64("org_id", cfg.OrgID).Msg("sso reconciliation: failed to build OIDC OAuth2 config") + return + } + + for _, identity := range identities { + _, refreshErr := oauthCfg.TokenSource(ctx, &oauth2.Token{RefreshToken: identity.RefreshToken}).Token() + if refreshErr != nil { + // The IdP rejected the refresh - invalid_grant/invalid_token is + // exactly what Okta/Azure/Google return for a disabled, deleted, + // or de-authorized account. Deactivate, same as a SCIM + // PATCH active:false would. If the deactivation itself fails, + // the identity is deliberately left unmarked (not MarkChecked) + // so it's picked up again next reconciliation cycle instead of + // silently staying active. + if err := userService.DeactivateUserInOrgBySCIM(identity.UserID); err != nil { + log.Error().Err(err).Int64("org_id", cfg.OrgID).Int64("user_id", identity.UserID). + Msg("sso reconciliation: IdP rejected refresh but deactivating the user failed") + } + continue + } + if err := identityStore.MarkChecked(identity.ID); err != nil { + log.Warn().Err(err).Int64("org_id", cfg.OrgID).Int64("identity_id", identity.ID).Msg("sso reconciliation: failed to mark identity checked") + } + } + if err := configStore.MarkReconciled(cfg.OrgID, sso.ProviderOIDC); err != nil { + log.Warn().Err(err).Int64("org_id", cfg.OrgID).Msg("sso reconciliation: failed to mark org reconciled") + } +} diff --git a/internal/license/payment/integration_test.go b/internal/license/payment/integration_test.go index eaaa3f2d7..2c912eb0c 100644 --- a/internal/license/payment/integration_test.go +++ b/internal/license/payment/integration_test.go @@ -13,6 +13,7 @@ import ( // TestSubscriptionIntegration tests the complete subscription flow // Run with: go test -v ./internal/license/payment -run TestSubscriptionIntegration func TestSubscriptionIntegration(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // Skip if not in integration test mode if testing.Short() { t.Skip("Skipping integration test in short mode") diff --git a/internal/license/payment/payment_test.go b/internal/license/payment/payment_test.go index e40073016..28c04d0ed 100644 --- a/internal/license/payment/payment_test.go +++ b/internal/license/payment/payment_test.go @@ -7,6 +7,7 @@ import ( ) func TestGetRazoarPayKeys(t *testing.T) { + skipWithoutRazorpayTestKeys(t) accessKey, secretKey, err := GetRazorpayKeys("test") if err != nil { t.Errorf("Error getting Razorpay keys: %v", err) @@ -20,6 +21,7 @@ func TestGetRazoarPayKeys(t *testing.T) { } func TestCreateMonthlyPlan(t *testing.T) { + skipWithoutRazorpayTestKeys(t) plan, err := CreatePlan("test", "monthly") if err != nil { t.Errorf("Error creating monthly plan: %v", err) @@ -41,6 +43,7 @@ func TestCreateMonthlyPlan(t *testing.T) { } func TestCreateYearlyPlan(t *testing.T) { + skipWithoutRazorpayTestKeys(t) plan, err := CreatePlan("test", "yearly") if err != nil { t.Errorf("Error creating yearly plan: %v", err) @@ -62,6 +65,7 @@ func TestCreateYearlyPlan(t *testing.T) { } func TestGetAllPlans(t *testing.T) { + skipWithoutRazorpayTestKeys(t) planList, err := GetAllPlans("test") if err != nil { t.Errorf("Error fetching all plans: %v", err) @@ -80,6 +84,7 @@ func TestGetAllPlans(t *testing.T) { } func TestGetPlanByID(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // First, get all plans to get a valid plan ID planList, err := GetAllPlans("test") if err != nil { diff --git a/internal/license/payment/razorpay_live_test.go b/internal/license/payment/razorpay_live_test.go new file mode 100644 index 000000000..96429a867 --- /dev/null +++ b/internal/license/payment/razorpay_live_test.go @@ -0,0 +1,18 @@ +package payment + +import ( + "os" + "testing" +) + +// skipWithoutRazorpayTestKeys skips a test that hits the real Razorpay test-mode +// API when RAZORPAY_TEST_KEY/RAZORPAY_TEST_SECRET aren't configured, rather than +// failing - these are manual verification scripts against a live sandbox +// account, not hermetic unit tests, and most environments (CI, local dev +// without sandbox credentials) have no reason to run them. +func skipWithoutRazorpayTestKeys(t *testing.T) { + t.Helper() + if os.Getenv("RAZORPAY_TEST_KEY") == "" || os.Getenv("RAZORPAY_TEST_SECRET") == "" { + t.Skip("skipping: RAZORPAY_TEST_KEY/RAZORPAY_TEST_SECRET not set (live Razorpay sandbox test)") + } +} diff --git a/internal/license/payment/setup_plans_test.go b/internal/license/payment/setup_plans_test.go index 6d8dfa6b5..623711df8 100644 --- a/internal/license/payment/setup_plans_test.go +++ b/internal/license/payment/setup_plans_test.go @@ -9,6 +9,7 @@ import ( // Run with: go test -v ./internal/license/payment -run TestSetupPlans // Use -args test or -args live to specify mode (default: test) func TestSetupPlans(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // Get mode from test args (defaults to "test") mode := "test" // Note: In real usage, you'd pass mode via environment variable: diff --git a/internal/license/payment/subscription_test.go b/internal/license/payment/subscription_test.go index d6accd9c9..e3a278e63 100644 --- a/internal/license/payment/subscription_test.go +++ b/internal/license/payment/subscription_test.go @@ -8,6 +8,7 @@ import ( ) func TestCreateSubscription(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // First, get a plan to subscribe to planList, err := GetAllPlans("test") if err != nil { @@ -56,6 +57,7 @@ func TestCreateSubscription(t *testing.T) { } func TestGetAllSubscriptions(t *testing.T) { + skipWithoutRazorpayTestKeys(t) subList, err := GetAllSubscriptions("test") if err != nil { t.Fatalf("Error fetching all subscriptions: %v", err) @@ -73,6 +75,7 @@ func TestGetAllSubscriptions(t *testing.T) { } func TestGetSubscriptionByID(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // First, get all subscriptions to get a valid subscription ID subList, err := GetAllSubscriptions("test") if err != nil { @@ -100,6 +103,7 @@ func TestGetSubscriptionByID(t *testing.T) { } func TestUpdateSubscriptionQuantity(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // First, create a subscription planList, err := GetAllPlans("test") if err != nil { @@ -157,6 +161,7 @@ func TestUpdateSubscriptionQuantity(t *testing.T) { } func TestCancelSubscription(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // First, create a subscription planList, err := GetAllPlans("test") if err != nil { @@ -208,6 +213,7 @@ func TestCancelSubscription(t *testing.T) { } func TestCancelSubscriptionAtCycleEnd(t *testing.T) { + skipWithoutRazorpayTestKeys(t) // First, create a subscription planList, err := GetAllPlans("test") if err != nil { diff --git a/internal/license/plans.go b/internal/license/plans.go index f87ced7ee..de5acaf8d 100644 --- a/internal/license/plans.go +++ b/internal/license/plans.go @@ -162,6 +162,7 @@ var PlanDefinitions = map[PlanType]PlanLimits{ "email_support", "priority_support", "sso", + "scim", }, }, } diff --git a/internal/mcpagent/agent.go b/internal/mcpagent/agent.go index 7773e3e42..0495bebfb 100644 --- a/internal/mcpagent/agent.go +++ b/internal/mcpagent/agent.go @@ -181,6 +181,15 @@ func (a *Agent) RunTurnWithArtifacts(ctx context.Context, history []HistoryEntry // single LLM call returns SQL + chart specs for up to 5 // interpretations. No plan→finalize two-step. clog.BranchSelected("analytics", 0, 0) + + // This branch never runs through runStepLoop (which otherwise + // does this swap - see swapSystemPrompt's comment), so it has + // to do the same history[0] swap itself. Without it, a turn + // landing here right after an action/product_guidance turn + // keeps that turn's stale system prompt (missing the SQL + // rules, or leaking the tool list into an analytics turn). + history = swapSystemPrompt(history, a.interpretPrompt()) + text, artifacts, debugArt, err := a.runMultiInterpret(ctx, userText, clog, a.mcpSession.OrgID, a.mcpSession.OrgName) if err != nil { return text, history, artifacts, debugArt, err @@ -305,6 +314,29 @@ func (a *Agent) RunTurnWithArtifacts(ctx context.Context, history []HistoryEntry return a.runStepLoop(ctx, history, userText, a.systemPrompt, a.providerTools, 0, false, false, "", clog) } +// swapSystemPrompt replaces (or inserts) history's leading system message +// with systemPrompt. Swapped every turn, not appended once: which prompt a +// turn needs is decided fresh per call, so a session that changes shape +// turn-to-turn (a data question, then "trigger a review for that repo", +// then another data question) must not keep whatever was baked in on turn +// one. The rest of history (actual conversation content) is untouched. +// Used by both runStepLoop and RunTurnWithArtifacts's analytics branch, +// which bypasses runStepLoop entirely but still needs the same swap. +func swapSystemPrompt(history []HistoryEntry, systemPrompt string) []HistoryEntry { + if systemPrompt == "" { + return history + } + switch { + case len(history) == 0: + return append(history, HistoryEntry{"role": "system", "content": systemPrompt}) + case history[0]["role"] == "system": + history[0] = HistoryEntry{"role": "system", "content": systemPrompt} + return history + default: + return append([]HistoryEntry{{"role": "system", "content": systemPrompt}}, history...) + } +} + // runStepLoop is the ReAct tool-calling loop. Extracted from RunTurnWithArtifacts // so the count_query branch can skip it entirely (using runMultiInterpret instead) // while action and chat branches continue through the step loop as before. @@ -320,25 +352,7 @@ func (a *Agent) runStepLoop( schemaTableText string, clog *logging.ChatTurnLogger, ) (string, []HistoryEntry, []Artifact, *DebugArtifacts, error) { - // Swapped every turn, not appended once: which prompt a turn needs is - // decided fresh above, so a session that changes shape turn-to-turn - // (a data question, then "trigger a review for that repo", then - // another data question) must not keep whatever was baked in on turn - // one. The rest of history (actual conversation content) is untouched. - switch { - case len(history) == 0: - if systemPrompt != "" { - history = append(history, HistoryEntry{"role": "system", "content": systemPrompt}) - } - case history[0]["role"] == "system": - if systemPrompt != "" { - history[0] = HistoryEntry{"role": "system", "content": systemPrompt} - } - default: - if systemPrompt != "" { - history = append([]HistoryEntry{{"role": "system", "content": systemPrompt}}, history...) - } - } + history = swapSystemPrompt(history, systemPrompt) history = append(history, HistoryEntry{"role": "user", "content": userText}) for step := 0; step < a.maxSteps; step++ { diff --git a/internal/mcpagent/analytics.go b/internal/mcpagent/analytics.go index 6d83955a3..ee7c96a81 100644 --- a/internal/mcpagent/analytics.go +++ b/internal/mcpagent/analytics.go @@ -1426,19 +1426,29 @@ var DefaultAIErrorSuggestedQuestions = []SuggestedQuestionCategory{ // right next to the correctly-delivered file - a contradiction a user would // rightly read as a bug, because it is one. func assembleAnalyticsResponse(reports []vlrender.VegaLiteReport, notes []string, hasArtifacts bool) string { - if len(reports) == 0 { - if hasArtifacts { - return "I've put the results in a file you can download." + if len(reports) > 0 { + payload, err := json.Marshal(map[string]any{"reports": reports}) + if err != nil { + log.Error().Err(err).Msg("failed to marshal analytics reports") + return strings.Join(notes, "\n\n") + } + out := string(payload) + if len(notes) > 0 { + // Notes precede the JSON: the render path strips the JSON block + // and keeps surrounding prose as the message text. + out = strings.Join(notes, "\n\n") + "\n" + out } - return NoDataAnalyticsResponseText + return out } - payload, err := json.Marshal(map[string]any{"reports": reports}) - if err != nil { - log.Error().Err(err).Msg("failed to marshal analytics reports") - return NoDataAnalyticsResponseText + if len(notes) > 0 { + return strings.Join(notes, "\n\n") + } + if hasArtifacts { + // A file-only turn still needs words. Returning "" renders as an + // empty bubble next to a download the user has no context for. + return "I've put the results in a file you can download." } - // Notes (skip reasons) are only in debug artifacts, not in chat response. - return string(payload) + return NoDataAnalyticsResponseText } // rowsToPreviewCSV converts rows to a CSV string for debug preview. @@ -1688,10 +1698,23 @@ func (a *Agent) runMultiInterpret( interps, ok := parseInterpretations(raw) if !ok { - log.Warn().Str("raw_preview", truncateContent(raw, 200)).Msg("could not parse interpretations from LLM response") + // One corrective retry, the same budget the SQL repair phases already + // get (see repairSQL) - recovers truncated/malformed replies instead + // of giving up the whole report on the first bad JSON. + log.Warn().Str("raw_preview", truncateContent(raw, 200)).Msg("could not parse interpretations from LLM response, retrying with correction") + retryUserMsg := userMsg + "\n\nYour previous reply could not be parsed:\n" + truncateContent(raw, 500) + + "\n\nReply again with ONLY a valid JSON object of the exact {\"analytics_plan\": [...]} shape described above - no prose, no truncation, nothing else." + retryRaw, retryErr := a.completeOnce(ctx, clog, 2, "interpret", "multi", 2, system, retryUserMsg) + if retryErr == nil { + raw = retryRaw + debug.LLMRawResponse = raw + interps, ok = parseInterpretations(raw) + } + } + if !ok { + log.Warn().Str("raw_preview", truncateContent(raw, 200)).Msg("could not parse interpretations from LLM response after retry") debug.RawLLMError = "Failed to parse LLM output as JSON (often caused by max token limits). Raw output:\n" + raw - formatMsg := "> **Model Output Error**\n> \n> The AI returned data in an unexpected format. Please try rephrasing your question." - return formatMsg, nil, debug, nil + return "I could not work out a full query for that - try rephrasing it.", nil, debug, nil } debug.Interpretations = interps diff --git a/internal/mcpagent/analytics_pipeline_test.go b/internal/mcpagent/analytics_pipeline_test.go index 9ad129b10..6e8f5c105 100644 --- a/internal/mcpagent/analytics_pipeline_test.go +++ b/internal/mcpagent/analytics_pipeline_test.go @@ -460,12 +460,12 @@ func TestAnalyticsPipelineProducesCSV(t *testing.T) { t.Fatalf("expected 1 CSV artifact, got %d", len(artifacts)) } // A CSV-only turn (no chart, no notes) must not also claim nothing was - // found - the file itself, delivered separately, IS the answer. Regression - // guard for the exact contradiction seen in production: a correctly - // attached CSV alongside the chat text "I could not find anything to - // answer that." - if text != "" { - t.Fatalf("expected empty response text alongside the CSV artifact, got %q", text) + // found - the file itself, delivered separately, IS the answer. An empty + // response text renders as a context-free empty bubble next to the + // download, so a short acknowledgement is used instead (never the + // contradictory "I could not find anything to answer that."). + if text != "I've put the results in a file you can download." { + t.Fatalf("expected the file-download acknowledgement alongside the CSV artifact, got %q", text) } art := artifacts[0] if art.Filename != "completed.csv" { diff --git a/internal/mcpagent/classify_test.go b/internal/mcpagent/classify_test.go index d84f2e22d..10dcaa772 100644 --- a/internal/mcpagent/classify_test.go +++ b/internal/mcpagent/classify_test.go @@ -6,6 +6,10 @@ import ( "strings" "testing" + "time" + + "github.com/livereview/internal/aiconnectors" + "github.com/livereview/internal/database" storageanalytics "github.com/livereview/storage/analytics" "github.com/tmc/langchaingo/llms" ) @@ -146,8 +150,24 @@ func (nopAnalyticsEngine) Query(context.Context, string, int) (*storageanalytics // database/tool-server behind it - every case below is designed to be // resolved by the guard's own validation (rejecting a table that isn't in // the catalog) or to never call a tool, so nothing here needs live -// infrastructure. -func dispatchTestAgent(prov *recordingProvider) *Agent { +// infrastructure. It still has to wait on the real package-wide schema +// index, though: RunTurnWithArtifacts refuses any turn up front when +// analyticsEnabled() && !schemaIndexReady() (agent.go), and that index is a +// process-wide singleton shared with the testAgent-based tests in +// analytics_pipeline_test.go - without this wait, this test is a race +// against whichever test happens to trigger the (slow, first-time) build. +func dispatchTestAgent(t *testing.T, prov *recordingProvider) *Agent { + t.Helper() + if dsn, err := database.LoadDatabaseURL(); err == nil { + InitSchemaIndex(dsn) + } + if schemaIdx != nil { + select { + case <-schemaIdx.Ready(): + case <-time.After(60 * time.Second): + } + } + session := &MCPSession{ OrgID: 1, OrgName: "TestOrg", @@ -171,21 +191,22 @@ func dispatchTestAgent(prov *recordingProvider) *Agent { // this replaced), turns 2 and 3 would still see turn 1's prompt. func TestDispatchSwapsSystemPromptPerTurn(t *testing.T) { prov := &recordingProvider{replies: []string{ - // Turn 1: chat - `{"response":"chat"}`, + // Turn 1: product_guidance (the old "chat" shape, renamed) + `{"response":"product_guidance"}`, "Hi! I'm Livi, how can I help you?", // Turn 2: action (no tool call in the reply - just confirms the // prompt/tools selection, not the tool-calling loop itself, which // is unchanged and already covered elsewhere) `{"response":"action"}`, "Which repository would you like me to review?", - // Turn 3: count_query - a table outside the catalog so the guard - // rejects it deterministically, without touching a database. - `{"response":"count_query"}`, + // Turn 3: analytics (the old "count_query" shape, renamed) - a table + // outside the catalog so the guard rejects it deterministically, + // without touching a database. + `{"response":"analytics"}`, `{"analytics_plan":[{"id":"r1","question":"bad","count_sql":"SELECT count(*) FROM secret_table"}]}`, "SELECT count(*) AS n FROM public.reviews", // repair attempt, also rejected }} - agent := dispatchTestAgent(prov) + agent := dispatchTestAgent(t, prov) var history []HistoryEntry @@ -300,13 +321,13 @@ func TestDispatchSwapsSystemPromptPerTurn(t *testing.T) { // UI would just render raw JSON. See looksLikeUnrecognizedJSON. func TestChatBranchRejectsFabricatedJSON(t *testing.T) { prov := &recordingProvider{replies: []string{ - `{"response":"chat"}`, + `{"response":"product_guidance"}`, // The model invents its own ad hoc chart shape instead of answering // in prose - none of parseToolCalls/parseAnalyticsPlan recognize it. `{"chart": {"title": "x"}, "learning": {"title": "y"}}`, "I don't have live data for that right now - try asking it as a data question.", }} - agent := dispatchTestAgent(prov) + agent := dispatchTestAgent(t, prov) text, _, _, _, err := agent.RunTurnWithArtifacts(context.Background(), nil, "are engineers using this daily?", "s1", "test") if err != nil { @@ -334,7 +355,7 @@ func TestCountQueryBranchRejectsTruncatedJSON(t *testing.T) { `{"analytics_plan": [{"id": "r1", "question": "q", "count_sql": "SELECT count(*) FROM reviews`, "I could not work out a full query for that - try rephrasing it.", }} - agent := dispatchTestAgent(prov) + agent := dispatchTestAgent(t, prov) text, _, _, _, err := agent.RunTurnWithArtifacts(context.Background(), nil, "what's our billing status?", "s1", "test") if err != nil { @@ -354,13 +375,20 @@ func TestDispatchClassifyFailureDegradesToChat(t *testing.T) { "Still not sure what you mean.", // classify call attempt 2 (the one corrective retry): also unparseable "Sure, I'm happy to help with that.", // the degraded chat-branch call }} - agent := dispatchTestAgent(prov) + agent := dispatchTestAgent(t, prov) text, history, _, _, err := agent.RunTurnWithArtifacts(context.Background(), nil, "hello", "s1", "test") if err != nil { t.Fatalf("expected the turn to degrade gracefully, not error: %v", err) } - if text != "Sure, I'm happy to help with that." { + // A genuine classify-call error (not just an unparseable shape) now + // surfaces an error banner ahead of the degraded reply, so the user + // knows call #0 failed instead of silently getting a plain chat answer + // - see agent.go's classifyErr handling. + wantPrefix := LLMErrorTemplates[aiconnectors.ErrCategoryUnknown].Message + "\n\n" + + "However, here is some general product guidance:\n\n---\n\n" + + "Sure, I'm happy to help with that." + if text != wantPrefix { t.Fatalf("unexpected response %q", text) } if len(prov.toolsPerCall) != 3 { diff --git a/internal/mcpagent/laws_contract_test.go b/internal/mcpagent/laws_contract_test.go index 828b5e185..ccf0c8b20 100644 --- a/internal/mcpagent/laws_contract_test.go +++ b/internal/mcpagent/laws_contract_test.go @@ -36,7 +36,7 @@ func TestLawbookCarriesMachineContracts(t *testing.T) { text string want []string }{ - {"classify", p.classify, []string{`{"response"`, "analytics", "product_guidance", "`action`", "unclassified"}}, + {"classify", p.classify, []string{`"response":`, "analytics", "product_guidance", "`action`", "unclassified"}}, {"plan", p.planTail, []string{"analytics_plan", "count_sql", "`id`"}}, {"finalize", p.finalizeTail, []string{"response_type", "data_sql", "encoding", "csv"}}, } { diff --git a/internal/provider_input/azuredevops/azuredevops_conversion_test.go b/internal/provider_input/azuredevops/azuredevops_conversion_test.go index da2ed7e4b..262f1f043 100644 --- a/internal/provider_input/azuredevops/azuredevops_conversion_test.go +++ b/internal/provider_input/azuredevops/azuredevops_conversion_test.go @@ -72,7 +72,7 @@ func TestConvertAzureDevOpsCommentEvent_RealPayload(t *testing.T) { if event.Comment == nil { t.Fatal("event.Comment is nil, want populated") } - if got, want := event.Comment.Body, "explain the issue\n"; got != want { + if got, want := event.Comment.Body, "explain the issue"; got != want { t.Errorf("Comment.Body = %q, want %q", got, want) } if got, want := event.Comment.ID, "2"; got != want { diff --git a/internal/provider_input/bitbucket/bitbucket_comment_hierarchy_test.go b/internal/provider_input/bitbucket/bitbucket_comment_hierarchy_test.go index bbe102bab..90efc6a59 100644 --- a/internal/provider_input/bitbucket/bitbucket_comment_hierarchy_test.go +++ b/internal/provider_input/bitbucket/bitbucket_comment_hierarchy_test.go @@ -40,7 +40,7 @@ func TestBitbucketCommentHierarchyFromCapture(t *testing.T) { provider := server.BitbucketProviderV2() fileStore := storagecore.NewFileOpsStore() - capturePath := filepath.Join("captures", "bitbucket", "20251014-194407", "bitbucket-webhook-pullrequest_comment_created-body-0025.json") + capturePath := filepath.Join("docs", "raw", "captures", "bitbucket", "20251014-194407", "bitbucket-webhook-pullrequest_comment_created-body-0025.json") body, err := fileStore.ReadFile(capturePath) if err != nil { t.Fatalf("failed to read capture file %s: %v", capturePath, err) diff --git a/internal/provider_output/github/output_client_test.go b/internal/provider_output/github/output_client_test.go index 7a2c381be..459d05d15 100644 --- a/internal/provider_output/github/output_client_test.go +++ b/internal/provider_output/github/output_client_test.go @@ -61,7 +61,7 @@ func TestPostCommentReply_IssueCommentUsesIssueEndpoint(t *testing.T) { require.NoError(t, client.PostCommentReply(event, "token", "hello")) require.Equal(t, "https://api.github.com/repos/owner/repo/issues/42/comments", capturedURL) - require.Equal(t, "hello", capturedBody["body"]) + require.Equal(t, "> @livereviewbot please help\n\n\nhello", capturedBody["body"]) require.NotContains(t, capturedBody, "in_reply_to") } diff --git a/internal/scim/token.go b/internal/scim/token.go new file mode 100644 index 000000000..a4644fe6c --- /dev/null +++ b/internal/scim/token.go @@ -0,0 +1,153 @@ +// Package scim implements the subset of SCIM 2.0 that enterprise IdPs +// (Okta, Azure AD, Google Workspace) actually use for user lifecycle sync: +// create, read, list-by-filter, and PATCH active:false/true. See +// docs/sso.md for the user-facing setup flow and internal/api/scim_handlers.go +// for the HTTP layer that wires this package into internal/api/users. +package scim + +import ( + "crypto/rand" + "crypto/sha256" + "database/sql" + "encoding/base32" + "encoding/hex" + "errors" + "fmt" + "strings" + "time" +) + +// tokenPrefix mirrors api_keys.go's "lr_" scheme (see +// internal/api/api_keys.go's GenerateAPIKey) so SCIM tokens are visually +// distinct from regular API keys and session tokens. +const tokenPrefix = "scim_" + +var ( + ErrTokenNotFound = errors.New("scim: token not found") + ErrTokenRevoked = errors.New("scim: token revoked") + ErrTokenExpired = errors.New("scim: token expired") +) + +// GenerateToken creates a new SCIM bearer token in the form "scim_". +func GenerateToken() (string, error) { + randomBytes := make([]byte, 32) + if _, err := rand.Read(randomBytes); err != nil { + return "", fmt.Errorf("failed to generate random bytes: %w", err) + } + encoded := base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(randomBytes) + return tokenPrefix + strings.ToLower(encoded), nil +} + +// HashToken returns the SHA-256 hash of a SCIM token, which is what's +// actually stored - the plaintext token is only ever shown once, at +// creation time, same as an API key. +func HashToken(token string) string { + hash := sha256.Sum256([]byte(token)) + return hex.EncodeToString(hash[:]) +} + +// TokenPrefix extracts a short, non-secret prefix for display in the +// settings UI (e.g. "scim_abcd1234..."), same convention as +// api_keys.go's GetKeyPrefix. +func TokenPrefix(token string) string { + stripped := strings.TrimPrefix(token, tokenPrefix) + if len(stripped) > 8 { + stripped = stripped[:8] + } + return tokenPrefix + stripped +} + +// Token is a SCIM bearer token record for one org. +type Token struct { + ID int64 + OrgID int64 + TokenPrefix string + CreatedAt time.Time + ExpiresAt *time.Time + RevokedAt *time.Time + CreatedByUserID *int64 +} + +// TokenStore manages the scim_tokens table. +type TokenStore struct { + db *sql.DB +} + +func NewTokenStore(db *sql.DB) *TokenStore { + return &TokenStore{db: db} +} + +// Create generates a new token for orgID, replacing any existing one (an +// org has at most one live SCIM token at a time - rotating means the old +// one stops working, same mental model as rotating an API key). Returns the +// plaintext token, which the caller must show to the user exactly once. +func (s *TokenStore) Create(orgID int64, createdByUserID int64) (plaintext string, tok *Token, err error) { + plaintext, err = GenerateToken() + if err != nil { + return "", nil, err + } + hash := HashToken(plaintext) + prefix := TokenPrefix(plaintext) + + tok = &Token{} + err = s.db.QueryRow(` + INSERT INTO scim_tokens (org_id, token_hash, token_prefix, created_by_user_id, created_at) + VALUES ($1, $2, $3, $4, NOW()) + RETURNING id, org_id, token_prefix, created_at, expires_at, revoked_at, created_by_user_id + `, orgID, hash, prefix, createdByUserID).Scan( + &tok.ID, &tok.OrgID, &tok.TokenPrefix, &tok.CreatedAt, &tok.ExpiresAt, &tok.RevokedAt, &tok.CreatedByUserID, + ) + if err != nil { + return "", nil, fmt.Errorf("failed to create scim token: %w", err) + } + return plaintext, tok, nil +} + +// Authenticate resolves a plaintext bearer token to the org it belongs to. +// Returns ErrTokenNotFound/ErrTokenRevoked/ErrTokenExpired so the HTTP layer +// can give a precise SCIM error response. +func (s *TokenStore) Authenticate(plaintext string) (orgID int64, err error) { + hash := HashToken(plaintext) + var revokedAt *time.Time + var expiresAt *time.Time + row := s.db.QueryRow(`SELECT org_id, revoked_at, expires_at FROM scim_tokens WHERE token_hash = $1`, hash) + if scanErr := row.Scan(&orgID, &revokedAt, &expiresAt); scanErr != nil { + if scanErr == sql.ErrNoRows { + return 0, ErrTokenNotFound + } + return 0, fmt.Errorf("failed to look up scim token: %w", scanErr) + } + if revokedAt != nil { + return 0, ErrTokenRevoked + } + if expiresAt != nil && expiresAt.Before(time.Now()) { + return 0, ErrTokenExpired + } + return orgID, nil +} + +// Get returns orgID's current live token, if any (nil, nil if none exists). +func (s *TokenStore) Get(orgID int64) (*Token, error) { + tok := &Token{} + err := s.db.QueryRow(` + SELECT id, org_id, token_prefix, created_at, expires_at, revoked_at, created_by_user_id + FROM scim_tokens WHERE org_id = $1 AND revoked_at IS NULL + ORDER BY created_at DESC LIMIT 1 + `, orgID).Scan(&tok.ID, &tok.OrgID, &tok.TokenPrefix, &tok.CreatedAt, &tok.ExpiresAt, &tok.RevokedAt, &tok.CreatedByUserID) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to load scim token: %w", err) + } + return tok, nil +} + +// Revoke invalidates orgID's current token immediately. +func (s *TokenStore) Revoke(orgID int64) error { + _, err := s.db.Exec(`UPDATE scim_tokens SET revoked_at = NOW() WHERE org_id = $1 AND revoked_at IS NULL`, orgID) + if err != nil { + return fmt.Errorf("failed to revoke scim token: %w", err) + } + return nil +} diff --git a/internal/scim/types.go b/internal/scim/types.go new file mode 100644 index 000000000..63d9594db --- /dev/null +++ b/internal/scim/types.go @@ -0,0 +1,78 @@ +package scim + +import "strconv" + +const ( + SchemaUser = "urn:ietf:params:scim:schemas:core:2.0:User" + SchemaListResponse = "urn:ietf:params:scim:api:messages:2.0:ListResponse" + SchemaPatchOp = "urn:ietf:params:scim:api:messages:2.0:PatchOp" + SchemaError = "urn:ietf:params:scim:api:messages:2.0:Error" +) + +// UserResource is the minimal SCIM User representation LiveReview speaks - +// enough for Okta/Azure AD/Google's lifecycle-sync use case (create, +// look up by userName/email, toggle active). Fields no customer IdP relies +// on for this use case (groups, x509Certificates, etc.) are intentionally +// omitted rather than stubbed out. +type UserResource struct { + Schemas []string `json:"schemas"` + ID string `json:"id"` + UserName string `json:"userName"` + Name *NameResource `json:"name,omitempty"` + Emails []EmailResource `json:"emails,omitempty"` + Active bool `json:"active"` + Meta *MetaResource `json:"meta,omitempty"` +} + +type NameResource struct { + GivenName string `json:"givenName,omitempty"` + FamilyName string `json:"familyName,omitempty"` +} + +type EmailResource struct { + Value string `json:"value"` + Primary bool `json:"primary,omitempty"` +} + +type MetaResource struct { + ResourceType string `json:"resourceType"` + Created string `json:"created,omitempty"` + LastModified string `json:"lastModified,omitempty"` +} + +type ListResponse struct { + Schemas []string `json:"schemas"` + TotalResults int `json:"totalResults"` + ItemsPerPage int `json:"itemsPerPage"` + StartIndex int `json:"startIndex"` + Resources []UserResource `json:"Resources"` +} + +// PatchRequest is the body of a SCIM PATCH /Users/:id call. IdPs send this +// to deprovision on offboarding: {"Operations":[{"op":"replace","value":{"active":false}}]}. +type PatchRequest struct { + Schemas []string `json:"schemas"` + Operations []PatchOperation `json:"Operations"` +} + +type PatchOperation struct { + Op string `json:"op"` + Path string `json:"path,omitempty"` + Value any `json:"value,omitempty"` +} + +// ErrorResponse is the SCIM-shaped error body every handler returns on +// failure, per RFC 7644 §3.12. +type ErrorResponse struct { + Schemas []string `json:"schemas"` + Status string `json:"status"` + Detail string `json:"detail"` +} + +func NewError(status int, detail string) ErrorResponse { + return ErrorResponse{ + Schemas: []string{SchemaError}, + Status: strconv.Itoa(status), + Detail: detail, + } +} diff --git a/internal/sso/config.go b/internal/sso/config.go index 43bcfc085..270da12b1 100644 --- a/internal/sso/config.go +++ b/internal/sso/config.go @@ -4,6 +4,7 @@ import ( "database/sql" "errors" "fmt" + "time" ) // ProviderType is the SSO protocol a config row speaks. @@ -31,6 +32,14 @@ type Config struct { SAMLIDPMetadataURL string SAMLIDPMetadataXML string SAMLIDPEntityID string + + // Reconciliation fields only apply to provider_type=oidc - see Phase 5 + // of docs/sso.md's deprovisioning section. SAML has no standing + // credential to silently re-check, so these are always false/zero for + // SAML configs. + ReconciliationEnabled bool + ReconciliationInterval string // "hourly" | "daily" | "weekly" + LastReconciledAt *time.Time } // Store persists org_sso_configs, transparently encrypting/decrypting the @@ -51,7 +60,8 @@ func (s *Store) Get(orgID int64, provider ProviderType) (*Config, error) { row := s.db.QueryRow(` SELECT id, org_id, provider_type, display_name, enabled, COALESCE(oidc_issuer, ''), COALESCE(oidc_client_id, ''), COALESCE(oidc_client_secret_enc, ''), COALESCE(oidc_scopes, ''), - COALESCE(saml_idp_metadata_url, ''), COALESCE(saml_idp_metadata_xml, ''), COALESCE(saml_idp_entity_id, '') + COALESCE(saml_idp_metadata_url, ''), COALESCE(saml_idp_metadata_xml, ''), COALESCE(saml_idp_entity_id, ''), + reconciliation_enabled, reconciliation_interval, last_reconciled_at FROM org_sso_configs WHERE org_id = $1 AND provider_type = $2 `, orgID, string(provider)) @@ -60,7 +70,8 @@ func (s *Store) Get(orgID int64, provider ProviderType) (*Config, error) { var encSecret string if err := row.Scan(&cfg.ID, &cfg.OrgID, &cfg.ProviderType, &cfg.DisplayName, &cfg.Enabled, &cfg.OIDCIssuer, &cfg.OIDCClientID, &encSecret, &cfg.OIDCScopes, - &cfg.SAMLIDPMetadataURL, &cfg.SAMLIDPMetadataXML, &cfg.SAMLIDPEntityID); err != nil { + &cfg.SAMLIDPMetadataURL, &cfg.SAMLIDPMetadataXML, &cfg.SAMLIDPEntityID, + &cfg.ReconciliationEnabled, &cfg.ReconciliationInterval, &cfg.LastReconciledAt); err != nil { if err == sql.ErrNoRows { return nil, ErrNotFound } @@ -122,13 +133,18 @@ func (s *Store) UpsertTx(exec sqlExecer, cfg *Config, createdByUserID int64) err if err != nil { return fmt.Errorf("sso: encrypting client secret: %w", err) } + reconciliationInterval := cfg.ReconciliationInterval + if reconciliationInterval == "" { + reconciliationInterval = "daily" + } _, err = exec.Exec(` INSERT INTO org_sso_configs ( org_id, provider_type, display_name, enabled, oidc_issuer, oidc_client_id, oidc_client_secret_enc, oidc_scopes, saml_idp_metadata_url, saml_idp_metadata_xml, saml_idp_entity_id, + reconciliation_enabled, reconciliation_interval, created_by_user_id - ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12) + ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14) ON CONFLICT (org_id, provider_type) DO UPDATE SET display_name = EXCLUDED.display_name, enabled = EXCLUDED.enabled, @@ -139,10 +155,13 @@ func (s *Store) UpsertTx(exec sqlExecer, cfg *Config, createdByUserID int64) err saml_idp_metadata_url = EXCLUDED.saml_idp_metadata_url, saml_idp_metadata_xml = EXCLUDED.saml_idp_metadata_xml, saml_idp_entity_id = EXCLUDED.saml_idp_entity_id, + reconciliation_enabled = EXCLUDED.reconciliation_enabled, + reconciliation_interval = EXCLUDED.reconciliation_interval, updated_at = NOW() `, cfg.OrgID, string(cfg.ProviderType), cfg.DisplayName, cfg.Enabled, nullable(cfg.OIDCIssuer), nullable(cfg.OIDCClientID), nullable(encSecret), nullable(cfg.OIDCScopes), nullable(cfg.SAMLIDPMetadataURL), nullable(cfg.SAMLIDPMetadataXML), nullable(cfg.SAMLIDPEntityID), + cfg.ReconciliationEnabled, reconciliationInterval, nullableID(createdByUserID)) if err != nil { return fmt.Errorf("sso: upserting config: %w", err) @@ -163,6 +182,49 @@ func (s *Store) DeleteTx(exec sqlExecer, orgID int64, provider ProviderType) err return nil } +// ListDueForReconciliation returns every OIDC config with the reconciliation +// poll enabled whose interval has elapsed since it last ran (or that has +// never run). Used by the coordinator job (internal/jobqueue/sso_reconciliation_worker.go) +// on a fixed short tick to decide which orgs to fan out one-shot work to - +// see Phase 5 of docs/sso.md's deprovisioning section for why this is a +// coordinator+fan-out job rather than one River PeriodicJob per org. +func (s *Store) ListDueForReconciliation() ([]*Config, error) { + rows, err := s.db.Query(` + SELECT id, org_id, provider_type, reconciliation_interval, last_reconciled_at + FROM org_sso_configs + WHERE provider_type = 'oidc' AND reconciliation_enabled = TRUE AND ( + last_reconciled_at IS NULL + OR (reconciliation_interval = 'hourly' AND last_reconciled_at < NOW() - INTERVAL '1 hour') + OR (reconciliation_interval = 'daily' AND last_reconciled_at < NOW() - INTERVAL '1 day') + OR (reconciliation_interval = 'weekly' AND last_reconciled_at < NOW() - INTERVAL '7 days') + ) + `) + if err != nil { + return nil, fmt.Errorf("sso: listing configs due for reconciliation: %w", err) + } + defer rows.Close() + + var out []*Config + for rows.Next() { + cfg := &Config{} + if err := rows.Scan(&cfg.ID, &cfg.OrgID, &cfg.ProviderType, &cfg.ReconciliationInterval, &cfg.LastReconciledAt); err != nil { + return nil, fmt.Errorf("sso: scanning config due for reconciliation: %w", err) + } + out = append(out, cfg) + } + return out, rows.Err() +} + +// MarkReconciled stamps last_reconciled_at = NOW() once a reconciliation +// sweep finishes for (orgID, provider). +func (s *Store) MarkReconciled(orgID int64, provider ProviderType) error { + _, err := s.db.Exec(`UPDATE org_sso_configs SET last_reconciled_at = NOW() WHERE org_id = $1 AND provider_type = $2`, orgID, string(provider)) + if err != nil { + return fmt.Errorf("sso: marking config reconciled: %w", err) + } + return nil +} + func nullable(s string) interface{} { if s == "" { return nil diff --git a/internal/sso/identity_store.go b/internal/sso/identity_store.go new file mode 100644 index 000000000..194143171 --- /dev/null +++ b/internal/sso/identity_store.go @@ -0,0 +1,110 @@ +package sso + +import ( + "database/sql" + "fmt" +) + +// IdentityStore persists sso_identities: the mapping from an IdP's own +// subject identifier (OIDC `sub` / SAML NameID) to a LiveReview user, per +// org and provider. Two things need this mapping, neither of which a +// LiveReview user id alone can satisfy: +// - An incoming OIDC back-channel logout token or SAML LogoutRequest only +// ever carries the IdP's subject, never a LiveReview user id. +// - The reconciliation poll (Phase 5) needs an encrypted refresh token per +// identity to silently re-check "is this account still valid at the IdP". +type IdentityStore struct { + db *sql.DB + box *SecretBox +} + +func NewIdentityStore(db *sql.DB, box *SecretBox) *IdentityStore { + return &IdentityStore{db: db, box: box} +} + +// Upsert records (or refreshes) the subject->user mapping for a successful +// login, and the refresh token if one was issued (empty string clears it - +// callers pass "" when the IdP didn't grant one, e.g. reconciliation isn't +// enabled for this config). +func (s *IdentityStore) Upsert(orgID, userID int64, provider ProviderType, subject, refreshToken string) error { + encRefresh, err := s.box.Encrypt(refreshToken) + if err != nil { + return fmt.Errorf("sso: encrypting refresh token: %w", err) + } + _, err = s.db.Exec(` + INSERT INTO sso_identities (org_id, user_id, provider_type, subject, oidc_refresh_token_enc, created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, NOW(), NOW()) + ON CONFLICT (org_id, provider_type, subject) DO UPDATE SET + user_id = EXCLUDED.user_id, + oidc_refresh_token_enc = CASE WHEN EXCLUDED.oidc_refresh_token_enc IS NOT NULL THEN EXCLUDED.oidc_refresh_token_enc ELSE sso_identities.oidc_refresh_token_enc END, + updated_at = NOW() + `, orgID, userID, string(provider), subject, nullable(encRefresh)) + if err != nil { + return fmt.Errorf("sso: upserting identity: %w", err) + } + return nil +} + +// LookupUserBySubject resolves an incoming logout event's subject to a +// LiveReview user id, for back-channel logout / SAML SLO (Phase 4). +func (s *IdentityStore) LookupUserBySubject(orgID int64, provider ProviderType, subject string) (userID int64, err error) { + err = s.db.QueryRow(` + SELECT user_id FROM sso_identities WHERE org_id = $1 AND provider_type = $2 AND subject = $3 + `, orgID, string(provider), subject).Scan(&userID) + if err != nil { + if err == sql.ErrNoRows { + return 0, fmt.Errorf("sso: no identity found for subject") + } + return 0, fmt.Errorf("sso: looking up identity: %w", err) + } + return userID, nil +} + +// IdentityWithRefreshToken is one row the reconciliation worker checks. +type IdentityWithRefreshToken struct { + ID int64 + UserID int64 + Subject string + RefreshToken string +} + +// ListWithRefreshToken returns every stored identity for orgID that has a +// refresh token on file, for the reconciliation worker +// (internal/jobqueue/sso_reconciliation_worker.go) to silently re-check. +func (s *IdentityStore) ListWithRefreshToken(orgID int64, provider ProviderType) ([]IdentityWithRefreshToken, error) { + rows, err := s.db.Query(` + SELECT id, user_id, subject, oidc_refresh_token_enc + FROM sso_identities + WHERE org_id = $1 AND provider_type = $2 AND oidc_refresh_token_enc IS NOT NULL + `, orgID, string(provider)) + if err != nil { + return nil, fmt.Errorf("sso: listing identities with refresh tokens: %w", err) + } + defer rows.Close() + + var out []IdentityWithRefreshToken + for rows.Next() { + var row IdentityWithRefreshToken + var encRefresh string + if err := rows.Scan(&row.ID, &row.UserID, &row.Subject, &encRefresh); err != nil { + return nil, fmt.Errorf("sso: scanning identity: %w", err) + } + refresh, err := s.box.Decrypt(encRefresh) + if err != nil { + return nil, fmt.Errorf("sso: decrypting refresh token: %w", err) + } + row.RefreshToken = refresh + out = append(out, row) + } + return out, rows.Err() +} + +// MarkChecked stamps last_checked_at = NOW() for one identity after a +// successful reconciliation refresh. +func (s *IdentityStore) MarkChecked(id int64) error { + _, err := s.db.Exec(`UPDATE sso_identities SET last_checked_at = NOW() WHERE id = $1`, id) + if err != nil { + return fmt.Errorf("sso: marking identity checked: %w", err) + } + return nil +} diff --git a/internal/sso/oidc.go b/internal/sso/oidc.go index 0c4fd72a2..894e8acc7 100644 --- a/internal/sso/oidc.go +++ b/internal/sso/oidc.go @@ -15,6 +15,17 @@ import ( type Identity struct { Email string Name string + // Subject is the IdP's own identifier for this identity (OIDC `sub` / + // SAML NameID). Needed to resolve an incoming OIDC back-channel logout + // token or SAML LogoutRequest - which only carry this, never a + // LiveReview user id - back to a user (see internal/sso identity_store.go + // and Phase 4 of docs/sso.md's deprovisioning section). + Subject string + // RefreshToken is only populated for OIDC configs with the + // reconciliation poll enabled (see Config.ReconciliationEnabled) - it + // requests the "offline_access" scope specifically so the poll can + // silently re-check "is this account still valid" between logins. + RefreshToken string } // oidcClaims mirrors the standard OIDC claims LiveReview needs. Most IdPs @@ -41,6 +52,9 @@ func BuildOIDCOAuth2Config(ctx context.Context, cfg *Config, redirectURL string) if strings.TrimSpace(cfg.OIDCScopes) != "" { scopes = strings.Fields(cfg.OIDCScopes) } + if cfg.ReconciliationEnabled && !containsFold(scopes, "offline_access") { + scopes = append(scopes, "offline_access") + } oauthCfg := &oauth2.Config{ ClientID: cfg.OIDCClientID, ClientSecret: cfg.OIDCClientSecret, @@ -74,7 +88,52 @@ func ExchangeOIDCCode(ctx context.Context, oauthCfg *oauth2.Config, verifier *oi if claims.Email == "" { return nil, fmt.Errorf("sso: oidc id_token has no email claim") } - return &Identity{Email: claims.Email, Name: claims.Name}, nil + return &Identity{ + Email: claims.Email, + Name: claims.Name, + Subject: idToken.Subject, + RefreshToken: token.RefreshToken, + }, nil +} + +// backChannelLogoutEvent is the OIDC Back-Channel Logout 1.0 event URI a +// valid logout_token's "events" claim must contain. +const backChannelLogoutEvent = "http://schemas.openid.net/event/backchannel-logout" + +type backChannelLogoutClaims struct { + Subject string `json:"sub"` + Events map[string]interface{} `json:"events"` +} + +// VerifyBackChannelLogoutToken verifies an incoming OIDC back-channel +// logout token (RFC: OIDC Back-Channel Logout 1.0) the same way an ID +// token is verified - same issuer, signature (JWKS), audience, and expiry +// checks via go-oidc - then confirms the logout-specific "events" claim is +// present, and returns the subject whose sessions should be revoked. +func VerifyBackChannelLogoutToken(ctx context.Context, cfg *Config, rawToken string) (string, error) { + if cfg.ProviderType != ProviderOIDC { + return "", fmt.Errorf("sso: config %d is not an OIDC config", cfg.ID) + } + provider, err := oidc.NewProvider(ctx, cfg.OIDCIssuer) + if err != nil { + return "", fmt.Errorf("sso: oidc discovery against %q failed: %w", cfg.OIDCIssuer, err) + } + verifier := provider.Verifier(&oidc.Config{ClientID: cfg.OIDCClientID}) + idToken, err := verifier.Verify(ctx, rawToken) + if err != nil { + return "", fmt.Errorf("sso: logout_token verification failed: %w", err) + } + var claims backChannelLogoutClaims + if err := idToken.Claims(&claims); err != nil { + return "", fmt.Errorf("sso: parsing logout_token claims: %w", err) + } + if _, ok := claims.Events[backChannelLogoutEvent]; !ok { + return "", fmt.Errorf("sso: logout_token missing required backchannel-logout event claim") + } + if claims.Subject == "" { + return "", fmt.Errorf("sso: logout_token has no sub claim") + } + return claims.Subject, nil } // TestOIDCDiscovery is used by the config "test connection" endpoint - it diff --git a/internal/sso/saml.go b/internal/sso/saml.go index 92f19802d..352fb87cd 100644 --- a/internal/sso/saml.go +++ b/internal/sso/saml.go @@ -95,6 +95,7 @@ func ExchangeSAMLResponse(sp *samllib.ServiceProvider, r *http.Request) (*Identi identity := &Identity{} if assertion.Subject != nil && assertion.Subject.NameID != nil { v := strings.TrimSpace(assertion.Subject.NameID.Value) + identity.Subject = v if strings.Contains(v, "@") { identity.Email = v } diff --git a/ui/src/pages/Settings/SSOSettingsTab.tsx b/ui/src/pages/Settings/SSOSettingsTab.tsx index ec4e12cbb..b8b5fd4ef 100644 --- a/ui/src/pages/Settings/SSOSettingsTab.tsx +++ b/ui/src/pages/Settings/SSOSettingsTab.tsx @@ -6,6 +6,8 @@ import { useOrgContext } from '../../hooks/useOrgContext'; type ProviderType = 'oidc' | 'saml'; +type ReconciliationInterval = 'hourly' | 'daily' | 'weekly'; + interface SSOConfigResponse { provider_type: ProviderType; display_name: string; @@ -17,6 +19,9 @@ interface SSOConfigResponse { saml_idp_metadata_url?: string; saml_idp_entity_id?: string; saml_metadata_set: boolean; + reconciliation_enabled?: boolean; + reconciliation_interval?: ReconciliationInterval; + last_reconciled_at?: string; } interface FormState { @@ -28,6 +33,8 @@ interface FormState { oidc_scopes: string; saml_idp_metadata_url: string; saml_idp_metadata_xml: string; + reconciliation_enabled: boolean; + reconciliation_interval: ReconciliationInterval; } const emptyForm: FormState = { @@ -39,6 +46,8 @@ const emptyForm: FormState = { oidc_scopes: '', saml_idp_metadata_url: '', saml_idp_metadata_xml: '', + reconciliation_enabled: false, + reconciliation_interval: 'daily', }; const ResultBanner: React.FC<{ type: 'success' | 'error'; message: string }> = ({ type, message }) => ( @@ -70,6 +79,8 @@ const ProviderPanel: React.FC<{ oidc_scopes: existing.oidc_scopes || '', saml_idp_metadata_url: existing.saml_idp_metadata_url || '', saml_idp_metadata_xml: '', // never round-tripped - left blank keeps the stored metadata + reconciliation_enabled: existing.reconciliation_enabled || false, + reconciliation_interval: existing.reconciliation_interval || 'daily', }); } }, [existing]); @@ -90,6 +101,8 @@ const ProviderPanel: React.FC<{ if (form.oidc_client_secret !== '') { body.oidc_client_secret = form.oidc_client_secret; } + body.reconciliation_enabled = form.reconciliation_enabled; + body.reconciliation_interval = form.reconciliation_interval; } else { body.saml_idp_metadata_url = form.saml_idp_metadata_url; if (form.saml_idp_metadata_xml !== '') { @@ -203,6 +216,46 @@ const ProviderPanel: React.FC<{ Redirect URI to register with your identity provider:{' '} {window.location.origin}/api/v1/auth/sso/oidc/callback

+ +
+
+ handleChange('reconciliation_enabled', e.target.checked)} + className="w-4 h-4 text-indigo-600 bg-slate-700 border-slate-600 rounded focus:ring-indigo-500 focus:ring-2" + /> + +
+

+ Fallback for orgs that don't configure SCIM below: periodically re-checks each signed-in + user against the identity provider and deactivates them here if the account was disabled + or removed there. SCIM (if configured) is real-time; this is a backstop, bounded by the + interval you pick. +

+ {form.reconciliation_enabled && ( +
+ + + {existing?.last_reconciled_at && ( +

+ Last checked: {new Date(existing.last_reconciled_at).toLocaleString()} +

+ )} +
+ )} +
) : ( <> @@ -261,6 +314,129 @@ const ProviderPanel: React.FC<{ ); }; +interface ScimTokenResponse { + exists?: boolean; + token_prefix?: string; + created_at?: string; + token?: string; // only present immediately after create/rotate +} + +const ScimProvisioningPanel: React.FC<{ orgId: number }> = ({ orgId }) => { + const [tokenInfo, setTokenInfo] = useState(null); + const [newToken, setNewToken] = useState(null); + const [isLoading, setIsLoading] = useState(true); + const [isWorking, setIsWorking] = useState(false); + + const fetchToken = async () => { + try { + const data = await apiClient.get(`/api/v1/orgs/${orgId}/scim-token`); + setTokenInfo(data); + } catch (error) { + console.error('Failed to fetch SCIM token status:', error); + } finally { + setIsLoading(false); + } + }; + + useEffect(() => { + if (orgId) { + fetchToken(); + } + }, [orgId]); + + const handleGenerate = async () => { + const verb = tokenInfo?.token_prefix ? 'rotate' : 'generate'; + if (tokenInfo?.token_prefix && !window.confirm('Rotating the SCIM token immediately invalidates the old one. Your identity provider will need the new token. Continue?')) { + return; + } + setIsWorking(true); + try { + const data = await apiClient.post(`/api/v1/orgs/${orgId}/scim-token`, {}); + setTokenInfo(data); + setNewToken(data.token || null); + notify.success(`SCIM token ${verb}d`); + } catch (error: any) { + notify.error(error?.message || `Failed to ${verb} SCIM token`); + } finally { + setIsWorking(false); + } + }; + + const handleRevoke = async () => { + if (!window.confirm('Revoke the SCIM token? Your identity provider will no longer be able to sync users until a new token is generated.')) { + return; + } + setIsWorking(true); + try { + await apiClient.delete(`/api/v1/orgs/${orgId}/scim-token`); + setTokenInfo({ exists: false }); + setNewToken(null); + notify.success('SCIM token revoked'); + } catch (error: any) { + notify.error(error?.message || 'Failed to revoke SCIM token'); + } finally { + setIsWorking(false); + } + }; + + if (isLoading) { + return null; + } + + const scimBaseURL = `${window.location.origin}/api/v1/scim/v2`; + + return ( +
+
+
+ +
+
+

SCIM Provisioning

+

+ Let Okta, Azure AD, or Google Workspace automatically create and deactivate users here when + they're added or removed at the identity provider - the real-time counterpart to the + reconciliation check above. +

+
+
+ + {newToken && ( +
+

+ Copy this token now - it won't be shown again. +

+ {newToken} +
+ )} + +

+ SCIM base URL: {scimBaseURL} +

+ + {tokenInfo?.token_prefix ? ( +

+ Current token: {tokenInfo.token_prefix}... + {tokenInfo.created_at && ` (created ${new Date(tokenInfo.created_at).toLocaleString()})`} +

+ ) : ( +

No SCIM token configured yet.

+ )} + +
+ + {tokenInfo?.token_prefix && ( + + )} +
+
+ ); +}; + const SSOSettingsTab: React.FC = () => { const { currentOrgId } = useOrgContext(); const orgId = currentOrgId ?? 0; @@ -336,6 +512,8 @@ const SSOSettingsTab: React.FC = () => { existing={existingForActive} onSaved={fetchConfigs} /> + + ); };