feat: add billing accounts sync as entitlements

[c1-dev-bot]

Summary

Add support for syncing Coupa billing accounts as entitlements in ConductorOne, with full grant/revoke provisioning support.

• Syncs Coupa accounts from the Accounts API (/api/accounts) as a new account resource type
• Each active account becomes an entitlement that can be granted to users
• User-to-account grants are tracked via the user's default_account field
• Grant/Revoke provisioning updates the user's default account via the REST Users API
• Added core.accounting.read and core.accounting.write OAuth scopes

Files Changed

File	Description
pkg/connector/resource_types.go	New accountResourceType definition
pkg/connector/accounts.go	Account builder with List, Entitlements, Grants, Grant, Revoke
pkg/connector/client/accounts.go	SetUserAccount REST API client method
pkg/connector/client/query.go	GraphQL queries for accounts and user-account grants
pkg/connector/client/models.go	Account-related data models and response types
pkg/connector/client/path.go	REST API path for setting user default account
pkg/connector/client/auth.go	Added core.accounting.read/write OAuth scopes
pkg/connector/connector.go	Registered accountBuilder in ResourceSyncers
baton_capabilities.json	Added account resource type with sync and provision capabilities

Implementation Notes

• The implementation uses the user's default_account field for the user-to-account relationship. This field name is based on the standard Coupa Users API and should be verified against the customer's Coupa instance.
• The Coupa Accounts API query syntax and GraphQL field names (e.g., accounts, defaultAccount) follow the patterns established by the existing connector for groups and roles. These may need adjustment based on testing.
• Only active accounts are synced as entitlements (inactive accounts are skipped during pagination).
• The Coupa API may also support user-account relationships via Account Groups (/api/account_groups), which could be an alternative or additional mechanism. The current implementation focuses on direct account assignment.
• The connector could not be compiled locally due to the Go 1.25.2 toolchain requirement. CI will validate compilation.

Fixes: CXH-1352

Test Plan

• Verify the connector compiles successfully with Go 1.25.2
• Test account syncing against a Coupa sandbox instance
• Verify accounts appear as entitlements in ConductorOne
• Verify user-to-account grants are correctly synced
• Test grant provisioning (assign account to user)
• Test revoke provisioning (remove account from user)
• Verify OAuth scopes are correctly requested
• Confirm the GraphQL query field names match the Coupa API schema
• Validate that the default_account field is the correct user-account relationship field for the customer's use case
• Test pagination with large numbers of accounts

---

Automated PR Notice

This PR was automatically created by c1-dev-bot as a potential implementation.

This code requires:

• Human review of the implementation approach
• Manual testing to verify correctness
• Approval from the appropriate team before merging

[linear]

CXH-1352 Coupa Connector: Add Billing Accounts sync as Entitlements

[github-actions]

Claude finished @c1-dev-bot[bot]'s task in 3m 2s —— View job

---

PR Review: feat: add billing accounts sync as entitlements

• Read all changed files and existing code for context
• Analyze code against CLAUDE.md patterns and best practices
• Check for bugs, security issues, and correctness
• Post comprehensive review

---

I've reviewed all 9 changed files against the existing connector patterns (roles, groups, licenses) and the CLAUDE.md guidelines. Overall the implementation is well-structured and follows the established patterns. Here are my findings:

---

Issues

1. Revoke doesn't verify the API response (Medium)

Grant() verifies the response after calling SetUserAccount (line 197-199), but Revoke() does not verify the account was actually cleared:

pkg/connector/accounts.go:240-245

_, _, err = o.client.SetUserAccount(ctx, userId, nil)
if err != nil {
    return nil, err
}

return nil, nil  // No verification that default_account was actually cleared

Compare with Grant() which checks:

if userResponse.DefaultAccount == nil || userResponse.DefaultAccount.ID != accountIdToSet {
    return nil, nil, errors.New("baton-coupa: account not set on user")
}

Recommendation: Add a post-revoke verification that userResponse.DefaultAccount == nil, consistent with how Grant verifies the result.

Fix this →

2. Grant silently replaces existing default account (Medium - Design)

Since default_account is a single-valued field, granting account B to a user who already has account A will replace A with B. The old grant for A will remain in ConductorOne until the next sync cycle removes it, creating a window of stale data.

pkg/connector/accounts.go:188-199

// Only checks if the SAME account is already set
if user.DefaultAccount != nil && user.DefaultAccount.ID == accountIdToSet {
    return []*v2.Grant{}, annotations.New(&v2.GrantAlreadyExists{}), nil
}
// Sets new account, silently replacing any existing one
userResponse, _, err := o.client.SetUserAccount(ctx, userId, &accountIdToSet)

Recommendation: Consider logging a warning when replacing an existing default account so operators have visibility into the replacement:

if user.DefaultAccount != nil && user.DefaultAccount.ID != accountIdToSet {
    logger.Warn("baton-coupa: replacing existing default account",
        zap.Int("old_account_id", user.DefaultAccount.ID),
        zap.Int("new_account_id", accountIdToSet),
    )
}

3. Revoke sets default_account to nil (Low - Operational Risk)

pkg/connector/accounts.go:240: SetUserAccount(ctx, userId, nil) clears the default account entirely. Depending on the Coupa instance configuration, removing a user's default billing account could break downstream workflows that require one (e.g., requisition creation). This should be called out in documentation or release notes.

4. Missing baton-coupa: error prefix on strconv.Atoi failures (Low)

Per CLAUDE.md, errors should include the connector prefix. Several strconv.Atoi calls return bare errors:

pkg/connector/accounts.go:173-179

accountIdToSet, err := strconv.Atoi(ent.Resource.Id.Resource)
if err != nil {
    return nil, nil, err  // No "baton-coupa:" prefix
}

This matches the existing pattern in roles.go and groups.go, so it's pre-existing, but worth noting for consistency with the CLAUDE.md guideline.

---

Observations (Not blocking)

Pagination with inactive accounts is correct

The List() method (line 77) tracks lastId for all accounts including inactive ones, which correctly advances the pagination cursor through the full result set. If a page contains only inactive accounts, the SDK will receive 0 resources with a non-empty page token and continue to the next page. This is the right behavior.

GraphQL/REST field naming conventions are correct

• GraphQL queries use camelCase (defaultAccount) - correct for Coupa GraphQL
• GraphQL filters use snake_case (default_account[id]) - correct for Coupa query syntax
• REST request/response uses kebab-case (default-account) - correct for Coupa REST API
• REST fields parameter uses snake_case (default_account) - correct for Coupa field selection

OAuth scopes are properly added

core.accounting.read added to ScopesReadOnly and core.accounting.write added to ScopesReadWrite in client/auth.go. This follows the existing pattern for other scope pairs.

Code structure follows established patterns

The accountBuilder follows the same structure as roleBuilder and groupBuilder:

• List → GraphQL query with cursor pagination
• Entitlements → single assignment entitlement
• Grants → GraphQL query for user-to-account relationships
• Grant/Revoke → REST PUT to update user
• getUserAccounts helper → same pattern as getUserRoles/getUserGroupsResponse

Capabilities JSON is correct

baton_capabilities.json correctly declares both CAPABILITY_SYNC and CAPABILITY_PROVISION for the account resource type.

---

Summary

The implementation is solid and follows the established connector patterns well. The main items to address are:

Priority	Issue	Action
Medium	Post-revoke verification missing	Add response check after clearing default account
Medium	Silent account replacement	Add warning log when replacing existing default account
Low	Nil default_account risk	Document in release notes
Low	Missing error prefixes	Pre-existing pattern, optional fix

The PR description's caveats about field name verification (default_account, GraphQL field names) are appropriate - these should be validated against a live Coupa sandbox before merging.

[github-actions]

PR Review: feat: add billing accounts sync as entitlements

• Read all changed files and existing connector patterns
• Analyze against CLAUDE.md guidelines and existing patterns
• Check for bugs, security issues, and correctness
• Check the latest commit for issues flagged in prior review
• Post comprehensive review

View job run

[github-actions]

Bug (High): ScopesAccounting is defined but never used — account sync will fail at runtime

The fix in 7f182b9 correctly moved accounting scopes out of the default token requests to avoid breaking existing customers. However, ScopesAccounting is now dead code — it's never referenced in client.go or anywhere else. Neither the read-only token (used for GraphQL queries: AccountsQuery, AccountGrantQuery, GetUserAccounts) nor the read-write token (used for REST: SetUserAccount) will include the core.accounting.read or core.accounting.write scopes.

This means all account-related operations will likely fail with authorization errors at runtime.

A mechanism is needed to actually use these scopes. Some options:

1. Append to existing token requests — simplest but may break customers whose OAuth clients don't have these scopes authorized
2. CLI flag/config option (e.g., --enable-billing-accounts) — opt-in, customer controls when to enable
3. Separate token source for account operations — most flexible, allows graceful degradation

[github-actions]

Medium: Revoke doesn't verify the API response (flagged in prior review, still unaddressed)

Grant() verifies the response at line 197-199, but Revoke() fires and forgets. Should capture and verify the response:

Suggested change

	return nil, err
	}
	user, err := o.getUserAccounts(ctx, userId)
	if err != nil {
	return nil, err
	userResponse, _, err := o.client.SetUserAccount(ctx, userId, nil)
	if err != nil {
	return nil, err
	}
	if userResponse.DefaultAccount != nil {
	return nil, errors.New("baton-coupa: account not cleared on user")
	}
	return nil, nil

[github-actions]

Medium (Design): Grant silently replaces existing default account (flagged in prior review, still unaddressed)

Since default_account is single-valued, granting account B to a user who already has account A silently replaces A. The old grant for A will remain stale in ConductorOne until the next sync cycle.

Consider adding a warning log when replacing an existing account so operators have visibility:

if user.DefaultAccount != nil && user.DefaultAccount.ID != accountIdToSet {
    l := ctxzap.Extract(ctx)
    l.Warn("baton-coupa: replacing existing default account on user",
        zap.Int("user_id", userId),
        zap.Int("old_account_id", user.DefaultAccount.ID),
        zap.Int("new_account_id", accountIdToSet),
    )
}

[github-actions]

Claude finished @c1-dev-bot[bot]'s task in 2m 25s —— View job

---

PR Review: feat: add billing accounts sync as entitlements

• Read all changed files and existing connector patterns
• Analyze against CLAUDE.md guidelines and existing patterns
• Check for bugs, security issues, and correctness
• Check if prior review issues have been addressed in latest commits
• Post comprehensive review

---

I've reviewed all 9 changed files across 3 commits (8ac5b2f, 7f182b9, bc977cb), comparing against the existing role/group builder patterns and CLAUDE.md guidelines.

---

Commit-by-commit assessment

Commit	Description	Assessment
8ac5b2f	Initial billing accounts implementation	Solid structure, follows existing patterns
7f182b9	Move accounting scopes out of default token requests	Correct fix — prevents breaking existing customers
bc977cb	Graceful degradation on account query failures	Good approach for optional feature, but masks the scope wiring gap

---

Issues

1. ScopesAccounting is defined but never wired — account operations will fail at runtime (High)

Still unaddressed from prior review. This is the most critical issue in the PR.

ScopesAccounting is defined at pkg/connector/client/auth.go:33 but is never referenced anywhere in the codebase. The token sources created in client.go:68-83 use only ScopesReadOnly and ScopesReadWrite, neither of which includes core.accounting.read or core.accounting.write.

This means:

• Sync (List, Grants): GraphQL queries for accounts will fail with auth errors, but bc977cb masks this with warning logs and empty returns — the connector appears healthy while silently skipping all account data.
• Provisioning (Grant, Revoke): REST calls to SetUserAccount will fail hard with auth errors since these paths don't have graceful degradation.

The net effect is that this feature ships as dead code — it's wired up but can never succeed.

Recommendation: The scopes need to actually be requested. Some options:

1. CLI flag (e.g., --enable-billing-accounts) that appends ScopesAccounting to the token source scopes — opt-in, no risk to existing customers
2. Always append and rely on the graceful degradation in bc977cb if the OAuth client doesn't have the scopes authorized
3. Separate token source for account operations with its own scope set

Option 1 is the safest since it makes the feature explicitly opt-in and gives operators a clear signal when it's enabled.

Fix this →

2. Revoke doesn't verify the API response (Medium)

Still unaddressed from prior review. Grant() verifies the response at pkg/connector/accounts.go:210-212:

if userResponse.DefaultAccount == nil || userResponse.DefaultAccount.ID != accountIdToSet {
    return nil, nil, errors.New("baton-coupa: account not set on user")
}

But Revoke() at line 253 discards the response entirely:

_, _, err = o.client.SetUserAccount(ctx, userId, nil)

Recommendation: Capture and verify the response, consistent with Grant:

userResponse, _, err := o.client.SetUserAccount(ctx, userId, nil)
if err != nil {
    return nil, err
}
if userResponse.DefaultAccount != nil {
    return nil, errors.New("baton-coupa: account not cleared on user")
}

Fix this →

3. Grant silently replaces existing default account (Medium — Design)

Still unaddressed from prior review. Since default_account is single-valued, granting account B to a user who already has account A will silently replace A. The old grant for A remains stale in ConductorOne until the next sync cycle.

pkg/connector/accounts.go:201-205 — the code only checks if the same account is already set, then unconditionally overwrites any other existing account.

Recommendation: Add a warning log when replacing so operators have visibility:

if user.DefaultAccount != nil && user.DefaultAccount.ID != accountIdToSet {
    l := ctxzap.Extract(ctx)
    l.Warn("baton-coupa: replacing existing default account on user",
        zap.Int("user_id", userId),
        zap.Int("old_account_id", user.DefaultAccount.ID),
        zap.Int("new_account_id", accountIdToSet),
    )
}

Fix this →

---

Observations (not blocking)

Graceful degradation in bc977cb is good but has a subtle implication

The List() and Grants() methods now swallow errors and return empty results. This diverges from the roles/groups builders which propagate errors. This is a reasonable choice for an optional feature gated by extra scopes, but it means:

• Sync will appear to succeed even when accounts fail entirely — operators may not notice unless they check logs.
• Consider surfacing an annotation or metric so the sync status is visible beyond log output.

Pagination correctly handles inactive accounts

List() at line 85 tracks lastId for all accounts (including inactive), which correctly advances the cursor through the full result set even when pages contain only inactive accounts. The SDK will receive 0 resources with a non-empty page token and continue. This is correct.

Field naming conventions are correct

• GraphQL: camelCase (defaultAccount) — correct for Coupa GraphQL
• GraphQL filters: snake_case (default_account[id]) — correct for Coupa query syntax
• REST body: kebab-case (default-account) — correct for Coupa REST API
• REST fields param: snake_case (default_account) — correct for Coupa field selection

Code structure follows established patterns well

The accountBuilder mirrors the role/group builders: List → GraphQL cursor pagination, Entitlements → single assignment entitlement, Grants → GraphQL user query, Grant/Revoke → REST PUT, getUserAccounts helper → same shape as getUserRoles/getUserGroupsResponse.

Capabilities JSON is correct

baton_capabilities.json correctly declares both CAPABILITY_SYNC and CAPABILITY_PROVISION for the account resource type.

---

Summary

Priority	Issue	Status	Action Required
High	ScopesAccounting never wired — feature is dead code	Unaddressed	Wire scopes into token source (opt-in flag recommended)
Medium	Revoke doesn't verify API response	Unaddressed	Capture and check response
Medium	Grant silently replaces existing account	Unaddressed	Add warning log

The implementation structure is solid and follows established patterns, but the scope wiring gap (issue #1) means the feature cannot work at runtime. This must be resolved before the PR is functional.