From a3a39fc918e4078244c0e410647c07b32b655452 Mon Sep 17 00:00:00 2001 From: Alan Lee Date: Wed, 15 Jul 2026 13:35:43 -0700 Subject: [PATCH 1/3] feat: report side-effect principal deletion from revoke 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 --- examples/mssql-revoke-deletes-user.yml | 166 +++++++++++ pkg/bsql/config.go | 18 ++ pkg/bsql/config_test.go | 13 + pkg/bsql/provisioning.go | 24 +- pkg/bsql/provisioning_revoke_deleted_test.go | 279 +++++++++++++++++++ pkg/bsql/query.go | 126 +++++++++ pkg/bsql/validate.go | 41 ++- 7 files changed, 650 insertions(+), 17 deletions(-) create mode 100644 examples/mssql-revoke-deletes-user.yml create mode 100644 pkg/bsql/provisioning_revoke_deleted_test.go diff --git a/examples/mssql-revoke-deletes-user.yml b/examples/mssql-revoke-deletes-user.yml new file mode 100644 index 00000000..aaa71821 --- /dev/null +++ b/examples/mssql-revoke-deletes-user.yml @@ -0,0 +1,166 @@ +--- +# Example: reporting a side-effect user deletion from a revoke. +# +# Some applications delete the user record when their last role is revoked. +# When that happens ConductorOne would otherwise not learn the account is gone +# until the next full sync, which blocks an immediate re-grant. Configuring +# `principal_deleted_check` under an entitlement's `revoke` makes the connector +# probe for the principal after the revoke queries run (on the same +# transaction). If the probe returns no rows the connector attaches a +# ResourceDeleted annotation to the revoke response so ConductorOne marks the +# account deleted right away. +app_name: SQL Server Revoke-Deletes-User Example +app_description: Demonstrates principal_deleted_check for apps that delete a user when their last role is revoked + +connect: + dsn: "sqlserver://${DB_HOST}:${DB_PORT}?database=${DB_DATABASE}" + user: "${DB_USER}" + password: "${DB_PASSWORD}" + +resource_types: + user: + name: "User" + description: "A user within the SQL Server system" + list: + query: | + SELECT + u.UserID AS id, + u.Username AS username, + u.Email AS email + FROM Users u + ORDER BY u.UserID + OFFSET ? ROWS FETCH NEXT ? ROWS ONLY + pagination: + strategy: "offset" + primary_key: "id" + map: + id: ".username" + display_name: ".username" + description: ".username" + traits: + user: + emails: + - ".email" + status: "enabled" + login: ".username" + + # Account provisioning so that a re-grant immediately after deletion + # recreates the user before the new role is assigned. + account_provisioning: + schema: + - name: "username" + description: "The username for the new user" + type: "string" + placeholder: "new_user" + required: true + - name: "email" + description: "Email address for the new user" + type: "string" + placeholder: "user@example.com" + required: true + credentials: + no_password: + preferred: true + validate: + vars: + username: "username" + query: | + SELECT u.UserID AS id, u.Username AS username, u.Email AS email + FROM Users u + WHERE u.Username = ? + create: + vars: + username: "input.username" + email: "input.email" + queries: + - | + IF NOT EXISTS (SELECT 1 FROM Users WHERE Username = ?) + INSERT INTO Users (Username, Email, IsActive, CreatedAt) + VALUES (?, ?, 1, GETDATE()) + + role: + name: "Role" + description: "A role within the SQL Server system" + list: + query: | + SELECT + RoleID AS id, + RoleName AS role_name, + Description AS description + FROM Roles + ORDER BY RoleID + OFFSET ? ROWS FETCH NEXT ? ROWS ONLY + pagination: + strategy: "offset" + primary_key: "id" + map: + id: ".role_name" + display_name: ".role_name" + description: ".description" + traits: + role: + profile: + role_name: ".role_name" + + static_entitlements: + - id: "member" + display_name: "resource.DisplayName + ' Membership'" + description: "'Member of the ' + resource.DisplayName + ' role'" + purpose: "assignment" + grantable_to: + - "user" + provisioning: + vars: + username: "principal.ID" + role_name: "resource.ID" + grant: + queries: + - | + INSERT INTO UserRoles (UserID, RoleID) + SELECT u.UserID, r.RoleID + FROM Users u, Roles r + WHERE u.Username = ? + AND r.RoleName = ? + revoke: + # Revoke removes the membership row, then deletes the user if this + # was their last role. The queries run in a single transaction. + queries: + - | + DELETE ur FROM UserRoles ur + INNER JOIN Users u ON ur.UserID = u.UserID + INNER JOIN Roles r ON ur.RoleID = r.RoleID + WHERE u.Username = ? + AND r.RoleName = ? + - | + DELETE FROM Users + WHERE Username = ? + AND NOT EXISTS ( + SELECT 1 FROM UserRoles ur + INNER JOIN Users u2 ON ur.UserID = u2.UserID + WHERE u2.Username = ? + ) + # After the revoke queries run, probe for the principal. No rows + # means the app deleted the user, so the connector reports a + # ResourceDeleted annotation on the revoke response. + principal_deleted_check: + query: | + SELECT 1 FROM Users WHERE Username = ? + + grants: + - query: | + SELECT + u.Username AS username, + r.RoleName AS role_name + FROM UserRoles ur + INNER JOIN Users u ON ur.UserID = u.UserID + INNER JOIN Roles r ON ur.RoleID = r.RoleID + ORDER BY r.RoleID + OFFSET ? ROWS FETCH NEXT ? ROWS ONLY + map: + - skip_if: ".role_name != resource.ID" + principal_id: ".username" + principal_type: "user" + entitlement_id: "member" + pagination: + strategy: "offset" + primary_key: "role_name" diff --git a/pkg/bsql/config.go b/pkg/bsql/config.go index e60f346b..056470c8 100644 --- a/pkg/bsql/config.go +++ b/pkg/bsql/config.go @@ -427,6 +427,24 @@ type EntitlementProvisioningQueries struct { // Queries is a list of SQL statements to execute for the provisioning operation. Queries []string `yaml:"queries,omitempty" json:"queries,omitempty"` + + // PrincipalDeletedCheck detects that a revoke deleted the principal itself + // (e.g. the downstream app deletes the user when their last role is + // removed). It is only consulted on the revoke path; the grant path ignores + // it. When configured and the probe finds the principal gone, the revoke + // response carries a ResourceDeleted annotation so ConductorOne can mark the + // account deleted without waiting for the next full sync. + PrincipalDeletedCheck *PrincipalDeletedCheck `yaml:"principal_deleted_check,omitempty" json:"principal_deleted_check,omitempty"` +} + +// PrincipalDeletedCheck configures a probe query that detects whether the +// principal was deleted as a side effect of a revoke. +type PrincipalDeletedCheck struct { + // Query runs after the revoke queries on the same executor/transaction with + // the same provisioning vars. If it returns no rows, the principal is + // considered deleted and a ResourceDeleted annotation is attached to the + // revoke response. If it returns any row, the principal still exists. + Query string `yaml:"query" json:"query"` } type GrantReplaceProvisioningQueries struct { diff --git a/pkg/bsql/config_test.go b/pkg/bsql/config_test.go index e966389b..db812591 100644 --- a/pkg/bsql/config_test.go +++ b/pkg/bsql/config_test.go @@ -248,6 +248,19 @@ resource_types: require.Equal(t, "'Grant cancelled by policy.'", grant.RejectIf.Reason) }, }, + { + name: "mssql-revoke-deletes-user-example", + input: loadExampleConfig(t, "mssql-revoke-deletes-user"), + validate: func(t *testing.T, c *Config) { + revoke := c.ResourceTypes["role"].StaticEntitlements[0].Provisioning.Revoke + require.NotNil(t, revoke) + require.NotNil(t, revoke.PrincipalDeletedCheck) + require.Equal(t, + normalizeQueryString("SELECT 1 FROM Users WHERE Username = ?"), + normalizeQueryString(revoke.PrincipalDeletedCheck.Query), + ) + }, + }, } for _, tt := range tests { diff --git a/pkg/bsql/provisioning.go b/pkg/bsql/provisioning.go index 1c256a72..7642ae1d 100644 --- a/pkg/bsql/provisioning.go +++ b/pkg/bsql/provisioning.go @@ -9,6 +9,7 @@ import ( v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/annotations" "github.com/conductorone/baton-sdk/pkg/crypto" + sdkResource "github.com/conductorone/baton-sdk/pkg/types/resource" "github.com/conductorone/baton-sql/pkg/helpers" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "go.uber.org/zap" @@ -134,10 +135,27 @@ func (s *SQLSyncer) Revoke(ctx context.Context, grant *v2.Grant) (annotations.An } useTx := !provisioningConfig.Revoke.NoTransaction - err = s.RunProvisioningQueries(ctx, provisioningConfig.Revoke.Queries, provisioningConfig.Revoke.ValidationQueries, provisioningVars, useTx) + principalDeleted, err := s.RunRevokeProvisioning( + ctx, + provisioningConfig.Revoke.Queries, + provisioningConfig.Revoke.ValidationQueries, + provisioningConfig.Revoke.PrincipalDeletedCheck, + provisioningVars, + useTx, + ) + + anno := annotations.Annotations{} + if principalDeleted { + anno.Append(sdkResource.NewResourceDeleted(grant.GetPrincipal().GetId())) + l.Info( + "revoke deleted principal; reporting ResourceDeleted", + zap.String("grant_id", grant.GetId()), + zap.String("principal_id", grant.GetPrincipal().GetId().GetResource()), + ) + } + if err != nil { if errors.Is(err, ErrQueryAffectedZeroRows) { - anno := annotations.Annotations{} anno.Update(&v2.GrantAlreadyRevoked{}) return anno, nil } @@ -146,7 +164,7 @@ func (s *SQLSyncer) Revoke(ctx context.Context, grant *v2.Grant) (annotations.An } l.Debug("revoked grant", zap.String("grant_id", grant.GetId())) - return nil, nil + return anno, nil } func (s *SQLSyncer) prepareProvisioningVars(ctx context.Context, vars map[string]string, principal *v2.Resource, entitlement *v2.Entitlement) (map[string]any, error) { diff --git a/pkg/bsql/provisioning_revoke_deleted_test.go b/pkg/bsql/provisioning_revoke_deleted_test.go new file mode 100644 index 00000000..e0d1ad44 --- /dev/null +++ b/pkg/bsql/provisioning_revoke_deleted_test.go @@ -0,0 +1,279 @@ +package bsql + +import ( + "database/sql" + "testing" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + sdkGrant "github.com/conductorone/baton-sdk/pkg/types/grant" + "github.com/conductorone/baton-sql/pkg/bcel" + "github.com/conductorone/baton-sql/pkg/database" + "github.com/stretchr/testify/require" + _ "modernc.org/sqlite" +) + +// revokeDeletesUserQueries removes the role membership, then deletes the user +// row when they have no remaining roles. +var revokeDeletesUserQueries = []string{ + `DELETE FROM user_roles WHERE user_id = ? AND role = ?`, + `DELETE FROM users WHERE id = ? AND NOT EXISTS (SELECT 1 FROM user_roles WHERE user_id = ?)`, +} + +func principalExistsCheck() *PrincipalDeletedCheck { + return &PrincipalDeletedCheck{Query: `SELECT 1 FROM users WHERE id = ?`} +} + +func newRevokeProvisioningTestSyncer(t *testing.T) (*SQLSyncer, *sql.DB) { + t.Helper() + + db, err := sql.Open("sqlite", ":memory:") + require.NoError(t, err) + db.SetMaxOpenConns(1) + t.Cleanup(func() { + require.NoError(t, db.Close()) + }) + + _, err = db.ExecContext(t.Context(), `CREATE TABLE users (id TEXT PRIMARY KEY)`) + require.NoError(t, err) + _, err = db.ExecContext(t.Context(), `CREATE TABLE user_roles (user_id TEXT, role TEXT)`) + require.NoError(t, err) + + env, err := bcel.NewEnv(t.Context()) + require.NoError(t, err) + + return &SQLSyncer{ + db: db, + dbs: map[string]*sql.DB{"primary": db}, + dbNames: []string{"primary"}, + primaryDBName: "primary", + currentDBName: "primary", + dbEngine: database.SQLite, + env: env, + }, db +} + +func seedUserWithRoles(t *testing.T, db *sql.DB, userID string, roles ...string) { + t.Helper() + _, err := db.ExecContext(t.Context(), `INSERT INTO users (id) VALUES (?)`, userID) + require.NoError(t, err) + for _, r := range roles { + _, err := db.ExecContext(t.Context(), `INSERT INTO user_roles (user_id, role) VALUES (?, ?)`, userID, r) + require.NoError(t, err) + } +} + +func countRows(t *testing.T, db *sql.DB, query string, args ...any) int { + t.Helper() + var count int + require.NoError(t, db.QueryRowContext(t.Context(), query, args...).Scan(&count)) + return count +} + +func TestRunRevokeProvisioning_LastRoleDeletesPrincipal(t *testing.T) { + s, db := newRevokeProvisioningTestSyncer(t) + seedUserWithRoles(t, db, "user-1", "admin") + + deleted, err := s.RunRevokeProvisioning( + t.Context(), + revokeDeletesUserQueries, + nil, + principalExistsCheck(), + map[string]any{"principal_id": "user-1", "role": "admin"}, + true, + ) + require.NoError(t, err) + require.True(t, deleted) + + require.Equal(t, 0, countRows(t, db, `SELECT COUNT(*) FROM users WHERE id = ?`, "user-1")) + require.Equal(t, 0, countRows(t, db, `SELECT COUNT(*) FROM user_roles WHERE user_id = ?`, "user-1")) +} + +func TestRunRevokeProvisioning_KeepsPrincipalWhenOtherRolesRemain(t *testing.T) { + s, db := newRevokeProvisioningTestSyncer(t) + seedUserWithRoles(t, db, "user-1", "admin", "viewer") + + deleted, err := s.RunRevokeProvisioning( + t.Context(), + revokeDeletesUserQueries, + nil, + principalExistsCheck(), + map[string]any{"principal_id": "user-1", "role": "admin"}, + true, + ) + require.NoError(t, err) + require.False(t, deleted) + + require.Equal(t, 1, countRows(t, db, `SELECT COUNT(*) FROM users WHERE id = ?`, "user-1")) + // only the viewer role remains + require.Equal(t, 1, countRows(t, db, `SELECT COUNT(*) FROM user_roles WHERE user_id = ?`, "user-1")) + require.Equal(t, 1, countRows(t, db, `SELECT COUNT(*) FROM user_roles WHERE user_id = ? AND role = ?`, "user-1", "viewer")) +} + +func TestRunRevokeProvisioning_AllZeroRowsWithDeletedPrincipal(t *testing.T) { + s, _ := newRevokeProvisioningTestSyncer(t) + // nothing seeded: the user and role are already gone + + deleted, err := s.RunRevokeProvisioning( + t.Context(), + revokeDeletesUserQueries, + nil, + principalExistsCheck(), + map[string]any{"principal_id": "user-1", "role": "admin"}, + true, + ) + require.ErrorIs(t, err, ErrQueryAffectedZeroRows) + require.True(t, deleted) +} + +func TestRunRevokeProvisioning_AllZeroRowsWithSurvivingPrincipal(t *testing.T) { + s, db := newRevokeProvisioningTestSyncer(t) + // user still has another role, and never had the role being revoked + seedUserWithRoles(t, db, "user-1", "viewer") + + deleted, err := s.RunRevokeProvisioning( + t.Context(), + revokeDeletesUserQueries, + nil, + principalExistsCheck(), + map[string]any{"principal_id": "user-1", "role": "admin"}, + true, + ) + require.ErrorIs(t, err, ErrQueryAffectedZeroRows) + require.False(t, deleted) + + require.Equal(t, 1, countRows(t, db, `SELECT COUNT(*) FROM users WHERE id = ?`, "user-1")) +} + +func TestRunRevokeProvisioning_NoDeletedCheckBehavesLikeBefore(t *testing.T) { + s, db := newRevokeProvisioningTestSyncer(t) + seedUserWithRoles(t, db, "user-1", "admin") + + // happy path: revoke a held role, no check configured + deleted, err := s.RunRevokeProvisioning( + t.Context(), + []string{`DELETE FROM user_roles WHERE user_id = ? AND role = ?`}, + nil, + nil, + map[string]any{"principal_id": "user-1", "role": "admin"}, + true, + ) + require.NoError(t, err) + require.False(t, deleted) + require.Equal(t, 0, countRows(t, db, `SELECT COUNT(*) FROM user_roles WHERE user_id = ?`, "user-1")) + + // zero-row path still returns ErrQueryAffectedZeroRows with deleted=false + deleted, err = s.RunRevokeProvisioning( + t.Context(), + []string{`DELETE FROM user_roles WHERE user_id = ? AND role = ?`}, + nil, + nil, + map[string]any{"principal_id": "user-1", "role": "admin"}, + true, + ) + require.ErrorIs(t, err, ErrQueryAffectedZeroRows) + require.False(t, deleted) +} + +func TestRunRevokeProvisioning_ProbeErrorRollsBack(t *testing.T) { + s, db := newRevokeProvisioningTestSyncer(t) + seedUserWithRoles(t, db, "user-1", "admin") + + deleted, err := s.RunRevokeProvisioning( + t.Context(), + []string{`DELETE FROM user_roles WHERE user_id = ? AND role = ?`}, + nil, + &PrincipalDeletedCheck{Query: `SELECT 1 FROM nonexistent_table WHERE id = ?`}, + map[string]any{"principal_id": "user-1", "role": "admin"}, + true, + ) + require.Error(t, err) + require.False(t, deleted) + + // the revoke DELETE was rolled back, so the role is still present + require.Equal(t, 1, countRows(t, db, `SELECT COUNT(*) FROM user_roles WHERE user_id = ? AND role = ?`, "user-1", "admin")) +} + +func withRevokeDeletesUserConfig(s *SQLSyncer) { + s.config = ResourceType{ + StaticEntitlements: []*EntitlementMapping{ + { + Id: "member", + Provisioning: &EntitlementProvisioning{ + Vars: map[string]string{ + "principal_id": "principal.ID", + "role": "resource.ID", + }, + Revoke: &EntitlementProvisioningQueries{ + Queries: revokeDeletesUserQueries, + PrincipalDeletedCheck: &PrincipalDeletedCheck{Query: `SELECT 1 FROM users WHERE id = ?`}, + }, + }, + }, + }, + } +} + +func revokeGrantFor(userID, role string) *v2.Grant { + roleResource := &v2.Resource{Id: &v2.ResourceId{ResourceType: "role", Resource: role}} + principal := &v2.Resource{Id: &v2.ResourceId{ResourceType: "user", Resource: userID}} + return sdkGrant.NewGrant(roleResource, "member", principal) +} + +func TestRevoke_ReportsResourceDeletedOnLastRole(t *testing.T) { + s, db := newRevokeProvisioningTestSyncer(t) + withRevokeDeletesUserConfig(s) + seedUserWithRoles(t, db, "user-1", "admin") + + annos, err := s.Revoke(t.Context(), revokeGrantFor("user-1", "admin")) + require.NoError(t, err) + + resourceDeleted := &v2.ResourceDeleted{} + ok, err := annos.Pick(resourceDeleted) + require.NoError(t, err) + require.True(t, ok) + require.Equal(t, "user", resourceDeleted.GetResourceId().GetResourceType()) + require.Equal(t, "user-1", resourceDeleted.GetResourceId().GetResource()) + + // not an "already revoked" case + alreadyRevoked := &v2.GrantAlreadyRevoked{} + ok, err = annos.Pick(alreadyRevoked) + require.NoError(t, err) + require.False(t, ok) + + require.Equal(t, 0, countRows(t, db, `SELECT COUNT(*) FROM users WHERE id = ?`, "user-1")) +} + +func TestRevoke_ReportsResourceDeletedAndAlreadyRevokedOnRetry(t *testing.T) { + s, _ := newRevokeProvisioningTestSyncer(t) + withRevokeDeletesUserConfig(s) + // nothing seeded: simulates a retry after the user was already deleted + + annos, err := s.Revoke(t.Context(), revokeGrantFor("user-1", "admin")) + require.NoError(t, err) + + resourceDeleted := &v2.ResourceDeleted{} + ok, err := annos.Pick(resourceDeleted) + require.NoError(t, err) + require.True(t, ok) + + alreadyRevoked := &v2.GrantAlreadyRevoked{} + ok, err = annos.Pick(alreadyRevoked) + require.NoError(t, err) + require.True(t, ok) +} + +func TestRevoke_NoDeletedCheckReturnsNoResourceDeleted(t *testing.T) { + s, db := newRevokeProvisioningTestSyncer(t) + withRevokeDeletesUserConfig(s) + // strip the deleted check to confirm the annotation is absent without it + s.config.StaticEntitlements[0].Provisioning.Revoke.PrincipalDeletedCheck = nil + seedUserWithRoles(t, db, "user-1", "admin") + + annos, err := s.Revoke(t.Context(), revokeGrantFor("user-1", "admin")) + require.NoError(t, err) + + resourceDeleted := &v2.ResourceDeleted{} + ok, err := annos.Pick(resourceDeleted) + require.NoError(t, err) + require.False(t, ok) +} diff --git a/pkg/bsql/query.go b/pkg/bsql/query.go index c13a0c53..b9185edf 100644 --- a/pkg/bsql/query.go +++ b/pkg/bsql/query.go @@ -441,6 +441,132 @@ func (s *SQLSyncer) RunProvisioningQueries( return nil } +// RunRevokeProvisioning runs revoke queries like RunProvisioningQueries, and +// additionally runs an optional principal-deleted probe on the same executor +// after the revoke queries. The probe detects the case where the revoke deleted +// the principal itself downstream (e.g. removing a user's last role deletes the +// user row). It returns principalDeleted=true when the probe finds no rows. +// +// When every revoke query affects zero rows the function still runs the probe +// and commits, then returns ErrQueryAffectedZeroRows so the caller can report +// GrantAlreadyRevoked — combined with ResourceDeleted when the principal is +// also gone, so retried revokes still surface the deletion. +func (s *SQLSyncer) RunRevokeProvisioning( + ctx context.Context, + queries, + validationQueries []string, + deletedCheck *PrincipalDeletedCheck, + vars map[string]any, + useTx bool, +) (bool, error) { + l := ctxzap.Extract(ctx).With( + zap.Bool("use_tx", useTx), + ) + + ctx = ctxzap.ToContext(ctx, l) + + target, err := s.resolveProvisioningDB(vars) + if err != nil { + return false, err + } + + var committed bool + var executor executor = target + + if useTx { + tx, err := target.BeginTx(ctx, nil) + if err != nil { + return false, err + } + executor = tx + + defer func() { + if !committed { + if err := tx.Rollback(); err != nil { + l.Error("failed to rollback revoke provisioning queries", zap.Error(err)) + } + } + }() + } + + // Tolerate the all-zero-rows case (already revoked) so we can still run the + // deleted-check probe and report a deletion on retried revokes. Any other + // error aborts before the probe and rolls the transaction back. + var allZero bool + err = s.RunProvisioningQueriesWithExecutor(ctx, queries, validationQueries, vars, executor) + if err != nil { + if !errors.Is(err, ErrQueryAffectedZeroRows) { + return false, err + } + allZero = true + } + + var principalDeleted bool + if deletedCheck != nil { + principalDeleted, err = s.runPrincipalDeletedCheck(ctx, executor, deletedCheck, vars) + if err != nil { + return false, err + } + } + + if useTx { + tx, ok := executor.(*sql.Tx) + if !ok { + return false, errors.New("transactional executor required") + } + if err := tx.Commit(); err != nil { + return false, err + } + committed = true + } + + if allZero { + return principalDeleted, ErrQueryAffectedZeroRows + } + return principalDeleted, nil +} + +// runPrincipalDeletedCheck executes the deleted-check probe on the given +// executor. No rows returned means the principal no longer exists. +func (s *SQLSyncer) runPrincipalDeletedCheck( + ctx context.Context, + executor executor, + deletedCheck *PrincipalDeletedCheck, + vars map[string]any, +) (bool, error) { + l := ctxzap.Extract(ctx) + + q, qArgs, err := s.prepareProvisioningQuery(deletedCheck.Query, vars) + if err != nil { + return false, fmt.Errorf("failed to prepare principal deleted check query: %w", err) + } + + l.Debug( + "running principal deleted check query", + zap.String("query", q), + zap.Any("args", qArgs), + ) + + rows, err := executor.QueryContext(ctx, q, qArgs...) + if err != nil { + return false, fmt.Errorf("failed to execute principal deleted check query: %w", err) + } + + exists := rows.Next() + + if err := rows.Err(); err != nil { + _ = rows.Close() + return false, fmt.Errorf("failed to read principal deleted check result: %w", err) + } + + if err := rows.Close(); err != nil { + return false, fmt.Errorf("failed to close principal deleted check result: %w", err) + } + + // No rows => the principal is gone => it was deleted by the revoke. + return !exists, nil +} + func (s *SQLSyncer) RunProvisioningQueriesWithExecutor( ctx context.Context, queries, diff --git a/pkg/bsql/validate.go b/pkg/bsql/validate.go index ce7f4c30..445b7bf1 100644 --- a/pkg/bsql/validate.go +++ b/pkg/bsql/validate.go @@ -63,13 +63,8 @@ func (l *EntitlementsQuery) staticValidate(ctx context.Context, s *SQLSyncer) er return err } - if mapping.Provisioning.Revoke != nil { - for _, query := range mapping.Provisioning.Revoke.Queries { - err := validateVarsInQuery(s, query, mapping.Provisioning.Vars) - if err != nil { - return err - } - } + if err := validateRevokeProvisioningQueries(s, mapping.Provisioning.Revoke, mapping.Provisioning.Vars); err != nil { + return err } } @@ -85,13 +80,8 @@ func (l *EntitlementMapping) staticValidate(ctx context.Context, s *SQLSyncer) e return err } - if l.Provisioning.Revoke != nil { - for _, query := range l.Provisioning.Revoke.Queries { - err := validateVarsInQuery(s, query, l.Provisioning.Vars) - if err != nil { - return err - } - } + if err := validateRevokeProvisioningQueries(s, l.Provisioning.Revoke, l.Provisioning.Vars); err != nil { + return err } return nil @@ -117,6 +107,29 @@ func validateGrantProvisioningQueries(s *SQLSyncer, grant *GrantEntitlementProvi return nil } +func validateRevokeProvisioningQueries(s *SQLSyncer, revoke *EntitlementProvisioningQueries, vars map[string]string) error { + if revoke == nil { + return nil + } + + for _, query := range revoke.Queries { + if err := validateVarsInQuery(s, query, vars); err != nil { + return err + } + } + + if revoke.PrincipalDeletedCheck != nil { + if revoke.PrincipalDeletedCheck.Query == "" { + return errors.New("principal_deleted_check requires a non-empty query") + } + if err := validateVarsInQuery(s, revoke.PrincipalDeletedCheck.Query, vars); err != nil { + return err + } + } + + return nil +} + func (l *GrantsQuery) staticValidate(ctx context.Context, s *SQLSyncer) error { if err := validateScope(l.Scope); err != nil { return err From 1a0cb5e954ae3765d8c34a5a43045b26af012098 Mon Sep 17 00:00:00 2001 From: Alan Lee Date: Wed, 15 Jul 2026 16:33:09 -0700 Subject: [PATCH 2/3] nest PrincipalDeletedCheck in a new revoke-only extension type --- pkg/bsql/config.go | 22 ++++++++++++-------- pkg/bsql/provisioning_revoke_deleted_test.go | 6 ++++-- pkg/bsql/validate.go | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/bsql/config.go b/pkg/bsql/config.go index 056470c8..fd943d84 100644 --- a/pkg/bsql/config.go +++ b/pkg/bsql/config.go @@ -411,7 +411,7 @@ type EntitlementProvisioning struct { Grant *GrantEntitlementProvisioningQueries `yaml:"grant,omitempty" json:"grant,omitempty"` // Revoke defines the SQL queries and settings for revoking this entitlement. - Revoke *EntitlementProvisioningQueries `yaml:"revoke,omitempty" json:"revoke,omitempty"` + Revoke *RevokeEntitlementProvisioningQueries `yaml:"revoke,omitempty" json:"revoke,omitempty"` // Vars provides variables that can be used within provisioning SQL queries. Vars map[string]string `yaml:"vars,omitempty" json:"vars,omitempty"` @@ -427,14 +427,6 @@ type EntitlementProvisioningQueries struct { // Queries is a list of SQL statements to execute for the provisioning operation. Queries []string `yaml:"queries,omitempty" json:"queries,omitempty"` - - // PrincipalDeletedCheck detects that a revoke deleted the principal itself - // (e.g. the downstream app deletes the user when their last role is - // removed). It is only consulted on the revoke path; the grant path ignores - // it. When configured and the probe finds the principal gone, the revoke - // response carries a ResourceDeleted annotation so ConductorOne can mark the - // account deleted without waiting for the next full sync. - PrincipalDeletedCheck *PrincipalDeletedCheck `yaml:"principal_deleted_check,omitempty" json:"principal_deleted_check,omitempty"` } // PrincipalDeletedCheck configures a probe query that detects whether the @@ -472,6 +464,18 @@ type GrantEntitlementProvisioningQueries struct { GrantReplace *GrantReplaceProvisioningQueries `yaml:"grant_replace,omitempty" json:"grant_replace,omitempty"` } +// RevokeEntitlementProvisioningQueries extends the shared provisioning query fields with revoke-only behavior +type RevokeEntitlementProvisioningQueries struct { + EntitlementProvisioningQueries `yaml:",inline" json:",inline"` + + // PrincipalDeletedCheck detects that a revoke deleted the principal itself + // (e.g. the downstream app deletes the user when their last role is + // removed). When configured and the probe finds the principal gone, the + // revoke response carries a ResourceDeleted annotation so ConductorOne can + // mark the account deleted without waiting for the next full sync. + PrincipalDeletedCheck *PrincipalDeletedCheck `yaml:"principal_deleted_check,omitempty" json:"principal_deleted_check,omitempty"` +} + // GrantsQuery defines the structure for querying existing entitlement grants. type GrantsQuery struct { // Vars provides variables that can be used within the grants query. diff --git a/pkg/bsql/provisioning_revoke_deleted_test.go b/pkg/bsql/provisioning_revoke_deleted_test.go index e0d1ad44..84f7ecb5 100644 --- a/pkg/bsql/provisioning_revoke_deleted_test.go +++ b/pkg/bsql/provisioning_revoke_deleted_test.go @@ -203,8 +203,10 @@ func withRevokeDeletesUserConfig(s *SQLSyncer) { "principal_id": "principal.ID", "role": "resource.ID", }, - Revoke: &EntitlementProvisioningQueries{ - Queries: revokeDeletesUserQueries, + Revoke: &RevokeEntitlementProvisioningQueries{ + EntitlementProvisioningQueries: EntitlementProvisioningQueries{ + Queries: revokeDeletesUserQueries, + }, PrincipalDeletedCheck: &PrincipalDeletedCheck{Query: `SELECT 1 FROM users WHERE id = ?`}, }, }, diff --git a/pkg/bsql/validate.go b/pkg/bsql/validate.go index 445b7bf1..e5f89cf0 100644 --- a/pkg/bsql/validate.go +++ b/pkg/bsql/validate.go @@ -107,7 +107,7 @@ func validateGrantProvisioningQueries(s *SQLSyncer, grant *GrantEntitlementProvi return nil } -func validateRevokeProvisioningQueries(s *SQLSyncer, revoke *EntitlementProvisioningQueries, vars map[string]string) error { +func validateRevokeProvisioningQueries(s *SQLSyncer, revoke *RevokeEntitlementProvisioningQueries, vars map[string]string) error { if revoke == nil { return nil } From 97a77d89a0766b7e8b82193d686e92f9aac0a6f5 Mon Sep 17 00:00:00 2001 From: Alan Lee Date: Wed, 15 Jul 2026 16:44:48 -0700 Subject: [PATCH 3/3] nest principal_deleted_check under revoke_options 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 --- examples/mssql-revoke-deletes-user.yml | 19 ++++++++++--------- pkg/bsql/config.go | 16 +++++++++------- pkg/bsql/config_test.go | 5 +++-- pkg/bsql/provisioning.go | 6 +++++- pkg/bsql/provisioning_revoke_deleted_test.go | 6 ++++-- pkg/bsql/validate.go | 6 +++--- 6 files changed, 34 insertions(+), 24 deletions(-) diff --git a/examples/mssql-revoke-deletes-user.yml b/examples/mssql-revoke-deletes-user.yml index aaa71821..82a3b16a 100644 --- a/examples/mssql-revoke-deletes-user.yml +++ b/examples/mssql-revoke-deletes-user.yml @@ -2,15 +2,15 @@ # Example: reporting a side-effect user deletion from a revoke. # # Some applications delete the user record when their last role is revoked. -# When that happens ConductorOne would otherwise not learn the account is gone +# When that happens C1 would otherwise not learn the account is gone # until the next full sync, which blocks an immediate re-grant. Configuring -# `principal_deleted_check` under an entitlement's `revoke` makes the connector -# probe for the principal after the revoke queries run (on the same -# transaction). If the probe returns no rows the connector attaches a -# ResourceDeleted annotation to the revoke response so ConductorOne marks the +# `revoke_options.principal_deleted_check` under an entitlement's `revoke` +# makes the connector probe for the principal after the revoke queries run (on +# the same transaction). If the probe returns no rows the connector attaches a +# ResourceDeleted annotation to the revoke response so C1 marks the # account deleted right away. app_name: SQL Server Revoke-Deletes-User Example -app_description: Demonstrates principal_deleted_check for apps that delete a user when their last role is revoked +app_description: Demonstrates revoke_options.principal_deleted_check for apps that delete a user when their last role is revoked connect: dsn: "sqlserver://${DB_HOST}:${DB_PORT}?database=${DB_DATABASE}" @@ -142,9 +142,10 @@ resource_types: # After the revoke queries run, probe for the principal. No rows # means the app deleted the user, so the connector reports a # ResourceDeleted annotation on the revoke response. - principal_deleted_check: - query: | - SELECT 1 FROM Users WHERE Username = ? + revoke_options: + principal_deleted_check: + query: | + SELECT 1 FROM Users WHERE Username = ? grants: - query: | diff --git a/pkg/bsql/config.go b/pkg/bsql/config.go index fd943d84..33377c17 100644 --- a/pkg/bsql/config.go +++ b/pkg/bsql/config.go @@ -429,6 +429,12 @@ type EntitlementProvisioningQueries struct { Queries []string `yaml:"queries,omitempty" json:"queries,omitempty"` } +// RevokeOptions holds optional revoke-only behavior beyond the shared provisioning queries. +type RevokeOptions struct { + // PrincipalDeletedCheck detects that a revoke deleted the principal itself (e.g. user is deleted when their last role is removed). + PrincipalDeletedCheck *PrincipalDeletedCheck `yaml:"principal_deleted_check,omitempty" json:"principal_deleted_check,omitempty"` +} + // PrincipalDeletedCheck configures a probe query that detects whether the // principal was deleted as a side effect of a revoke. type PrincipalDeletedCheck struct { @@ -464,16 +470,12 @@ type GrantEntitlementProvisioningQueries struct { GrantReplace *GrantReplaceProvisioningQueries `yaml:"grant_replace,omitempty" json:"grant_replace,omitempty"` } -// RevokeEntitlementProvisioningQueries extends the shared provisioning query fields with revoke-only behavior +// RevokeEntitlementProvisioningQueries extends the shared provisioning query fields with revoke-only behavior. type RevokeEntitlementProvisioningQueries struct { EntitlementProvisioningQueries `yaml:",inline" json:",inline"` - // PrincipalDeletedCheck detects that a revoke deleted the principal itself - // (e.g. the downstream app deletes the user when their last role is - // removed). When configured and the probe finds the principal gone, the - // revoke response carries a ResourceDeleted annotation so ConductorOne can - // mark the account deleted without waiting for the next full sync. - PrincipalDeletedCheck *PrincipalDeletedCheck `yaml:"principal_deleted_check,omitempty" json:"principal_deleted_check,omitempty"` + // RevokeOptions groups optional revoke-only settings such as principal_deleted_check. + RevokeOptions *RevokeOptions `yaml:"revoke_options,omitempty" json:"revoke_options,omitempty"` } // GrantsQuery defines the structure for querying existing entitlement grants. diff --git a/pkg/bsql/config_test.go b/pkg/bsql/config_test.go index db812591..27fbd12e 100644 --- a/pkg/bsql/config_test.go +++ b/pkg/bsql/config_test.go @@ -254,10 +254,11 @@ resource_types: validate: func(t *testing.T, c *Config) { revoke := c.ResourceTypes["role"].StaticEntitlements[0].Provisioning.Revoke require.NotNil(t, revoke) - require.NotNil(t, revoke.PrincipalDeletedCheck) + require.NotNil(t, revoke.RevokeOptions) + require.NotNil(t, revoke.RevokeOptions.PrincipalDeletedCheck) require.Equal(t, normalizeQueryString("SELECT 1 FROM Users WHERE Username = ?"), - normalizeQueryString(revoke.PrincipalDeletedCheck.Query), + normalizeQueryString(revoke.RevokeOptions.PrincipalDeletedCheck.Query), ) }, }, diff --git a/pkg/bsql/provisioning.go b/pkg/bsql/provisioning.go index 7642ae1d..40460010 100644 --- a/pkg/bsql/provisioning.go +++ b/pkg/bsql/provisioning.go @@ -135,11 +135,15 @@ func (s *SQLSyncer) Revoke(ctx context.Context, grant *v2.Grant) (annotations.An } useTx := !provisioningConfig.Revoke.NoTransaction + var deletedCheck *PrincipalDeletedCheck + if provisioningConfig.Revoke.RevokeOptions != nil { + deletedCheck = provisioningConfig.Revoke.RevokeOptions.PrincipalDeletedCheck + } principalDeleted, err := s.RunRevokeProvisioning( ctx, provisioningConfig.Revoke.Queries, provisioningConfig.Revoke.ValidationQueries, - provisioningConfig.Revoke.PrincipalDeletedCheck, + deletedCheck, provisioningVars, useTx, ) diff --git a/pkg/bsql/provisioning_revoke_deleted_test.go b/pkg/bsql/provisioning_revoke_deleted_test.go index 84f7ecb5..a0ccd4e3 100644 --- a/pkg/bsql/provisioning_revoke_deleted_test.go +++ b/pkg/bsql/provisioning_revoke_deleted_test.go @@ -207,7 +207,9 @@ func withRevokeDeletesUserConfig(s *SQLSyncer) { EntitlementProvisioningQueries: EntitlementProvisioningQueries{ Queries: revokeDeletesUserQueries, }, - PrincipalDeletedCheck: &PrincipalDeletedCheck{Query: `SELECT 1 FROM users WHERE id = ?`}, + RevokeOptions: &RevokeOptions{ + PrincipalDeletedCheck: &PrincipalDeletedCheck{Query: `SELECT 1 FROM users WHERE id = ?`}, + }, }, }, }, @@ -268,7 +270,7 @@ func TestRevoke_NoDeletedCheckReturnsNoResourceDeleted(t *testing.T) { s, db := newRevokeProvisioningTestSyncer(t) withRevokeDeletesUserConfig(s) // strip the deleted check to confirm the annotation is absent without it - s.config.StaticEntitlements[0].Provisioning.Revoke.PrincipalDeletedCheck = nil + s.config.StaticEntitlements[0].Provisioning.Revoke.RevokeOptions = nil seedUserWithRoles(t, db, "user-1", "admin") annos, err := s.Revoke(t.Context(), revokeGrantFor("user-1", "admin")) diff --git a/pkg/bsql/validate.go b/pkg/bsql/validate.go index e5f89cf0..c4981f86 100644 --- a/pkg/bsql/validate.go +++ b/pkg/bsql/validate.go @@ -118,11 +118,11 @@ func validateRevokeProvisioningQueries(s *SQLSyncer, revoke *RevokeEntitlementPr } } - if revoke.PrincipalDeletedCheck != nil { - if revoke.PrincipalDeletedCheck.Query == "" { + if revoke.RevokeOptions != nil && revoke.RevokeOptions.PrincipalDeletedCheck != nil { + if revoke.RevokeOptions.PrincipalDeletedCheck.Query == "" { return errors.New("principal_deleted_check requires a non-empty query") } - if err := validateVarsInQuery(s, revoke.PrincipalDeletedCheck.Query, vars); err != nil { + if err := validateVarsInQuery(s, revoke.RevokeOptions.PrincipalDeletedCheck.Query, vars); err != nil { return err } }