Skip to content

feat: report side-effect principal deletion from revoke#141

Draft
alan-lee-12 wants to merge 4 commits into
mainfrom
fde-296-revoke-deletes-user
Draft

feat: report side-effect principal deletion from revoke#141
alan-lee-12 wants to merge 4 commits into
mainfrom
fde-296-revoke-deletes-user

Conversation

@alan-lee-12

@alan-lee-12 alan-lee-12 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What

Some downstream apps delete the user record when their last role is revoked. Today ConductorOne doesn't learn the account is gone until the next full sync (~1h), so an immediate role change (revoke old → request new) fails downstream because the account no longer exists.

This adds an optional provisioning.revoke.revoke_options.principal_exists_check probe. After the revoke queries run (same transaction), the probe checks whether the principal still exists — a positive existence query, the same shape as account_provisioning.validate. If it returns no rows, the principal was deleted as a side effect, and the connector attaches a ResourceDeleted annotation to the revoke response so ConductorOne can mark the account deleted immediately.

Changes

  • config (pkg/bsql/config.go, pkg/bsql/validate.go): a revoke-only RevokeEntitlementProvisioningQueries type with a revoke_options block carrying principal_exists_check { query }, plus validation (non-empty query + var check)
  • pkg/bsql/query.go: RunRevokeProvisioning + runPrincipalExistsCheck — runs the probe on the same executor/tx after the revoke queries; no rows ⇒ principal deleted. Tolerates the all-zero-rows (already-revoked) case so retried revokes still report the deletion.
  • pkg/bsql/provisioning.go: Revoke() attaches ResourceDeleted{principal.Id}, combined with GrantAlreadyRevoked on the already-revoked retry path.
  • examples/mssql-revoke-deletes-user.yml: worked example (delete-user-on-last-role revoke + probe + re-provisioning)
  • tests: pkg/bsql/provisioning_revoke_deleted_test.go + config round-trip in config_test.go

Design note

The probe is a positive existence query (rows present = principal still exists), mirroring account_provisioning.validate; "no rows ⇒ deleted" is the single interpretation. This keeps the SQL natural (no NOT EXISTS gymnastics) and consistent with the existing validate pattern.

Dependency / merge order

Depends on the ResourceDeleted annotation in ConductorOne/baton-sdk#1010. This branch is intentionally source-onlygo.mod / go.sum / vendor/ are not included, so it won't build standalone until the SDK PR is released and the dependency is bumped here (dropping the local replace directive used during development).

Test plan (built against the local SDK via replace)

  • go build ./... && go test ./... -count=1 — pass
  • gofmt clean; golangci-lint run ./pkg/bsql/... — clean aside from the dev-only replace directive

Refs FDE-296

🤖 Generated with Claude Code

Some apps delete the user when their last role is revoked. Add an optional
provisioning.revoke.principal_deleted_check probe that runs after the revoke
queries on the same transaction; when it finds the principal gone, the revoke
response carries a ResourceDeleted annotation so ConductorOne marks the
account deleted immediately instead of waiting for a full sync.

- config: PrincipalDeletedCheck struct/field + validation
- query.go: RunRevokeProvisioning + runPrincipalDeletedCheck probe
- provisioning.go: attach ResourceDeleted (combined with GrantAlreadyRevoked
  on the already-revoked retry path)
- examples/mssql-revoke-deletes-user.yml + tests

Depends on the baton-sdk ResourceDeleted annotation; the go.mod/vendor bump to
the released SDK is handled separately.

Refs FDE-296

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 15, 2026

Copy link
Copy Markdown

FDE-296

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: feat: report side-effect principal deletion from revoke

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 88650be8ea0c.
Review mode: incremental since 97a77d8
View review run

Review Summary

The new commit renames principal_deleted_check to principal_exists_check across config, provisioning, validation, tests, and the example YAML, and inverts the probe helper to return "principal exists" (the caller now computes principalDeleted = !exists). This is semantics-preserving: verified no leftover PrincipalDeletedCheck / principal_deleted_check references remain, and the Revoke flow (pkg/bsql/provisioning.go:142-171) still attaches ResourceDeleted correctly. Since the entire feature including the config key is new in this PR, the rename is not a breaking change against released config. Scanned the full PR diff for security and correctness; no new issues found.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

alan-lee-12 and others added 2 commits July 15, 2026 16:33
Group revoke-only probe config under revoke.revoke_options so future revoke
knobs have a clear home without polluting the shared provisioning query fields.

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

The probe is a positive existence query — it returns the principal when it
still exists, mirroring account_provisioning.validate. The previous name
described the downstream consequence rather than the query, which forced a
double-negative read ("write an exists query, but zero rows means deleted").

Behavior is unchanged: the exists-check returning no rows still means the
principal was deleted by the revoke, yielding a ResourceDeleted annotation.

Refs FDE-296

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant