diff --git a/apps/console/src/pages/organizations/access-reviews/campaigns/CampaignDetailPage.tsx b/apps/console/src/pages/organizations/access-reviews/campaigns/CampaignDetailPage.tsx index 9da25c8cb..f840d1845 100644 --- a/apps/console/src/pages/organizations/access-reviews/campaigns/CampaignDetailPage.tsx +++ b/apps/console/src/pages/organizations/access-reviews/campaigns/CampaignDetailPage.tsx @@ -154,9 +154,15 @@ export const campaignDetailPageQuery = graphql` id } name - fetchStatus - fetchedAccountsCount - lastError + fetchAttempts(first: 1) { + edges { + node { + status + fetchedAccountsCount + error + } + } + } entries(first: 500) { edges { node { @@ -504,6 +510,10 @@ function CampaignSourceCard({ source, isPendingActions }: { source: CampaignSour const entries = source.entries?.edges ?? []; const entryIds = entries.map(edge => edge.node.id); + const latestAttempt = source.fetchAttempts.edges[0]?.node; + const fetchStatus = latestAttempt?.status ?? "QUEUED"; + const fetchedAccountsCount = latestAttempt?.fetchedAccountsCount ?? 0; + const lastError = latestAttempt?.error; const handleBulkDecision = (value: string) => { const decision = value as AccessReviewEntryDecision; @@ -639,22 +649,22 @@ function CampaignSourceCard({ source, isPendingActions }: { source: CampaignSour : } {source.name} - {source.fetchedAccountsCount} + {fetchedAccountsCount} {" "} {__("accounts")} - - {formatStatus(source.fetchStatus)} + + {formatStatus(fetchStatus)} - {source.fetchStatus === "FAILED" && source.lastError && ( + {fetchStatus === "FAILED" && lastError && (

{__("Fetch failed")}

-

{source.lastError}

+

{lastError}

)} diff --git a/apps/console/src/pages/organizations/access-reviews/dialogs/AddCampaignSourceDialog.tsx b/apps/console/src/pages/organizations/access-reviews/dialogs/AddCampaignSourceDialog.tsx index 752cf33fa..ffa59cc39 100644 --- a/apps/console/src/pages/organizations/access-reviews/dialogs/AddCampaignSourceDialog.tsx +++ b/apps/console/src/pages/organizations/access-reviews/dialogs/AddCampaignSourceDialog.tsx @@ -41,8 +41,15 @@ const addScopeMutation = graphql` sources { id name - fetchStatus - fetchedAccountsCount + fetchAttempts(first: 1) { + edges { + node { + status + fetchedAccountsCount + error + } + } + } entries(first: 50) { edges { node { diff --git a/e2e/console/access_review_test.go b/e2e/console/access_review_test.go index 2f0bb808f..ec22fad15 100644 --- a/e2e/console/access_review_test.go +++ b/e2e/console/access_review_test.go @@ -824,6 +824,182 @@ func TestAccessReviewCampaign_AddAndRemoveCampaignSource(t *testing.T) { }) } +func TestAccessReviewCampaignSource_Node(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + orgID := owner.GetOrganizationID().String() + + sourceID := factory.NewAccessReviewSource(owner, orgID). + WithName("Node Query Source"). + WithCsvData(testCsvData). + Create() + + campaignID := factory.NewAccessReviewCampaign(owner, orgID). + WithName("Node Query Campaign"). + WithAccessReviewSourceIDs([]string{sourceID}). + Create() + + const campaignQuery = ` + query($id: ID!) { + node(id: $id) { + ... on AccessReviewCampaign { + sources { + id + name + } + } + } + } + ` + + var campaignResult struct { + Node struct { + Sources []struct { + ID string `json:"id"` + Name string `json:"name"` + } `json:"sources"` + } `json:"node"` + } + + err := owner.Execute(campaignQuery, map[string]any{"id": campaignID}, &campaignResult) + require.NoError(t, err) + require.Len(t, campaignResult.Node.Sources, 1) + + campaignSourceID := campaignResult.Node.Sources[0].ID + + const nodeQuery = ` + query($id: ID!) { + node(id: $id) { + __typename + ... on AccessReviewCampaignSource { + id + campaign { + id + } + name + entries(first: 10) { + totalCount + } + } + } + } + ` + + var nodeResult struct { + Node struct { + Typename string `json:"__typename"` + ID string `json:"id"` + Campaign struct { + ID string `json:"id"` + } `json:"campaign"` + Name string `json:"name"` + Entries struct { + TotalCount int `json:"totalCount"` + } `json:"entries"` + } `json:"node"` + } + + err = owner.Execute(nodeQuery, map[string]any{"id": campaignSourceID}, &nodeResult) + require.NoError(t, err) + assert.Equal(t, "AccessReviewCampaignSource", nodeResult.Node.Typename) + assert.Equal(t, campaignSourceID, nodeResult.Node.ID) + assert.Equal(t, campaignID, nodeResult.Node.Campaign.ID) + assert.Equal(t, "Node Query Source", nodeResult.Node.Name) + assert.Greater(t, nodeResult.Node.Entries.TotalCount, 0) +} + +func TestAccessReviewCampaignSource_NameSurvivesSourceDeletion(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + orgID := owner.GetOrganizationID().String() + + const snapshotName = "Archived Snapshot Source" + sourceID := factory.NewAccessReviewSource(owner, orgID). + WithName(snapshotName). + WithCsvData(testCsvData). + Create() + + campaignID := factory.NewAccessReviewCampaign(owner, orgID). + WithName("Archival Campaign"). + WithAccessReviewSourceIDs([]string{sourceID}). + Create() + + const sourcesQuery = ` + query($id: ID!) { + node(id: $id) { + ... on AccessReviewCampaign { + sources { + id + name + sourceId + source { + id + } + } + } + } + } + ` + + var before struct { + Node struct { + Sources []struct { + ID string `json:"id"` + Name string `json:"name"` + SourceID *string `json:"sourceId"` + Source *struct { + ID string `json:"id"` + } `json:"source"` + } `json:"sources"` + } `json:"node"` + } + + err := owner.Execute(sourcesQuery, map[string]any{"id": campaignID}, &before) + require.NoError(t, err) + require.Len(t, before.Node.Sources, 1) + assert.Equal(t, snapshotName, before.Node.Sources[0].Name) + require.NotNil(t, before.Node.Sources[0].SourceID) + assert.Equal(t, sourceID, *before.Node.Sources[0].SourceID) + require.NotNil(t, before.Node.Sources[0].Source) + assert.Equal(t, sourceID, before.Node.Sources[0].Source.ID) + + const deleteQuery = ` + mutation($input: DeleteAccessReviewSourceInput!) { + deleteAccessReviewSource(input: $input) { + deletedAccessReviewSourceId + } + } + ` + + err = owner.Execute(deleteQuery, map[string]any{ + "input": map[string]any{ + "accessReviewSourceId": sourceID, + }, + }, nil) + require.NoError(t, err) + + var after struct { + Node struct { + Sources []struct { + ID string `json:"id"` + Name string `json:"name"` + SourceID *string `json:"sourceId"` + Source *struct { + ID string `json:"id"` + } `json:"source"` + } `json:"sources"` + } `json:"node"` + } + + err = owner.Execute(sourcesQuery, map[string]any{"id": campaignID}, &after) + require.NoError(t, err) + require.Len(t, after.Node.Sources, 1) + assert.Equal(t, before.Node.Sources[0].ID, after.Node.Sources[0].ID) + assert.Equal(t, snapshotName, after.Node.Sources[0].Name) + assert.Nil(t, after.Node.Sources[0].SourceID) + assert.Nil(t, after.Node.Sources[0].Source) +} + func TestAccessReviewCampaign_Cancel(t *testing.T) { t.Parallel() owner := testutil.NewClient(t, testutil.RoleOwner) diff --git a/pkg/accessreview/campaign_service.go b/pkg/accessreview/campaign_service.go index 679e9a918..27345e97f 100644 --- a/pkg/accessreview/campaign_service.go +++ b/pkg/accessreview/campaign_service.go @@ -152,12 +152,12 @@ func (s *Service) UpdateCampaign( return fmt.Errorf("cannot update campaign: status is %s, expected DRAFT", campaign.Status) } - if req.Name != nil { - campaign.Name = *req.Name + if req.Name != nil && *req.Name != nil { + campaign.Name = **req.Name } - if req.Description != nil { - campaign.Description = *req.Description + if req.Description != nil && *req.Description != nil { + campaign.Description = **req.Description } if req.FrameworkControls != nil { @@ -425,7 +425,6 @@ func (s *Service) upsertCampaignSource( AccessReviewCampaignID: campaignID, AccessReviewSourceID: &sourceID, Name: source.Name, - Category: source.Category, ConnectorID: source.ConnectorID, CreatedAt: now, UpdatedAt: now, @@ -553,41 +552,18 @@ func (s *Service) ListCampaignSources( return sources, nil } -func (s *Service) ListLatestFetchAttempts( - ctx context.Context, - scope coredata.Scoper, - campaignID gid.GID, -) (coredata.AccessReviewCampaignSourceFetchAttempts, error) { - var attempts coredata.AccessReviewCampaignSourceFetchAttempts - - err := s.pg.WithConn( - ctx, - func(ctx context.Context, conn pg.Querier) error { - if err := attempts.LoadLatestByCampaignID(ctx, conn, scope, campaignID); err != nil { - return fmt.Errorf("cannot load latest fetch attempts by campaign: %w", err) - } - - return nil - }, - ) - if err != nil { - return nil, err - } - - return attempts, nil -} - -func (s *Service) ListFetchAttempts( +func (s *Service) ListFetchAttemptsForCampaignSourceID( ctx context.Context, scope coredata.Scoper, campaignSourceID gid.GID, -) (coredata.AccessReviewCampaignSourceFetchAttempts, error) { + cursor *page.Cursor[coredata.AccessReviewCampaignSourceFetchAttemptOrderField], +) (*page.Page[*coredata.AccessReviewCampaignSourceFetchAttempt, coredata.AccessReviewCampaignSourceFetchAttemptOrderField], error) { var attempts coredata.AccessReviewCampaignSourceFetchAttempts err := s.pg.WithConn( ctx, func(ctx context.Context, conn pg.Querier) error { - if err := attempts.LoadByCampaignSourceID(ctx, conn, scope, campaignSourceID); err != nil { + if err := attempts.LoadByCampaignSourceID(ctx, conn, scope, campaignSourceID, cursor); err != nil { return fmt.Errorf("cannot load fetch attempts: %w", err) } @@ -598,7 +574,35 @@ func (s *Service) ListFetchAttempts( return nil, err } - return attempts, nil + return page.NewPage(attempts, cursor), nil +} + +func (s *Service) CountFetchAttemptsForCampaignSourceID( + ctx context.Context, + scope coredata.Scoper, + campaignSourceID gid.GID, +) (int, error) { + var count int + + err := s.pg.WithConn( + ctx, + func(ctx context.Context, conn pg.Querier) error { + var attempts coredata.AccessReviewCampaignSourceFetchAttempts + + var err error + count, err = attempts.CountByCampaignSourceID(ctx, conn, scope, campaignSourceID) + if err != nil { + return fmt.Errorf("cannot count fetch attempts: %w", err) + } + + return nil + }, + ) + if err != nil { + return 0, err + } + + return count, nil } func (s *Service) CountCampaignsForOrganizationID( diff --git a/pkg/accessreview/campaign_types.go b/pkg/accessreview/campaign_types.go index 616446de9..0dab93b8d 100644 --- a/pkg/accessreview/campaign_types.go +++ b/pkg/accessreview/campaign_types.go @@ -33,8 +33,8 @@ type ( UpdateAccessReviewCampaignRequest struct { CampaignID gid.GID - Name *string - Description *string + Name **string + Description **string FrameworkControls *[]string } diff --git a/pkg/accessreview/entry_service.go b/pkg/accessreview/entry_service.go index 285973bca..6d5dadde4 100644 --- a/pkg/accessreview/entry_service.go +++ b/pkg/accessreview/entry_service.go @@ -51,11 +51,15 @@ func (s *Service) GetEntry( err := s.pg.WithConn( ctx, func(ctx context.Context, conn pg.Querier) error { - return entry.LoadByID(ctx, conn, scope, entryID) + if err := entry.LoadByID(ctx, conn, scope, entryID); err != nil { + return fmt.Errorf("cannot load access entry: %w", err) + } + + return nil }, ) if err != nil { - return nil, fmt.Errorf("cannot get access entry: %w", err) + return nil, err } return entry, nil @@ -66,86 +70,12 @@ func (s *Service) RecordDecision( scope coredata.Scoper, req RecordAccessReviewEntryDecisionRequest, ) (*coredata.AccessReviewEntry, error) { - if req.Decision == coredata.AccessReviewEntryDecisionPending { - return nil, fmt.Errorf("cannot decide access entry: invalid decision %q", req.Decision) - } - - if req.Decision != coredata.AccessReviewEntryDecisionApproved { - if req.DecisionNote == nil || strings.TrimSpace(*req.DecisionNote) == "" { - return nil, fmt.Errorf("cannot decide access entry: note is required for non-approved decisions") - } - } - - entry := &coredata.AccessReviewEntry{} - - err := s.pg.WithTx( - ctx, - func(ctx context.Context, conn pg.Tx) error { - if err := entry.LoadByID(ctx, conn, scope, req.EntryID); err != nil { - return fmt.Errorf("cannot load access entry: %w", err) - } - - campaign := &coredata.AccessReviewCampaign{} - if err := campaign.LoadByID(ctx, conn, scope, entry.AccessReviewCampaignID); err != nil { - return fmt.Errorf("cannot load campaign: %w", err) - } - - if campaign.Status != coredata.AccessReviewCampaignStatusPendingActions { - return fmt.Errorf("cannot decide access entry: campaign status is %s, expected PENDING_ACTIONS", campaign.Status) - } - - now := time.Now() - entry.Decision = req.Decision - entry.DecisionNote = req.DecisionNote - entry.DecidedBy = req.DecidedByID - entry.DecidedAt = &now - - entry.UpdatedAt = now - if entry.Flags == nil { - entry.Flags = []coredata.AccessReviewEntryFlag{} - } - - if entry.FlagReasons == nil { - entry.FlagReasons = []string{} - } - - if req.Decision == coredata.AccessReviewEntryDecisionRevoke || req.Decision == coredata.AccessReviewEntryDecisionEscalate { - if len(entry.Flags) == 0 { - entry.Flags = []coredata.AccessReviewEntryFlag{coredata.AccessReviewEntryFlagExcessive} - } - } - - if err := entry.Update(ctx, conn, scope); err != nil { - return fmt.Errorf("cannot record access entry decision: %w", err) - } - - history := &coredata.AccessReviewEntryDecisionHistory{ - ID: gid.New(scope.GetTenantID(), coredata.AccessReviewEntryDecisionHistoryEntityType), - OrganizationID: entry.OrganizationID, - AccessReviewEntry: entry.ID, - Decision: entry.Decision, - DecisionNote: entry.DecisionNote, - DecidedBy: entry.DecidedBy, - DecidedAt: *entry.DecidedAt, - CreatedAt: now, - } - if err := history.Insert(ctx, conn, scope); err != nil { - return fmt.Errorf("cannot insert decision history: %w", err) - } - - return nil - }, - ) + entries, err := s.RecordDecisions(ctx, scope, []RecordAccessReviewEntryDecisionRequest{req}) if err != nil { - return nil, fmt.Errorf("cannot record access entry decision: %w", err) + return nil, err } - updatedEntry, err := s.GetEntry(ctx, scope, req.EntryID) - if err != nil { - return nil, fmt.Errorf("cannot reload access entry after decision: %w", err) - } - - return updatedEntry, nil + return entries[0], nil } func (s *Service) RecordDecisions( @@ -179,6 +109,7 @@ func (s *Service) RecordDecisions( // Track verified campaigns to avoid repeated loads within the // same transaction. verifiedCampaigns := make(map[gid.GID]bool) + decidedByCache := make(map[gid.GID]*gid.GID) for _, d := range decisions { entry := &coredata.AccessReviewEntry{} @@ -199,10 +130,23 @@ func (s *Service) RecordDecisions( verifiedCampaigns[entry.AccessReviewCampaignID] = true } + decidedByID, ok := decidedByCache[entry.OrganizationID] + if !ok { + decidedByID = nil + if d.DecidedByID != nil { + profile := &coredata.MembershipProfile{} + if err := profile.LoadByIdentityIDAndOrganizationID(ctx, conn, scope, *d.DecidedByID, entry.OrganizationID); err == nil { + decidedByID = &profile.ID + } + } + + decidedByCache[entry.OrganizationID] = decidedByID + } + now := time.Now() entry.Decision = d.Decision entry.DecisionNote = d.DecisionNote - entry.DecidedBy = d.DecidedByID + entry.DecidedBy = decidedByID entry.DecidedAt = &now entry.UpdatedAt = now @@ -279,11 +223,9 @@ func (s *Service) FlagEntry( } if campaign.Status != coredata.AccessReviewCampaignStatusPendingActions { - return fmt.Errorf("cannot flag access entry: campaign status is %s, expected PENDING_ACTIONS", campaign.Status) + return fmt.Errorf("cannot flag access entry: campaign status is %q, expected PENDING_ACTIONS", campaign.Status) } - now := time.Now() - entry.Flags = req.Flags if entry.Flags == nil { entry.Flags = []coredata.AccessReviewEntryFlag{} @@ -294,16 +236,20 @@ func (s *Service) FlagEntry( entry.FlagReasons = []string{} } - entry.UpdatedAt = now + entry.UpdatedAt = time.Now() - return entry.UpdateFlags(ctx, conn, scope) + if err := entry.UpdateFlags(ctx, conn, scope); err != nil { + return fmt.Errorf("cannot update access entry flags: %w", err) + } + + return nil }, ) if err != nil { - return nil, fmt.Errorf("cannot flag access entry: %w", err) + return nil, err } - return s.GetEntry(ctx, scope, req.EntryID) + return entry, nil } func (s *Service) ListEntriesForCampaignID( @@ -318,11 +264,22 @@ func (s *Service) ListEntriesForCampaignID( err := s.pg.WithConn( ctx, func(ctx context.Context, conn pg.Querier) error { - return entries.LoadByCampaignID(ctx, conn, scope, campaignID, cursor, filter) + if err := entries.LoadByCampaignID( + ctx, + conn, + scope, + campaignID, + cursor, + filter, + ); err != nil { + return fmt.Errorf("cannot load access entries: %w", err) + } + + return nil }, ) if err != nil { - return nil, fmt.Errorf("cannot list access entries: %w", err) + return nil, err } return page.NewPage(entries, cursor), nil @@ -341,11 +298,23 @@ func (s *Service) ListEntriesForCampaignIDAndSourceID( err := s.pg.WithConn( ctx, func(ctx context.Context, conn pg.Querier) error { - return entries.LoadByCampaignIDAndSourceID(ctx, conn, scope, campaignID, sourceID, cursor, filter) + if err := entries.LoadByCampaignIDAndSourceID( + ctx, + conn, + scope, + campaignID, + sourceID, + cursor, + filter, + ); err != nil { + return fmt.Errorf("cannot load access entries: %w", err) + } + + return nil }, ) if err != nil { - return nil, fmt.Errorf("cannot list access entries: %w", err) + return nil, err } return page.NewPage(entries, cursor), nil @@ -373,7 +342,7 @@ func (s *Service) CountEntriesForCampaignID( }, ) if err != nil { - return 0, fmt.Errorf("cannot count access entries: %w", err) + return 0, err } return count, nil @@ -402,7 +371,7 @@ func (s *Service) CountEntriesForCampaignIDAndSourceID( }, ) if err != nil { - return 0, fmt.Errorf("cannot count access entries: %w", err) + return 0, err } return count, nil @@ -429,7 +398,7 @@ func (s *Service) CountPendingEntriesForCampaignID( }, ) if err != nil { - return 0, fmt.Errorf("cannot count pending access entries: %w", err) + return 0, err } return count, nil @@ -465,7 +434,11 @@ func (s *Service) CampaignStatistics( err := s.pg.WithConn( ctx, func(ctx context.Context, conn pg.Querier) error { - return stats.LoadByCampaignID(ctx, conn, scope, campaignID) + if err := stats.LoadByCampaignID(ctx, conn, scope, campaignID); err != nil { + return fmt.Errorf("cannot load campaign statistics: %w", err) + } + + return nil }, ) if err != nil { @@ -486,11 +459,15 @@ func (s *Service) CampaignSourceStatistics( err := s.pg.WithConn( ctx, func(ctx context.Context, conn pg.Querier) error { - return stats.LoadByCampaignIDAndSourceID(ctx, conn, scope, campaignID, sourceID) + if err := stats.LoadByCampaignIDAndSourceID(ctx, conn, scope, campaignID, sourceID); err != nil { + return fmt.Errorf("cannot load source statistics: %w", err) + } + + return nil }, ) if err != nil { - return nil, fmt.Errorf("cannot load source statistics: %w", err) + return nil, err } return stats, nil diff --git a/pkg/accessreview/service.go b/pkg/accessreview/service.go index 54852d6f0..539c1a94a 100644 --- a/pkg/accessreview/service.go +++ b/pkg/accessreview/service.go @@ -16,7 +16,6 @@ package accessreview import ( "context" - "fmt" "time" "go.gearno.de/kit/log" @@ -26,7 +25,6 @@ import ( "go.probo.inc/probo/pkg/connector/provider" "go.probo.inc/probo/pkg/coredata" "go.probo.inc/probo/pkg/crypto/cipher" - "go.probo.inc/probo/pkg/gid" "golang.org/x/sync/errgroup" ) @@ -102,34 +100,6 @@ func NewService( return s } -// ResolveEntryOrganizationID resolves the organization ID for an access entry. -// This is unscoped because it is used by resolvers before authorization to -// find the organization from an entry ID. -func (s *Service) ResolveEntryOrganizationID(ctx context.Context, entryID gid.GID) (gid.GID, error) { - var organizationID gid.GID - - err := s.pg.WithConn( - ctx, - func(ctx context.Context, conn pg.Querier) error { - var err error - - entry := &coredata.AccessReviewEntry{} - - organizationID, err = entry.LoadOrganizationID(ctx, conn, entryID) - if err != nil { - return fmt.Errorf("cannot load organization id: %w", err) - } - - return nil - }, - ) - if err != nil { - return gid.GID{}, fmt.Errorf("cannot resolve organization id: %w", err) - } - - return organizationID, nil -} - func (s *Service) Run(ctx context.Context) error { g, gCtx := errgroup.WithContext(ctx) diff --git a/pkg/cmd/access-review/entry/decide/decide.go b/pkg/cmd/access-review/entry/decide/decide.go index 5c00f4a04..5fe2a986c 100644 --- a/pkg/cmd/access-review/entry/decide/decide.go +++ b/pkg/cmd/access-review/entry/decide/decide.go @@ -26,7 +26,7 @@ import ( const decideMutation = ` mutation($input: RecordAccessReviewEntryDecisionInput!) { recordAccessReviewEntryDecision(input: $input) { - accessEntry { + accessReviewEntry { id email fullName @@ -47,7 +47,7 @@ type decideResponse struct { Decision string `json:"decision"` DecisionNote *string `json:"decisionNote"` DecidedAt *string `json:"decidedAt"` - } `json:"accessEntry"` + } `json:"accessReviewEntry"` } `json:"recordAccessReviewEntryDecision"` } diff --git a/pkg/cmd/access-review/entry/decideall/decideall.go b/pkg/cmd/access-review/entry/decideall/decideall.go index 8e75750ae..3ac5feef4 100644 --- a/pkg/cmd/access-review/entry/decideall/decideall.go +++ b/pkg/cmd/access-review/entry/decideall/decideall.go @@ -26,7 +26,7 @@ import ( const decideAllMutation = ` mutation($input: RecordAccessReviewEntryDecisionsInput!) { recordAccessReviewEntryDecisions(input: $input) { - accessEntries { + accessReviewEntries { id email decision @@ -41,7 +41,7 @@ type decideAllResponse struct { ID string `json:"id"` Email string `json:"email"` Decision string `json:"decision"` - } `json:"accessEntries"` + } `json:"accessReviewEntries"` } `json:"recordAccessReviewEntryDecisions"` } diff --git a/pkg/cmd/access-review/entry/list/list.go b/pkg/cmd/access-review/entry/list/list.go index c24d9f034..f2dd6958d 100644 --- a/pkg/cmd/access-review/entry/list/list.go +++ b/pkg/cmd/access-review/entry/list/list.go @@ -30,7 +30,6 @@ query( $first: Int, $after: CursorKey, $orderBy: AccessReviewEntryOrder, - $campaignSourceId: ID, $filter: AccessReviewEntryFilter ) { node(id: $id) { @@ -40,7 +39,6 @@ query( first: $first, after: $after, orderBy: $orderBy, - campaignSourceId: $campaignSourceId, filter: $filter ) { totalCount @@ -63,8 +61,60 @@ query( flagReasons decision decisionNote - accessSource { - id + campaignSource { + name + } + createdAt + } + } + pageInfo { + hasNextPage + endCursor + } + } + } + } +} +` + +const listBySourceQuery = ` +query( + $id: ID!, + $first: Int, + $after: CursorKey, + $orderBy: AccessReviewEntryOrder, + $filter: AccessReviewEntryFilter +) { + node(id: $id) { + __typename + ... on AccessReviewCampaignSource { + entries( + first: $first, + after: $after, + orderBy: $orderBy, + filter: $filter + ) { + totalCount + edges { + node { + id + email + fullName + role + jobTitle + isAdmin + active + mfaStatus + authMethod + accountType + lastLogin + externalId + incrementalTag + flags + flagReasons + decision + decisionNote + campaignSource { name } createdAt @@ -81,27 +131,26 @@ query( ` type entryNode struct { - ID string `json:"id"` - Email string `json:"email"` - FullName string `json:"fullName"` - Role string `json:"role"` - JobTitle string `json:"jobTitle"` - IsAdmin bool `json:"isAdmin"` - Active *bool `json:"active"` - MfaStatus string `json:"mfaStatus"` - AuthMethod string `json:"authMethod"` - AccountType string `json:"accountType"` - LastLogin *string `json:"lastLogin"` - ExternalID string `json:"externalId"` - IncrementalTag string `json:"incrementalTag"` - Flags []string `json:"flags"` - FlagReasons []string `json:"flagReasons"` - Decision string `json:"decision"` - DecisionNote *string `json:"decisionNote"` - AccessReviewSource struct { - ID string `json:"id"` + ID string `json:"id"` + Email string `json:"email"` + FullName string `json:"fullName"` + Role string `json:"role"` + JobTitle string `json:"jobTitle"` + IsAdmin bool `json:"isAdmin"` + Active *bool `json:"active"` + MfaStatus string `json:"mfaStatus"` + AuthMethod string `json:"authMethod"` + AccountType string `json:"accountType"` + LastLogin *string `json:"lastLogin"` + ExternalID string `json:"externalId"` + IncrementalTag string `json:"incrementalTag"` + Flags []string `json:"flags"` + FlagReasons []string `json:"flagReasons"` + Decision string `json:"decision"` + DecisionNote *string `json:"decisionNote"` + CampaignSource struct { Name string `json:"name"` - } `json:"accessSource"` + } `json:"campaignSource"` CreatedAt string `json:"createdAt"` } @@ -122,14 +171,14 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { ) cmd := &cobra.Command{ - Use: "list ", - Short: "List access entries for a campaign", - Args: cobra.ExactArgs(1), + Use: "list []", + Short: "List access entries for a campaign or campaign source", + Args: cobra.MaximumNArgs(1), Example: ` # List all entries for a campaign prb access-review entry list - # List entries for a specific source - prb access-review entry list --campaign-source-id + # List entries for a specific campaign source + prb access-review entry list --source-id # List only pending entries prb access-review entry list --decision PENDING @@ -141,6 +190,10 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { return err } + if flagCampaignSourceID == "" && len(args) == 0 { + return fmt.Errorf("campaign ID is required when --source-id is not set") + } + cfg, err := f.Config() if err != nil { return err @@ -159,8 +212,14 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { cmdutil.TokenRefreshOption(cfg, host, hc), ) - variables := map[string]any{ - "id": args[0], + variables := map[string]any{} + + if len(args) > 0 { + variables["id"] = args[0] + } + + if flagCampaignSourceID != "" { + variables["id"] = flagCampaignSourceID } if err := cmdutil.ValidateEnum("order-direction", flagOrderDir, []string{"ASC", "DESC"}); err != nil { @@ -178,10 +237,6 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { } } - if flagCampaignSourceID != "" { - variables["campaignSourceId"] = flagCampaignSourceID - } - filter := map[string]any{} if flagDecision != "" { @@ -261,9 +316,19 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { variables["filter"] = filter } + query := listQuery + expectedTypename := "AccessReviewCampaign" + notFoundLabel := "campaign" + + if flagCampaignSourceID != "" { + query = listBySourceQuery + expectedTypename = "AccessReviewCampaignSource" + notFoundLabel = "campaign source" + } + entries, totalCount, err := api.Paginate( client, - listQuery, + query, variables, flagLimit, func(data json.RawMessage) (*api.Connection[entryNode], error) { @@ -278,11 +343,11 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { } if resp.Node == nil { - return nil, fmt.Errorf("campaign %s not found", args[0]) + return nil, fmt.Errorf("%s %s not found", notFoundLabel, variables["id"]) } - if resp.Node.Typename != "AccessReviewCampaign" { - return nil, fmt.Errorf("expected AccessReviewCampaign node, got %s", resp.Node.Typename) + if resp.Node.Typename != expectedTypename { + return nil, fmt.Errorf("expected %s node, got %s", expectedTypename, resp.Node.Typename) } return &resp.Node.Entries, nil @@ -326,7 +391,7 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { e.ID, e.Email, e.FullName, - e.AccessReviewSource.Name, + e.CampaignSource.Name, e.Decision, strings.Join(e.Flags, ","), admin, @@ -354,7 +419,7 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of entries to list") cmd.Flags().StringVar(&flagOrderBy, "order-by", "", "Order by field (CREATED_AT)") cmd.Flags().StringVar(&flagOrderDir, "order-direction", "DESC", "Sort direction (ASC, DESC)") - cmd.Flags().StringVar(&flagCampaignSourceID, "source-id", "", "Filter by access source ID") + cmd.Flags().StringVar(&flagCampaignSourceID, "source-id", "", "Campaign source ID to list entries for") cmd.Flags().StringVar(&flagDecision, "decision", "", "Filter by decision (PENDING, APPROVED, REVOKE, DEFER, ESCALATE)") cmd.Flags().StringVar(&flagFlag, "flag", "", "Filter by flag (NONE, ORPHANED, INACTIVE, EXCESSIVE, ROLE_MISMATCH, NEW)") cmd.Flags().StringVar(&flagIncTag, "incremental-tag", "", "Filter by incremental tag (NEW, REMOVED, UNCHANGED)") diff --git a/pkg/cmd/access-review/entry/setflag/flag.go b/pkg/cmd/access-review/entry/setflag/flag.go index 6279e1f8a..6ed59d56e 100644 --- a/pkg/cmd/access-review/entry/setflag/flag.go +++ b/pkg/cmd/access-review/entry/setflag/flag.go @@ -27,7 +27,7 @@ import ( const flagMutation = ` mutation($input: FlagAccessReviewEntryInput!) { flagAccessReviewEntry(input: $input) { - accessEntry { + accessReviewEntry { id email fullName @@ -48,7 +48,7 @@ type flagResponse struct { Flags []string `json:"flags"` FlagReasons []string `json:"flagReasons"` Decision string `json:"decision"` - } `json:"accessEntry"` + } `json:"accessReviewEntry"` } `json:"flagAccessReviewEntry"` } diff --git a/pkg/cmd/access-review/source/create/create.go b/pkg/cmd/access-review/source/create/create.go index b87946fe9..f867a8dc4 100644 --- a/pkg/cmd/access-review/source/create/create.go +++ b/pkg/cmd/access-review/source/create/create.go @@ -27,7 +27,7 @@ import ( const createMutation = ` mutation($input: CreateAccessReviewSourceInput!) { createAccessReviewSource(input: $input) { - accessSourceEdge { + accessReviewSourceEdge { node { id name @@ -44,7 +44,7 @@ type createResponse struct { ID string `json:"id"` Name string `json:"name"` } `json:"node"` - } `json:"accessSourceEdge"` + } `json:"accessReviewSourceEdge"` } `json:"createAccessReviewSource"` } diff --git a/pkg/cmd/access-review/source/update/update.go b/pkg/cmd/access-review/source/update/update.go index 515b43303..7a90ae3a1 100644 --- a/pkg/cmd/access-review/source/update/update.go +++ b/pkg/cmd/access-review/source/update/update.go @@ -27,7 +27,7 @@ import ( const updateMutation = ` mutation($input: UpdateAccessReviewSourceInput!) { updateAccessReviewSource(input: $input) { - accessSource { + accessReviewSource { id name } @@ -40,7 +40,7 @@ type updateResponse struct { AccessReviewSource struct { ID string `json:"id"` Name string `json:"name"` - } `json:"accessSource"` + } `json:"accessReviewSource"` } `json:"updateAccessReviewSource"` } diff --git a/pkg/coredata/access_review_campaign_source_fetch_attempt.go b/pkg/coredata/access_review_campaign_source_fetch_attempt.go index 6ff3b25ff..86344df6c 100644 --- a/pkg/coredata/access_review_campaign_source_fetch_attempt.go +++ b/pkg/coredata/access_review_campaign_source_fetch_attempt.go @@ -24,13 +24,14 @@ import ( "github.com/jackc/pgx/v5" "go.gearno.de/kit/pg" "go.probo.inc/probo/pkg/gid" + "go.probo.inc/probo/pkg/page" ) type ( // AccessReviewCampaignSourceFetchAttempt is a single, append-only fetch run for a // campaign source snapshot. Each retry produces a new row, so the error of // every attempt is retained. The current state of a snapshot is the latest - // attempt (highest attempt_number). Terminal rows (SUCCESS / FAILED) are + // attempt (most recently created). Terminal rows (SUCCESS / FAILED) are // immutable; only the in-flight attempt is updated. // // TenantID is retained on the struct because the background worker claims @@ -40,7 +41,6 @@ type ( ID gid.GID `db:"id"` TenantID gid.TenantID `db:"tenant_id"` AccessReviewCampaignSourceID gid.GID `db:"access_review_campaign_source_id"` - AttemptNumber int `db:"attempt_number"` Status AccessReviewCampaignSourceFetchStatus `db:"status"` FetchedAccountsCount int `db:"fetched_accounts_count"` Error *string `db:"error"` @@ -48,11 +48,23 @@ type ( CompletedAt *time.Time `db:"completed_at"` CreatedAt time.Time `db:"created_at"` UpdatedAt time.Time `db:"updated_at"` + AttemptNumber int `db:"attempt_number"` } AccessReviewCampaignSourceFetchAttempts []*AccessReviewCampaignSourceFetchAttempt ) +func (a AccessReviewCampaignSourceFetchAttempt) CursorKey( + orderBy AccessReviewCampaignSourceFetchAttemptOrderField, +) page.CursorKey { + switch orderBy { + case AccessReviewCampaignSourceFetchAttemptOrderFieldCreatedAt: + return page.NewCursorKey(a.ID, a.CreatedAt) + } + + panic(fmt.Sprintf("unsupported order by: %s", orderBy)) +} + var ( ErrNoAccessReviewCampaignSourceFetchAttemptAvailable = errors.New("no access review source fetch attempt available") ) @@ -185,7 +197,7 @@ SELECT updated_at FROM access_review_campaign_source_fetch_attempts WHERE status = @status -ORDER BY created_at ASC +ORDER BY created_at ASC, id ASC LIMIT 1 FOR UPDATE SKIP LOCKED ` @@ -263,13 +275,76 @@ ORDER BY access_review_campaign_source_id, attempt_number DESC return nil } -// LoadByCampaignSourceID returns the full attempt history for a snapshot, -// newest first. +// LoadByCampaignSourceID returns a page of fetch attempts for a snapshot. func (attempts *AccessReviewCampaignSourceFetchAttempts) LoadByCampaignSourceID( ctx context.Context, conn pg.Querier, scope Scoper, campaignSourceID gid.GID, + cursor *page.Cursor[AccessReviewCampaignSourceFetchAttemptOrderField], +) error { + q := ` +SELECT + id, + tenant_id, + access_review_campaign_source_id, + status, + fetched_accounts_count, + error, + started_at, + completed_at, + created_at, + updated_at, + attempt_number +FROM ( + SELECT + id, + tenant_id, + access_review_campaign_source_id, + status, + fetched_accounts_count, + error, + started_at, + completed_at, + created_at, + updated_at, + ROW_NUMBER() OVER (ORDER BY created_at ASC, id ASC)::int AS attempt_number + FROM access_review_campaign_source_fetch_attempts + WHERE + %s + AND access_review_campaign_source_id = @access_review_campaign_source_id +) fetch_attempts +WHERE + %s +` + q = fmt.Sprintf(q, scope.SQLFragment(), cursor.SQLFragment()) + + args := pgx.StrictNamedArgs{"access_review_campaign_source_id": campaignSourceID} + maps.Copy(args, scope.SQLArguments()) + maps.Copy(args, cursor.SQLArguments()) + + rows, err := conn.Query(ctx, q, args) + if err != nil { + return fmt.Errorf("cannot query fetch attempts: %w", err) + } + + result, err := pgx.CollectRows(rows, pgx.RowToAddrOfStructByName[AccessReviewCampaignSourceFetchAttempt]) + if err != nil { + return fmt.Errorf("cannot collect fetch attempts: %w", err) + } + + *attempts = result + + return nil +} + +// LoadAllByCampaignSourceID returns the full attempt history for a snapshot, +// newest first. +func (attempts *AccessReviewCampaignSourceFetchAttempts) LoadAllByCampaignSourceID( + ctx context.Context, + conn pg.Querier, + scope Scoper, + campaignSourceID gid.GID, ) error { q := ` SELECT @@ -310,6 +385,33 @@ ORDER BY attempt_number DESC return nil } +func (attempts *AccessReviewCampaignSourceFetchAttempts) CountByCampaignSourceID( + ctx context.Context, + conn pg.Querier, + scope Scoper, + campaignSourceID gid.GID, +) (int, error) { + q := ` +SELECT COUNT(*) +FROM access_review_campaign_source_fetch_attempts +WHERE + %s + AND access_review_campaign_source_id = @access_review_campaign_source_id +` + q = fmt.Sprintf(q, scope.SQLFragment()) + + args := pgx.StrictNamedArgs{"access_review_campaign_source_id": campaignSourceID} + maps.Copy(args, scope.SQLArguments()) + + var count int + + if err := conn.QueryRow(ctx, q, args).Scan(&count); err != nil { + return 0, fmt.Errorf("cannot count fetch attempts: %w", err) + } + + return count, nil +} + // RecoverStale fails attempts stuck in FETCHING past the threshold and queues a // fresh retry attempt for each, preserving the stale attempt's history. It is // intentionally cross-tenant. Returns the number of recovered attempts. diff --git a/pkg/coredata/access_review_campaign_source_fetch_attempt_order_field.go b/pkg/coredata/access_review_campaign_source_fetch_attempt_order_field.go new file mode 100644 index 000000000..1c1b1c3e2 --- /dev/null +++ b/pkg/coredata/access_review_campaign_source_fetch_attempt_order_field.go @@ -0,0 +1,80 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package coredata + +import ( + "encoding" + "fmt" + + "go.probo.inc/probo/pkg/page" +) + +type ( + AccessReviewCampaignSourceFetchAttemptOrderField string +) + +const ( + AccessReviewCampaignSourceFetchAttemptOrderFieldCreatedAt AccessReviewCampaignSourceFetchAttemptOrderField = "CREATED_AT" +) + +var ( + _ page.OrderField = AccessReviewCampaignSourceFetchAttemptOrderField("") + _ fmt.Stringer = AccessReviewCampaignSourceFetchAttemptOrderField("") + _ encoding.TextMarshaler = AccessReviewCampaignSourceFetchAttemptOrderField("") + _ encoding.TextUnmarshaler = (*AccessReviewCampaignSourceFetchAttemptOrderField)(nil) +) + +func AccessReviewCampaignSourceFetchAttemptOrderFields() []AccessReviewCampaignSourceFetchAttemptOrderField { + return []AccessReviewCampaignSourceFetchAttemptOrderField{ + AccessReviewCampaignSourceFetchAttemptOrderFieldCreatedAt, + } +} + +func (v AccessReviewCampaignSourceFetchAttemptOrderField) IsValid() bool { + switch v { + case AccessReviewCampaignSourceFetchAttemptOrderFieldCreatedAt: + return true + } + + return false +} + +func (v AccessReviewCampaignSourceFetchAttemptOrderField) String() string { + return string(v) +} + +func (v AccessReviewCampaignSourceFetchAttemptOrderField) MarshalText() ([]byte, error) { + return []byte(v.String()), nil +} + +func (v *AccessReviewCampaignSourceFetchAttemptOrderField) UnmarshalText(text []byte) error { + val := AccessReviewCampaignSourceFetchAttemptOrderField(text) + if !val.IsValid() { + return fmt.Errorf("invalid AccessReviewCampaignSourceFetchAttemptOrderField value: %q", string(text)) + } + + *v = val + + return nil +} + +func (p AccessReviewCampaignSourceFetchAttemptOrderField) Column() string { + switch p { + case AccessReviewCampaignSourceFetchAttemptOrderFieldCreatedAt: + return "created_at" + } + + panic(fmt.Sprintf("unsupported order by: %s", p)) +} diff --git a/pkg/coredata/access_review_campaign_source_test.go b/pkg/coredata/access_review_campaign_source_test.go index 0212afd4e..0b25403fb 100644 --- a/pkg/coredata/access_review_campaign_source_test.go +++ b/pkg/coredata/access_review_campaign_source_test.go @@ -119,7 +119,6 @@ func TestSourceFetchAttempts_AppendOnly(t *testing.T) { if err := first.Insert(ctx, tx, fx.scope); err != nil { return err } - require.Equal(t, 1, first.AttemptNumber) second := &coredata.AccessReviewCampaignSourceFetchAttempt{ ID: gid.New(tenantID, coredata.AccessReviewCampaignSourceFetchAttemptEntityType), @@ -133,17 +132,16 @@ func TestSourceFetchAttempts_AppendOnly(t *testing.T) { if err := second.Insert(ctx, tx, fx.scope); err != nil { return err } - require.Equal(t, 2, second.AttemptNumber) return nil })) var history coredata.AccessReviewCampaignSourceFetchAttempts require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error { - return history.LoadByCampaignSourceID(ctx, conn, fx.scope, fx.campaignSourceID) + return history.LoadAllByCampaignSourceID(ctx, conn, fx.scope, fx.campaignSourceID) })) require.Len(t, history, 2, "both attempts must be retained") - assert.Equal(t, 2, history[0].AttemptNumber, "history is newest first") + assert.Equal(t, coredata.AccessReviewCampaignSourceFetchStatusSuccess, history[0].Status, "history is newest first") assert.Equal(t, coredata.AccessReviewCampaignSourceFetchStatusFailed, history[1].Status) require.NotNil(t, history[1].Error) assert.Equal(t, failureMsg, *history[1].Error, "the failed attempt's error is retained") diff --git a/pkg/server/api/authn/context.go b/pkg/server/api/authn/context.go index 67e22eee0..9106cd925 100644 --- a/pkg/server/api/authn/context.go +++ b/pkg/server/api/authn/context.go @@ -18,6 +18,7 @@ import ( "context" "go.probo.inc/probo/pkg/coredata" + "go.probo.inc/probo/pkg/gid" ) type ( @@ -45,6 +46,15 @@ func IdentityFromContext(ctx context.Context) *coredata.Identity { return identity } +func IdentityIDFromContext(ctx context.Context) *gid.GID { + identity := IdentityFromContext(ctx) + if identity == nil { + return nil + } + + return &identity.ID +} + func ContextWithIdentity(ctx context.Context, identity *coredata.Identity) context.Context { return context.WithValue(ctx, identityContextKey, identity) } diff --git a/pkg/server/api/console/v1/access_review_campaign_resolvers.go b/pkg/server/api/console/v1/access_review_campaign_resolvers.go index 2278a3c44..9d7924c3b 100644 --- a/pkg/server/api/console/v1/access_review_campaign_resolvers.go +++ b/pkg/server/api/console/v1/access_review_campaign_resolvers.go @@ -14,7 +14,6 @@ import ( "go.gearno.de/kit/log" "go.probo.inc/probo/pkg/accessreview" "go.probo.inc/probo/pkg/coredata" - "go.probo.inc/probo/pkg/gid" "go.probo.inc/probo/pkg/page" "go.probo.inc/probo/pkg/probo" "go.probo.inc/probo/pkg/server/api/authn" @@ -55,29 +54,21 @@ func (r *accessReviewCampaignResolver) Sources(ctx context.Context, obj *types.A campaignSources, err := r.accessReview.ListCampaignSources(ctx, scope, obj.ID) if err != nil { - panic(fmt.Errorf("cannot list campaign sources: %w", err)) - } + r.logger.ErrorCtx(ctx, "cannot list campaign sources", log.Error(err)) - attempts, err := r.accessReview.ListLatestFetchAttempts(ctx, scope, obj.ID) - if err != nil { - panic(fmt.Errorf("cannot list latest fetch attempts: %w", err)) - } - - attemptByCampaignSourceID := make(map[gid.GID]*coredata.AccessReviewCampaignSourceFetchAttempt, len(attempts)) - for _, attempt := range attempts { - attemptByCampaignSourceID[attempt.AccessReviewCampaignSourceID] = attempt + return nil, gqlutils.Internal(ctx) } result := make([]*types.AccessReviewCampaignSource, len(campaignSources)) for i, campaignSource := range campaignSources { - result[i] = types.NewAccessReviewCampaignSource(campaignSource, attemptByCampaignSourceID[campaignSource.ID]) + result[i] = types.NewAccessReviewCampaignSource(campaignSource) } return result, nil } // Entries is the resolver for the entries field. -func (r *accessReviewCampaignResolver) Entries(ctx context.Context, obj *types.AccessReviewCampaign, first *int, after *page.CursorKey, last *int, before *page.CursorKey, orderBy *types.AccessReviewEntryOrder, campaignSourceID *gid.GID, filter *coredata.AccessReviewEntryFilter) (*types.AccessReviewEntryConnection, error) { +func (r *accessReviewCampaignResolver) Entries(ctx context.Context, obj *types.AccessReviewCampaign, first *int, after *page.CursorKey, last *int, before *page.CursorKey, orderBy *types.AccessReviewEntryOrder, filter *coredata.AccessReviewEntryFilter) (*types.AccessReviewEntryConnection, error) { scope, err := r.authorize(ctx, obj.ID, accessreview.ActionEntryList) if err != nil { return nil, err @@ -97,21 +88,14 @@ func (r *accessReviewCampaignResolver) Entries(ctx context.Context, obj *types.A cursor := types.NewCursor(first, after, last, before, pageOrderBy) - var ( - p *page.Page[*coredata.AccessReviewEntry, coredata.AccessReviewEntryOrderField] - ) - - if campaignSourceID != nil { - p, err = r.accessReview.ListEntriesForCampaignIDAndSourceID(ctx, scope, obj.ID, *campaignSourceID, cursor, filter) - } else { - p, err = r.accessReview.ListEntriesForCampaignID(ctx, scope, obj.ID, cursor, filter) - } - + p, err := r.accessReview.ListEntriesForCampaignID(ctx, scope, obj.ID, cursor, filter) if err != nil { - panic(fmt.Errorf("cannot list access entries: %w", err)) + r.logger.ErrorCtx(ctx, "cannot list access entries", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } - return types.NewAccessReviewEntryConnection(p, r, obj.ID, campaignSourceID, filter), nil + return types.NewAccessReviewEntryConnection(p, r, obj.ID, nil, filter), nil } // PendingEntryCount is the resolver for the pendingEntryCount field. @@ -123,7 +107,9 @@ func (r *accessReviewCampaignResolver) PendingEntryCount(ctx context.Context, ob count, err := r.accessReview.CountPendingEntriesForCampaignID(ctx, scope, obj.ID) if err != nil { - panic(fmt.Errorf("cannot count pending access entries: %w", err)) + r.logger.ErrorCtx(ctx, "cannot count pending access entries", log.Error(err)) + + return 0, gqlutils.Internal(ctx) } return count, nil @@ -138,7 +124,9 @@ func (r *accessReviewCampaignResolver) Statistics(ctx context.Context, obj *type stats, err := r.accessReview.CampaignStatistics(ctx, scope, obj.ID) if err != nil { - panic(fmt.Errorf("cannot get campaign statistics: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get campaign statistics", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return types.NewAccessReviewStatistics(stats), nil @@ -160,13 +148,38 @@ func (r *accessReviewCampaignConnectionResolver) TotalCount(ctx context.Context, case *organizationResolver: count, err := r.accessReview.CountCampaignsForOrganizationID(ctx, scope, obj.ParentID) if err != nil { - panic(fmt.Errorf("cannot count access review campaigns: %w", err)) + r.logger.ErrorCtx(ctx, "cannot count access review campaigns", log.Error(err)) + + return 0, gqlutils.Internal(ctx) } return count, nil } - panic(fmt.Errorf("unsupported resolver: %T", obj.Resolver)) + r.logger.ErrorCtx(ctx, "unsupported resolver", log.String("resolver", fmt.Sprintf("%T", obj.Resolver))) + + return 0, gqlutils.Internal(ctx) +} + +// Campaign is the resolver for the campaign field. +func (r *accessReviewCampaignSourceResolver) Campaign(ctx context.Context, obj *types.AccessReviewCampaignSource) (*types.AccessReviewCampaign, error) { + scope, err := r.authorize(ctx, obj.Campaign.ID, accessreview.ActionCampaignGet) + if err != nil { + return nil, err + } + + campaign, err := r.accessReview.GetCampaign(ctx, scope, obj.Campaign.ID) + if err != nil { + if errors.Is(err, coredata.ErrResourceNotFound) { + return nil, gqlutils.NotFound(ctx, err) + } + + r.logger.ErrorCtx(ctx, "cannot get access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) + } + + return types.NewAccessReviewCampaign(campaign), nil } // Source is the resolver for the source field. @@ -175,7 +188,7 @@ func (r *accessReviewCampaignSourceResolver) Source(ctx context.Context, obj *ty return nil, nil } - scope, err := r.authorize(ctx, obj.CampaignID, accessreview.ActionCampaignGet) + scope, err := r.authorize(ctx, obj.Campaign.ID, accessreview.ActionCampaignGet) if err != nil { return nil, err } @@ -186,35 +199,48 @@ func (r *accessReviewCampaignSourceResolver) Source(ctx context.Context, obj *ty return nil, nil } - panic(fmt.Errorf("cannot get access review source: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get access review source", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return types.NewAccessReviewSource(source), nil } // FetchAttempts is the resolver for the fetchAttempts field. -func (r *accessReviewCampaignSourceResolver) FetchAttempts(ctx context.Context, obj *types.AccessReviewCampaignSource) ([]*types.AccessReviewCampaignSourceFetchAttempt, error) { - scope, err := r.authorize(ctx, obj.CampaignID, accessreview.ActionCampaignGet) +func (r *accessReviewCampaignSourceResolver) FetchAttempts(ctx context.Context, obj *types.AccessReviewCampaignSource, first *int, after *page.CursorKey, last *int, before *page.CursorKey, orderBy *types.AccessReviewCampaignSourceFetchAttemptOrder) (*types.AccessReviewCampaignSourceFetchAttemptConnection, error) { + scope, err := r.authorize(ctx, obj.Campaign.ID, accessreview.ActionCampaignGet) if err != nil { return nil, err } - attempts, err := r.accessReview.ListFetchAttempts(ctx, scope, obj.ID) + pageOrderBy := page.OrderBy[coredata.AccessReviewCampaignSourceFetchAttemptOrderField]{ + Field: coredata.AccessReviewCampaignSourceFetchAttemptOrderFieldCreatedAt, + Direction: page.OrderDirectionDesc, + } + + if orderBy != nil { + pageOrderBy = page.OrderBy[coredata.AccessReviewCampaignSourceFetchAttemptOrderField]{ + Field: orderBy.Field, + Direction: orderBy.Direction, + } + } + + cursor := types.NewCursor(first, after, last, before, pageOrderBy) + + p, err := r.accessReview.ListFetchAttemptsForCampaignSourceID(ctx, scope, obj.ID, cursor) if err != nil { - panic(fmt.Errorf("cannot list fetch attempts: %w", err)) + r.logger.ErrorCtx(ctx, "cannot list fetch attempts", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } - result := make([]*types.AccessReviewCampaignSourceFetchAttempt, len(attempts)) - for i, attempt := range attempts { - result[i] = types.NewAccessReviewCampaignSourceFetchAttempt(attempt) - } - - return result, nil + return types.NewAccessReviewCampaignSourceFetchAttemptConnection(p, r, obj.ID), nil } // Entries is the resolver for the entries field. func (r *accessReviewCampaignSourceResolver) Entries(ctx context.Context, obj *types.AccessReviewCampaignSource, first *int, after *page.CursorKey, last *int, before *page.CursorKey, orderBy *types.AccessReviewEntryOrder, filter *coredata.AccessReviewEntryFilter) (*types.AccessReviewEntryConnection, error) { - scope, err := r.authorize(ctx, obj.CampaignID, accessreview.ActionEntryList) + scope, err := r.authorize(ctx, obj.Campaign.ID, accessreview.ActionEntryList) if err != nil { return nil, err } @@ -233,31 +259,52 @@ func (r *accessReviewCampaignSourceResolver) Entries(ctx context.Context, obj *t cursor := types.NewCursor(first, after, last, before, pageOrderBy) - p, err := r.accessReview.ListEntriesForCampaignIDAndSourceID(ctx, scope, obj.CampaignID, obj.ID, cursor, filter) + p, err := r.accessReview.ListEntriesForCampaignIDAndSourceID(ctx, scope, obj.Campaign.ID, obj.ID, cursor, filter) if err != nil { - panic(fmt.Errorf("cannot list access entries: %w", err)) + r.logger.ErrorCtx(ctx, "cannot list access entries", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } sourceID := obj.ID - return types.NewAccessReviewEntryConnection(p, r, obj.CampaignID, &sourceID, filter), nil + return types.NewAccessReviewEntryConnection(p, r, obj.Campaign.ID, &sourceID, filter), nil } // Statistics is the resolver for the statistics field. func (r *accessReviewCampaignSourceResolver) Statistics(ctx context.Context, obj *types.AccessReviewCampaignSource) (*types.AccessReviewStatistics, error) { - scope, err := r.authorize(ctx, obj.CampaignID, accessreview.ActionEntryList) + scope, err := r.authorize(ctx, obj.Campaign.ID, accessreview.ActionEntryList) if err != nil { return nil, err } - stats, err := r.accessReview.CampaignSourceStatistics(ctx, scope, obj.CampaignID, obj.ID) + stats, err := r.accessReview.CampaignSourceStatistics(ctx, scope, obj.Campaign.ID, obj.ID) if err != nil { - panic(fmt.Errorf("cannot get source statistics: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get source statistics", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return types.NewAccessReviewStatistics(stats), nil } +// TotalCount is the resolver for the totalCount field. +func (r *accessReviewCampaignSourceFetchAttemptConnectionResolver) TotalCount(ctx context.Context, obj *types.AccessReviewCampaignSourceFetchAttemptConnection) (int, error) { + scope, err := r.authorize(ctx, obj.ParentID, accessreview.ActionCampaignGet) + if err != nil { + return 0, err + } + + count, err := r.accessReview.CountFetchAttemptsForCampaignSourceID(ctx, scope, obj.ParentID) + if err != nil { + r.logger.ErrorCtx(ctx, "cannot count fetch attempts", log.Error(err)) + + return 0, gqlutils.Internal(ctx) + } + + return count, nil +} + // Campaign is the resolver for the campaign field. func (r *accessReviewEntryResolver) Campaign(ctx context.Context, obj *types.AccessReviewEntry) (*types.AccessReviewCampaign, error) { scope, err := r.authorize(ctx, obj.Campaign.ID, accessreview.ActionCampaignGet) @@ -271,51 +318,33 @@ func (r *accessReviewEntryResolver) Campaign(ctx context.Context, obj *types.Acc return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot get access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return types.NewAccessReviewCampaign(campaign), nil } -// AccessReviewSource is the resolver for the accessReviewSource field. -func (r *accessReviewEntryResolver) AccessReviewSource(ctx context.Context, obj *types.AccessReviewEntry) (*types.AccessReviewSource, error) { - scope, err := r.authorize(ctx, obj.ID, accessreview.ActionEntryGet) +// CampaignSource is the resolver for the campaignSource field. +func (r *accessReviewEntryResolver) CampaignSource(ctx context.Context, obj *types.AccessReviewEntry) (*types.AccessReviewCampaignSource, error) { + scope, err := r.authorize(ctx, obj.Campaign.ID, accessreview.ActionCampaignGet) if err != nil { return nil, err } - entry, err := r.accessReview.GetEntry(ctx, scope, obj.ID) + campaignSource, err := r.accessReview.GetCampaignSource(ctx, scope, obj.CampaignSource.ID) if err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot get access review entry: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get campaign source", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } - campaignSource, err := r.accessReview.GetCampaignSource(ctx, scope, entry.AccessReviewCampaignSourceID) - if err != nil { - if errors.Is(err, coredata.ErrResourceNotFound) { - return nil, nil - } - - panic(fmt.Errorf("cannot get campaign source: %w", err)) - } - - if campaignSource.AccessReviewSourceID == nil { - return nil, nil - } - - source, err := r.accessReview.GetSource(ctx, scope, *campaignSource.AccessReviewSourceID) - if err != nil { - if errors.Is(err, coredata.ErrResourceNotFound) { - return nil, nil - } - - panic(fmt.Errorf("cannot get access review source: %w", err)) - } - - return types.NewAccessReviewSource(source), nil + return types.NewAccessReviewCampaignSource(campaignSource), nil } // DecisionHistory is the resolver for the decisionHistory field. @@ -327,7 +356,9 @@ func (r *accessReviewEntryResolver) DecisionHistory(ctx context.Context, obj *ty histories, err := r.accessReview.EntryDecisionHistory(ctx, scope, obj.ID) if err != nil { - panic(fmt.Errorf("cannot get decision history: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get decision history", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } result := make([]*types.AccessReviewEntryDecisionHistoryEntry, len(histories)) @@ -352,24 +383,28 @@ func (r *accessReviewEntryConnectionResolver) TotalCount(ctx context.Context, ob switch obj.Resolver.(type) { case *accessReviewCampaignResolver: - if obj.SourceID != nil { - count, err := r.accessReview.CountEntriesForCampaignIDAndSourceID(ctx, scope, obj.ParentID, *obj.SourceID, obj.Filter) - if err != nil { - panic(fmt.Errorf("cannot count access entries: %w", err)) - } - - return count, nil - } - count, err := r.accessReview.CountEntriesForCampaignID(ctx, scope, obj.ParentID, obj.Filter) if err != nil { - panic(fmt.Errorf("cannot count access entries: %w", err)) + r.logger.ErrorCtx(ctx, "cannot count access entries", log.Error(err)) + + return 0, gqlutils.Internal(ctx) + } + + return count, nil + case *accessReviewCampaignSourceResolver: + count, err := r.accessReview.CountEntriesForCampaignIDAndSourceID(ctx, scope, obj.ParentID, *obj.SourceID, obj.Filter) + if err != nil { + r.logger.ErrorCtx(ctx, "cannot count access entries", log.Error(err)) + + return 0, gqlutils.Internal(ctx) } return count, nil } - panic(fmt.Errorf("unsupported resolver: %T", obj.Resolver)) + r.logger.ErrorCtx(ctx, "unsupported resolver", log.String("resolver", fmt.Sprintf("%T", obj.Resolver))) + + return 0, gqlutils.Internal(ctx) } // Organization is the resolver for the organization field. @@ -411,7 +446,9 @@ func (r *accessReviewSourceResolver) Connector(ctx context.Context, obj *types.A return nil, nil } - panic(fmt.Errorf("cannot get connector: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get connector", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return types.NewConnector(connector), nil @@ -477,7 +514,9 @@ func (r *accessReviewSourceResolver) NeedsConfiguration(ctx context.Context, obj return false, nil } - panic(fmt.Errorf("cannot get connector: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get connector", log.Error(err)) + + return false, gqlutils.Internal(ctx) } cfg, ok := providerOrgConfigs[dbConnector.Provider] @@ -536,7 +575,9 @@ func (r *accessReviewSourceResolver) SelectedOrganization(ctx context.Context, o return nil, nil } - panic(fmt.Errorf("cannot get connector: %w", err)) + r.logger.ErrorCtx(ctx, "cannot get connector", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } cfg, ok := providerOrgConfigs[dbConnector.Provider] @@ -568,13 +609,17 @@ func (r *accessReviewSourceConnectionResolver) TotalCount(ctx context.Context, o case *organizationResolver: count, err := r.accessReview.CountSourcesForOrganizationID(ctx, scope, obj.ParentID) if err != nil { - panic(fmt.Errorf("cannot count access sources: %w", err)) + r.logger.ErrorCtx(ctx, "cannot count access sources", log.Error(err)) + + return 0, gqlutils.Internal(ctx) } return count, nil } - panic(fmt.Errorf("unsupported resolver: %T", obj.Resolver)) + r.logger.ErrorCtx(ctx, "unsupported resolver", log.String("resolver", fmt.Sprintf("%T", obj.Resolver))) + + return 0, gqlutils.Internal(ctx) } // CreateAccessReviewSource is the resolver for the createAccessReviewSource field. @@ -584,15 +629,20 @@ func (r *mutationResolver) CreateAccessReviewSource(ctx context.Context, input t return nil, err } - source, err := r.accessReview.CreateSource(ctx, scope, accessreview.CreateAccessReviewSourceRequest{ - OrganizationID: input.OrganizationID, - ConnectorID: input.ConnectorID, - Name: input.Name, - Category: coredata.AccessReviewSourceCategorySaaS, - CsvData: input.CSVData, - }) + source, err := r.accessReview.CreateSource( + ctx, + scope, + accessreview.CreateAccessReviewSourceRequest{ + OrganizationID: input.OrganizationID, + ConnectorID: input.ConnectorID, + Name: input.Name, + CsvData: input.CSVData, + }, + ) if err != nil { - panic(fmt.Errorf("cannot create access source: %w", err)) + r.logger.ErrorCtx(ctx, "cannot create access source", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.CreateAccessReviewSourcePayload{ @@ -607,29 +657,24 @@ func (r *mutationResolver) UpdateAccessReviewSource(ctx context.Context, input t return nil, err } - req := accessreview.UpdateAccessReviewSourceRequest{ - AccessReviewSourceID: input.AccessReviewSourceID, - } - - if input.Name.IsSet() { - req.Name = input.Name.Value() - } - - if input.ConnectorID.IsSet() { - req.ConnectorID = gqlutils.UnwrapOmittable(input.ConnectorID) - } - - if input.CSVData.IsSet() { - req.CsvData = gqlutils.UnwrapOmittable(input.CSVData) - } - - source, err := r.accessReview.UpdateSource(ctx, scope, req) + source, err := r.accessReview.UpdateSource( + ctx, + scope, + accessreview.UpdateAccessReviewSourceRequest{ + AccessReviewSourceID: input.AccessReviewSourceID, + Name: gqlutils.UnwrapOmittable(input.Name), + ConnectorID: gqlutils.UnwrapOmittable(input.ConnectorID), + CsvData: gqlutils.UnwrapOmittable(input.CSVData), + }, + ) if err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot update access source: %w", err)) + r.logger.ErrorCtx(ctx, "cannot update access source", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.UpdateAccessReviewSourcePayload{ @@ -649,7 +694,9 @@ func (r *mutationResolver) DeleteAccessReviewSource(ctx context.Context, input t return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot delete access source: %w", err)) + r.logger.ErrorCtx(ctx, "cannot delete access source", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.DeleteAccessReviewSourcePayload{ @@ -677,7 +724,9 @@ func (r *mutationResolver) ConfigureAccessReviewSource(ctx context.Context, inpu return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot configure access source: %w", err)) + r.logger.ErrorCtx(ctx, "cannot configure access source", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.ConfigureAccessReviewSourcePayload{ @@ -697,15 +746,21 @@ func (r *mutationResolver) CreateAccessReviewCampaign(ctx context.Context, input description = *input.Description } - campaign, err := r.accessReview.CreateCampaign(ctx, scope, accessreview.CreateAccessReviewCampaignRequest{ - OrganizationID: input.OrganizationID, - Name: input.Name, - Description: description, - FrameworkControls: input.FrameworkControls, - AccessReviewSourceIDs: input.AccessReviewSourceIds, - }) + campaign, err := r.accessReview.CreateCampaign( + ctx, + scope, + accessreview.CreateAccessReviewCampaignRequest{ + OrganizationID: input.OrganizationID, + Name: input.Name, + Description: description, + FrameworkControls: input.FrameworkControls, + AccessReviewSourceIDs: input.AccessReviewSourceIds, + }, + ) if err != nil { - panic(fmt.Errorf("cannot create access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot create access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.CreateAccessReviewCampaignPayload{ @@ -720,30 +775,24 @@ func (r *mutationResolver) UpdateAccessReviewCampaign(ctx context.Context, input return nil, err } - req := accessreview.UpdateAccessReviewCampaignRequest{ - CampaignID: input.AccessReviewCampaignID, - } - - if input.Name.IsSet() { - req.Name = input.Name.Value() - } - - if input.Description.IsSet() { - req.Description = input.Description.Value() - } - - if input.FrameworkControls.IsSet() { - controls := input.FrameworkControls.Value() - req.FrameworkControls = &controls - } - - campaign, err := r.accessReview.UpdateCampaign(ctx, scope, req) + campaign, err := r.accessReview.UpdateCampaign( + ctx, + scope, + accessreview.UpdateAccessReviewCampaignRequest{ + CampaignID: input.AccessReviewCampaignID, + Name: gqlutils.UnwrapOmittable(input.Name), + Description: gqlutils.UnwrapOmittable(input.Description), + FrameworkControls: gqlutils.UnwrapOmittable(input.FrameworkControls), + }, + ) if err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot update access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot update access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.UpdateAccessReviewCampaignPayload{ @@ -763,7 +812,9 @@ func (r *mutationResolver) DeleteAccessReviewCampaign(ctx context.Context, input return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot delete access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot delete access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.DeleteAccessReviewCampaignPayload{ @@ -780,7 +831,9 @@ func (r *mutationResolver) StartAccessReviewCampaign(ctx context.Context, input campaign, err := r.accessReview.StartCampaign(ctx, scope, input.AccessReviewCampaignID) if err != nil { - panic(fmt.Errorf("cannot start access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot start access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.StartAccessReviewCampaignPayload{ @@ -797,7 +850,9 @@ func (r *mutationResolver) CloseAccessReviewCampaign(ctx context.Context, input campaign, err := r.accessReview.CloseCampaign(ctx, scope, input.AccessReviewCampaignID) if err != nil { - panic(fmt.Errorf("cannot close access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot close access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.CloseAccessReviewCampaignPayload{ @@ -814,7 +869,9 @@ func (r *mutationResolver) CancelAccessReviewCampaign(ctx context.Context, input campaign, err := r.accessReview.CancelCampaign(ctx, scope, input.AccessReviewCampaignID) if err != nil { - panic(fmt.Errorf("cannot cancel access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot cancel access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.CancelAccessReviewCampaignPayload{ @@ -829,12 +886,18 @@ func (r *mutationResolver) AddAccessReviewCampaignSource(ctx context.Context, in return nil, err } - campaign, err := r.accessReview.AddCampaignSource(ctx, scope, accessreview.AddCampaignSourceRequest{ - CampaignID: input.AccessReviewCampaignID, - AccessReviewSourceID: input.AccessReviewSourceID, - }) + campaign, err := r.accessReview.AddCampaignSource( + ctx, + scope, + accessreview.AddCampaignSourceRequest{ + CampaignID: input.AccessReviewCampaignID, + AccessReviewSourceID: input.AccessReviewSourceID, + }, + ) if err != nil { - panic(fmt.Errorf("cannot add scope source to access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot add scope source to access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.AddAccessReviewCampaignSourcePayload{ @@ -849,12 +912,18 @@ func (r *mutationResolver) RemoveAccessReviewCampaignSource(ctx context.Context, return nil, err } - campaign, err := r.accessReview.RemoveCampaignSource(ctx, scope, accessreview.RemoveCampaignSourceRequest{ - CampaignID: input.AccessReviewCampaignID, - AccessReviewSourceID: input.AccessReviewSourceID, - }) + campaign, err := r.accessReview.RemoveCampaignSource( + ctx, + scope, + accessreview.RemoveCampaignSourceRequest{ + CampaignID: input.AccessReviewCampaignID, + AccessReviewSourceID: input.AccessReviewSourceID, + }, + ) if err != nil { - panic(fmt.Errorf("cannot remove scope source from access review campaign: %w", err)) + r.logger.ErrorCtx(ctx, "cannot remove scope source from access review campaign", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.RemoveAccessReviewCampaignSourcePayload{ @@ -869,35 +938,24 @@ func (r *mutationResolver) RecordAccessReviewEntryDecision(ctx context.Context, return nil, err } - // Resolve the profile ID from the session's identity. - // The profile may not exist for every identity, in which - // case decided_by will be left nil. - identity := authn.IdentityFromContext(ctx) - if identity == nil { - return nil, fmt.Errorf("no identity in context") - } - - req := accessreview.RecordAccessReviewEntryDecisionRequest{ - EntryID: input.AccessReviewEntryID, - Decision: input.Decision, - DecisionNote: input.DecisionNote, - } - - organizationID, err := r.accessReview.ResolveEntryOrganizationID(ctx, input.AccessReviewEntryID) - if err == nil { - profile, err := r.iam.OrganizationService.GetProfileForIdentityAndOrganization(ctx, identity.ID, organizationID) - if err == nil { - req.DecidedByID = &profile.ID - } - } - - entry, err := r.accessReview.RecordDecision(ctx, scope, req) + entry, err := r.accessReview.RecordDecision( + ctx, + scope, + accessreview.RecordAccessReviewEntryDecisionRequest{ + EntryID: input.AccessReviewEntryID, + Decision: input.Decision, + DecisionNote: input.DecisionNote, + DecidedByID: authn.IdentityIDFromContext(ctx), + }, + ) if err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot record access entry decision: %w", err)) + r.logger.ErrorCtx(ctx, "cannot record access entry decision", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.RecordAccessReviewEntryDecisionPayload{ @@ -933,29 +991,10 @@ func (r *mutationResolver) RecordAccessReviewEntryDecisions(ctx context.Context, tenantID := input.Decisions[0].AccessReviewEntryID.TenantID() scope := coredata.NewScope(tenantID) - - // Cache profile lookups per organization so we resolve the correct - // decidedByID for each entry even when a batch spans multiple orgs. - profileCache := make(map[gid.GID]*gid.GID) + decidedByID := &identity.ID decisions := make([]accessreview.RecordAccessReviewEntryDecisionRequest, len(input.Decisions)) for i, d := range input.Decisions { - var decidedByID *gid.GID - - organizationID, err := r.accessReview.ResolveEntryOrganizationID(ctx, d.AccessReviewEntryID) - if err == nil { - if cached, ok := profileCache[organizationID]; ok { - decidedByID = cached - } else { - profile, err := r.iam.OrganizationService.GetProfileForIdentityAndOrganization(ctx, identity.ID, organizationID) - if err == nil { - decidedByID = &profile.ID - } - - profileCache[organizationID] = decidedByID - } - } - decisions[i] = accessreview.RecordAccessReviewEntryDecisionRequest{ EntryID: d.AccessReviewEntryID, Decision: d.Decision, @@ -970,7 +1009,9 @@ func (r *mutationResolver) RecordAccessReviewEntryDecisions(ctx context.Context, return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot record access entry decisions: %w", err)) + r.logger.ErrorCtx(ctx, "cannot record access entry decisions", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } accessEntries := make([]*types.AccessReviewEntry, len(entries)) @@ -1000,7 +1041,9 @@ func (r *mutationResolver) FlagAccessReviewEntry(ctx context.Context, input type return nil, gqlutils.NotFound(ctx, err) } - panic(fmt.Errorf("cannot flag access entry: %w", err)) + r.logger.ErrorCtx(ctx, "cannot flag access entry", log.Error(err)) + + return nil, gqlutils.Internal(ctx) } return &types.FlagAccessReviewEntryPayload{ @@ -1023,6 +1066,11 @@ func (r *Resolver) AccessReviewCampaignSource() schema.AccessReviewCampaignSourc return &accessReviewCampaignSourceResolver{r} } +// AccessReviewCampaignSourceFetchAttemptConnection returns schema.AccessReviewCampaignSourceFetchAttemptConnectionResolver implementation. +func (r *Resolver) AccessReviewCampaignSourceFetchAttemptConnection() schema.AccessReviewCampaignSourceFetchAttemptConnectionResolver { + return &accessReviewCampaignSourceFetchAttemptConnectionResolver{r} +} + // AccessReviewEntry returns schema.AccessReviewEntryResolver implementation. func (r *Resolver) AccessReviewEntry() schema.AccessReviewEntryResolver { return &accessReviewEntryResolver{r} @@ -1046,6 +1094,7 @@ func (r *Resolver) AccessReviewSourceConnection() schema.AccessReviewSourceConne type accessReviewCampaignResolver struct{ *Resolver } type accessReviewCampaignConnectionResolver struct{ *Resolver } type accessReviewCampaignSourceResolver struct{ *Resolver } +type accessReviewCampaignSourceFetchAttemptConnectionResolver struct{ *Resolver } type accessReviewEntryResolver struct{ *Resolver } type accessReviewEntryConnectionResolver struct{ *Resolver } type accessReviewSourceResolver struct{ *Resolver } diff --git a/pkg/server/api/console/v1/base_resolvers.go b/pkg/server/api/console/v1/base_resolvers.go index 014f2b6ae..34d1de8fc 100644 --- a/pkg/server/api/console/v1/base_resolvers.go +++ b/pkg/server/api/console/v1/base_resolvers.go @@ -394,6 +394,16 @@ func (r *queryResolver) Node(ctx context.Context, id gid.GID) (types.Node, error return types.NewAccessReviewCampaign(campaign), nil } + case coredata.AccessReviewCampaignSourceEntityType: + action = accessreview.ActionCampaignGet + loadNode = func(ctx context.Context, scope *coredata.Scope, id gid.GID) (types.Node, error) { + campaignSource, err := r.accessReview.GetCampaignSource(ctx, scope, id) + if err != nil { + return nil, err + } + + return types.NewAccessReviewCampaignSource(campaignSource), nil + } case coredata.AccessReviewSourceEntityType: action = accessreview.ActionSourceGet loadNode = func(ctx context.Context, scope *coredata.Scope, id gid.GID) (types.Node, error) { diff --git a/pkg/server/api/console/v1/graphql/access_review_campaign.graphql b/pkg/server/api/console/v1/graphql/access_review_campaign.graphql index e2555d7e2..1e7a8cb7d 100644 --- a/pkg/server/api/console/v1/graphql/access_review_campaign.graphql +++ b/pkg/server/api/console/v1/graphql/access_review_campaign.graphql @@ -24,28 +24,6 @@ enum AccessReviewCampaignStatus ) } -enum AccessReviewSourceCategory - @goModel( - model: "go.probo.inc/probo/pkg/coredata.AccessReviewSourceCategory" - ) { - SAAS - @goEnum( - value: "go.probo.inc/probo/pkg/coredata.AccessReviewSourceCategorySaaS" - ) - CLOUD_INFRA - @goEnum( - value: "go.probo.inc/probo/pkg/coredata.AccessReviewSourceCategoryCloudInfra" - ) - SOURCE_CODE - @goEnum( - value: "go.probo.inc/probo/pkg/coredata.AccessReviewSourceCategorySourceCode" - ) - OTHER - @goEnum( - value: "go.probo.inc/probo/pkg/coredata.AccessReviewSourceCategoryOther" - ) -} - enum AccessReviewCampaignSourceFetchStatus @goModel( model: "go.probo.inc/probo/pkg/coredata.AccessReviewCampaignSourceFetchStatus" @@ -251,6 +229,21 @@ enum AccessReviewEntryOrderField CREATED_AT } +enum AccessReviewCampaignSourceFetchAttemptOrderField + @goModel( + model: "go.probo.inc/probo/pkg/coredata.AccessReviewCampaignSourceFetchAttemptOrderField" + ) { + CREATED_AT + @goEnum( + value: "go.probo.inc/probo/pkg/coredata.AccessReviewCampaignSourceFetchAttemptOrderFieldCreatedAt" + ) +} + +input AccessReviewCampaignSourceFetchAttemptOrder { + direction: OrderDirection! + field: AccessReviewCampaignSourceFetchAttemptOrderField! +} + enum AccessReviewSourceConnectionStatus { CONNECTED DISCONNECTED @@ -307,10 +300,7 @@ type AccessReviewSource implements Node { permission(action: String!): Boolean! @goField(forceResolver: true) } -type AccessReviewCampaignSourceFetchAttempt - @goModel( - model: "go.probo.inc/probo/pkg/server/api/console/v1/types.AccessReviewCampaignSourceFetchAttempt" - ) { +type AccessReviewCampaignSourceFetchAttempt { id: ID! attemptNumber: Int! status: AccessReviewCampaignSourceFetchStatus! @@ -322,25 +312,22 @@ type AccessReviewCampaignSourceFetchAttempt updatedAt: Datetime! } -type AccessReviewCampaignSource - @goModel( - model: "go.probo.inc/probo/pkg/server/api/console/v1/types.AccessReviewCampaignSource" - ) { +type AccessReviewCampaignSource implements Node { id: ID! + campaign: AccessReviewCampaign! @goField(forceResolver: true) + sourceId: ID "The live access source this snapshot was taken from. Null once the source is deleted." source: AccessReviewSource @goField(forceResolver: true) name: String! - "Current fetch status, derived from the latest fetch attempt." - fetchStatus: AccessReviewCampaignSourceFetchStatus! - fetchedAccountsCount: Int! - attemptCount: Int! - "Generic, user-facing error from the latest attempt. Raw errors stay in logs." - lastError: String - fetchStartedAt: Datetime - fetchCompletedAt: Datetime - "Full append-only history of fetch attempts, most recent first." - fetchAttempts: [AccessReviewCampaignSourceFetchAttempt!]! @goField(forceResolver: true) + fetchAttempts( + first: Int + after: CursorKey + last: Int + before: CursorKey + orderBy: AccessReviewCampaignSourceFetchAttemptOrder + ): AccessReviewCampaignSourceFetchAttemptConnection! + @goField(forceResolver: true) entries( first: Int @@ -374,7 +361,6 @@ type AccessReviewCampaign implements Node { last: Int before: CursorKey orderBy: AccessReviewEntryOrder - campaignSourceId: ID filter: AccessReviewEntryFilter ): AccessReviewEntryConnection! @goField(forceResolver: true) @@ -388,8 +374,7 @@ type AccessReviewCampaign implements Node { type AccessReviewEntry implements Node { id: ID! campaign: AccessReviewCampaign! @goField(forceResolver: true) - "The live access source this entry came from. Null once the source is deleted." - accessReviewSource: AccessReviewSource @goField(forceResolver: true) + campaignSource: AccessReviewCampaignSource! @goField(forceResolver: true) email: String! fullName: String! role: String! @@ -468,6 +453,20 @@ type AccessReviewEntryEdge { node: AccessReviewEntry! } +type AccessReviewCampaignSourceFetchAttemptConnection + @goModel( + model: "go.probo.inc/probo/pkg/server/api/console/v1/types.AccessReviewCampaignSourceFetchAttemptConnection" + ) { + totalCount: Int! @goField(forceResolver: true) + edges: [AccessReviewCampaignSourceFetchAttemptEdge!]! + pageInfo: PageInfo! +} + +type AccessReviewCampaignSourceFetchAttemptEdge { + cursor: CursorKey! + node: AccessReviewCampaignSourceFetchAttempt! +} + type AccessReviewStatistics { totalCount: Int! decisionCounts: [AccessReviewEntryDecisionCount!]! diff --git a/pkg/server/api/console/v1/types/access_review.go b/pkg/server/api/console/v1/types/access_review.go index ef2ec5338..ce44947af 100644 --- a/pkg/server/api/console/v1/types/access_review.go +++ b/pkg/server/api/console/v1/types/access_review.go @@ -15,17 +15,16 @@ package types import ( - "time" - "go.probo.inc/probo/pkg/coredata" "go.probo.inc/probo/pkg/gid" "go.probo.inc/probo/pkg/page" ) type ( - AccessReviewSourceOrderBy OrderBy[coredata.AccessReviewSourceOrderField] - AccessReviewCampaignOrderBy OrderBy[coredata.AccessReviewCampaignOrderField] - AccessReviewEntryOrderBy OrderBy[coredata.AccessReviewEntryOrderField] + AccessReviewSourceOrderBy OrderBy[coredata.AccessReviewSourceOrderField] + AccessReviewCampaignOrderBy OrderBy[coredata.AccessReviewCampaignOrderField] + AccessReviewEntryOrderBy OrderBy[coredata.AccessReviewEntryOrderField] + AccessReviewCampaignSourceFetchAttemptOrderBy OrderBy[coredata.AccessReviewCampaignSourceFetchAttemptOrderField] AccessReviewSourceConnection struct { TotalCount int @@ -55,6 +54,15 @@ type ( SourceID *gid.GID Filter *coredata.AccessReviewEntryFilter } + + AccessReviewCampaignSourceFetchAttemptConnection struct { + TotalCount int + Edges []*AccessReviewCampaignSourceFetchAttemptEdge + PageInfo PageInfo + + Resolver any + ParentID gid.GID + } ) // AccessReviewSource helpers @@ -100,53 +108,37 @@ func NewAccessReviewSource(s *coredata.AccessReviewSource) *AccessReviewSource { } } -// NewAccessReviewCampaignSource builds the GraphQL scope source from a -// campaign source snapshot. The current fetch state is derived from the latest -// fetch attempt (nil when the source has never been fetched). The live access -// source is resolved lazily via the source field resolver from SourceID. +// NewAccessReviewCampaignSource builds the GraphQL campaign source from a +// snapshot row. Fetch state is resolved lazily via field resolvers on +// AccessReviewCampaignSource. The live access source is resolved via the +// source field resolver from SourceID. func NewAccessReviewCampaignSource( campaignSource *coredata.AccessReviewCampaignSource, - latestAttempt *coredata.AccessReviewCampaignSourceFetchAttempt, ) *AccessReviewCampaignSource { - status := coredata.AccessReviewCampaignSourceFetchStatusQueued - fetchedAccountsCount := 0 - attemptCount := 0 - - var ( - lastError *string - fetchStartedAt *time.Time - fetchCompletedAt *time.Time - ) - - if latestAttempt != nil { - status = latestAttempt.Status - fetchedAccountsCount = latestAttempt.FetchedAccountsCount - attemptCount = latestAttempt.AttemptNumber - lastError = latestAttempt.Error - fetchStartedAt = latestAttempt.StartedAt - fetchCompletedAt = latestAttempt.CompletedAt - } - return &AccessReviewCampaignSource{ - ID: campaignSource.ID, - CampaignID: campaignSource.AccessReviewCampaignID, - SourceID: campaignSource.AccessReviewSourceID, - Name: campaignSource.Name, - FetchStatus: status, - FetchedAccountsCount: fetchedAccountsCount, - AttemptCount: attemptCount, - LastError: lastError, - FetchStartedAt: fetchStartedAt, - FetchCompletedAt: fetchCompletedAt, + ID: campaignSource.ID, + Campaign: &AccessReviewCampaign{ + ID: campaignSource.AccessReviewCampaignID, + }, + SourceID: campaignSource.AccessReviewSourceID, + Name: campaignSource.Name, } } // NewAccessReviewCampaignSourceFetchAttempt builds the GraphQL representation of a -// single append-only fetch attempt. -func NewAccessReviewCampaignSourceFetchAttempt(a *coredata.AccessReviewCampaignSourceFetchAttempt) *AccessReviewCampaignSourceFetchAttempt { +// single append-only fetch attempt. attemptNumber is the 1-based position in the +// snapshot's history, counting up from the oldest attempt. +func NewAccessReviewCampaignSourceFetchAttempt( + a *coredata.AccessReviewCampaignSourceFetchAttempt, + attemptNumber int, +) *AccessReviewCampaignSourceFetchAttempt { + if a.AttemptNumber > 0 { + attemptNumber = a.AttemptNumber + } + return &AccessReviewCampaignSourceFetchAttempt{ ID: a.ID, - AttemptNumber: a.AttemptNumber, + AttemptNumber: attemptNumber, Status: a.Status, FetchedAccountsCount: a.FetchedAccountsCount, Error: a.Error, @@ -157,6 +149,37 @@ func NewAccessReviewCampaignSourceFetchAttempt(a *coredata.AccessReviewCampaignS } } +func NewAccessReviewCampaignSourceFetchAttemptConnection( + p *page.Page[*coredata.AccessReviewCampaignSourceFetchAttempt, coredata.AccessReviewCampaignSourceFetchAttemptOrderField], + parentType any, + parentID gid.GID, +) *AccessReviewCampaignSourceFetchAttemptConnection { + edges := make([]*AccessReviewCampaignSourceFetchAttemptEdge, len(p.Data)) + + for i := range edges { + edges[i] = NewAccessReviewCampaignSourceFetchAttemptEdge(p.Data[i], p.Cursor.OrderBy.Field, 0) + } + + return &AccessReviewCampaignSourceFetchAttemptConnection{ + Edges: edges, + PageInfo: *NewPageInfo(p), + + Resolver: parentType, + ParentID: parentID, + } +} + +func NewAccessReviewCampaignSourceFetchAttemptEdge( + a *coredata.AccessReviewCampaignSourceFetchAttempt, + orderBy coredata.AccessReviewCampaignSourceFetchAttemptOrderField, + attemptNumber int, +) *AccessReviewCampaignSourceFetchAttemptEdge { + return &AccessReviewCampaignSourceFetchAttemptEdge{ + Cursor: a.CursorKey(orderBy), + Node: NewAccessReviewCampaignSourceFetchAttempt(a, attemptNumber), + } +} + // AccessReviewCampaign helpers func NewAccessReviewCampaignConnection( @@ -260,6 +283,9 @@ func NewAccessReviewEntry(e *coredata.AccessReviewEntry) *AccessReviewEntry { Campaign: &AccessReviewCampaign{ ID: e.AccessReviewCampaignID, }, + CampaignSource: &AccessReviewCampaignSource{ + ID: e.AccessReviewCampaignSourceID, + }, Email: e.Email, FullName: e.FullName, Role: e.Role, diff --git a/pkg/server/api/console/v1/types/access_review_campaign_source.go b/pkg/server/api/console/v1/types/access_review_campaign_source.go deleted file mode 100644 index 0a8614a62..000000000 --- a/pkg/server/api/console/v1/types/access_review_campaign_source.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) 2026 Probo Inc . -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR -// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - -package types - -import ( - "time" - - "go.probo.inc/probo/pkg/coredata" - "go.probo.inc/probo/pkg/gid" -) - -type AccessReviewCampaignSource struct { - ID gid.GID `json:"id"` - CampaignID gid.GID `json:"-"` - // SourceID is the live access source this snapshot points at, or nil once - // that source has been deleted. The source field is resolved lazily from it. - SourceID *gid.GID `json:"-"` - Name string `json:"name"` - FetchStatus coredata.AccessReviewCampaignSourceFetchStatus `json:"fetchStatus"` - FetchedAccountsCount int `json:"fetchedAccountsCount"` - AttemptCount int `json:"attemptCount"` - LastError *string `json:"lastError,omitempty"` - FetchStartedAt *time.Time `json:"fetchStartedAt,omitempty"` - FetchCompletedAt *time.Time `json:"fetchCompletedAt,omitempty"` -} - -type AccessReviewCampaignSourceFetchAttempt struct { - ID gid.GID `json:"id"` - AttemptNumber int `json:"attemptNumber"` - Status coredata.AccessReviewCampaignSourceFetchStatus `json:"status"` - FetchedAccountsCount int `json:"fetchedAccountsCount"` - Error *string `json:"error,omitempty"` - StartedAt *time.Time `json:"startedAt,omitempty"` - CompletedAt *time.Time `json:"completedAt,omitempty"` - CreatedAt time.Time `json:"createdAt"` - UpdatedAt time.Time `json:"updatedAt"` -} diff --git a/pkg/server/api/console/v1/types/access_review_campaign_source_test.go b/pkg/server/api/console/v1/types/access_review_campaign_source_test.go index 0539f1770..c7b024fa9 100644 --- a/pkg/server/api/console/v1/types/access_review_campaign_source_test.go +++ b/pkg/server/api/console/v1/types/access_review_campaign_source_test.go @@ -16,7 +16,6 @@ package types import ( "testing" - "time" "go.probo.inc/probo/pkg/coredata" "go.probo.inc/probo/pkg/gid" @@ -29,28 +28,23 @@ func newTestCampaignSource(tenantID gid.TenantID, sourceID *gid.GID, name string AccessReviewCampaignID: gid.New(tenantID, coredata.AccessReviewCampaignEntityType), AccessReviewSourceID: sourceID, Name: name, - Category: coredata.AccessReviewSourceCategorySaaS, } } -func TestNewAccessReviewCampaignSource_DefaultFetchState(t *testing.T) { +func TestNewAccessReviewCampaignSource(t *testing.T) { t.Parallel() tenantID := gid.NewTenantID() sourceID := gid.New(tenantID, coredata.AccessReviewSourceEntityType) campaignSource := newTestCampaignSource(tenantID, &sourceID, "Google Workspace") - got := NewAccessReviewCampaignSource(campaignSource, nil) - if got.FetchStatus != coredata.AccessReviewCampaignSourceFetchStatusQueued { - t.Fatalf("fetch status = %q, want QUEUED", got.FetchStatus) + got := NewAccessReviewCampaignSource(campaignSource) + if got.ID != campaignSource.ID { + t.Fatalf("id = %v, want %v", got.ID, campaignSource.ID) } - if got.FetchedAccountsCount != 0 { - t.Fatalf("fetched accounts count = %d, want 0", got.FetchedAccountsCount) - } - - if got.AttemptCount != 0 { - t.Fatalf("attempt count = %d, want 0", got.AttemptCount) + if got.Campaign == nil || got.Campaign.ID != campaignSource.AccessReviewCampaignID { + t.Fatalf("campaign id = %v, want %v", got.Campaign, campaignSource.AccessReviewCampaignID) } if got.SourceID == nil || *got.SourceID != sourceID { @@ -62,41 +56,6 @@ func TestNewAccessReviewCampaignSource_DefaultFetchState(t *testing.T) { } } -func TestNewAccessReviewCampaignSource_UsesLatestAttempt(t *testing.T) { - t.Parallel() - - now := time.Now() - errMsg := "We couldn't fetch accounts from this source." - tenantID := gid.NewTenantID() - sourceID := gid.New(tenantID, coredata.AccessReviewSourceEntityType) - campaignSource := newTestCampaignSource(tenantID, &sourceID, "Linear") - attempt := &coredata.AccessReviewCampaignSourceFetchAttempt{ - Status: coredata.AccessReviewCampaignSourceFetchStatusFailed, - FetchedAccountsCount: 42, - AttemptNumber: 3, - Error: &errMsg, - StartedAt: &now, - CompletedAt: &now, - } - - got := NewAccessReviewCampaignSource(campaignSource, attempt) - if got.FetchStatus != coredata.AccessReviewCampaignSourceFetchStatusFailed { - t.Fatalf("fetch status = %q, want FAILED", got.FetchStatus) - } - - if got.FetchedAccountsCount != 42 { - t.Fatalf("fetched accounts count = %d, want 42", got.FetchedAccountsCount) - } - - if got.AttemptCount != 3 { - t.Fatalf("attempt count = %d, want 3", got.AttemptCount) - } - - if got.LastError == nil || *got.LastError != errMsg { - t.Fatalf("last error = %v, want %q", got.LastError, errMsg) - } -} - // TestNewAccessReviewCampaignSource_DeletedSource verifies a snapshot whose // live source has been deleted still renders (with a nil source link) so the // historical review data remains visible. @@ -106,7 +65,7 @@ func TestNewAccessReviewCampaignSource_DeletedSource(t *testing.T) { tenantID := gid.NewTenantID() campaignSource := newTestCampaignSource(tenantID, nil, "Deleted Source") - got := NewAccessReviewCampaignSource(campaignSource, nil) + got := NewAccessReviewCampaignSource(campaignSource) if got.SourceID != nil { t.Fatalf("source id = %v, want nil for deleted source", got.SourceID) } diff --git a/pkg/server/api/mcp/v1/schema.resolvers.go b/pkg/server/api/mcp/v1/schema.resolvers.go index a2732e2b8..026e17633 100644 --- a/pkg/server/api/mcp/v1/schema.resolvers.go +++ b/pkg/server/api/mcp/v1/schema.resolvers.go @@ -3410,11 +3410,39 @@ func (r *Resolver) ListAccessReviewCampaignsTool(ctx context.Context, req *mcp.C } // ListAccessEntriesTool handles the listAccessEntries tool -// List access entries for a campaign with optional filters +// List access entries for a campaign or campaign source with optional filters func (r *Resolver) ListAccessEntriesTool(ctx context.Context, req *mcp.CallToolRequest, input *types.ListAccessEntriesInput) (*mcp.CallToolResult, types.ListAccessEntriesOutput, error) { - scope, err := r.Authorize(ctx, input.CampaignID, accessreview.ActionEntryList) - if err != nil { - return nil, types.ListAccessEntriesOutput{}, err + if input.AccessReviewCampaignSourceID == nil && input.CampaignID == nil { + return nil, types.ListAccessEntriesOutput{}, fmt.Errorf("campaign_id or access_review_campaign_source_id is required") + } + + var ( + scope *coredata.Scope + campaignID gid.GID + sourceID *gid.GID + err error + ) + + if input.AccessReviewCampaignSourceID != nil { + scope, err = r.Authorize(ctx, *input.AccessReviewCampaignSourceID, accessreview.ActionEntryList) + if err != nil { + return nil, types.ListAccessEntriesOutput{}, err + } + + campaignSource, err := r.accessReview.GetCampaignSource(ctx, scope, *input.AccessReviewCampaignSourceID) + if err != nil { + panic(fmt.Errorf("cannot get campaign source: %w", err)) + } + + campaignID = campaignSource.AccessReviewCampaignID + sourceID = input.AccessReviewCampaignSourceID + } else { + scope, err = r.Authorize(ctx, *input.CampaignID, accessreview.ActionEntryList) + if err != nil { + return nil, types.ListAccessEntriesOutput{}, err + } + + campaignID = *input.CampaignID } pageOrderBy := page.OrderBy[coredata.AccessReviewEntryOrderField]{ @@ -3446,14 +3474,12 @@ func (r *Resolver) ListAccessEntriesTool(ctx context.Context, req *mcp.CallToolR var p *page.Page[*coredata.AccessReviewEntry, coredata.AccessReviewEntryOrderField] - if input.AccessReviewCampaignSourceID != nil { - var err error - + if sourceID != nil { p, err = r.accessReview.ListEntriesForCampaignIDAndSourceID( ctx, scope, - input.CampaignID, - *input.AccessReviewCampaignSourceID, + campaignID, + *sourceID, cursor, filter, ) @@ -3461,9 +3487,7 @@ func (r *Resolver) ListAccessEntriesTool(ctx context.Context, req *mcp.CallToolR panic(fmt.Errorf("cannot list access entries: %w", err)) } } else { - var err error - - p, err = r.accessReview.ListEntriesForCampaignID(ctx, scope, input.CampaignID, cursor, filter) + p, err = r.accessReview.ListEntriesForCampaignID(ctx, scope, campaignID, cursor, filter) if err != nil { panic(fmt.Errorf("cannot list access entries: %w", err)) } @@ -3498,26 +3522,16 @@ func (r *Resolver) RecordAccessReviewEntryDecisionTool(ctx context.Context, req return nil, types.RecordAccessReviewEntryDecisionOutput{}, err } - identity := authn.IdentityFromContext(ctx) - if identity == nil { - return nil, types.RecordAccessReviewEntryDecisionOutput{}, fmt.Errorf("no identity in context") - } - - decisionReq := accessreview.RecordAccessReviewEntryDecisionRequest{ - EntryID: input.AccessReviewEntryID, - Decision: input.Decision, - DecisionNote: input.DecisionNote, - } - - organizationID, err := r.accessReview.ResolveEntryOrganizationID(ctx, input.AccessReviewEntryID) - if err == nil { - profile, err := r.iamSvc.OrganizationService.GetProfileForIdentityAndOrganization(ctx, identity.ID, organizationID) - if err == nil { - decisionReq.DecidedByID = &profile.ID - } - } - - entry, err := r.accessReview.RecordDecision(ctx, scope, decisionReq) + entry, err := r.accessReview.RecordDecision( + ctx, + scope, + accessreview.RecordAccessReviewEntryDecisionRequest{ + EntryID: input.AccessReviewEntryID, + Decision: input.Decision, + DecisionNote: input.DecisionNote, + DecidedByID: authn.IdentityIDFromContext(ctx), + }, + ) if err != nil { return nil, types.RecordAccessReviewEntryDecisionOutput{}, fmt.Errorf("cannot record decision: %w", err) } @@ -3555,28 +3569,10 @@ func (r *Resolver) RecordAccessReviewEntryDecisionsTool(ctx context.Context, req return nil, types.RecordAccessReviewEntryDecisionsOutput{}, fmt.Errorf("no identity in context") } - // Cache profile lookups per organization so we resolve the correct - // decidedByID for each entry even when a batch spans multiple orgs. - profileCache := make(map[gid.GID]*gid.GID) + decidedByID := &identity.ID decisions := make([]accessreview.RecordAccessReviewEntryDecisionRequest, len(input.Decisions)) for i, d := range input.Decisions { - var decidedByID *gid.GID - - organizationID, err := r.accessReview.ResolveEntryOrganizationID(ctx, d.AccessReviewEntryID) - if err == nil { - if cached, ok := profileCache[organizationID]; ok { - decidedByID = cached - } else { - profile, err := r.iamSvc.OrganizationService.GetProfileForIdentityAndOrganization(ctx, identity.ID, organizationID) - if err == nil { - decidedByID = &profile.ID - } - - profileCache[organizationID] = decidedByID - } - } - decisions[i] = accessreview.RecordAccessReviewEntryDecisionRequest{ EntryID: d.AccessReviewEntryID, Decision: d.Decision, @@ -3660,7 +3656,6 @@ func (r *Resolver) CreateAccessReviewSourceTool(ctx context.Context, req *mcp.Ca OrganizationID: input.OrganizationID, ConnectorID: input.ConnectorID, Name: input.Name, - Category: coredata.AccessReviewSourceCategorySaaS, CsvData: input.CsvData, }) if err != nil { @@ -3682,7 +3677,10 @@ func (r *Resolver) UpdateAccessReviewSourceTool(ctx context.Context, req *mcp.Ca updateReq := accessreview.UpdateAccessReviewSourceRequest{ AccessReviewSourceID: input.AccessReviewSourceID, - Name: input.Name, + } + + if input.Name != nil { + updateReq.Name = &input.Name } if rawConnectorID := UnwrapOmittable(input.ConnectorID); rawConnectorID != nil { @@ -3770,9 +3768,15 @@ func (r *Resolver) UpdateAccessReviewCampaignTool(ctx context.Context, req *mcp. } updateReq := accessreview.UpdateAccessReviewCampaignRequest{ - CampaignID: input.CampaignID, - Name: input.Name, - Description: input.Description, + CampaignID: input.CampaignID, + } + + if input.Name != nil { + updateReq.Name = &input.Name + } + + if input.Description != nil { + updateReq.Description = &input.Description } if rawControls := UnwrapOmittable(input.FrameworkControls); rawControls != nil { diff --git a/pkg/server/api/mcp/v1/specification.yaml b/pkg/server/api/mcp/v1/specification.yaml index f6d37ab00..094af4d37 100644 --- a/pkg/server/api/mcp/v1/specification.yaml +++ b/pkg/server/api/mcp/v1/specification.yaml @@ -7851,15 +7851,13 @@ components: ListAccessEntriesInput: type: object - required: - - campaign_id properties: campaign_id: $ref: "#/components/schemas/GID" - description: Campaign ID + description: Campaign ID. Required when access_review_campaign_source_id is omitted. access_review_campaign_source_id: $ref: "#/components/schemas/GID" - description: Filter by per-campaign source snapshot ID + description: Campaign source snapshot ID. When set, lists entries for that source only and campaign_id is ignored. order_by: $ref: "#/components/schemas/AccessReviewEntryOrderBy" description: Order by @@ -8026,15 +8024,6 @@ components: campaign: $ref: "#/components/schemas/AccessReviewCampaign" - AccessReviewSourceCategory: - type: string - go.probo.inc/mcpgen/type: go.probo.inc/probo/pkg/coredata.AccessReviewSourceCategory - enum: - - SAAS - - CLOUD_INFRA - - SOURCE_CODE - - OTHER - AccessReviewSourceOrderField: type: string go.probo.inc/mcpgen/type: go.probo.inc/probo/pkg/coredata.AccessReviewSourceOrderField