feat: add billing accounts sync as entitlements

[c1-dev-bot]

Summary

• Adds support for syncing Coupa Billing Accounts as Entitlements in ConductorOne
• Implements full provisioning lifecycle: List, Entitlements, Grants, Grant, and Revoke
• Follows the established patterns used by roles and groups for consistent behavior

Changes

New Files

• pkg/connector/billing_accounts.go — billingAccountBuilder implementing the ResourceSyncer interface with all five methods (List, Entitlements, Grants, Grant, Revoke)
• pkg/connector/client/billing_accounts.go — SetUserAccounts REST client method for assigning/removing billing accounts via PUT /api/users/{id}

Modified Files

• pkg/connector/client/models.go — Added Account, AccountsQueryResponse, AccountGrantsQueryResponse, UserAccounts, UserAccountsResponse, and UserAccountsPutResponse structs
• pkg/connector/client/query.go — Added GraphQL query templates: getAccountsQuery, getAccountGrantListQuery, getUserAccountsQuery and their corresponding functions
• pkg/connector/client/path.go — Added setAccountPath for REST billing account assignment
• pkg/connector/client/auth.go — Added core.accounting.read and core.accounting.write OAuth scopes
• pkg/connector/resource_types.go — Added billingAccountResourceType definition
• pkg/connector/connector.go — Registered newBillingAccountBuilder in ResourceSyncers(), updated connector description
• baton_capabilities.json — Added billing_account resource type with CAPABILITY_SYNC and CAPABILITY_PROVISION

Important Notes

The exact GraphQL field names for billing account queries need validation against a live Coupa instance:

• The accounts resource name in GraphQL
• The account[id] filter for querying users by billing account
• The account field on the user object for current assignments
• The REST API field name (account) in the PUT request body

The implementation uses the same two-step revoke pattern (clear all, then re-set) used by roles and groups, as Coupa may have the same API constraint.

Test plan

• Validate GraphQL field names against a live Coupa instance
• Test billing account listing syncs correctly as resources
• Test entitlements are created for each billing account
• Test grants correctly map users to billing accounts
• Test granting a billing account to a user (provisioning)
• Test revoking a billing account from a user (deprovisioning)
• Test idempotency: granting an already-assigned account returns GrantAlreadyExists
• Test idempotency: revoking an unassigned account returns GrantAlreadyRevoked

Fixes: CXH-1353

---

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-1353 Coupa Connector: Add Billing Accounts sync as Entitlements

[github-actions]

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

---

PR Review: feat: add billing accounts sync as entitlements

• Read all changed files and understand the implementation
• Review against existing patterns (roles, groups builders)
• Check for bugs, security issues, and CLAUDE.md adherence
• Post comprehensive review

---

Overall Assessment

This is a well-structured PR that closely follows the established patterns from the roles and groups builders. The implementation is consistent and easy to follow. There are a few issues to address, ranging from a potential data model mismatch to minor improvements.

---

Issues

1. Singular vs Plural: account field may be wrong for user accounts (Medium)

The GraphQL query and response models use the singular account for the user's billing account assignments:

• getUserAccountsQuery (query.go:83): id account { id name code }
• UserAccounts (models.go:92): json:"account"
• UserAccountsPutResponse (models.go:101): json:"account"
• SetUserAccounts request body (billing_accounts.go:28): json:"account"

However, the code treats accounts as a collection (slice) — []Account. In Coupa, a user's account field may be a single object (the user's default account), not an array of accounts like roles (plural) or userGroups (plural).

This needs validation against the Coupa API. If account is indeed singular (one account per user), then the entire grant/revoke model changes — it becomes a simple set/replace rather than the current add-to-collection/remove-from-collection pattern. If the Coupa API actually supports multiple accounts via a plural field (e.g., accounts), then the JSON tags and query fields need to be updated to match.

The PR description already calls this out, but it's the most critical item to verify.

2. billingAccountResource uses resourceSdk.NewResource instead of a trait-specific constructor (Low)

pkg/connector/billing_accounts.go:37-43 — The roles builder uses resourceSdk.NewRoleResource() and the groups builder uses resourceSdk.NewGroupResource(), both of which set appropriate traits. The billing account builder uses the generic resourceSdk.NewResource(), which means the resource won't have any traits set.

This is consistent with how billingAccountResourceType is defined without traits in resource_types.go:31-34, so it's intentionally trait-less. This is fine — just noting the deliberate difference. However, it may be worth considering whether billing accounts should have a trait (e.g., TRAIT_GROUP since they represent a collection with membership).

3. List pagination always returns lastId even when no results (Very Low)

pkg/connector/billing_accounts.go:73-83 — If the query returns an empty target.Accounts slice, lastId stays "" and we return ("", "", annotations, nil). This is actually correct behavior for pagination termination (empty token = no more pages). However, this means the pagination relies on the API eventually returning an empty page to terminate. The roles builder has the same pattern, so this is consistent — but worth understanding that pagination terminates only when a page returns zero results.

4. Missing strconv.Itoa for account.ID in billingAccountResource (Bug)

pkg/connector/billing_accounts.go:37-43:

return resourceSdk.NewResource(
    displayName,
    billingAccountResourceType,
    account.ID,  // This is an int
    resourceSdk.WithParentResourceID(parentResourceID),
)

Looking at Account in models.go, ID is an int. The NewResource function likely expects a string or has an any parameter. Let me check — in the roles builder, NewRoleResource takes role.ID which is also int. The SDK likely handles this via an any parameter, but this should be verified. If NewResource expects a string, this would be a compilation error. This needs verification via build.

Verify this compiles →

5. Revoke has a race condition window (Inherited, Low)

pkg/connector/billing_accounts.go:258-263 — The two-step revoke (clear all, then re-set) has an inherent race condition where if the second SetUserAccounts call fails, the user is left with zero accounts. This is the same pattern used by roles and groups, so it's a known limitation. The error logging at line 266 is appropriate.

6. Account.Active field is fetched but never used (Very Low)

pkg/connector/client/models.go:77 — The Active field is included in the Account struct and fetched in getAccountsQuery, but the billingAccountResource function doesn't filter or use it. The roles and groups builders also don't filter by active status, so this is consistent. However, you may want to consider filtering out inactive billing accounts in List or at least noting the active status.

---

What Looks Good

• Pattern consistency: The implementation closely mirrors roles.go and groups.go for Grant, Revoke, List, Entitlements, and Grants. The two-step revoke pattern, the getUserAccounts helper, and the error handling all follow established conventions.
• Idempotency: Grant correctly returns GrantAlreadyExists (line 183) and Revoke correctly returns GrantAlreadyRevoked (line 240) — both following the CLAUDE.md guidance.
• Error prefixes: All errors use the baton-coupa: prefix consistently.
• OAuth scopes: core.accounting.read and core.accounting.write are correctly added to the respective scope lists.
• Capabilities: baton_capabilities.json properly registers the new resource type with both CAPABILITY_SYNC and CAPABILITY_PROVISION.
• HTTP response handling: Response bodies are properly deferred for close after error checks.

---

Summary

The PR is well-implemented and follows existing patterns. The primary concern is validating the Coupa API's billing account model — specifically whether account is singular (one per user) or plural (many per user). This fundamentally affects the grant/revoke logic. Everything else is minor and consistent with existing code patterns.

Recommendation: Validate against a live Coupa instance before merging, particularly the GraphQL field names and the singular/plural semantics of the account field.

[laurenleach]

we should document the required scopes on the resource type capabilities (like https://github.com/ConductorOne/baton-slack-enterprise/blob/main/pkg/connector/resource_types.go#L25-L34)