Skip to content

Add access review campaigns#935

Open
aureliensibiril wants to merge 158 commits intogetprobo:mainfrom
aureliensibiril:aureliensibiril/eng-136-access-review
Open

Add access review campaigns#935
aureliensibiril wants to merge 158 commits intogetprobo:mainfrom
aureliensibiril:aureliensibiril/eng-136-access-review

Conversation

@aureliensibiril
Copy link
Copy Markdown
Contributor

@aureliensibiril aureliensibiril commented Mar 26, 2026

Summary

Add access review campaigns with full lifecycle management (create, start, close, cancel) and a source fetch worker that pulls account data from connected providers.

Access source drivers (18 providers)

Provider Connection Fields
Google Workspace OAuth2 Email, name, role, admin, MFA, last login, created at
Slack OAuth2 Email, name, role, job title, admin, MFA, bot support
Linear OAuth2 Email, name, role, admin, active, created at
Cloudflare API key Email, name, roles (concatenated), MFA, active
Brex API key Email, name, role, active
Tally API key Email, name, role
1Password API key Email, name, role, active, MFA, created at
DocuSign OAuth2 Email, name, role, job title, admin, active, created at
HubSpot OAuth2 Email, name, role (resolved via roles endpoint)
Notion OAuth2 Email, name, role, bot support (ServiceAccount)
Figma OAuth2 Email, name
OpenAI API key Email, name, role, MFA
Sentry API key Email, name, role, MFA, active
GitHub OAuth2 Email, name, role, admin, MFA, 2FA status
Supabase API key Email, name, role, MFA, active
Intercom OAuth2 Email, name, role
Resend API key Email, name, role
Probo memberships Internal Email, name, role, admin, active, created at
CSV File upload Email, name, role, job title, admin, active, external ID

API surface (GraphQL + MCP + CLI)

  • Campaign CRUD, start, close, cancel
  • Access source CRUD with OAuth2 and API key connector support
  • Campaign scope source management (add/remove)
  • Access entry listing, flagging, and decision recording (approve/revoke)
  • Bulk decide-all for batch processing

Console UI

  • Campaign list with pagination and status badges
  • Campaign detail page with entry table (flag, decision columns)
  • Access source creation page with OAuth2 flow and API key dialog
  • CSV upload source creation
  • Vendor icon components for all supported providers

Bug fixes included

  • Fix source name worker retry on resolution failure
  • Fix React hooks ordering in access source page (lint)
  • Fix connector dropdown showing non-unique labels
  • Fix OAuth callback retry when mutation fails
  • Handle DocuSign pagination parse errors
  • Return error when driver pagination limit is reached
  • Accept variable-length rows in CSV driver
  • Fix scope sources query returning all org sources after removal

Test plan

  • make test passes
  • make lint passes (0 errors)
  • make test-e2e — access review e2e tests pass
  • Create a campaign via CLI, add a source, start it, verify source fetch completes
  • Verify campaign lifecycle transitions (draft → in_progress → pending_actions → completed)
  • Test cancel and delete flows

gearnode and others added 30 commits March 25, 2026 23:39
Add SQL migration for access reviews, campaigns, sources, and
entries. Implement coredata types for access entries, reviews,
campaigns, source fetches, and supporting enums (decision, flag,
MFA status, auth method, connector provider/protocol).

Signed-off-by: Bryan Frimin <[email protected]>
Introduce an API key-based connector protocol alongside the
existing OAuth flow. This allows access source drivers to
authenticate with third-party services using static API keys.

Signed-off-by: Bryan Frimin <[email protected]>
Implement core domain logic for access reviews: entry service
for CRUD and decisions, source service for managing access
sources, review service for campaign orchestration, and a review
engine that flags anomalies (inactive accounts, missing MFA,
admin privileges, unknown entries).

Signed-off-by: Bryan Frimin <[email protected]>
Define the Driver interface that all access source integrations
must implement: Name, Snapshot, and IsAdmin methods.

Signed-off-by: Bryan Frimin <[email protected]>
Signed-off-by: Bryan Frimin <[email protected]>
Signed-off-by: Bryan Frimin <[email protected]>
Implement campaign lifecycle management (create, initialize,
cancel, complete) with optimistic locking for state transitions.
Add a poll-based worker that fetches access snapshots from
configured sources with bounded concurrency, then finalizes
campaigns when all fetches complete.

Wire the access review service into probod.

Signed-off-by: Bryan Frimin <[email protected]>
Add GraphQL types, queries, mutations, and resolvers for access
reviews, campaigns, sources, and entries. Includes connection-based
pagination for sources and entries, campaign lifecycle mutations
(initialize, cancel, record decision), and access source CRUD.

Signed-off-by: Bryan Frimin <[email protected]>
Add pages for viewing the access review, managing access sources,
creating sources (OAuth and CSV), and a sidebar navigation link.
Includes Relay fragments, mutations, and pagination for the
access source list.

Signed-off-by: Bryan Frimin <[email protected]>
Expose access review operations through the MCP interface:
list/get access sources, list/get campaigns, list/get entries,
record entry decisions, and initialize/cancel campaigns.

Signed-off-by: Bryan Frimin <[email protected]>
Test the full access review lifecycle through the GraphQL API:
source CRUD, campaign initialization, entry listing with
pagination, decision recording, and campaign completion.

Signed-off-by: Bryan Frimin <[email protected]>
Remove identity_source_id from campaigns (no longer needed), collapse
migrations into a single file, remove dead code functions, and add a
campaign detail page with collapsible source entries.

Signed-off-by: Bryan Frimin <[email protected]>
Replace ref.Ref() with new() pointer syntax, update access source
driver interface to use typed source names, and clean up GraphQL
schema and MCP specification for access review campaigns.

Signed-off-by: Bryan Frimin <[email protected]>
Every return from WithTx/WithConn now wraps the error with
context. Also removes the redundant lockCampaignForUpdate
method, fixes duplicate source loading in Start, and drops
useless doc comments.

Signed-off-by: Aurélien Sibiril <[email protected]>
Convert ProviderDisplayName to map, remove dead resolvers
(Figma, Notion, 1Password) that always returned empty, and
drop unused HubSpot CompanyName field with wrong json tag.

Signed-off-by: Aurélien Sibiril <[email protected]>
Error handlers reset the processed ref but left connector_id
in the URL, causing the effect to re-trigger indefinitely.
Clear URL params in both error paths.

Signed-off-by: Aurélien Sibiril <[email protected]>
Bulk flagging showed a success toast before mutations
completed. Track per-mutation outcomes and show an error
toast with count when any fail. Bump entry limit from 50
to 500 with a CLI fallback notice.

Signed-off-by: Aurélien Sibiril <[email protected]>
Denormalize organization_id onto access_entries and
access_entry_decision_history to eliminate JOINs in
AuthorizationAttributes. This follows the established
pattern used by all other coredata entities.

The migration backfills existing rows from the campaign.

Signed-off-by: Aurélien Sibiril <[email protected]>
The __() i18n function takes a single string; format
interpolation uses sprintf() from @probo/helpers.

Signed-off-by: Aurélien Sibiril <[email protected]>
The --flag filter only accepted 6 of the 15 flag values.
Add the 9 missing flags: DORMANT, TERMINATED_USER,
CONTRACTOR_EXPIRED, SOD_CONFLICT, PRIVILEGED_ACCESS,
ROLE_CREEP, NO_BUSINESS_JUSTIFICATION, OUT_OF_DEPARTMENT,
SHARED_ACCOUNT.

Signed-off-by: Aurélien Sibiril <[email protected]>
The openai section was replaced by the agents config
with providers and per-agent settings.

Signed-off-by: Aurélien Sibiril <[email protected]>
The SnapshotSource name is confusing because the codebase
already has a snapshot entity with a source_id field.
Rename to FetchSource which better describes the action.

Signed-off-by: Aurélien Sibiril <[email protected]>
The convention is to set values in Go code, not via SQL
defaults. Drop the default after the backfill ALTER.

Signed-off-by: Aurélien Sibiril <[email protected]>
Go code is responsible for setting values, not SQL defaults.
Remove DEFAULT clauses from access_entries and
access_review_campaign_source_fetches.

Signed-off-by: Aurélien Sibiril <[email protected]>
FlagReasons was left as nil (SQL NULL) when creating access
entries. Now that the column has no DEFAULT, set it to an
empty slice explicitly.

Signed-off-by: Aurélien Sibiril <[email protected]>
The confirm dialog defaulted to "Delete" with danger variant.
Set label to "Complete" with primary variant since this is a
completion action, not a deletion.

Signed-off-by: Aurélien Sibiril <[email protected]>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/accessreview/drivers/github.go">

<violation number="1" location="pkg/accessreview/drivers/github.go:63">
P1: Custom agent: **Avoid Logging Sensitive Information**

Avoid logging member identifiers (`m.Login`) in warnings; this exposes PII in logs and violates the Avoid Logging Sensitive Information rule.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

The GitHub driver silently skipped members when membership
or profile fetches failed. Add a kit/log logger to the
driver and log warnings so failures are visible.

Signed-off-by: Aurélien Sibiril <[email protected]>
@aureliensibiril aureliensibiril force-pushed the aureliensibiril/eng-136-access-review branch from 877cce5 to 31a2ae4 Compare March 30, 2026 13:46
Do not log member login identifiers; only log the error.

Signed-off-by: Aurélien Sibiril <[email protected]>
Fix operator-linebreak style errors, move ref update into
effect to avoid render-time mutation, and remove setState
call from effect body in EntryFlagSelect.

Signed-off-by: Aurélien Sibiril <[email protected]>
Every error return must be wrapped with fmt.Errorf so that
call sites are traceable in logs. Wrap ~23 bare return err
in campaign_service, worker, source_name_worker, review
engine, and entry/source service code.

Signed-off-by: Aurélien Sibiril <[email protected]>
Signed-off-by: Aurélien Sibiril <[email protected]>
RecordDecision was identical to Update on AccessEntry.
Remove it and use Update directly. Also remove RETURNING
from source fetch Update and use Exec with RowsAffected
check instead, matching the standard Update pattern.

Signed-off-by: Aurélien Sibiril <[email protected]>
Signed-off-by: Aurélien Sibiril <[email protected]>
The source field can be null when a scope source references
a deleted access source. Add a null guard when collecting
existing scope source IDs.

Signed-off-by: Aurélien Sibiril <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants