diff --git a/.github/workflows/capabilities_and_config.yaml b/.github/workflows/capabilities_and_config.yaml index 55d9d19d..e646789e 100644 --- a/.github/workflows/capabilities_and_config.yaml +++ b/.github/workflows/capabilities_and_config.yaml @@ -36,10 +36,11 @@ jobs: - name: Run and save config output run: ./connector config > config_schema.json + # No credentials and no flags: the connector answers this from its + # default capabilities builder, which lists every resource type rather + # than only the ones a given configuration switches on. - name: Run and save capabilities output - env: - BATON_TOKEN: test - run: ./connector --sync-secrets capabilities > baton_capabilities.json + run: ./connector capabilities > baton_capabilities.json - name: Commit changes uses: EndBug/add-and-commit@v9 diff --git a/README.md b/README.md index 98ce535b..1455d3d6 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,11 @@ baton resources - Users - Teams - Repositories +- Organization roles +- Invitations (users invited to an organization who have not accepted yet) - GitHub Apps (installed in organizations, synced as non-human identities) +- Enterprise roles, only when `--enterprises` is set. Provisioning the built-in Enterprise Owner role additionally requires `--enable-enterprise-owner-provisioning` and a GitHub App installed on the enterprise account +- Enterprise licenses, only when `--enterprises` is set and the connector uses a personal access token. The API behind them is not available to GitHub Apps By default, `baton-github` will sync information from any organizations that the provided credential has Administrator permissions on. You can specify exactly which organizations you would like to sync using the `--orgs` flag. @@ -78,6 +82,7 @@ Flags: --app-privatekey-path string Path to private key that is used to connect to the GitHub App. Ignored when app-privatekey is set. ($BATON_APP_PRIVATEKEY_PATH) --client-id string The client ID used to authenticate with ConductorOne ($BATON_CLIENT_ID) --client-secret string The client secret used to authenticate with ConductorOne ($BATON_CLIENT_SECRET) + --enable-enterprise-owner-provisioning Sync and provision the built-in Enterprise Owner role. Requires the GitHub App to be installed on the enterprise account as well as on the organization, with the "Enterprise people: read and write" permission, and requires --enterprises to name that enterprise. Not available with a personal access token. ($BATON_ENABLE_ENTERPRISE_OWNER_PROVISIONING) --enterprises strings Sync enterprise roles, must be an admin of the enterprise. ($BATON_ENTERPRISES) --external-resource-c1z string The path to the c1z file to sync external baton resources with ($BATON_EXTERNAL_RESOURCE_C1Z) --external-resource-entitlement-id-filter string The entitlement that external users, groups must have access to sync external baton resources ($BATON_EXTERNAL_RESOURCE_ENTITLEMENT_ID_FILTER) diff --git a/baton_capabilities.json b/baton_capabilities.json index ee90c5b4..6b741942 100644 --- a/baton_capabilities.json +++ b/baton_capabilities.json @@ -55,6 +55,29 @@ ] } }, + { + "resourceType": { + "id": "enterprise_role", + "displayName": "Enterprise Role", + "traits": [ + "TRAIT_ROLE" + ], + "annotations": [ + { + "@type": "type.googleapis.com/c1.connector.v2.V1Identifier", + "id": "enterprise_role" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.SkipEntitlements" + } + ] + }, + "capabilities": [ + "CAPABILITY_SYNC", + "CAPABILITY_PROVISION" + ], + "permissions": {} + }, { "resourceType": { "id": "invitation", @@ -80,6 +103,32 @@ "permissions": {}, "skipSyncAnomalyDetection": true }, + { + "resourceType": { + "id": "license", + "displayName": "License", + "traits": [ + "TRAIT_LICENSE_PROFILE" + ], + "annotations": [ + { + "@type": "type.googleapis.com/c1.connector.v2.V1Identifier", + "id": "license" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.SkipEntitlements" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.OptInRequired" + } + ] + }, + "capabilities": [ + "CAPABILITY_SYNC" + ], + "permissions": {}, + "optInRequired": true + }, { "resourceType": { "id": "org", @@ -166,6 +215,24 @@ ], "permissions": {} }, + { + "resourceType": { + "id": "usage-app", + "displayName": "GitHub Activity", + "traits": [ + "TRAIT_APP" + ], + "annotations": [ + { + "@type": "type.googleapis.com/c1.connector.v2.SkipGrants" + } + ] + }, + "capabilities": [ + "CAPABILITY_SYNC" + ], + "permissions": {} + }, { "resourceType": { "id": "user", @@ -191,7 +258,8 @@ "CAPABILITY_PROVISION", "CAPABILITY_SYNC", "CAPABILITY_ACCOUNT_PROVISIONING", - "CAPABILITY_RESOURCE_DELETE" + "CAPABILITY_RESOURCE_DELETE", + "CAPABILITY_EVENT_FEED_V2" ], "credentialDetails": { "capabilityAccountProvisioning": { diff --git a/cmd/baton-github/main.go b/cmd/baton-github/main.go index 56742ec8..9063fd69 100644 --- a/cmd/baton-github/main.go +++ b/cmd/baton-github/main.go @@ -14,5 +14,8 @@ var version = "dev" func main() { ctx := context.Background() - config.RunConnector(ctx, "baton-github", version, cfg.Config, connector.NewLambdaConnector, connectorrunner.WithSessionStoreEnabled()) + config.RunConnector(ctx, "baton-github", version, cfg.Config, connector.NewLambdaConnector, + connectorrunner.WithSessionStoreEnabled(), + connectorrunner.WithDefaultCapabilitiesConnectorBuilderV2(&connector.DefaultCapabilitiesBuilder{}), + ) } diff --git a/config_schema.json b/config_schema.json index 6b0dce9b..dec08605 100644 --- a/config_schema.json +++ b/config_schema.json @@ -169,6 +169,12 @@ "description": "Whether to sync secrets or not", "boolField": {} }, + { + "name": "enable-enterprise-owner-provisioning", + "displayName": "Enable enterprise owner provisioning", + "description": "Sync and provision the built-in Enterprise Owner role. Requires the GitHub App to be installed on the enterprise account as well as on the organization, with the \"Enterprise people: read and write\" permission, and requires --enterprises to name that enterprise. Not available with a personal access token.", + "boolField": {} + }, { "name": "omit-archived-repositories", "displayName": "Omit syncing archived repositories", @@ -207,7 +213,9 @@ "app-privatekey-path", "app-privatekey", "org", + "enterprises", "sync-secrets", + "enable-enterprise-owner-provisioning", "omit-archived-repositories", "direct-collaborators-only" ] diff --git a/docs/connector.mdx b/docs/connector.mdx index 7d4b2a25..e8ff1761 100644 --- a/docs/connector.mdx +++ b/docs/connector.mdx @@ -22,6 +22,9 @@ Use this integration if your organization accesses GitHub at `github.com`. If yo | Orgs | | | | GitHub Apps (NHI) | | | | Secrets - API keys | | | +| Enterprise roles | | | + +Enterprise roles are opt-in and off by default. Syncing them requires the **Enterprises** setting; provisioning the built-in **Enterprise Owner** role additionally requires **Enable enterprise owner provisioning** and a GitHub app installed on both the enterprise account and the organization. See the setup steps below. The GitHub connector supports [automatic account provisioning and deprovisioning](/product/admin/account-provisioning). New accounts will send an invitation to the account owner; while an invitation is pending, the account status will be shown as **Pending**. Expired invitations also report **Pending**, distinguished by `invitation_expired` in the status details. @@ -158,6 +161,10 @@ This process creates a GitHub app that is only available to your GitHub organiza A user with the **Org Owner** permission in the GitHub organization to be integrated with C1 must perform this task. + +If you want C1 to manage the built-in **Enterprise Owner** role, create the app under your **enterprise account** rather than under the organization — start from your enterprise's **Settings** > **GitHub Apps** > **New GitHub App** instead of Step 1 below — and install it twice: once on the enterprise account and once on the organization the connector syncs. The rest of the steps are the same. + + In GitHub, navigate to **Your organizations** > **Settings**. @@ -199,7 +206,11 @@ In the **Permissions** section of the page, give the app the following permissio - **Administration**: Read-only access (required to detect SAML/SSO configuration) - **Custom organization roles**: Read and write access - **Members**: Read and write access - + + - Enterprise permissions, **only** if you want C1 to manage the built-in Enterprise Owner role: + + - **People**: Read and write access + For details, see the GitHub docs on [Permissions required for GitHub Apps](https://docs.github.com/en/enterprise-cloud@latest/rest/authentication/permissions-required-for-github-apps). @@ -239,6 +250,9 @@ Select the repositories the app can act on. Click **Install**. + +**Optional.** If you want C1 to manage the built-in Enterprise Owner role, install the app a second time, on the **enterprise account**, from your enterprise's **Settings** > **GitHub Apps**. Granting the enterprise permission above is not enough on its own: without that second installation the connector cannot read or assign owners. + **Done.** Next, move on to the connector configuration instructions. @@ -314,6 +328,12 @@ If you're using a GitHub app to set up the connector: 1. **Optional.** Click to enable **Sync secrets**. [Synced secrets](/product/admin/inventory) are displayed on the **Inventory** page. + 1. **Optional.** If you want C1 to manage the built-in Enterprise Owner role, enter your enterprise's slug in the **Enterprises** field and click to enable **Enable enterprise owner provisioning**. Both are required together, and the app must be installed on the enterprise account with the **Enterprise → People: Read and write** permission, as described above. + + Enter a single enterprise. An organization belongs to exactly one enterprise, and owners are read through the organization you configured above, so naming more than one is rejected. + + Leave these unset unless you need the role. While the option is off, the connector syncs exactly as it did before, and the Enterprise Owner role does not appear in C1. While it is on, the connector deliberately fails the sync if it cannot reach the enterprise — reporting no owners would make C1 delete the role and every grant on it. + 1. **Optional.** If you do not want to include archived repos in syncs, click to enable **Omit archived repositories**. 1. **Optional.** If your GitHub organization has thousands of repositories or members, click to enable **Optimize sync for large organizations**. See [Optimize sync for large organizations](#optimize-sync-for-large-organizations) for what changes when this option is enabled. @@ -406,6 +426,13 @@ stringData: # Optional: include if you want C1 to provision access using this connector BATON_PROVISIONING: true + # Optional: GitHub app only. Include both if you want C1 to manage the built-in + # Enterprise Owner role. Requires the app to be installed on the enterprise + # account as well as on the org, with "Enterprise people: read and write". + # Name a single enterprise: the org you configured belongs to exactly one. + BATON_ENTERPRISES: + BATON_ENABLE_ENTERPRISE_OWNER_PROVISIONING: true + # Optional: include if you do not want to sync archived repos BATON_OMIT_ARCHIVED_REPOSITORIES: true diff --git a/docs/docs-info.md b/docs/docs-info.md new file mode 100644 index 00000000..d6cc1707 --- /dev/null +++ b/docs/docs-info.md @@ -0,0 +1,354 @@ +# GitHub Connector Setup Guide + +--- + +## Requirements + +- A **GitHub** organization, and for enterprise features a **GitHub Enterprise Cloud** account +- Either a **personal access token (classic)** or a **GitHub App** owned by the organization or the enterprise +- For the built-in Enterprise Owner role: a GitHub App installed on **both** the enterprise account and the organization + +--- + +## Connector capabilities + +1. **What resources does the connector sync?** + This connector syncs: + - Organizations (the orgs the credential can administer, or the ones named in `--orgs` / `--org`) + - Users (organization members, with SAML identity emails when SAML is configured) + - Invitations (users invited to an organization who have not accepted, and invitations that expired) + - Teams (including nested teams, with their parent team as the parent resource) + - Repositories (optionally excluding archived ones) + - Organization roles (GitHub's built-in and custom org roles, also called "enterprise licenses" in GitHub's docs) + - Enterprise roles (only when `--enterprises` is set; under App authentication the built-in Owner role also needs `--enable-enterprise-owner-provisioning`) + - Licenses (enterprise seat consumption, only when `--enterprises` is set) + - GitHub Apps installed on the organization + - API keys (fine-grained personal access tokens with access to the org, only when `--sync-secrets` is set) + +2. **Can the connector provision any resources? If so, which ones?** + The connector can provision: + - Organization membership and admin role via Grant and Revoke. Granting to a user who is not yet a member sends an org invitation + - Team membership (`member`, `maintainer`) via Grant and Revoke + - Repository access (`pull`, `triage`, `push`, `maintain`, `admin`) via Grant and Revoke, for users and for teams + - Organization role assignment via Grant and Revoke + - The built-in Enterprise **Owner** role via Grant and Revoke, GitHub App authentication only + - Accounts, via the invitation resource type: `CreateAccount` sends an org invitation, and `Delete` cancels it + - User removal from the organization via `Delete` on the user resource type + +3. **Does the connector emit any event feeds?** + Yes, when `--sync-last-activity` is set. `github_usage_event_feed` streams member activity from each organization's audit log as usage events, which is what drives last-login and activity reporting. The flag also registers a synthetic app resource that exists only to carry those events. + +4. **Does the connector support grant expansion?** + Yes, in three places: + - Repository permissions implied by the organization's `default_repository_permission` are emitted against the organization and expanded through the org's `member` and `admin` entitlements, so every member's baseline repository access surfaces without enumerating collaborators + - An organization role assigned to a team is expanded through that team's `member` and `maintainer` entitlements + - An enterprise license held by a role is expanded through that role's `assigned` entitlement + + All three are `Shallow`, so the SDK does not recurse further. `--direct-collaborators-only` leans on this expansion instead of fetching per-team repository detail, which cuts API calls on large organizations. + +--- + +## Connector credentials + +1. **What credentials or information are needed to set up the connector?** + This connector accepts one of two authentication methods. + + **Personal access token (classic)** + + **Args**: + `--token` — the GitHub personal access token + `--instance-url` — the GitHub instance URL, defaults to `https://github.com` + `--orgs` — optional, limits syncing to specific organizations + + **GitHub App** + + **Args**: + `--app-id` — the GitHub App ID + `--app-privatekey-path` — path to the App's private key `.pem` + `--org` — required, the single organization the App is installed on + `--instance-url` — the GitHub instance URL, defaults to `https://github.com` + + Common to both: + `--enterprises` — enterprises to sync enterprise roles and licenses for + `--enable-enterprise-owner-provisioning` — sync and provision the built-in Enterprise Owner role. Off by default, App authentication only, and requires the App installed on the enterprise account as well as the organization + `--sync-secrets` — sync fine-grained personal access tokens as API keys + `--sync-last-activity` — emit the audit-log usage event feed. Hidden from `--help` and from the GUI config here, because it only applies to GitHub Enterprise audit-log access; `baton-github-enterprise` sets it directly instead of going through this CLI layer + `--omit-archived-repositories` — skip archived repositories + `--direct-collaborators-only` — reduce API calls on large organizations + +2. **For each item in the list above:** + - **How does a user create or look up that credential or info?** + + **Personal access token (classic):** + 1. In GitHub, click your profile photo, then **Settings** + 2. Go to **Developer settings** > **Personal access tokens** > **Tokens (classic)** + 3. Click **Generate new token** > **Generate new token (classic)** + 4. Name the token, optionally set an expiration, and select the scopes below + 5. Click **Generate token** and copy it — it is shown only once + + **GitHub App:** + 1. For enterprise features, create the App under the enterprise account: **Settings** > **GitHub Apps** > **New GitHub App**. Otherwise create it under the organization + 2. Give it a globally unique name, and use a placeholder URL for Homepage and Callback + 3. Uncheck **Active** under Webhook + 4. Select the permissions below + 5. Under **Where can this app be installed?** choose **Only on this account** + 6. Create the App, copy the **App ID**, then generate and save a **private key** + 7. Install the App. For enterprise features install it twice: once on the **enterprise account**, once on the **organization** the connector syncs + + - **Does the credential need any specific scopes or permissions?** + + **Personal access token (classic)** scopes: + - `repo` — all + - `admin:org` — all for organization-level provisioning, otherwise `read:org` + - `user` — all + - `admin:enterprise` — `read:enterprise`, for enterprise roles and licenses + + If the organization uses SAML single sign-on, the token must also be authorized for that organization. + + **GitHub App** permissions: + - Repository: **Administration** read and write (implies **Metadata** read) + - Organization: **Administration** read-only (detects SAML/SSO configuration), **Members** read and write, **Custom organization roles** read and write + - Enterprise: **Enterprise people** read and write, required to sync and provision the built-in Owner role + + - **Is the list of scopes or permissions different to sync (read) versus provision (read-write)?** + Yes. Read-only syncing needs `read:org` rather than `admin:org` on a PAT, and read-only equivalents of the App's organization permissions. Provisioning the built-in Enterprise Owner role requires **Enterprise people: read and write**; read-only is not enough because the connector issues invitations and role mutations. + + - **What level of access or permissions does the user need in order to create the credentials?** + A personal access token must be created by a user with **Enterprise Owner** access when enterprise features are used, and organization admin access otherwise. Creating an enterprise-owned GitHub App requires someone who can manage GitHub Apps for the enterprise; installing it on an organization requires **Org Owner** on that organization. + +--- + +## Resource Details + +### Organizations + +- **Resource type ID**: `org` +- **Description**: The GitHub organizations the credential can administer, or the ones named in `--orgs` / `--org` +- **Traits**: None +- **Entitlements**: `member` (assignment) and `admin` (permission) +- **Grants**: One grant per organization member for their role. Members are read from the members list; the connector distinguishes admins from plain members +- **Children**: Users, Invitations, Teams, Repositories, Organization roles, GitHub Apps, API keys +- **Provisioning**: Grant adds the member or promotes them to admin. A user who is not yet a member is sent an organization invitation instead, so the membership only exists once they accept. Revoke removes the organization membership + +### Users + +- **Resource type ID**: `user` +- **Description**: Members of the synced organizations +- **Traits**: User trait with login, email and profile +- **Parent**: Organization +- **Entitlements**: None +- **Grants**: None. Access is emitted by the organization, team, repository and role builders +- **Provisioning**: `Delete` removes the user from the organization +- **Note**: When the organization has SAML single sign-on, emails are read from the SAML identity rather than the public profile. Enterprise-level SAML is read from the enterprise consumed-licenses API, which is PAT-only; when that is unavailable the connector falls back to the REST email + +### Invitations + +- **Resource type ID**: `invitation` +- **Description**: Users invited to an organization who have not accepted, and invitations GitHub expired +- **Traits**: User trait with `RESOURCE_STATUS_PENDING`, plus `invitation_status` and `invitation_expires_at` profile fields +- **Parent**: Organization +- **Entitlements**: None +- **Grants**: None +- **Provisioning**: `CreateAccount` sends an organization invitation; `Delete` cancels it +- **Note**: Organization invitations expire seven days after creation. Because a pending invitation disappears once accepted, this resource type opts out of sync anomaly detection + +### Teams + +- **Resource type ID**: `team` +- **Description**: GitHub teams, including nested teams +- **Traits**: Group trait +- **Parent**: Organization, or the parent team for a nested team +- **Entitlements**: `member`, `maintainer` (permission) +- **Grants**: One grant per team member for their role +- **Provisioning**: Grant and Revoke add or remove team membership + +### Repositories + +- **Resource type ID**: `repository` +- **Description**: Repositories of the synced organizations +- **Traits**: None +- **Parent**: Organization +- **Entitlements**: `pull`, `triage`, `push`, `maintain`, `admin` (permission), grantable to users and teams, and declared as an exclusion group because a principal holds one level at a time +- **Grants**: One grant per collaborator for their permission level, and one per team with repository access. The organization's `default_repository_permission` is expanded into the cumulative levels it implies and emitted against the organization, annotated as expandable through the org's `member` and `admin` entitlements +- **Provisioning**: Grant and Revoke add or remove a collaborator, or a team's repository access +- **Note**: `--omit-archived-repositories` skips archived repositories. `--direct-collaborators-only` relies on grant expansion for team access instead of fetching per-team detail + +### Organization roles + +- **Resource type ID**: `org_role` +- **Description**: GitHub's built-in and custom organization roles. GitHub's documentation also calls these "enterprise licenses" +- **Traits**: Role trait +- **Parent**: Organization +- **Entitlements**: `assigned` (assignment) +- **Grants**: One grant per user and per team assigned to the role. A team's grant is expandable through that team's `member` and `maintainer` entitlements +- **Provisioning**: Grant and Revoke assign or unassign the role + +### Enterprise roles + +- **Resource type ID**: `enterprise_role` +- **Description**: Roles of an enterprise account. Only synced when `--enterprises` is set. Under App authentication the built-in Owner role is only synced when `--enable-enterprise-owner-provisioning` is set as well +- **Traits**: Role trait +- **Entitlements**: `assigned` (assignment) +- **Grants**: Under PAT authentication, one grant per user holding each role, read from the enterprise consumed-licenses API. Under GitHub App authentication, only the built-in **Owner** role is visible, and its grants are the users who hold it plus the users who have been invited and have not accepted. The two are emitted against the same entitlement and C1 cannot tell them apart +- **Provisioning**: Only the built-in **Owner** role, only with GitHub App authentication, and only when `--enable-enterprise-owner-provisioning` is set. See [Enterprise Owner provisioning](#enterprise-owner-provisioning) +- **Limitation**: A GitHub App cannot read `Enterprise.ownerInfo`, so under App authentication the connector sees only the Owner role, not billing managers or custom enterprise roles + +### Licenses + +- **Resource type ID**: `license` +- **Description**: Enterprise seat consumption. Only synced when `--enterprises` is set +- **Traits**: License profile trait +- **Entitlements**: `assigned` (assignment) +- **Grants**: One grant for the enterprise member role holding the license, expandable through that role's `assigned` entitlement +- **Limitation**: Requires a personal access token. GitHub does not offer the enterprise administration permission to GitHub Apps, so this resource type cannot sync with an App installation token, and its failure fails the whole sync. Customers using a GitHub App with `--enterprises` set must disable this resource type in the connector's resource capabilities in C1 + +### GitHub Apps + +- **Resource type ID**: `app` +- **Description**: GitHub Apps installed on the organization +- **Traits**: App trait, annotated as a non-human identity of type app registration +- **Parent**: Organization +- **Entitlements**: None +- **Grants**: None + +### API keys + +- **Resource type ID**: `api-key` +- **Description**: Fine-grained personal access tokens with access to the organization. Only synced when `--sync-secrets` is set +- **Traits**: Secret trait +- **Parent**: Organization +- **Entitlements**: None +- **Grants**: None + +--- + +## Enterprise Owner provisioning + +Only the built-in **Owner** role of an enterprise is provisionable, and only under GitHub App authentication. The design is shaped by three GitHub constraints: + +**`Enterprise.ownerInfo` is invisible to an App.** It holds `admins` and `pendingAdminInvitations`, and it resolves to `null` for an installation token regardless of which permissions the App declares. + +**`Enterprise.members(role: OWNER)` is the wrong list.** That argument is an `EnterpriseUserAccountMembershipRole`, whose `OWNER` means "owner of an *organization* in the enterprise" — a different enum from the `EnterpriseAdministratorRole` the mutations take. Owners are read from `Organization.enterpriseOwners` instead, which returns every owner of the organization's enterprise account annotated with their role in that organization. The query must not pass `organizationRole`, because that would drop owners who are not owners of the organization. + +**There is no single operation that safely assigns Owner in every case.** A member becomes an Owner by accepting an invitation, but GitHub rejects that invitation when the user already has an enterprise administrator role such as Billing Manager. A GitHub App cannot read that prior role. The connector therefore returns `FailedPrecondition` instead of promoting the administrator in place: otherwise Revoke could only demote them to `UNAFFILIATED`, permanently discarding the role they held before the grant. + +Consequences worth knowing: + +- Grant returns the grant when it creates an invitation. `Grants()` emits pending invitations alongside accepted Owners, so C1 keeps a record of the request from the moment it is made +- Revoke clears both states rather than treating them as alternatives: it demotes an active Owner to `UNAFFILIATED`, which keeps them as a member of the enterprise rather than evicting them, and cancels an unaccepted invitation. A `NOT_FOUND` on either is success, because it means the state being asked for is already in place +- Reading owners uses the **organization** installation token and every mutation uses the **enterprise** installation token; the enterprise token is rejected on organization fields. Startup verifies that the configured organization belongs to the configured enterprise, and a failure there fails the sync +- Only one enterprise can be served under App authentication, because the owners are read through the single configured organization and an organization belongs to exactly one enterprise. A configuration naming several is rejected while the clients are built, with an explanatory error rather than a later failure on a check the operator cannot satisfy. The PAT path does accept a list + +### Enterprise owner provisioning is opt-in + +`--enable-enterprise-owner-provisioning` is off by default, and while it is off the connector behaves exactly as it did before this capability existed. Enterprise roles come from the consumed-licenses cache, which is PAT-only, so an App deployment reports none of them, and the `license` resource type still fails the sync on that same 403 — as it did before this change. Nothing about an upgrade is different until the capability is asked for. + +The flag exists because turning the capability on requires setup nobody has done yet — a second installation of the App, on the enterprise account. Without the flag that requirement would reach every deployment that already passes `--enterprises` under App authentication, and the failure below would turn their working sync into a failing one on upgrade. With it, the failure is only reachable by an operator who asked for the capability. + +### A missing enterprise installation fails the sync rather than emitting nothing + +Once the capability is enabled, the connector refuses to sync when it cannot build an enterprise administration client: the app is not installed on the enterprise account, several enterprises are configured, or the organization does not belong to the configured one. The whole sync fails, not just this resource type. + +That is deliberate, and the alternative is worse. C1 deletes every resource of a type that a completed sync did not report, and it applies that to a resource type whose list came back empty for any reason. Letting the sync finish while reading no owners would therefore delete the Owner role and every grant on it, silently, and GitHub answers `404` for an uninstalled app, a revoked permission and a slug typo alike — the connector cannot tell a genuine uninstall from a blip. An error keeps the sync from completing, so nothing is deleted. + +The cost falls on a deployment that enabled the capability without installing the app on the enterprise account, and the sync stops until the app is installed or the flag is turned back off. + +### Pending invitations look the same as real access + +C1 has no pending state for a grant, so an invitation nobody has accepted and an accepted Owner are emitted as the same grant on the same entitlement, and nothing distinguishes them. That is a deliberate trade: emitting nothing until the invitee accepts would leave the request invisible for up to seven days and leave reviewers no record that it was made. + +- An access review or an offboarding sweep counts an invitee as holding Owner. They do not hold it — GitHub assigns the role only on acceptance +- Nothing tracks an expiry. GitHub stops resolving an invitation once it is accepted, cancelled, or expired, so it simply stops being emitted and C1 drops the grant on that sync. `EnterpriseAdministratorInvitation` exposes no `expiresAt`, so there is nothing to compute from either +- **Time-bound access is measured from the invitation, not from acceptance.** C1 starts the clock when Grant reports success, which is when the invitation is sent. Someone who accepts three hours into a four-hour window holds the role for one hour, and the record still reads four. If the window closes first, Revoke cancels the unaccepted invitation — the right action, since the request expired, but C1 logs a completed Owner grant for someone who never held the role. Prefer windows comfortably longer than the invitee takes to accept + +### The list of pending invitations cannot be complete + +`Enterprise.ownerInfo.pendingAdminInvitations` is the only connection of invitations GitHub offers, and it resolves to `null` for an installation token. The root `enterpriseAdministratorInvitation` field answers for one login at a time, so the sync resolves invitations by asking about the enterprise members, batching up to 100 logins into one aliased request — GitHub charges that whole request a single rate-limit point. + +An invitation sent to someone who is not a member of the enterprise is therefore invisible to the sync, and that is not hypothetical: an owner invitation can be addressed to any GitHub user. Invitations that C1 itself creates are always visible, because C1 grants to a user it has already synced. + +### Owner grants can reference a user the sync did not emit + +The `user` resource type is populated from the members of the **configured organization**, while `Organization.enterpriseOwners` returns owners of the whole **enterprise account** and the invitation candidates come from `Enterprise.members`, which spans every organization in it. An owner who belongs to a different organization in the same enterprise therefore produces a grant whose principal this sync never created. Widening the user sync is out of scope here — it would change the connector's user population for every deployment — so the grant is emitted and this limitation is recorded instead. + +--- + +## Authentication + +The connector supports two methods, selected by which credentials are supplied. + +1. **Personal access token (classic)**: a single bearer token used for REST and GraphQL. + +2. **GitHub App**: the App's private key signs a JWT, which is exchanged for installation access tokens. Tokens are refreshed automatically when they expire. A connector using enterprise features holds two installation tokens at once, one for the organization and one for the enterprise account, because GitHub splits the data between them. + +GraphQL is used for SAML identity lookups, the audit log, and all enterprise owner reads and mutations. Everything else is REST. + +--- + +## API Endpoints Used + +**REST** (via `go-github`): + +- `GET /user`, `GET /users/{username}`, `GET /user/{id}` — resolve users +- `GET /organizations`, `GET /orgs/{org}`, `GET /organizations/{id}` — list and resolve organizations +- `GET /orgs/{org}/members` — organization members +- `GET /orgs/{org}/memberships/{username}` — a member's role +- `PUT /orgs/{org}/memberships/{username}` — promote to admin (Grant) +- `DELETE /orgs/{org}/memberships/{username}` — remove membership (Revoke, user Delete) +- `POST /orgs/{org}/invitations` — invite a user (Grant, CreateAccount) +- `GET /orgs/{org}/invitations`, `GET /orgs/{org}/failed_invitations` — pending and expired invitations +- `DELETE /orgs/{org}/invitations/{invitation_id}` — cancel an invitation (invitation Delete) +- `GET /orgs/{org}/teams`, `GET /teams/{team_id}` — teams +- `GET /teams/{team_id}/members` — team members +- `PUT /teams/{team_id}/memberships/{username}`, `DELETE /teams/{team_id}/memberships/{username}` — team membership (Grant, Revoke) +- `GET /orgs/{org}/repos`, `GET /repositories/{id}` — repositories +- `GET /repos/{owner}/{repo}/collaborators`, `GET /repos/{owner}/{repo}/collaborators/{username}/permission` — repository access +- `PUT /repos/{owner}/{repo}/collaborators/{username}`, `DELETE /repos/{owner}/{repo}/collaborators/{username}` — repository access (Grant, Revoke) +- `GET /repos/{owner}/{repo}/teams` — teams with repository access +- `PUT /orgs/{org}/teams/{team_slug}/repos/{owner}/{repo}`, `DELETE /orgs/{org}/teams/{team_slug}/repos/{owner}/{repo}` — team repository access (Grant, Revoke) +- `GET /orgs/{org}/organization-roles` — organization roles +- `GET /orgs/{org}/organization-roles/{role_id}/users`, `.../teams` — role assignments +- `GET /orgs/{org}/installations` — installed GitHub Apps, requires `organization_administration=read` +- `GET /orgs/{org}/personal-access-tokens` — fine-grained PATs, only with `--sync-secrets` +- `GET /orgs/{org}/audit-log` — organization audit log +- `GET /enterprises/{enterprise}/installation` — this App's installation on one enterprise, authenticated with the App JWT +- `GET /enterprises/{enterprise}/consumed-licenses` — enterprise license consumption and enterprise SAML identities. **PAT only** + +**GraphQL**: + +- `organization(login:) { samlIdentityProvider { externalIdentities } }` — SAML identity emails +- `organization(login:) { enterpriseOwners }` — the enterprise account's owners, read with the organization token +- `enterprise(slug:) { id }`, `enterprise(slug:) { organizations }` — enterprise node ID, and the organization-belongs-to-enterprise check +- `enterpriseAdministratorInvitation(enterpriseSlug:, userLogin:, role:)` — a pending Owner invitation, for one login; the sync aliases up to 100 of these into a single request +- `enterprise(slug:).members` — the candidate logins the pending-invitation lookup asks about +- `inviteEnterpriseAdmin`, `updateEnterpriseAdministratorRole`, `cancelEnterpriseAdminInvitation` — Owner Grant and Revoke + +--- + +## Pagination + +- REST endpoints use GitHub's `page` and `per_page` parameters, 100 per page, driven one page per SDK call +- `GET /enterprises/{enterprise}/consumed-licenses` is 1-indexed; page 0 is undocumented and can repeat page 1, producing duplicates +- GraphQL connections use cursor pagination, 100 per page, with the `endCursor` passed through as the SDK page token +- The audit log event feed keeps its own cursor, which carries both the current organization index and GitHub's `after` token, so the feed resumes mid-organization + +--- + +## Rate Limits + +- REST: 5,000 requests per hour for a PAT. A GitHub App installation gets a larger budget that scales with the account; installations on this connector's test enterprise reported 15,000 +- GraphQL: a separate points-based budget, reported per query in the `rateLimit` field; App installations reported 10,000 +- The connector returns GitHub's rate limit headers to the SDK as rate limit annotations, so it backs off rather than failing the sync. The enterprise owner queries additionally report the GraphQL `rateLimit` field, and are the ones that classify the errors GitHub returns inside an HTTP 200 body, so a rate limit there surfaces as retryable rather than as an opaque failure + +--- + +## API Documentation + +**Official GitHub API references:** + +- **REST**: https://docs.github.com/en/rest +- **GraphQL**: https://docs.github.com/en/graphql +- **Enterprise administration (GraphQL)**: https://docs.github.com/en/graphql/reference/enterprise-admin +- **Permissions required for GitHub Apps**: https://docs.github.com/en/rest/authentication/permissions-required-for-github-apps +- **Inviting people to manage your enterprise**: https://docs.github.com/en/enterprise-cloud@latest/admin/managing-accounts-and-repositories/managing-users-in-your-enterprise/inviting-people-to-manage-your-enterprise +- **Enterprise licensing (REST)**: https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/license diff --git a/pkg/config/conf.gen.go b/pkg/config/conf.gen.go index 6541f0f2..0c4cf034 100644 --- a/pkg/config/conf.gen.go +++ b/pkg/config/conf.gen.go @@ -13,6 +13,7 @@ type Github struct { AppPrivatekey string `mapstructure:"app-privatekey"` Org string `mapstructure:"org"` SyncSecrets bool `mapstructure:"sync-secrets"` + EnableEnterpriseOwnerProvisioning bool `mapstructure:"enable-enterprise-owner-provisioning"` OmitArchivedRepositories bool `mapstructure:"omit-archived-repositories"` DirectCollaboratorsOnly bool `mapstructure:"direct-collaborators-only"` SyncLastActivity bool `mapstructure:"sync-last-activity"` diff --git a/pkg/config/config.go b/pkg/config/config.go index 60856c56..3b10b1d3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -69,6 +69,18 @@ var ( field.WithDisplayName("Sync secrets"), field.WithDescription(`Whether to sync secrets or not`), ) + // Off by default because it needs setup nobody has done yet: turning it on + // without the enterprise installation fails the sync, which is only a fair + // trade for someone who asked for the capability. + enableEnterpriseOwnerProvisioning = field.BoolField( + "enable-enterprise-owner-provisioning", + field.WithDisplayName("Enable enterprise owner provisioning"), + field.WithDescription( + "Sync and provision the built-in Enterprise Owner role. Requires the GitHub App to be installed on "+ + "the enterprise account as well as on the organization, with the \"Enterprise people: read and write\" "+ + "permission, and requires --enterprises to name that enterprise. Not available with a personal access token.", + ), + ) omitArchivedRepositories = field.BoolField( "omit-archived-repositories", field.WithDisplayName("Omit syncing archived repositories"), @@ -120,6 +132,7 @@ var Config = field.NewConfiguration( appPrivateKey, orgField, syncSecrets, + enableEnterpriseOwnerProvisioning, omitArchivedRepositories, directCollaboratorsOnly, syncLastActivity, @@ -139,8 +152,11 @@ var Config = field.NewConfiguration( Name: GithubAppGroup, DisplayName: "GitHub app", HelpText: "Use a github app for authentication", - Fields: []field.SchemaField{appIDField, appPrivateKeyPath, appPrivateKey, orgField, syncSecrets, omitArchivedRepositories, directCollaboratorsOnly}, - Default: false, + Fields: []field.SchemaField{ + appIDField, appPrivateKeyPath, appPrivateKey, orgField, EnterprisesField, syncSecrets, + enableEnterpriseOwnerProvisioning, omitArchivedRepositories, directCollaboratorsOnly, + }, + Default: false, }, }), ) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index b1bd1017..8bd88fbf 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "net/http" - "net/url" "strings" "time" @@ -127,6 +126,7 @@ type GitHub struct { omitArchivedRepositories bool directCollaboratorsOnly bool enterprises []string + newEnterpriseRoleClients enterpriseClientProvider syncLastActivity bool } @@ -155,10 +155,33 @@ func (gh *GitHub) ResourceSyncers(ctx context.Context) []connectorbuilder.Resour } if len(gh.enterprises) > 0 { - resourceSyncers = append(resourceSyncers, - EnterpriseRoleBuilder(gh.client, gh.appClient, gh.customClient, gh.enterprises), - LicenseBuilder(gh.customClient, gh.enterprises), - ) + // The provisioning-capable syncer is registered only where the role can + // actually be provisioned. The SDK reads CAPABILITY_PROVISION off the + // methods a syncer implements, so registering it everywhere would offer + // the role as requestable to PAT deployments, where every request fails. + if gh.newEnterpriseRoleClients != nil { + resourceSyncers = append(resourceSyncers, EnterpriseRoleProvisioningBuilder( + gh.client, gh.appClient, gh.customClient, gh.enterprises, + gh.newEnterpriseRoleClients, + )) + } else { + resourceSyncers = append(resourceSyncers, EnterpriseRoleBuilder( + gh.client, gh.appClient, gh.customClient, gh.enterprises, nil, + )) + } + // The consumed-licenses API behind this type is PAT-only, so under app + // auth its 403 fails the whole sync and it has never emitted a + // resource. It is dropped only once the operator opts into enterprise + // owner provisioning, which is the setup whose sync it would break. + // + // Keyed on the provider rather than on the credential on purpose: + // with the opt-in off this type has to stay registered, because its + // failure is what stops the run. Dropping it there would let the sync + // complete while reporting no enterprise roles, and C1 deletes the + // stored resources of a type a completed sync did not report. + if gh.newEnterpriseRoleClients == nil { + resourceSyncers = append(resourceSyncers, LicenseBuilder(gh.customClient, gh.enterprises)) + } } return resourceSyncers } @@ -288,9 +311,9 @@ func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annot l := ctxzap.Extract(ctx) _, _, err := gh.customClient.ListEnterpriseConsumedLicenses(ctx, gh.enterprises[0], 1) if err != nil { - l.Debug("baton-github: enterprise features (--enterprises) require a Personal Access Token. "+ - "GitHub App authentication cannot access the consumed-licenses API. "+ - "Either switch to PAT auth or remove the --enterprises flag.", + l.Debug("baton-github: enterprise license data requires a Personal Access Token. "+ + "GitHub App authentication cannot access the consumed-licenses API, "+ + "so the license resource type cannot sync.", zap.Error(err)) } } @@ -340,6 +363,7 @@ func NewLambdaConnector(ctx context.Context, ghc *cfg.Github, cliOpts *cli.Conne } func newWithGithubPAT(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { + enterprises := distinctEnterprises(ghc.Enterprises) ts := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: ghc.Token}, ) @@ -356,7 +380,7 @@ func newWithGithubPAT(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { customClient: customclient.New(ghClient), instanceURL: ghc.InstanceUrl, orgs: ghc.Orgs, - enterprises: ghc.Enterprises, + enterprises: enterprises, graphqlClient: graphqlClient, orgCache: newOrgNameCache(ghClient), syncSecrets: ghc.SyncSecrets, @@ -381,6 +405,7 @@ func appPrivateKeyPEM(ghc *cfg.Github) (string, error) { } func newWithGithubApp(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { + enterprises := distinctEnterprises(ghc.Enterprises) privateKey, err := appPrivateKeyPEM(ghc) if err != nil { return nil, err @@ -457,13 +482,44 @@ func newWithGithubApp(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { return nil, err } + // Enterprise administration needs its own installation token: the org + // installation token above carries no enterprise permissions. Reading the + // owners needs the org token, so both are handed to the client. + // + // Built on first use rather than here, so connector construction and + // Validate do not depend on the enterprise installation and a later sync + // retries the build. It does not narrow the blast radius of a failure: + // the error surfaces from List, which fails the whole sync, and that is + // deliberate — a sync that completed without owners would read to C1 as a + // revoke of every owner assignment. + // + // The construction context is captured separately: the clients are + // memoized for the process lifetime, so the token refresher inside them + // must not hold the context of whichever RPC happened to build them. + connectorCtx := ctx + // Left nil unless the operator opted in. Reading enterprise owners needs + // the app installed on the enterprise account too, which no existing + // deployment has done, and the connector fails the sync when it cannot + // read them. Nil keeps that path inert: enterprise roles then come from + // the consumed-licenses cache exactly as they did before this capability, + // which under app auth means nothing, so an upgrade changes no behaviour + // until it is asked for. + var newEnterpriseRoleClientsFn enterpriseClientProvider + if ghc.EnableEnterpriseOwnerProvisioning { + newEnterpriseRoleClientsFn = func(ctx context.Context) (map[string]*githubEnterpriseAdministratorClient, error) { + return newEnterpriseRoleClients( + ctx, connectorCtx, ghc.InstanceUrl, appClient, jwtts, enterprises, appHTTPClient, ghc.Org) + } + } + gh := &GitHub{ client: ghClient, appClient: appClient, customClient: customclient.New(ghClient), instanceURL: ghc.InstanceUrl, orgs: []string{ghc.Org}, - enterprises: ghc.Enterprises, + enterprises: enterprises, + newEnterpriseRoleClients: newEnterpriseRoleClientsFn, graphqlClient: graphqlClient, orgCache: newOrgNameCache(ghClient), syncSecrets: ghc.SyncSecrets, @@ -474,17 +530,129 @@ func newWithGithubApp(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { return gh, nil } -func newGitHubGraphqlClient(ctx context.Context, instanceURL string, ts oauth2.TokenSource) (*githubv4.Client, error) { - instanceURL = strings.TrimSuffix(instanceURL, "/") +// newEnterpriseRoleClients builds one client per configured enterprise, each +// with that enterprise's own installation token: enterprise and organization +// installations are separate, and an enterprise mutation rejects the org token. +// +// Fails closed when the app is not installed on an enterprise, or when the +// organization does not belong to it. Returning no clients instead would let +// the sync complete while reading no owners, and C1 deletes every resource of +// a type that a completed sync did not report — the Owner role and every +// grant on it. Failing is what stops a sync from being completed at all, so +// an operator who has not installed the app on the enterprise account, or who +// configured several, hears about it instead of losing the assignments. +// +// ctx scopes the discovery requests to the caller. connectorCtx outlives them +// and is what the memoized clients keep for refreshing the installation token, +// which expires after an hour or on the first 401. +func newEnterpriseRoleClients( + ctx context.Context, + connectorCtx context.Context, + instanceURL string, + appClient *github.Client, + jwtTokenSource oauth2.TokenSource, + enterprises []string, + orgHTTPClient *http.Client, + org string, +) (map[string]*githubEnterpriseAdministratorClient, error) { + // Naming the same enterprise twice, which happens when the flag is set in + // both the environment and the command line, is one enterprise. + enterprises = distinctEnterprises(enterprises) + if len(enterprises) == 0 { + return nil, nil + } + // The owners are read through the single configured organization, and an + // organization belongs to exactly one enterprise, so app auth cannot serve + // a list and picking one of them would be a guess. The PAT path does + // support a list. + if len(enterprises) > 1 { + return nil, status.Errorf(codes.FailedPrecondition, + "github-connector: GitHub App authentication serves one enterprise at a time, "+ + "because the owners are read through organization %q, which belongs to a single enterprise; "+ + "%d were configured", org, len(enterprises)) + } + + // NewBaseHttpClient reports a failed cache setup by returning nil, which + // only panics later inside Do. + installationClient := customclient.New(appClient) + if installationClient.BaseHttpClient == nil { + return nil, fmt.Errorf("github-connector: error building the enterprise installation client") + } + + clients := make(map[string]*githubEnterpriseAdministratorClient, len(enterprises)) + for _, enterprise := range enterprises { + installation, _, err := installationClient.GetEnterpriseInstallation(ctx, enterprise) + if err != nil { + // A 404 is the app not being installed on the enterprise, which is + // the misconfiguration worth naming. + if status.Code(err) == codes.NotFound { + return nil, status.Errorf(codes.FailedPrecondition, + "github-connector: GitHub App is not installed on enterprise %q; install it on the enterprise account "+ + "with the Enterprise people read and write permission", enterprise) + } + return nil, err + } + installationID := installation.ID - var enterpriseGqlURL string - if instanceURL != "" && instanceURL != githubDotCom { - parsed, err := url.Parse(instanceURL) + token, err := getInstallationToken(ctx, appClient, installationID) + if err != nil { + return nil, err + } + + ts := newRefreshableTokenSource( + &oauth2.Token{ + AccessToken: token.GetToken(), + Expiry: token.GetExpiresAt().Time, + }, + &appTokenRefresher{ + ctx: connectorCtx, + instanceURL: instanceURL, + installationID: installationID, + jwtTokenSource: jwtTokenSource, + }, + ) + + httpClient, err := newGitHubAppHTTPClient(connectorCtx, ts) if err != nil { return nil, err } - parsed.Path = "/api/graphql" - enterpriseGqlURL = parsed.String() + + client, err := newEnterpriseAdministratorClient(instanceURL, httpClient, orgHTTPClient, org) + if err != nil { + return nil, err + } + if err := client.verifyOrganization(ctx, enterprise); err != nil { + return nil, err + } + if err := client.resolveEnterpriseNodeID(ctx, enterprise); err != nil { + return nil, err + } + clients[enterprise] = client + } + + return clients, nil +} + +// distinctEnterprises folds the slugs, which GitHub matches case-insensitively, +// so a repeated value does not read as several enterprises. +func distinctEnterprises(enterprises []string) []string { + seen := make(map[string]struct{}, len(enterprises)) + distinct := make([]string, 0, len(enterprises)) + for _, enterprise := range enterprises { + key := strings.ToLower(enterprise) + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + distinct = append(distinct, enterprise) + } + return distinct +} + +func newGitHubGraphqlClient(ctx context.Context, instanceURL string, ts oauth2.TokenSource) (*githubv4.Client, error) { + endpoint, err := enterpriseGraphQLEndpoint(instanceURL) + if err != nil { + return nil, err } httpClient, err := uhttp.NewClient(ctx, uhttp.WithLogger(true, ctxzap.Extract(ctx))) @@ -496,10 +664,7 @@ func newGitHubGraphqlClient(ctx context.Context, instanceURL string, ts oauth2.T ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient) tc := oauth2.NewClient(ctx, ts) - if enterpriseGqlURL != "" { - return githubv4.NewEnterpriseClient(enterpriseGqlURL, tc), nil - } - return githubv4.NewClient(tc), nil + return githubv4.NewEnterpriseClient(endpoint.String(), tc), nil } // escapedLineBreaks unescapes LF-, CRLF-, and CR-escaped line breaks (`\r\n`, diff --git a/pkg/connector/default_capabilities.go b/pkg/connector/default_capabilities.go new file mode 100644 index 00000000..f54021e5 --- /dev/null +++ b/pkg/connector/default_capabilities.go @@ -0,0 +1,64 @@ +package connector + +import ( + "context" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + "github.com/conductorone/baton-sdk/pkg/connectorbuilder" +) + +// DefaultCapabilitiesBuilder describes everything this connector can do, for +// the `capabilities` command alone. +// +// That command runs without credentials, so a connector built from real +// config would omit whatever its configuration did not switch on: enterprise +// roles and licenses need --enterprises, API keys need --sync-secrets, the +// usage app and its event feed need --sync-last-activity. The published +// metadata would then understate the connector for every tenant. +// +// It does not change what a running connector reports. C1 refreshes the +// capabilities it stores from the live connector on Validate and on every +// sync, so a deployment that cannot provision enterprise roles still +// advertises sync alone. +type DefaultCapabilitiesBuilder struct{} + +// Metadata delegates to the real implementation so the account creation +// schema behind CAPABILITY_ACCOUNT_PROVISIONING cannot drift from it. +func (d *DefaultCapabilitiesBuilder) Metadata(ctx context.Context) (*v2.ConnectorMetadata, error) { + return (&GitHub{}).Metadata(ctx) +} + +func (d *DefaultCapabilitiesBuilder) Validate(_ context.Context) (annotations.Annotations, error) { + return nil, nil +} + +// ResourceSyncers lists every syncer, including the ones a given deployment +// may not register. The nil dependencies are never dereferenced: this builder +// only answers what methods each type implements. +func (d *DefaultCapabilitiesBuilder) ResourceSyncers(_ context.Context) []connectorbuilder.ResourceSyncerV2 { + return []connectorbuilder.ResourceSyncerV2{ + OrgBuilder(nil, nil, nil, nil, false), + TeamBuilder(nil, nil, false), + UserBuilder(nil, nil, nil, nil, nil, nil), + RepositoryBuilder(nil, nil, false, false), + OrgRoleBuilder(nil, nil), + InvitationBuilder(InvitationBuilderParams{}), + AppBuilder(nil, nil), + APITokenBuilder(nil, nil), + newUsageAppBuilder(), + // The provisioning-capable type: the published metadata describes what + // the connector can do once configured for it. + EnterpriseRoleProvisioningBuilder(nil, nil, nil, nil, nil), + LicenseBuilder(nil, nil), + } +} + +// EventFeeds is what CAPABILITY_EVENT_FEED_V2 is derived from, so omitting it +// would drop the capability from the published metadata the same way a +// missing syncer drops a resource type. +func (d *DefaultCapabilitiesBuilder) EventFeeds(_ context.Context) []connectorbuilder.EventFeed { + return []connectorbuilder.EventFeed{ + newUsageEventFeed(nil, nil), + } +} diff --git a/pkg/connector/default_capabilities_test.go b/pkg/connector/default_capabilities_test.go new file mode 100644 index 00000000..0a122d2f --- /dev/null +++ b/pkg/connector/default_capabilities_test.go @@ -0,0 +1,59 @@ +package connector + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +// The default builder's list is hand-maintained, and anything a real +// deployment can register but it omits is silently dropped from the published +// metadata -- which is the exact understatement the builder exists to fix. +// Two GitHub values are needed to cover everything: the enterprise role +// client provider decides between the read-only and the provisioning syncer, +// and its absence is also what keeps the license type registered. +func TestDefaultCapabilitiesCoverEverySyncerADeploymentCanRegister(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + advertised := make(map[string]bool) + for _, syncer := range (&DefaultCapabilitiesBuilder{}).ResourceSyncers(ctx) { + advertised[syncer.ResourceType(ctx).GetId()] = true + } + + for _, gh := range []*GitHub{ + {syncSecrets: true, syncLastActivity: true, enterprises: []string{testEnterprise}}, + {syncSecrets: true, syncLastActivity: true, enterprises: []string{testEnterprise}, + newEnterpriseRoleClients: func(context.Context) (map[string]*githubEnterpriseAdministratorClient, error) { + return nil, nil + }}, + } { + for _, syncer := range gh.ResourceSyncers(ctx) { + id := syncer.ResourceType(ctx).GetId() + require.True(t, advertised[id], + "resource type %q is registered by a real deployment but missing from DefaultCapabilitiesBuilder", id) + } + } +} + +// CAPABILITY_EVENT_FEED_V2 is derived from the registered feeds, so a feed the +// default builder does not list disappears from the published metadata. +func TestDefaultCapabilitiesCoverEveryEventFeed(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + advertised := make(map[string]bool) + for _, feed := range (&DefaultCapabilitiesBuilder{}).EventFeeds(ctx) { + advertised[feed.EventFeedMetadata(ctx).GetId()] = true + } + + gh := &GitHub{syncLastActivity: true} + for _, feed := range gh.EventFeeds(ctx) { + id := feed.EventFeedMetadata(ctx).GetId() + require.True(t, advertised[id], + "event feed %q is registered by a real deployment but missing from DefaultCapabilitiesBuilder", id) + } +} diff --git a/pkg/connector/enterprise_administrator_client.go b/pkg/connector/enterprise_administrator_client.go new file mode 100644 index 00000000..128aa170 --- /dev/null +++ b/pkg/connector/enterprise_administrator_client.go @@ -0,0 +1,694 @@ +package connector + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "strings" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + "github.com/conductorone/baton-sdk/pkg/uhttp" + "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "github.com/shurcooL/githubv4" + "go.uber.org/zap" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/timestamppb" +) + +const ( + // GraphQL variable names, shared by the query text and the variable map so + // the two cannot drift: a typo'd map key reaches the API as a missing + // variable rather than failing to compile. + enterpriseSlugVariable = "slug" + enterpriseLoginVariable = "login" + enterpriseOrgVariable = "org" + enterpriseRoleVariable = "role" + enterpriseQueryVariable = "query" + enterpriseFirstVariable = "first" + enterpriseAfterVariable = "after" + + enterpriseGraphQLPath = "/api/graphql" + githubDotComGraphQL = "https://api.github.com/graphql" + enterpriseRateLimitField = "rateLimit" + + // GitHub caps every connection used here at 100 per page. + enterpriseOwnerPageSize = 100 + enterpriseOrganizationPageSize = 100 + enterpriseMemberPageSize = 100 + // One aliased invitation lookup per member of a page. + enterpriseInvitationBatchSize = enterpriseMemberPageSize + // Bounds the walks that happen inside one call rather than across page + // tokens, so a mispaginating API cannot pin a provisioning task. + enterpriseMaxPages = 1000 +) + +// enterpriseOwnerState is what the connector can observe about a user's Owner +// access: whether they hold the role today, and the invitation that would give +// it to them once they accept it. +type enterpriseOwnerState struct { + enterpriseID string + isOwner bool + pendingInvitationID string +} + +// githubEnterpriseAdministratorClient reads and writes the built-in Owner role +// of one enterprise. It needs both of the app's installations, because GitHub +// splits the data across them: +// +// - Enterprise.ownerInfo, which holds admins and pendingAdminInvitations, +// resolves to null for an installation token, so the owners are read from +// Organization.enterpriseOwners with the organization token. The +// enterprise token gets FORBIDDEN on any organization field. +// - The invitation lookup and every mutation are enterprise fields, so they +// go through the enterprise token. +// +// Enterprise.members(role: OWNER) is deliberately not used: that role is +// EnterpriseUserAccountMembershipRole, whose OWNER means "owner of an +// organization in the enterprise", not owner of the enterprise account. +type githubEnterpriseAdministratorClient struct { + enterpriseClient *githubv4.Client + orgClient *githubv4.Client + org string + // Immutable for a given slug, so it is resolved once at construction + // rather than on each Grant and Revoke. + enterpriseNodeID string + // batchClient serves the aliased invitation lookup, and omits + // enterpriseGraphQLTransport because that batch always reports NOT_FOUND + // entries the classifier would read as a failure. + endpoint *url.URL + batchClient *uhttp.BaseHttpClient +} + +// newEnterpriseAdministratorClient returns the client for one enterprise +// installation, with a GraphQL client per token because the two installations +// are separate credentials. +// +// The HTTP clients arrive unwrapped because that is what both consumers take: +// githubv4 builds its own client over one, uhttp wraps one. They already carry +// the installation token, its 401 refresh, and uhttp's transport underneath. +func newEnterpriseAdministratorClient( + instanceURL string, + enterpriseHTTPClient *http.Client, + orgHTTPClient *http.Client, + org string, +) (*githubEnterpriseAdministratorClient, error) { + endpoint, err := enterpriseGraphQLEndpoint(instanceURL) + if err != nil { + return nil, err + } + + // NewBaseHttpClient reports a failed cache setup by returning nil, which + // only panics later inside Do. + batchClient := uhttp.NewBaseHttpClient(enterpriseHTTPClient) + if batchClient == nil { + return nil, fmt.Errorf("baton-github: error building the enterprise GraphQL batch client") + } + + return &githubEnterpriseAdministratorClient{ + enterpriseClient: newEnterpriseGraphQLClient(endpoint.String(), enterpriseHTTPClient), + orgClient: newEnterpriseGraphQLClient(endpoint.String(), orgHTTPClient), + org: org, + endpoint: endpoint, + batchClient: batchClient, + }, nil +} + +// newEnterpriseGraphQLClient returns a GraphQL client that classifies both the +// HTTP status and the errors[] GitHub returns alongside an HTTP 200. Both +// layers are needed, because the SDK only backs off on a typed Unavailable and +// a rate limit arrives as a successful status. +// +// The connector's shared GraphQL client keeps only the status classifier: +// user.go detects enterprise SAML by matching the text of its error. +func newEnterpriseGraphQLClient(endpoint string, httpClient *http.Client) *githubv4.Client { + base := httpClient.Transport + if base == nil { + base = http.DefaultTransport + } + + return githubv4.NewEnterpriseClient(endpoint, &http.Client{ + Timeout: httpClient.Timeout, + Transport: &enterpriseGraphQLTransport{ + base: &statusClassifyingTransport{base: base}, + }, + }) +} + +// enterpriseGraphQLEndpoint returns the GraphQL URL of the instance, which is +// api.github.com for GitHub.com and /api/graphql on a self-hosted host. +// +// The trailing slash is trimmed before the comparison, so "https://github.com/" +// resolves to GitHub.com rather than to a /api/graphql path on that host. +func enterpriseGraphQLEndpoint(instanceURL string) (*url.URL, error) { + instanceURL = strings.TrimSuffix(instanceURL, "/") + if instanceURL == "" || instanceURL == githubDotCom { + return url.Parse(githubDotComGraphQL) + } + + gqlURL, err := url.Parse(instanceURL) + if err != nil { + return nil, err + } + gqlURL.Path = enterpriseGraphQLPath + + return gqlURL, nil +} + +type graphQLRateLimit struct { + Limit githubv4.Int + Remaining githubv4.Int + ResetAt githubv4.DateTime +} + +// annotations returns the remaining GraphQL budget, or nil when the response +// carried no rateLimit block: the zero value would otherwise be reported as an +// exhausted budget with no reset time. +func (r graphQLRateLimit) annotations() annotations.Annotations { + if r.Limit == 0 && r.ResetAt.IsZero() { + return nil + } + + rateLimit := &v2.RateLimitDescription{ + Status: v2.RateLimitDescription_STATUS_OK, + Limit: int64(r.Limit), + Remaining: int64(r.Remaining), + } + if r.Remaining <= 0 { + rateLimit.Status = v2.RateLimitDescription_STATUS_OVERLIMIT + } + if !r.ResetAt.IsZero() { + rateLimit.ResetAt = timestamppb.New(r.ResetAt.Time) + } + + return annotations.New(rateLimit) +} + +// enterpriseOwnersQuery reads one page of the enterprise account's owners +// through the organization the app is installed on. +type enterpriseOwnersQuery struct { + Organization struct { + EnterpriseOwners struct { + Nodes []struct { + DatabaseID githubv4.Int + Login githubv4.String + } + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + } `graphql:"enterpriseOwners(first: $first, after: $after)"` + } `graphql:"organization(login: $org)"` + RateLimit graphQLRateLimit +} + +// enterpriseUser is a user account as the enterprise reports it, whether as an +// owner or as a plain member. +type enterpriseUser struct { + databaseID int64 + login string +} + +// owners returns one page of the users who currently own the enterprise +// account. +func (c *githubEnterpriseAdministratorClient) owners( + ctx context.Context, + after *githubv4.String, +) ([]enterpriseUser, string, annotations.Annotations, error) { + var query enterpriseOwnersQuery + err := c.orgClient.Query(ctx, &query, map[string]any{ + enterpriseOrgVariable: githubv4.String(c.org), + enterpriseFirstVariable: githubv4.Int(enterpriseOwnerPageSize), + enterpriseAfterVariable: after, + }) + if err != nil { + return nil, "", nil, fmt.Errorf("baton-github: error listing enterprise owners of org %s: %w", c.org, err) + } + + owners := make([]enterpriseUser, 0, len(query.Organization.EnterpriseOwners.Nodes)) + for _, node := range query.Organization.EnterpriseOwners.Nodes { + owners = append(owners, enterpriseUser{ + databaseID: int64(node.DatabaseID), + login: string(node.Login), + }) + } + + nextCursor := "" + if query.Organization.EnterpriseOwners.PageInfo.HasNextPage { + nextCursor = string(query.Organization.EnterpriseOwners.PageInfo.EndCursor) + } + + return owners, nextCursor, query.RateLimit.annotations(), nil +} + +// resolveEnterpriseNodeID looks up the node ID every enterprise mutation takes +// and stores it on the client. It is called once at construction: the ID is +// immutable for a given slug, and resolving it per operation would add two +// requests to every Grant and Revoke, which each read OwnerState twice. +// +// It doubles as the check that the enterprise is visible to this installation. +func (c *githubEnterpriseAdministratorClient) resolveEnterpriseNodeID(ctx context.Context, enterprise string) error { + var query struct { + Enterprise struct { + ID githubv4.String + } `graphql:"enterprise(slug: $slug)"` + } + err := c.enterpriseClient.Query(ctx, &query, map[string]any{ + enterpriseSlugVariable: githubv4.String(enterprise), + }) + if err != nil { + return fmt.Errorf("baton-github: error getting enterprise %s: %w", enterprise, err) + } + if query.Enterprise.ID == "" { + return fmt.Errorf("baton-github: enterprise %s is not visible to the GitHub App", enterprise) + } + c.enterpriseNodeID = string(query.Enterprise.ID) + + return nil +} + +// enterpriseOrganizationsQuery reads one page of an enterprise's +// organizations, narrowed by a search term. +type enterpriseOrganizationsQuery struct { + Enterprise struct { + Organizations struct { + Nodes []struct { + Login githubv4.String + } + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + } `graphql:"organizations(first: $first, after: $after, query: $query)"` + } `graphql:"enterprise(slug: $slug)"` +} + +// verifyOrganization checks that the organization the owners are read through +// belongs to this enterprise; otherwise the connector would report another +// enterprise's owners as owners of this one. +// +// organizations(query:) is a substring search rather than an exact filter, so +// every page is read before concluding the organization is not there. GitHub +// offers no direct way to ask an organization which enterprise owns it. +func (c *githubEnterpriseAdministratorClient) verifyOrganization(ctx context.Context, enterprise string) error { + var after *githubv4.String + walked := false + for page := 0; page < enterpriseMaxPages; page++ { + var query enterpriseOrganizationsQuery + err := c.enterpriseClient.Query(ctx, &query, map[string]any{ + enterpriseSlugVariable: githubv4.String(enterprise), + enterpriseFirstVariable: githubv4.Int(enterpriseOrganizationPageSize), + enterpriseAfterVariable: after, + enterpriseQueryVariable: githubv4.String(c.org), + }) + if err != nil { + return fmt.Errorf("baton-github: error listing organizations of enterprise %s: %w", enterprise, err) + } + + for _, node := range query.Enterprise.Organizations.Nodes { + if strings.EqualFold(string(node.Login), c.org) { + return nil + } + } + + if !query.Enterprise.Organizations.PageInfo.HasNextPage { + walked = true + break + } + after = githubv4.NewString(query.Enterprise.Organizations.PageInfo.EndCursor) + } + // Running out of budget is not the same answer as reading every page and + // not finding the organization: reporting the membership error there would + // send the operator to fix a membership that is already correct. + if !walked { + return status.Errorf(codes.Internal, + "baton-github: gave up looking for organization %s in enterprise %s after %d pages", + c.org, enterprise, enterpriseMaxPages) + } + + // FailedPrecondition so the caller remembers it: only a configuration + // change can make this organization belong to that enterprise. + return status.Errorf(codes.FailedPrecondition, + "baton-github: organization %s does not belong to enterprise %s, so its owners cannot be synced", + c.org, enterprise) +} + +// enterpriseMembersQuery reads one page of the enterprise's member accounts. +// members is a union: an enterprise with Enterprise Managed Users returns +// EnterpriseUserAccount, a regular enterprise can also return User, so both +// shapes are selected and each field falls back to the other branch. +type enterpriseMembersQuery struct { + Enterprise struct { + Members struct { + Nodes []struct { + EnterpriseUserAccount struct { + Login githubv4.String + User struct { + DatabaseID githubv4.Int + Login githubv4.String + } + } `graphql:"... on EnterpriseUserAccount"` + User struct { + DatabaseID githubv4.Int + Login githubv4.String + } `graphql:"... on User"` + } + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + } `graphql:"members(first: $first, after: $after)"` + } `graphql:"enterprise(slug: $slug)"` + RateLimit graphQLRateLimit +} + +// members returns one page of the enterprise's member accounts and the cursor +// of the next page, skipping any node without a database ID because guessing +// one would re-key the identity on the next sync. +func (c *githubEnterpriseAdministratorClient) members( + ctx context.Context, + enterprise string, + after *githubv4.String, +) ([]enterpriseUser, string, annotations.Annotations, error) { + var query enterpriseMembersQuery + err := c.enterpriseClient.Query(ctx, &query, map[string]any{ + enterpriseSlugVariable: githubv4.String(enterprise), + enterpriseFirstVariable: githubv4.Int(enterpriseMemberPageSize), + enterpriseAfterVariable: after, + }) + if err != nil { + return nil, "", nil, fmt.Errorf("baton-github: error listing members of enterprise %s: %w", enterprise, err) + } + + members := make([]enterpriseUser, 0, len(query.Enterprise.Members.Nodes)) + for _, node := range query.Enterprise.Members.Nodes { + member := enterpriseUser{ + databaseID: int64(node.EnterpriseUserAccount.User.DatabaseID), + login: string(node.EnterpriseUserAccount.Login), + } + if member.databaseID == 0 { + member.databaseID = int64(node.User.DatabaseID) + } + if member.login == "" { + member.login = string(node.User.Login) + } + if member.databaseID == 0 || member.login == "" { + // Skipping costs this member their pending invitation lookup, so + // their Owner grant would never appear. Say so rather than drop + // them silently. + ctxzap.Extract(ctx).Debug("baton-github: skipping an enterprise member with no database ID or login", + zap.String("enterprise", enterprise), + zap.Int64("database_id", member.databaseID), + zap.String("login", member.login), + ) + continue + } + members = append(members, member) + } + + nextCursor := "" + if query.Enterprise.Members.PageInfo.HasNextPage { + nextCursor = string(query.Enterprise.Members.PageInfo.EndCursor) + } + + return members, nextCursor, query.RateLimit.annotations(), nil +} + +// pendingOwnerInvitations returns the pending Owner invitation of each given +// login that has one, keyed by login, resolved in a single request. +// +// Invitations are not enumerable for an installation token, because +// Enterprise.ownerInfo resolves to null, so they can only be asked for one +// login at a time. Aliasing collapses that into one request, which GitHub +// charges a single rate-limit point. +// +// Logins travel as variables and only generated alias names reach the query +// text, so a login cannot alter the query. A login with no invitation answers +// with a NOT_FOUND entry, so those are dropped before the remaining errors are +// classified: left in, they would claim the code for the whole response, and +// NOT_FOUND is the one code the SDK downgrades to a warning. The rate limit is +// returned on the failure paths too, since that is when C1 needs it. +func (c *githubEnterpriseAdministratorClient) pendingOwnerInvitations( + ctx context.Context, + enterprise string, + logins []string, +) (map[string]string, annotations.Annotations, error) { + if len(logins) == 0 { + return map[string]string{}, nil, nil + } + if len(logins) > enterpriseInvitationBatchSize { + return nil, nil, fmt.Errorf( + "baton-github: pending owner invitation lookup takes at most %d logins, got %d", + enterpriseInvitationBatchSize, len(logins)) + } + + declarations := []string{"$" + enterpriseSlugVariable + ":String!", "$" + enterpriseRoleVariable + ":EnterpriseAdministratorRole!"} + selections := make([]string, 0, len(logins)) + variables := map[string]any{ + enterpriseSlugVariable: enterprise, + enterpriseRoleVariable: string(githubv4.EnterpriseAdministratorRoleOwner), + } + aliasLogin := make(map[string]string, len(logins)) + for i, login := range logins { + alias := fmt.Sprintf("i%d", i) + variable := fmt.Sprintf("l%d", i) + aliasLogin[alias] = login + variables[variable] = login + declarations = append(declarations, "$"+variable+":String!") + selections = append(selections, fmt.Sprintf( + "%s: enterpriseAdministratorInvitation(enterpriseSlug: $%s, userLogin: $%s, role: $%s){id}", + alias, enterpriseSlugVariable, variable, enterpriseRoleVariable)) + } + + query := fmt.Sprintf("query(%s){%s %s{limit remaining resetAt}}", + strings.Join(declarations, ","), strings.Join(selections, " "), enterpriseRateLimitField) + + aliases, graphQLErrors, err := c.doGraphQL(ctx, query, variables) + + var rateLimit graphQLRateLimit + if raw, ok := aliases[enterpriseRateLimitField]; ok { + if unmarshalErr := json.Unmarshal(raw, &rateLimit); unmarshalErr != nil { + return nil, nil, fmt.Errorf("baton-github: error decoding the rate limit of enterprise %s: %w", enterprise, unmarshalErr) + } + } + annos := rateLimit.annotations() + if err != nil { + return nil, annos, fmt.Errorf("baton-github: error listing pending owner invitations of enterprise %s: %w", enterprise, err) + } + unexpected := make([]graphQLError, 0, len(graphQLErrors)) + for _, graphQLErr := range graphQLErrors { + if graphQLErrorType(graphQLErr) != graphQLErrorNotFound { + unexpected = append(unexpected, graphQLErr) + } + } + if len(unexpected) > 0 { + return nil, annos, status.Errorf(graphQLErrorsCode(unexpected), + "baton-github: error listing pending owner invitations of enterprise %s: %s", + enterprise, unexpected[0].Message) + } + + invitations := make(map[string]string, len(aliases)) + for alias, raw := range aliases { + login, ok := aliasLogin[alias] + if !ok { + continue + } + var node struct { + ID string `json:"id"` + } + if err := json.Unmarshal(raw, &node); err != nil || node.ID == "" { + continue + } + invitations[login] = node.ID + } + + return invitations, annos, nil +} + +// doGraphQL runs a query whose selection set is built at runtime, which the +// typed client cannot express. It returns the top-level fields undecoded plus +// the errors array, so the caller decides which fields to read and which +// errors are expected. A non-2xx arrives already mapped onto a gRPC code by +// uhttp, so its error is returned unwrapped. +func (c *githubEnterpriseAdministratorClient) doGraphQL( + ctx context.Context, + query string, + variables map[string]any, +) (map[string]json.RawMessage, []graphQLError, error) { + req, err := c.batchClient.NewRequest(ctx, http.MethodPost, c.endpoint, + uhttp.WithContentTypeJSONHeader(), + uhttp.WithAcceptJSONHeader(), + uhttp.WithJSONBody(map[string]any{"query": query, "variables": variables}), + ) + if err != nil { + return nil, nil, fmt.Errorf("creating GraphQL request: %w", err) + } + + var envelope graphQLEnvelope + resp, err := c.batchClient.Do(req, uhttp.WithJSONResponse(&envelope)) + if err != nil { + if resp != nil { + _ = resp.Body.Close() + } + return nil, nil, err + } + defer resp.Body.Close() + + return envelope.Data, envelope.Errors, nil +} + +// pendingOwnerInvitation returns the ID of the pending Owner invitation for a +// login, or an empty string when there is none. GitHub reports a login without +// an invitation as NOT_FOUND rather than as a null field. +func (c *githubEnterpriseAdministratorClient) pendingOwnerInvitation( + ctx context.Context, + enterprise string, + login string, +) (string, error) { + var query struct { + EnterpriseAdministratorInvitation struct { + ID githubv4.String + } `graphql:"enterpriseAdministratorInvitation(enterpriseSlug: $slug, userLogin: $login, role: $role)"` + } + err := c.enterpriseClient.Query(ctx, &query, map[string]any{ + enterpriseSlugVariable: githubv4.String(enterprise), + enterpriseLoginVariable: githubv4.String(login), + enterpriseRoleVariable: githubv4.EnterpriseAdministratorRoleOwner, + }) + if err != nil { + if status.Code(err) == codes.NotFound { + return "", nil + } + return "", fmt.Errorf("baton-github: error getting owner invitation for %s: %w", login, err) + } + + return string(query.EnterpriseAdministratorInvitation.ID), nil +} + +// OwnerState reports whether a login owns the enterprise account and whether +// an invitation for it is still pending, together with the remaining budget. +// +// Both facts are resolved even for an active owner, because Revoke acts on +// each separately: stopping at the role would let a stale invitation survive +// the demotion and then fail the verification that follows it. +// +// The owners connection does take a query argument, but it searches profile +// text rather than filtering on the login: it also matches the display name. +// A search index carries no freshness guarantee, and Grant and Revoke call +// this again right after their mutation to confirm it landed, so an empty +// result cannot be trusted to mean "not an owner" — on Revoke that reading +// would report GrantAlreadyRevoked while the user keeps the role. The pages +// are walked and matched on the login instead, which in practice is one +// request: owners are a small set. +func (c *githubEnterpriseAdministratorClient) OwnerState( + ctx context.Context, + enterprise string, + login string, +) (enterpriseOwnerState, annotations.Annotations, error) { + state := enterpriseOwnerState{enterpriseID: c.enterpriseNodeID} + + var annos annotations.Annotations + var after *githubv4.String + walked := false + for page := 0; page < enterpriseMaxPages; page++ { + owners, nextCursor, pageAnnos, err := c.owners(ctx, after) + if err != nil { + return state, annos, err + } + // The freshest budget, not one descriptor per page. + if len(pageAnnos) > 0 { + annos = pageAnnos + } + for _, owner := range owners { + if strings.EqualFold(owner.login, login) { + state.isOwner = true + break + } + } + if state.isOwner || nextCursor == "" { + walked = true + break + } + after = githubv4.NewString(githubv4.String(nextCursor)) + } + // Falling through the bound would report "not an owner" for someone the + // walk never finished reading, which Revoke would answer with + // GrantAlreadyRevoked while they still hold the role. + if !walked { + return state, annos, status.Errorf(codes.Internal, + "baton-github: gave up reading the owners of enterprise %s after %d pages", + enterprise, enterpriseMaxPages) + } + + invitationID, err := c.pendingOwnerInvitation(ctx, enterprise, login) + if err != nil { + return state, annos, err + } + state.pendingInvitationID = invitationID + + return state, annos, nil +} + +// UpdateRole changes the role of someone who already administers the +// enterprise. It cannot promote a plain member. +func (c *githubEnterpriseAdministratorClient) UpdateRole( + ctx context.Context, + enterpriseID string, + login string, + role githubv4.EnterpriseAdministratorRole, +) error { + // GraphQL rejects a mutation without a selection set, and rateLimit exists + // only on Query, so every mutation selects clientMutationId. + var mutation struct { + UpdateEnterpriseAdministratorRole struct { + ClientMutationID githubv4.String + } `graphql:"updateEnterpriseAdministratorRole(input: $input)"` + } + input := githubv4.UpdateEnterpriseAdministratorRoleInput{ + EnterpriseID: githubv4.ID(enterpriseID), + Login: githubv4.String(login), + Role: role, + } + if err := c.enterpriseClient.Mutate(ctx, &mutation, input, nil); err != nil { + return fmt.Errorf("baton-github: error setting enterprise role of %s to %s: %w", login, role, err) + } + + return nil +} + +// InviteOwner sends the Owner invitation a member has to accept. +func (c *githubEnterpriseAdministratorClient) InviteOwner(ctx context.Context, enterpriseID string, login string) error { + var mutation struct { + InviteEnterpriseAdmin struct { + ClientMutationID githubv4.String + } `graphql:"inviteEnterpriseAdmin(input: $input)"` + } + role := githubv4.EnterpriseAdministratorRoleOwner + input := githubv4.InviteEnterpriseAdminInput{ + EnterpriseID: githubv4.ID(enterpriseID), + Invitee: githubv4.NewString(githubv4.String(login)), + Role: &role, + } + if err := c.enterpriseClient.Mutate(ctx, &mutation, input, nil); err != nil { + return fmt.Errorf("baton-github: error inviting %s as enterprise owner: %w", login, err) + } + + return nil +} + +func (c *githubEnterpriseAdministratorClient) CancelInvitation(ctx context.Context, invitationID string) error { + var mutation struct { + CancelEnterpriseAdminInvitation struct { + ClientMutationID githubv4.String + } `graphql:"cancelEnterpriseAdminInvitation(input: $input)"` + } + input := githubv4.CancelEnterpriseAdminInvitationInput{InvitationID: githubv4.ID(invitationID)} + if err := c.enterpriseClient.Mutate(ctx, &mutation, input, nil); err != nil { + return fmt.Errorf("baton-github: error cancelling enterprise owner invitation: %w", err) + } + + return nil +} diff --git a/pkg/connector/enterprise_installations_test.go b/pkg/connector/enterprise_installations_test.go new file mode 100644 index 00000000..c872bc26 --- /dev/null +++ b/pkg/connector/enterprise_installations_test.go @@ -0,0 +1,273 @@ +package connector + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/google/go-github/v69/github" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + cfg "github.com/conductorone/baton-github/pkg/config" + "github.com/conductorone/baton-github/pkg/customclient" + "github.com/conductorone/baton-sdk/pkg/connectorbuilder" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" +) + +// newGitHubAPITestClient points the client at a test server through BaseURL +// alone. Nothing rewrites the host, so a customclient endpoint that ignored +// BaseURL would leave the test reaching for api.github.com. +func newGitHubAPITestClient(t *testing.T, handler http.Handler) *github.Client { + t.Helper() + + return newGitHubAPITestClientAt(t, handler, "/") +} + +func newGitHubAPITestClientAt(t *testing.T, handler http.Handler, basePath string) *github.Client { + t.Helper() + + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + + baseURL, err := url.Parse(srv.URL + basePath) + require.NoError(t, err) + + client := github.NewClient(srv.Client()) + client.BaseURL = baseURL + + return client +} + +func TestGetEnterpriseInstallation(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := newGitHubAPITestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/enterprises/example-enterprise/installation", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "id": int64(22), + })) + })) + + installation, _, err := customclient.New(client).GetEnterpriseInstallation(ctx, "example-enterprise") + require.NoError(t, err) + require.Equal(t, int64(22), installation.ID) +} + +// A slug is operator-supplied, so it has to survive as one path segment +// instead of being pasted into the URL. +func TestGetEnterpriseInstallationEscapesTheSlug(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := newGitHubAPITestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/enterprises/a%2Fb/installation", r.URL.EscapedPath()) + w.WriteHeader(http.StatusNotFound) + })) + + _, _, err := customclient.New(client).GetEnterpriseInstallation(ctx, "a/b") + require.Error(t, err) +} + +// On GitHub Enterprise Server, WithEnterpriseURLs puts the REST API under +// /api/v3. Asking api.github.com instead would report the app as uninstalled +// on an enterprise that does have it. +func TestGetEnterpriseInstallationUsesTheInstanceBaseURL(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := newGitHubAPITestClientAt(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/api/v3/enterprises/ghes-enterprise/installation", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "id": int64(33), + })) + }), "/api/v3/") + + installation, _, err := customclient.New(client).GetEnterpriseInstallation(ctx, "ghes-enterprise") + require.NoError(t, err) + require.Equal(t, int64(33), installation.ID) +} + +// Without the opt-in the enterprise owner path is left unwired, and this is +// what that has to mean: the same answer the connector gave before the +// capability existed. Reading owners needs the app installed on the enterprise +// account, which no existing deployment has done, and the connector fails the +// sync when it cannot read them — so if the unwired path did anything else, +// upgrading would turn a working sync into a failing one for everyone already +// passing --enterprises under app auth. Under app auth that answer is no +// resources, because the consumed-licenses API it falls back to is PAT-only +// and answers 403. +func TestEnterpriseRoleListIsInertWithoutTheOptIn(t *testing.T) { + t.Parallel() + + ctx := context.Background() + apiClient := newGitHubAPITestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"message": "Resource not accessible by integration"})) + })) + + // nil provider is how newWithGithubApp leaves it when the flag is off. + builder := EnterpriseRoleBuilder(apiClient, apiClient, customclient.New(apiClient), + []string{"example-enterprise"}, nil) + + resources, _, err := builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Empty(t, resources) +} + +// GitHub answers 404 when the app is not installed on the enterprise. Failing +// is what protects the data: C1 deletes every resource of a type that a +// completed sync did not report, so letting the sync finish while reading no +// owners would drop the Owner role and every grant on it. An error keeps the +// sync from completing at all. +func TestNewEnterpriseRoleClientsRequiresEnterpriseInstall(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := newGitHubAPITestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/enterprises/example-enterprise/installation", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"message": "Not Found"})) + })) + + _, err := newEnterpriseRoleClients( + ctx, + ctx, + "https://github.com", + client, + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "unused"}), + []string{"example-enterprise"}, + nil, + "example-org", + ) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.Contains(t, err.Error(), `not installed on enterprise "example-enterprise"`) + require.NotContains(t, err.Error(), "personal access token") +} + +// The SDK derives CAPABILITY_PROVISION by type-asserting each registered +// syncer, so the split between the two types is the whole mechanism keeping +// provisioning off PAT deployments. Nothing else fails if it is undone: moving +// Grant and Revoke back onto the read-only type, or dropping the check in +// ResourceSyncers, would re-advertise provisioning to PAT with every other +// test still green. +func TestResourceSyncersOfferProvisioningOnlyWhenItWorks(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + provider enterpriseClientProvider + provisioned bool + }{ + {"pat or opt-in off", nil, false}, + {"app auth opted in", func(context.Context) (map[string]*githubEnterpriseAdministratorClient, error) { + return nil, nil + }, true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + gh := &GitHub{ + enterprises: []string{testEnterprise}, + newEnterpriseRoleClients: tc.provider, + } + + var found bool + for _, syncer := range gh.ResourceSyncers(context.Background()) { + if syncer.ResourceType(context.Background()).GetId() != resourceTypeEnterpriseRole.Id { + continue + } + found = true + _, provisions := syncer.(connectorbuilder.ResourceProvisionerV2Limited) + require.Equal(t, tc.provisioned, provisions) + } + require.True(t, found, "enterprise_role must be synced either way") + }) + } +} + +// Setting the flag in both the environment and the command line lands the +// same enterprise twice, which is still one enterprise. Counting raw entries +// would reject a configuration the operator wrote correctly, with a message +// naming a number they never chose. +func TestNewEnterpriseRoleClientsFoldsRepeatedEnterprises(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := newGitHubAPITestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/enterprises/example-enterprise/installation", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"message": "Not Found"})) + })) + + _, err := newEnterpriseRoleClients( + ctx, + ctx, + "https://github.com", + client, + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "unused"}), + []string{"example-enterprise", "Example-Enterprise"}, + nil, + "example-org", + ) + // Reaches the installation lookup rather than the several-enterprises + // guard, which is what proves the fold happened. + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.Contains(t, err.Error(), "not installed on enterprise") + require.NotContains(t, err.Error(), "one enterprise at a time") +} + +func TestConnectorFoldsRepeatedEnterprisesBeforeBuildingSyncers(t *testing.T) { + t.Parallel() + + gh, err := newWithGithubPAT(context.Background(), &cfg.Github{ + Token: "test-token", + InstanceUrl: githubDotCom, + Enterprises: []string{"example-enterprise", "example-enterprise"}, + }) + require.NoError(t, err) + require.Equal(t, []string{"example-enterprise"}, gh.enterprises) + + resources, _, err := appList( + gh.enterprises, + map[string]*githubEnterpriseAdministratorClient{"example-enterprise": nil}, + ) + require.NoError(t, err) + require.Len(t, resources, 1) +} + +// The owners are read through one organization, which belongs to one +// enterprise, so app auth cannot serve a list and picking one would be a +// guess. This fails for the same reason as a missing installation: a sync that +// completes without owners costs the operator every owner grant. +func TestNewEnterpriseRoleClientsRejectsSeveralEnterprises(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := newGitHubAPITestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("no request expected, got %s", r.URL.Path) + })) + + _, err := newEnterpriseRoleClients( + ctx, + ctx, + "https://github.com", + client, + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "unused"}), + []string{"one-enterprise", "another-enterprise"}, + nil, + "example-org", + ) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.Contains(t, err.Error(), "one enterprise at a time") +} diff --git a/pkg/connector/enterprise_role.go b/pkg/connector/enterprise_role.go index a6e79aab..74289bac 100644 --- a/pkg/connector/enterprise_role.go +++ b/pkg/connector/enterprise_role.go @@ -2,23 +2,44 @@ package connector import ( "context" - "errors" "fmt" + "strconv" "strings" "sync" "github.com/conductorone/baton-github/pkg/customclient" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + "github.com/conductorone/baton-sdk/pkg/connectorbuilder" + "github.com/conductorone/baton-sdk/pkg/pagination" "github.com/conductorone/baton-sdk/pkg/types/entitlement" "github.com/conductorone/baton-sdk/pkg/types/grant" resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" "github.com/google/go-github/v69/github" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "github.com/shurcooL/githubv4" "go.uber.org/zap" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) +const ( + enterpriseRoleAssigned = "assigned" + enterpriseRoleOwner = "Owner" + + // Sync phases of the Owner grants. The invitations cannot be listed, so + // they are resolved from the members in a second pass over the same token. + enterpriseOwnersPhase = "enterprise-owners" + enterprisePendingPhase = "enterprise-pending-invitations" + + // UNAFFILIATED demotes an administrator while keeping their enterprise + // membership; removeEnterpriseAdmin would evict them from the enterprise. + enterpriseAdministratorRoleUnaffiliated githubv4.EnterpriseAdministratorRole = "UNAFFILIATED" +) + +// enterpriseClientProvider builds the per-enterprise administration clients. +type enterpriseClientProvider func(ctx context.Context) (map[string]*githubEnterpriseAdministratorClient, error) + type enterpriseRoleResourceType struct { resourceType *v2.ResourceType client *github.Client @@ -27,12 +48,65 @@ type enterpriseRoleResourceType struct { enterprises []string roleUsersCache map[string][]string mu *sync.Mutex + // newEnterpriseClients builds the per-enterprise administration clients. + // It is nil under PAT auth. + newEnterpriseClients enterpriseClientProvider + // enterpriseClients is keyed by enterprise slug and memoized after the + // first successful build. A build error is returned rather than stored, + // so the sync fails instead of completing without owners and the next + // call tries again. + enterpriseClients map[string]*githubEnterpriseAdministratorClient } func (o *enterpriseRoleResourceType) ResourceType(_ context.Context) *v2.ResourceType { return o.resourceType } +// clients returns the per-enterprise administration clients, building them on +// first use and memoizing them once they are built. +// +// Only success is remembered. A failure is retried on the next call, which +// costs one discovery attempt on a path that is already failing the sync, and +// buys two things: a startup blip does not disable this resource type for the +// lifetime of the process, and an operator who installs the app or restores +// the permission is picked up by the next sync instead of needing a restart. +// GitHub answers 404 for an uninstalled app, a revoked permission and a typo +// alike, so no error here can be trusted to be permanent. +func (o *enterpriseRoleResourceType) clients( + ctx context.Context, +) (map[string]*githubEnterpriseAdministratorClient, error) { + if o.newEnterpriseClients == nil { + return nil, nil + } + + o.mu.Lock() + defer o.mu.Unlock() + + if o.enterpriseClients != nil { + return o.enterpriseClients, nil + } + + clients, err := o.newEnterpriseClients(ctx) + if err != nil { + return nil, err + } + o.enterpriseClients = clients + + return o.enterpriseClients, nil +} + +// noClientReason explains why no administration client exists, in the terms of +// the fix the operator has to make. Under app auth every reason already +// surfaced as a build error, so what is left is the credential and an +// enterprise this connector was not configured for. +func (o *enterpriseRoleResourceType) noClientReason() string { + if o.newEnterpriseClients == nil { + return "a personal access token can sync enterprise roles but cannot provision them, " + + "which needs GitHub App authentication" + } + return "it is not one of the configured enterprises" +} + func (o *enterpriseRoleResourceType) cacheRole(roleId string, userLogin string) { o.mu.Lock() defer o.mu.Unlock() @@ -97,6 +171,17 @@ func (o *enterpriseRoleResourceType) List( parentID *v2.ResourceId, opts resourceSdk.SyncOpAttrs, ) ([]*v2.Resource, *resourceSdk.SyncOpResults, error) { + enterpriseClients, err := o.clients(ctx) + if err != nil { + return nil, nil, err + } + + // The consumed-licenses API that backs the cache is PAT-only, so a GitHub + // App can only see the built-in Owner role it is able to read and mutate. + if len(enterpriseClients) > 0 { + return appList(o.enterprises, enterpriseClients) + } + var ret []*v2.Resource cache, err := o.getRoleUsersCache(ctx) if err != nil { @@ -135,7 +220,7 @@ func (o *enterpriseRoleResourceType) StaticEntitlements( _ resourceSdk.SyncOpAttrs, ) ([]*v2.Entitlement, *resourceSdk.SyncOpResults, error) { rv := []*v2.Entitlement{} - rv = append(rv, entitlement.NewAssignmentEntitlement(nil, "assigned", + rv = append(rv, entitlement.NewAssignmentEntitlement(nil, enterpriseRoleAssigned, entitlement.WithDisplayName("Role Assigned"), entitlement.WithDescription("Assignment to enterprise role in GitHub"), entitlement.WithGrantableTo(resourceTypeUser), @@ -149,6 +234,14 @@ func (o *enterpriseRoleResourceType) Grants( resource *v2.Resource, opts resourceSdk.SyncOpAttrs, ) ([]*v2.Grant, *resourceSdk.SyncOpResults, error) { + enterpriseClients, err := o.clients(ctx) + if err != nil { + return nil, nil, err + } + if len(enterpriseClients) > 0 { + return o.appGrants(ctx, enterpriseClients, resource, opts) + } + cache, err := o.getRoleUsersCache(ctx) if err != nil { return nil, nil, fmt.Errorf("baton-github: error getting user roles cache: %w", err) @@ -168,7 +261,7 @@ func (o *enterpriseRoleResourceType) Grants( ret = append(ret, grant.NewGrant( resource, - "assigned", + enterpriseRoleAssigned, principalId, )) } @@ -176,22 +269,429 @@ func (o *enterpriseRoleResourceType) Grants( return ret, &resourceSdk.SyncOpResults{}, nil } -func EnterpriseRoleBuilder(client *github.Client, appClient *github.Client, customClient *customclient.Client, enterprises []string) *enterpriseRoleResourceType { +// Asserted here because the SDK reads the capability off these types: if the +// provisioner ever stops satisfying the interface the build breaks, rather +// than the capability quietly disappearing. +var _ connectorbuilder.ResourceProvisionerV2 = (*enterpriseRoleProvisioner)(nil) + +// enterpriseRoleProvisioner adds Grant and Revoke to the read-only syncer. +// +// It is a separate type because the SDK derives CAPABILITY_PROVISION from the +// methods a syncer implements, with no way to vary it at runtime. Registering +// it only when the enterprise clients can be built keeps C1 from offering the +// role as requestable on a deployment that could never satisfy the request: +// under a PAT there is no App to install, and every request would fail. +type enterpriseRoleProvisioner struct { + *enterpriseRoleResourceType +} + +// EnterpriseRoleProvisioningBuilder returns the syncer with Grant and Revoke. +// Only for deployments whose enterprise administration clients can be built. +func EnterpriseRoleProvisioningBuilder( + client *github.Client, + appClient *github.Client, + customClient *customclient.Client, + enterprises []string, + newEnterpriseClients enterpriseClientProvider, +) *enterpriseRoleProvisioner { + return &enterpriseRoleProvisioner{ + enterpriseRoleResourceType: EnterpriseRoleBuilder( + client, appClient, customClient, enterprises, newEnterpriseClients), + } +} + +// EnterpriseRoleBuilder returns the read-only enterprise role syncer. +// newEnterpriseClients is nil under PAT authentication, where only the read +// path is available. +func EnterpriseRoleBuilder( + client *github.Client, + appClient *github.Client, + customClient *customclient.Client, + enterprises []string, + newEnterpriseClients enterpriseClientProvider, +) *enterpriseRoleResourceType { return &enterpriseRoleResourceType{ - resourceType: resourceTypeEnterpriseRole, - client: client, - appClient: appClient, - customClient: customClient, - enterprises: enterprises, - roleUsersCache: make(map[string][]string), - mu: &sync.Mutex{}, + resourceType: resourceTypeEnterpriseRole, + client: client, + appClient: appClient, + customClient: customClient, + enterprises: enterprises, + roleUsersCache: make(map[string][]string), + mu: &sync.Mutex{}, + newEnterpriseClients: newEnterpriseClients, + } +} + +// appList emits only the built-in Owner role. The consumed-licenses API that +// discovers the other roles is PAT-only. +func appList( + enterprises []string, + enterpriseClients map[string]*githubEnterpriseAdministratorClient, +) ([]*v2.Resource, *resourceSdk.SyncOpResults, error) { + var ret []*v2.Resource + for _, enterprise := range enterprises { + if _, ok := enterpriseClients[enterprise]; !ok { + continue + } + + roleResource, err := resourceSdk.NewRoleResource( + enterpriseRoleOwner, + resourceTypeEnterpriseRole, + fmt.Sprintf("%s:%s", enterprise, enterpriseRoleOwner), + []resourceSdk.RoleTraitOption{}, + ) + if err != nil { + return nil, nil, fmt.Errorf("baton-github: error creating role resource for %s in enterprise %s: %w", + enterpriseRoleOwner, enterprise, err) + } + ret = append(ret, roleResource) + } + + return ret, &resourceSdk.SyncOpResults{}, nil +} + +// appGrants emits the users who hold the Owner role today and the ones invited +// but not yet accepted, against the same entitlement. C1 has no pending grant +// state, so the two are indistinguishable there; the connector still tells them +// apart, because revoking an owner and cancelling an invitation are different +// mutations. +// +// Nothing tracks an expiry: an invitation nobody accepts stops resolving on +// GitHub's side, so it stops being emitted and C1 drops the grant that sync. +// +// Owners and invitations are two phases of one page token, because invitations +// are not enumerable and have to be resolved from the enterprise members. An +// empty cursor drops the current phase, emptying the token once both are done. +func (o *enterpriseRoleResourceType) appGrants( + ctx context.Context, + enterpriseClients map[string]*githubEnterpriseAdministratorClient, + resource *v2.Resource, + opts resourceSdk.SyncOpAttrs, +) ([]*v2.Grant, *resourceSdk.SyncOpResults, error) { + enterprise, ok := provisionableEnterpriseOwner(resource.Id.Resource) + if !ok { + return nil, &resourceSdk.SyncOpResults{}, nil + } + client, ok := enterpriseClients[enterprise] + if !ok { + return nil, &resourceSdk.SyncOpResults{}, nil + } + + bag := &pagination.Bag{} + if err := bag.Unmarshal(opts.PageToken.Token); err != nil { + return nil, nil, fmt.Errorf("baton-github: error parsing enterprise owner page token: %w", err) + } + if bag.Current() == nil { + // Reverse order: the owners are walked first. + bag.Push(pagination.PageState{ResourceTypeID: enterprisePendingPhase}) + bag.Push(pagination.PageState{ResourceTypeID: enterpriseOwnersPhase}) + } + + var after *githubv4.String + if cursor := bag.PageToken(); cursor != "" { + after = githubv4.NewString(githubv4.String(cursor)) + } + + var ( + ret []*v2.Grant + nextCursor string + annos annotations.Annotations + err error + ) + switch phase := bag.ResourceTypeID(); phase { + case enterpriseOwnersPhase: + ret, nextCursor, annos, err = o.ownerGrants(ctx, client, resource, after) + case enterprisePendingPhase: + ret, nextCursor, annos, err = o.pendingInvitationGrants(ctx, client, resource, enterprise, after) + default: + return nil, nil, fmt.Errorf("baton-github: unexpected enterprise owner sync phase %q", phase) + } + if err != nil { + return nil, &resourceSdk.SyncOpResults{Annotations: annos}, err + } + + if err := bag.Next(nextCursor); err != nil { + return nil, &resourceSdk.SyncOpResults{Annotations: annos}, + fmt.Errorf("baton-github: error advancing the enterprise owner page token: %w", err) + } + pageToken, err := bag.Marshal() + if err != nil { + return nil, &resourceSdk.SyncOpResults{Annotations: annos}, + fmt.Errorf("baton-github: error building the enterprise owner page token: %w", err) + } + + return ret, &resourceSdk.SyncOpResults{Annotations: annos, NextPageToken: pageToken}, nil +} + +// ownerGrants emits one page of the users who hold the role today. +func (o *enterpriseRoleResourceType) ownerGrants( + ctx context.Context, + client *githubEnterpriseAdministratorClient, + resource *v2.Resource, + after *githubv4.String, +) ([]*v2.Grant, string, annotations.Annotations, error) { + owners, nextCursor, annos, err := client.owners(ctx, after) + if err != nil { + return nil, "", annos, err + } + + ret := make([]*v2.Grant, 0, len(owners)) + for _, owner := range owners { + principalId, err := enterpriseOwnerPrincipalID(owner) + if err != nil { + return nil, "", annos, err + } + ret = append(ret, grant.NewGrant(resource, enterpriseRoleAssigned, principalId)) + } + + return ret, nextCursor, annos, nil +} + +// pendingInvitationGrants emits one page worth of users who have been invited +// to the role and have not accepted, in member order so a page emits the same +// grants in the same sequence on every sync. +// +// GitHub exposes no connection of pending administrator invitations to an +// installation token, so they cannot be listed: they are resolved by asking +// about known logins. The enterprise members are that candidate set, which +// means an invitation sent to someone who is not a member of the enterprise is +// invisible to the sync. +func (o *enterpriseRoleResourceType) pendingInvitationGrants( + ctx context.Context, + client *githubEnterpriseAdministratorClient, + resource *v2.Resource, + enterprise string, + after *githubv4.String, +) ([]*v2.Grant, string, annotations.Annotations, error) { + members, nextCursor, annos, err := client.members(ctx, enterprise, after) + if err != nil { + return nil, "", annos, err + } + if len(members) == 0 { + return nil, nextCursor, annos, nil + } + + logins := make([]string, 0, len(members)) + for _, member := range members { + logins = append(logins, member.login) + } + + invitations, invitationAnnos, err := client.pendingOwnerInvitations(ctx, enterprise, logins) + annos = freshestRateLimit(annos, invitationAnnos) + if err != nil { + return nil, "", annos, err + } + + ret := make([]*v2.Grant, 0, len(invitations)) + for _, member := range members { + if _, invited := invitations[member.login]; !invited { + continue + } + principalId, err := enterpriseOwnerPrincipalID(member) + if err != nil { + return nil, "", annos, err + } + ret = append(ret, grant.NewGrant(resource, enterpriseRoleAssigned, principalId)) + } + + return ret, nextCursor, annos, nil +} + +// Grant gives a user the built-in Owner role and returns the resulting grant. +// +// A member can only become an owner by accepting an invitation. GitHub rejects +// that invitation for existing administrators, but an installation token +// cannot read their current role. Promoting one in place would therefore make +// Revoke destructive: it could only demote them to UNAFFILIATED rather than +// restore the role they held before the grant. +// +// An unaccepted invitation counts as held and returns a grant, the same way +// Grants() emits it: returning nothing would make C1 drop an overlay that the +// next sync puts straight back. +func (o *enterpriseRoleProvisioner) Grant( + ctx context.Context, + principal *v2.Resource, + ent *v2.Entitlement, +) ([]*v2.Grant, annotations.Annotations, error) { + enterprise, client, err := o.provisioningTarget(ctx, principal, ent) + if err != nil { + return nil, nil, err + } + login, err := o.userLogin(ctx, principal.Id.Resource) + if err != nil { + return nil, nil, err + } + + result := []*v2.Grant{grant.NewGrant(ent.GetResource(), ent.GetSlug(), principal.Id)} + annos := annotations.New() + state, stateAnnos, err := client.OwnerState(ctx, enterprise, login) + annos = freshestRateLimit(annos, stateAnnos) + if err != nil { + return nil, annos, err + } + if state.isOwner || state.pendingInvitationID != "" { + annos.Append(&v2.GrantAlreadyExists{}) + return result, annos, nil + } + + if inviteErr := client.InviteOwner(ctx, state.enterpriseID, login); inviteErr != nil { + if status.Code(inviteErr) != codes.FailedPrecondition { + return nil, annos, inviteErr + } + ctxzap.Extract(ctx).Debug("baton-github: invitation rejected; refusing unsafe in-place promotion", + zap.String("login", login), + zap.Error(inviteErr), + ) + // The rejection is not always "already an administrator" — a seat + // limit or an SSO restriction lands here too — so the message states + // the policy and lets GitHub give the reason. + return nil, annos, fmt.Errorf( + "baton-github: cannot grant enterprise Owner to %s, and promoting in place is not attempted "+ + "because a prior administrator role cannot be read back and revoke would discard it: %w", + login, inviteErr) + } + + state, stateAnnos, err = client.OwnerState(ctx, enterprise, login) + annos = freshestRateLimit(annos, stateAnnos) + if err != nil { + return nil, annos, err + } + switch { + case state.isOwner, state.pendingInvitationID != "": + return result, annos, nil + default: + return nil, annos, status.Errorf(codes.Unavailable, + "baton-github: enterprise owner grant for %s is not visible in GitHub", login) } } -func isPermissionDenied(err error) bool { - var grpcErr interface{ GRPCStatus() *status.Status } - if errors.As(err, &grpcErr) { - return grpcErr.GRPCStatus().Code() == codes.PermissionDenied +// Revoke takes the built-in Owner role away from a user. +// +// Holding the role and carrying an invitation are not alternatives, so both +// are cleared: either one left behind would fail the verification that +// follows. Demotion uses UNAFFILIATED, which keeps the user as a member of the +// enterprise instead of evicting them. A NOT_FOUND from either mutation is +// success, because it means the state being asked for is already in place. +func (o *enterpriseRoleProvisioner) Revoke( + ctx context.Context, + grantObj *v2.Grant, +) (annotations.Annotations, error) { + enterprise, client, err := o.provisioningTarget(ctx, grantObj.GetPrincipal(), grantObj.GetEntitlement()) + if err != nil { + return nil, err } - return false + login, err := o.userLogin(ctx, grantObj.GetPrincipal().GetId().GetResource()) + if err != nil { + return nil, err + } + + annos := annotations.New() + state, stateAnnos, err := client.OwnerState(ctx, enterprise, login) + annos = freshestRateLimit(annos, stateAnnos) + if err != nil { + return annos, err + } + if !state.isOwner && state.pendingInvitationID == "" { + annos.Append(&v2.GrantAlreadyRevoked{}) + return annos, nil + } + + if state.isOwner { + if err := client.UpdateRole( + ctx, state.enterpriseID, login, enterpriseAdministratorRoleUnaffiliated, + ); err != nil && status.Code(err) != codes.NotFound { + return annos, err + } + } + if state.pendingInvitationID != "" { + if err := client.CancelInvitation(ctx, state.pendingInvitationID); err != nil && status.Code(err) != codes.NotFound { + return annos, err + } + } + + state, stateAnnos, err = client.OwnerState(ctx, enterprise, login) + annos = freshestRateLimit(annos, stateAnnos) + if err != nil { + return annos, err + } + if state.isOwner || state.pendingInvitationID != "" { + return annos, status.Errorf(codes.Unavailable, + "baton-github: enterprise owner revoke for %s is not visible in GitHub", login) + } + + return annos, nil +} + +func (o *enterpriseRoleProvisioner) provisioningTarget( + ctx context.Context, + principal *v2.Resource, + ent *v2.Entitlement, +) (string, *githubEnterpriseAdministratorClient, error) { + if principal.GetId().GetResourceType() != resourceTypeUser.Id { + return "", nil, status.Error(codes.InvalidArgument, + "baton-github: enterprise role can only be granted to a user") + } + enterprise, ok := provisionableEnterpriseOwner(ent.GetResource().GetId().GetResource()) + if !ok { + return "", nil, status.Error(codes.InvalidArgument, + "baton-github: only the built-in enterprise Owner role can be provisioned") + } + // The construction error comes first: without it the operator is told the + // app is not installed even when the real cause was a transient failure + // or a mismatched organization, and FailedPrecondition reads to C1 as + // non-retryable. + enterpriseClients, err := o.clients(ctx) + if err != nil { + return "", nil, err + } + client, ok := enterpriseClients[enterprise] + if !ok { + return "", nil, status.Errorf(codes.FailedPrecondition, + "baton-github: cannot provision enterprise %s: %s", enterprise, o.noClientReason()) + } + return enterprise, client, nil +} + +func (o *enterpriseRoleResourceType) userLogin(ctx context.Context, userID string) (string, error) { + id, err := strconv.ParseInt(userID, 10, 64) + if err != nil { + return "", status.Errorf(codes.InvalidArgument, "baton-github: invalid GitHub user ID %q", userID) + } + user, resp, err := o.client.Users.GetByID(ctx, id) + if err != nil { + return "", wrapGitHubError(err, resp, fmt.Sprintf("baton-github: failed to get user %d", id)) + } + if user.GetLogin() == "" { + return "", fmt.Errorf("baton-github: GitHub user %d has no login", id) + } + return user.GetLogin(), nil +} + +func enterpriseOwnerPrincipalID(owner enterpriseUser) (*v2.ResourceId, error) { + if owner.databaseID <= 0 { + return nil, fmt.Errorf("baton-github: enterprise owner %q has no database ID", owner.login) + } + principalId, err := resourceSdk.NewResourceID(resourceTypeUser, owner.databaseID) + if err != nil { + return nil, fmt.Errorf("baton-github: error creating resource ID for user %s: %w", owner.login, err) + } + return principalId, nil +} + +// provisionableEnterpriseOwner returns the enterprise of a resource ID when it +// names the one role this connector can read and write through the GitHub App. +// Sync and provisioning share it so they cannot disagree on which roles are in +// the catalog. +func provisionableEnterpriseOwner(resourceID string) (string, bool) { + enterprise, role, ok := parseEnterpriseRoleID(resourceID) + if !ok || !strings.EqualFold(role, enterpriseRoleOwner) { + return "", false + } + + return enterprise, true +} + +func parseEnterpriseRoleID(resourceID string) (string, string, bool) { + enterprise, role, ok := strings.Cut(resourceID, ":") + return enterprise, role, ok && enterprise != "" && role != "" } diff --git a/pkg/connector/enterprise_role_test.go b/pkg/connector/enterprise_role_test.go new file mode 100644 index 00000000..a6d03c25 --- /dev/null +++ b/pkg/connector/enterprise_role_test.go @@ -0,0 +1,1064 @@ +package connector + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + "github.com/conductorone/baton-sdk/pkg/pagination" + entitlementSdk "github.com/conductorone/baton-sdk/pkg/types/entitlement" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/google/go-github/v69/github" + "github.com/shurcooL/githubv4" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/conductorone/baton-github/test/mocks" +) + +const ( + testEnterprise = "example-enterprise" + testEnterpriseID = "E_example" + // The seeded mock user has ID 56 and login "56". + testLogin = "56" + testLoginID = 56 + testOrg = "example-org" +) + +// enterpriseStub is an in-memory GitHub GraphQL enterprise: it answers the +// owner and invitation queries from its own state and applies the mutations to +// it, so the tests exercise the documents the connector actually sends. +type enterpriseStub struct { + // ownerPages holds the users that currently hold the Owner role, split + // into the pages the members connection returns. + ownerPages [][]enterpriseStubOwner + // invitations maps a login to its pending Owner invitation ID. + invitations map[string]string + // memberPages holds the enterprise member accounts, split into the pages + // the members connection returns. They are the candidate set the pending + // invitation lookup asks about. + memberPages [][]enterpriseStubOwner + + // inviteErrorType makes inviteEnterpriseAdmin fail with that GraphQL error + // type. UNPROCESSABLE is what GitHub returns for someone who already + // administers the enterprise. + inviteErrorType string + inviteErrorMessage string + updateFails bool + cancelNotFound bool + // ownersRateLimited makes the owners read answer the way GitHub reports a + // GraphQL budget error: HTTP 200 carrying errors[]. + ownersRateLimited bool + // batchErrorType adds one entry of that type to the invitation batch, + // alongside the NOT_FOUND entries the batch always produces. + batchErrorType string + // silentMutations make the mutations report success without changing any + // state, which is how a phantom grant or revoke would look. + silentMutations bool + + ownerQueries int + invitationQueries int + invitationBatches int + memberQueries int + organizationChecks int + + updatedRole githubv4.EnterpriseAdministratorRole + invitedLogin string + cancelledID string +} + +type enterpriseStubOwner struct { + id int64 + login string +} + +func (s *enterpriseStub) handle(t *testing.T, w http.ResponseWriter, r *http.Request) { + t.Helper() + + var body struct { + Query string `json:"query"` + Variables map[string]any `json:"variables"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + + w.Header().Set("Content-Type", "application/json") + switch { + case strings.Contains(body.Query, "enterpriseAdministratorInvitation("): + // The sync resolves many logins in one aliased request; Grant and + // Revoke ask about a single login through the typed client. + if _, single := body.Variables["login"]; single { + s.writeInvitation(t, w, body.Variables) + } else { + s.writeInvitationBatch(t, w, body.Query, body.Variables) + } + + case strings.Contains(body.Query, "enterpriseOwners("): + require.Contains(t, body.Query, "databaseId") + require.NotContains(t, body.Query, "members(", + "owners must come from Organization.enterpriseOwners, not Enterprise.members") + // An organizationRole filter would drop every enterprise owner whose + // role in this organization is not OWNER, which is the bug this read + // path replaced. + require.NotContains(t, body.Query, "organizationRole", + "the owners query must not filter by the owner's role in the organization") + s.writeOwners(t, w, body.Variables) + + case strings.Contains(body.Query, "members("): + s.writeMembers(t, w, body.Variables) + + case strings.Contains(body.Query, "organizations("): + s.organizationChecks++ + _, _ = fmt.Fprintf(w, `{"data":{"enterprise":{"organizations":{"nodes":[{"login":%q}]}}}}`, testOrg) + + case strings.Contains(body.Query, "updateEnterpriseAdministratorRole("): + requireMutationShape(t, body.Query, "updateEnterpriseAdministratorRole") + input := mutationInput(t, body.Variables) + login, _ := input["login"].(string) + role, _ := input["role"].(string) + if s.updateFails { + _, _ = w.Write([]byte(`{"data":{"updateEnterpriseAdministratorRole":null},` + + `"errors":[{"type":"RATE_LIMITED","message":"rate limit exceeded"}]}`)) + return + } + s.updatedRole = githubv4.EnterpriseAdministratorRole(role) + if !s.silentMutations { + if role == string(enterpriseAdministratorRoleUnaffiliated) { + s.removeOwner(login) + } else { + s.ownerPages = [][]enterpriseStubOwner{{{id: testLoginID, login: login}}} + } + } + _, _ = w.Write([]byte(`{"data":{"updateEnterpriseAdministratorRole":{"clientMutationId":null}}}`)) + + case strings.Contains(body.Query, "inviteEnterpriseAdmin("): + requireMutationShape(t, body.Query, "inviteEnterpriseAdmin") + input := mutationInput(t, body.Variables) + login, _ := input["invitee"].(string) + if s.inviteErrorType != "" { + message := s.inviteErrorMessage + if message == "" { + // Observed live against GitHub for an existing administrator. + message = "Invitee is already an owner of this enterprise" + } + _, _ = fmt.Fprintf(w, + `{"data":{"inviteEnterpriseAdmin":null},"errors":[{"type":%q,"message":%q}]}`, + s.inviteErrorType, message) + return + } + s.invitedLogin = login + if !s.silentMutations { + s.invitations[login] = "EAI_" + login + } + _, _ = w.Write([]byte(`{"data":{"inviteEnterpriseAdmin":{"clientMutationId":null}}}`)) + + case strings.Contains(body.Query, "cancelEnterpriseAdminInvitation("): + requireMutationShape(t, body.Query, "cancelEnterpriseAdminInvitation") + input := mutationInput(t, body.Variables) + id, _ := input["invitationId"].(string) + s.cancelledID = id + for login, invitation := range s.invitations { + if invitation == id { + delete(s.invitations, login) + } + } + if s.cancelNotFound { + _, _ = w.Write([]byte(`{"data":{"cancelEnterpriseAdminInvitation":null},` + + `"errors":[{"type":"NOT_FOUND","message":"invitation not found"}]}`)) + return + } + _, _ = w.Write([]byte(`{"data":{"cancelEnterpriseAdminInvitation":{"clientMutationId":null}}}`)) + + case strings.Contains(body.Query, "enterprise(slug: $slug){id}"): + _, _ = fmt.Fprintf(w, `{"data":{"enterprise":{"id":%q}}}`, testEnterpriseID) + + default: + t.Fatalf("unexpected GraphQL operation: %s", body.Query) + } +} + +func (s *enterpriseStub) removeOwner(login string) { + for pageIndex, page := range s.ownerPages { + remaining := make([]enterpriseStubOwner, 0, len(page)) + for _, owner := range page { + if !strings.EqualFold(owner.login, login) { + remaining = append(remaining, owner) + } + } + s.ownerPages[pageIndex] = remaining + } +} + +func (s *enterpriseStub) writeOwners(t *testing.T, w http.ResponseWriter, variables map[string]any) { + t.Helper() + s.ownerQueries++ + + if s.ownersRateLimited { + _, err := w.Write([]byte(`{"data":{"organization":null},` + + `"errors":[{"type":"RATE_LIMITED","message":"API rate limit exceeded"}]}`)) + require.NoError(t, err) + return + } + + pageIndex := 0 + if after, ok := variables["after"].(string); ok && after != "" { + _, err := fmt.Sscanf(after, "cursor-%d", &pageIndex) + require.NoError(t, err) + } + + owners := []enterpriseStubOwner{} + if pageIndex < len(s.ownerPages) { + owners = s.ownerPages[pageIndex] + } + hasNextPage := pageIndex+1 < len(s.ownerPages) + + nodes := make([]string, 0, len(owners)) + for _, owner := range owners { + nodes = append(nodes, fmt.Sprintf(`{"databaseId":%d,"login":%q}`, owner.id, owner.login)) + } + + _, err := fmt.Fprintf( + w, + `{"data":{"organization":{"enterpriseOwners":{"nodes":[%s],`+ + `"pageInfo":{"hasNextPage":%t,"endCursor":"cursor-%d"}}},`+ + `"rateLimit":{"limit":5000,"remaining":4999,"resetAt":"2026-09-18T23:00:00Z"}}}`, + strings.Join(nodes, ","), hasNextPage, pageIndex+1, + ) + require.NoError(t, err) +} + +// writeInvitation answers the way GitHub does: the invitation when there is +// one, and a NOT_FOUND error when there is not. +// writeMembers answers one page of the enterprise member accounts, in the +// EnterpriseUserAccount shape GitHub returns for this connection. +func (s *enterpriseStub) writeMembers(t *testing.T, w http.ResponseWriter, variables map[string]any) { + t.Helper() + s.memberQueries++ + + pageIndex := 0 + if after, ok := variables["after"].(string); ok && after != "" { + _, err := fmt.Sscanf(after, "member-cursor-%d", &pageIndex) + require.NoError(t, err) + } + + members := []enterpriseStubOwner{} + if pageIndex < len(s.memberPages) { + members = s.memberPages[pageIndex] + } + hasNextPage := pageIndex+1 < len(s.memberPages) + + nodes := make([]string, 0, len(members)) + for _, member := range members { + // Only the selected fields come back, so no __typename here: the query + // resolves the union with inline fragments instead. + nodes = append(nodes, fmt.Sprintf( + `{"login":%q,"user":{"databaseId":%d,"login":%q}}`, + member.login, member.id, member.login)) + } + + _, err := fmt.Fprintf(w, + `{"data":{"enterprise":{"members":{"nodes":[%s],`+ + `"pageInfo":{"hasNextPage":%t,"endCursor":"member-cursor-%d"}}},`+ + `"rateLimit":{"limit":5000,"remaining":4998,"resetAt":"2026-09-18T23:00:00Z"}}}`, + strings.Join(nodes, ","), hasNextPage, pageIndex+1) + require.NoError(t, err) +} + +// writeInvitationBatch answers the aliased lookup the sync uses. Every login +// without an invitation contributes a NOT_FOUND entry to errors[] next to the +// aliases that did resolve, which is how GitHub answers a partial batch. +func (s *enterpriseStub) writeInvitationBatch(t *testing.T, w http.ResponseWriter, query string, variables map[string]any) { + t.Helper() + s.invitationBatches++ + + require.NotContains(t, query, `userLogin: "`, "logins must travel as variables, not in the query text") + require.Equal(t, string(githubv4.EnterpriseAdministratorRoleOwner), variables["role"]) + + aliases, failures := []string{}, []string{} + for i := 0; ; i++ { + login, ok := variables[fmt.Sprintf("l%d", i)].(string) + if !ok { + break + } + alias := fmt.Sprintf("i%d", i) + require.Contains(t, query, alias+": enterpriseAdministratorInvitation") + if invitationID, invited := s.invitations[login]; invited { + aliases = append(aliases, fmt.Sprintf(`%q:{"id":%q}`, alias, invitationID)) + continue + } + aliases = append(aliases, fmt.Sprintf(`%q:null`, alias)) + failures = append(failures, fmt.Sprintf( + `{"type":"NOT_FOUND","message":"Could not resolve to a pending invitation for %s."}`, login)) + } + + if s.batchErrorType != "" { + failures = append(failures, fmt.Sprintf( + `{"type":%q,"message":"the app lost access to the enterprise"}`, s.batchErrorType)) + } + + aliases = append(aliases, `"rateLimit":{"limit":5000,"remaining":4997,"resetAt":"2026-09-18T23:00:00Z"}`) + body := fmt.Sprintf(`{"data":{%s}`, strings.Join(aliases, ",")) + if len(failures) > 0 { + body += fmt.Sprintf(`,"errors":[%s]`, strings.Join(failures, ",")) + } + _, err := w.Write([]byte(body + "}")) + require.NoError(t, err) +} + +func (s *enterpriseStub) writeInvitation(t *testing.T, w http.ResponseWriter, variables map[string]any) { + t.Helper() + s.invitationQueries++ + + login, ok := variables["login"].(string) + require.True(t, ok, "invitation lookup must send the login as a variable") + require.Equal(t, string(githubv4.EnterpriseAdministratorRoleOwner), variables["role"]) + + invitationID, invited := s.invitations[login] + if !invited { + _, _ = fmt.Fprintf(w, + `{"data":{"enterpriseAdministratorInvitation":null},`+ + `"errors":[{"type":"NOT_FOUND","message":"Could not resolve to an invitation for %s."}]}`, login) + return + } + _, _ = fmt.Fprintf(w, `{"data":{"enterpriseAdministratorInvitation":{"id":%q}}}`, invitationID) +} + +// requireMutationShape rejects the two mutation bodies GitHub answers with a +// 200 plus an errors array: a payload without a selection set, and a +// Query-only rateLimit field selected on Mutation. +func requireMutationShape(t *testing.T, query string, field string) { + t.Helper() + + _, payload, ok := strings.Cut(query, field+"(input: $input)") + require.True(t, ok, "mutation %s must take its input as a variable", field) + require.True(t, strings.HasPrefix(payload, "{"), "mutation %s must select payload fields", field) + require.NotEqual(t, "{}", strings.TrimSuffix(payload, "}"), "mutation %s selection set is empty", field) + require.NotContains(t, query, "rateLimit", "rateLimit does not exist on type Mutation") +} + +func mutationInput(t *testing.T, variables map[string]any) map[string]any { + t.Helper() + + input, ok := variables["input"].(map[string]any) + require.True(t, ok, "mutation must send its input as a variable") + // The enterprise node ID is resolved once at construction. An empty one + // here means that step was skipped, which GitHub would reject at runtime. + if enterpriseID, present := input["enterpriseId"]; present { + require.Equal(t, testEnterpriseID, enterpriseID, + "mutation must carry the enterprise node ID resolved at construction") + } + return input +} + +// requireNoIdempotencyClaim asserts the operation actually acted instead of +// reporting the state as already correct. Emptiness is not the assertion: +// every path also carries the GraphQL budget left after reading the owners. +func requireNoIdempotencyClaim(t *testing.T, annos annotations.Annotations) { + t.Helper() + + var alreadyExists v2.GrantAlreadyExists + var alreadyRevoked v2.GrantAlreadyRevoked + require.False(t, annos.Contains(&alreadyExists)) + require.False(t, annos.Contains(&alreadyRevoked)) +} + +func newTestEnterpriseRoleBuilder( + t *testing.T, + stub *enterpriseStub, +) (*enterpriseRoleProvisioner, *v2.Resource, *v2.Entitlement) { + t.Helper() + + if stub.invitations == nil { + stub.invitations = make(map[string]string) + } + + graphqlSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + stub.handle(t, w, r) + })) + t.Cleanup(graphqlSrv.Close) + + // Both installations point at the same stub: the test exercises the + // queries, not the two-token split. + enterpriseClient, err := newEnterpriseAdministratorClient( + graphqlSrv.URL, graphqlSrv.Client(), graphqlSrv.Client(), testOrg) + require.NoError(t, err) + // Mirrors construction: the node ID is resolved once, not per operation. + require.NoError(t, enterpriseClient.resolveEnterpriseNodeID(context.Background(), testEnterprise)) + + mgh := mocks.NewMockGitHub() + _, _, _, githubUser, _, err := mgh.Seed() + require.NoError(t, err) + + builder := EnterpriseRoleProvisioningBuilder( + github.NewClient(mgh.Server()), + nil, + nil, + []string{testEnterprise}, + func(context.Context) (map[string]*githubEnterpriseAdministratorClient, error) { + return map[string]*githubEnterpriseAdministratorClient{testEnterprise: enterpriseClient}, nil + }, + ) + + principalID, err := resourceSdk.NewResourceID(resourceTypeUser, githubUser.GetID()) + require.NoError(t, err) + + roleResource, err := resourceSdk.NewRoleResource( + enterpriseRoleOwner, + resourceTypeEnterpriseRole, + testEnterprise+":"+enterpriseRoleOwner, + []resourceSdk.RoleTraitOption{}, + ) + require.NoError(t, err) + + ent := &v2.Entitlement{ + Id: entitlementSdk.NewEntitlementID(roleResource, enterpriseRoleAssigned), + Slug: enterpriseRoleAssigned, + Resource: roleResource, + } + + return builder, &v2.Resource{Id: principalID}, ent +} + +func TestEnterpriseRoleGrant(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // updateEnterpriseAdministratorRole rejects anyone who is not already an + // administrator, so a plain member can only be invited. + // The invitation is reported as the grant it will become, which is what + // the sync emits too: returning nothing here would make C1 drop an overlay + // that the next sync puts straight back. + t.Run("invites a member and reports the grant", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + grants, annos, err := builder.Grant(ctx, principal, ent) + require.NoError(t, err) + requireNoIdempotencyClaim(t, annos) + require.Len(t, grants, 1) + require.Equal(t, testLogin, grants[0].GetPrincipal().GetId().GetResource()) + require.Equal(t, testLogin, stub.invitedLogin) + require.Empty(t, stub.updatedRole) + }) + + // A billing manager cannot be invited. Which existing role caused the + // rejection is unreadable for a GitHub App, so promoting in place would + // make Revoke unable to restore the user's previous role. + t.Run("rejects an unsafe promotion after the invitation is rejected", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{inviteErrorType: "UNPROCESSABLE"} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + grants, _, err := builder.Grant(ctx, principal, ent) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.ErrorContains(t, err, "promoting in place is not attempted") + require.Empty(t, grants) + require.Empty(t, stub.updatedRole) + require.Empty(t, stub.invitedLogin) + }) + + // Any other invitation failure must surface instead of triggering a second + // mutation the user never asked for. + t.Run("does not promote after an unrelated invitation failure", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + inviteErrorType: "RATE_LIMITED", + inviteErrorMessage: "rate limit exceeded", + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + _, _, err := builder.Grant(ctx, principal, ent) + require.Equal(t, codes.Unavailable, status.Code(err)) + require.Empty(t, stub.updatedRole) + }) + + // UNPROCESSABLE covers more than "already an administrator": seat limits + // and SSO or EMU restrictions land here too. Preserve GitHub's reason so + // the operator can distinguish those cases. + t.Run("preserves the invitation rejection reason", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + inviteErrorType: "UNPROCESSABLE", + inviteErrorMessage: "enterprise owner seat limit reached", + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + _, _, err := builder.Grant(ctx, principal, ent) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.ErrorContains(t, err, stub.inviteErrorMessage) + require.Empty(t, stub.updatedRole) + }) + + t.Run("reports an owner as already granted", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{{{id: testLoginID, login: testLogin}}}, + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + grants, annos, err := builder.Grant(ctx, principal, ent) + require.NoError(t, err) + require.Len(t, grants, 1) + + var alreadyExists v2.GrantAlreadyExists + require.True(t, annos.Contains(&alreadyExists)) + require.Empty(t, stub.updatedRole) + require.Empty(t, stub.invitedLogin) + + // Reading the owners spends GraphQL budget, so the remaining budget + // travels with the idempotency annotation instead of being dropped. + var rateLimit v2.RateLimitDescription + require.True(t, annos.Contains(&rateLimit)) + }) + + // The owner check has to page: the owners connection has no login filter. + t.Run("finds an owner on a later page", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{ + {{id: 99, login: "another-owner"}}, + {{id: testLoginID, login: testLogin}}, + }, + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + grants, annos, err := builder.Grant(ctx, principal, ent) + require.NoError(t, err) + require.Len(t, grants, 1) + + var alreadyExists v2.GrantAlreadyExists + require.True(t, annos.Contains(&alreadyExists)) + require.Equal(t, 2, stub.ownerQueries) + require.Empty(t, stub.invitedLogin) + }) + + // Re-inviting returns the same invitation, so a repeat grant must not send + // a second one. + t.Run("does not resend a pending invitation", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{invitations: map[string]string{testLogin: "EAI_existing"}} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + grants, annos, err := builder.Grant(ctx, principal, ent) + require.NoError(t, err) + require.Len(t, grants, 1) + + var alreadyExists v2.GrantAlreadyExists + require.True(t, annos.Contains(&alreadyExists)) + require.Empty(t, stub.invitedLogin, "an existing invitation must not be sent again") + }) + + // The mutation reporting success is not evidence that GitHub applied it. + // Without this guard C1 would record access that does not exist. + t.Run("rejects a grant GitHub did not apply", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{silentMutations: true} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + grants, _, err := builder.Grant(ctx, principal, ent) + require.Equal(t, codes.Unavailable, status.Code(err)) + require.Empty(t, grants) + require.Equal(t, testLogin, stub.invitedLogin) + }) + + t.Run("rejects a role other than owner", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + ent.Resource.Id.Resource = testEnterprise + ":Member" + + _, _, err := builder.Grant(ctx, principal, ent) + require.Equal(t, codes.InvalidArgument, status.Code(err)) + require.Empty(t, stub.invitedLogin) + }) +} + +func TestEnterpriseRoleRevoke(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // UNAFFILIATED demotes the administrator but keeps enterprise membership; + // removeEnterpriseAdmin would evict them from the enterprise. + t.Run("demotes an active owner", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{{{id: testLoginID, login: testLogin}}}, + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + annos, err := builder.Revoke(ctx, &v2.Grant{Principal: principal, Entitlement: ent}) + require.NoError(t, err) + requireNoIdempotencyClaim(t, annos) + require.Equal(t, enterpriseAdministratorRoleUnaffiliated, stub.updatedRole) + }) + + // Holding the role and carrying an invitation are not alternatives. If the + // revoke only demoted, the invitation would survive and the verification + // that follows would report a retryable failure for a demotion that had + // already gone through. + t.Run("clears both the role and a leftover invitation", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{{{id: testLoginID, login: testLogin}}}, + invitations: map[string]string{testLogin: "EAI_leftover"}, + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + annos, err := builder.Revoke(ctx, &v2.Grant{Principal: principal, Entitlement: ent}) + require.NoError(t, err) + requireNoIdempotencyClaim(t, annos) + require.Equal(t, enterpriseAdministratorRoleUnaffiliated, stub.updatedRole) + require.Equal(t, "EAI_leftover", stub.cancelledID) + }) + + t.Run("cancels an invitation that was never accepted", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{invitations: map[string]string{testLogin: "EAI_existing"}} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + annos, err := builder.Revoke(ctx, &v2.Grant{Principal: principal, Entitlement: ent}) + require.NoError(t, err) + requireNoIdempotencyClaim(t, annos) + require.Equal(t, "EAI_existing", stub.cancelledID) + require.Empty(t, stub.updatedRole) + }) + + // GitHub expires an invitation after seven days, so a time-bound revoke can + // arrive once it is already gone. + t.Run("tolerates an invitation that expired mid-revoke", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + invitations: map[string]string{testLogin: "EAI_existing"}, + cancelNotFound: true, + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + annos, err := builder.Revoke(ctx, &v2.Grant{Principal: principal, Entitlement: ent}) + require.NoError(t, err) + requireNoIdempotencyClaim(t, annos) + require.Equal(t, "EAI_existing", stub.cancelledID) + }) + + // The mirror of the grant guard: reporting a revoke that GitHub did not + // apply would let C1 believe the access is gone while it is still there. + t.Run("rejects a revoke GitHub did not apply", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{{{id: testLoginID, login: testLogin}}}, + silentMutations: true, + } + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + _, err := builder.Revoke(ctx, &v2.Grant{Principal: principal, Entitlement: ent}) + require.Equal(t, codes.Unavailable, status.Code(err)) + require.Equal(t, enterpriseAdministratorRoleUnaffiliated, stub.updatedRole) + }) + + t.Run("reports no owner access as already revoked", func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + annos, err := builder.Revoke(ctx, &v2.Grant{Principal: principal, Entitlement: ent}) + require.NoError(t, err) + require.Empty(t, stub.updatedRole) + require.Empty(t, stub.cancelledID) + + var alreadyRevoked v2.GrantAlreadyRevoked + require.True(t, annos.Contains(&alreadyRevoked)) + }) +} + +// drainGrants runs the sync the way the SDK does, following the page token +// until it empties, and returns every grant across both phases. +func drainGrants( + t *testing.T, + builder *enterpriseRoleProvisioner, + resource *v2.Resource, +) []*v2.Grant { + t.Helper() + + var all []*v2.Grant + token := "" + for calls := 0; ; calls++ { + require.Less(t, calls, 20, "the page token never emptied") + grants, result, err := builder.Grants(context.Background(), resource, + resourceSdk.SyncOpAttrs{PageToken: pagination.Token{Token: token}}) + require.NoError(t, err) + all = append(all, grants...) + if result.NextPageToken == "" { + return all + } + token = result.NextPageToken + } +} + +// principals reduces grants to the principal IDs, which is what C1 keys access +// on and therefore what these tests care about. +func principals(grants []*v2.Grant) []string { + out := make([]string, 0, len(grants)) + for _, g := range grants { + out = append(out, g.GetPrincipal().GetId().GetResource()) + } + return out +} + +// An invitation nobody has accepted is emitted as a grant so C1 keeps a record +// of the request. C1 has no pending state, so it is the same entitlement an +// accepted owner gets; the connector tells them apart only when revoking. +func TestEnterpriseRoleGrantsIncludePendingInvitations(t *testing.T) { + t.Parallel() + + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{{{id: 101, login: "accepted-owner"}}}, + memberPages: [][]enterpriseStubOwner{{{id: 202, login: "invited-member"}, {id: 303, login: "plain-member"}}}, + invitations: map[string]string{"invited-member": "EAI_invited"}, + } + builder, _, ent := newTestEnterpriseRoleBuilder(t, stub) + + require.ElementsMatch(t, []string{"101", "202"}, principals(drainGrants(t, builder, ent.Resource))) + // One request answered both members, rather than one lookup per login. + require.Equal(t, 1, stub.invitationBatches) + require.Zero(t, stub.invitationQueries, "the sync must not use the single-login lookup") +} + +// GitHub stops resolving an invitation that expires or is cancelled, so it +// simply stops being emitted and C1 drops the grant. Nothing tracks an expiry. +func TestEnterpriseRoleGrantsDropInvitationsThatStoppedResolving(t *testing.T) { + t.Parallel() + + stub := &enterpriseStub{ + memberPages: [][]enterpriseStubOwner{{{id: 202, login: "invited-member"}}}, + invitations: map[string]string{"invited-member": "EAI_invited"}, + } + builder, _, ent := newTestEnterpriseRoleBuilder(t, stub) + require.Equal(t, []string{"202"}, principals(drainGrants(t, builder, ent.Resource))) + + // The invitation lapses on GitHub's side. + delete(stub.invitations, "invited-member") + require.Empty(t, drainGrants(t, builder, ent.Resource)) + + // Had the invitee accepted instead, they would surface as an owner. + stub.ownerPages = [][]enterpriseStubOwner{{{id: 202, login: "invited-member"}}} + require.Equal(t, []string{"202"}, principals(drainGrants(t, builder, ent.Resource))) +} + +// A member who was never invited must not produce a grant, even though the +// batch reports them as NOT_FOUND alongside the invitation that did resolve. +func TestEnterpriseRoleGrantsIgnoreMembersWithoutAnInvitation(t *testing.T) { + t.Parallel() + + stub := &enterpriseStub{ + memberPages: [][]enterpriseStubOwner{{ + {id: 202, login: "invited-member"}, + {id: 303, login: "plain-member"}, + {id: 404, login: "another-plain-member"}, + }}, + invitations: map[string]string{"invited-member": "EAI_invited"}, + } + builder, _, ent := newTestEnterpriseRoleBuilder(t, stub) + + require.Equal(t, []string{"202"}, principals(drainGrants(t, builder, ent.Resource))) +} + +// The invitation batch always reports NOT_FOUND for the members with no +// invitation, so those entries must not decide the code for the whole +// response. NOT_FOUND is the one code the SDK downgrades to a warning: if a +// real failure were reported as NOT_FOUND, the sync would finish green having +// emitted no pending invitations, and C1 would read that as a revoke. +func TestEnterpriseRoleGrantsFailOnARealErrorInsideTheInvitationBatch(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + errorType string + want codes.Code + }{ + {errorType: "FORBIDDEN", want: codes.PermissionDenied}, + {errorType: "UNAUTHENTICATED", want: codes.Unauthenticated}, + {errorType: "RATE_LIMITED", want: codes.Unavailable}, + // An error type the classifier has no rule for must still fail the + // batch rather than inherit NOT_FOUND from its neighbours. + {errorType: "SERVICE_UNAVAILABLE", want: codes.Internal}, + } { + t.Run(tc.errorType, func(t *testing.T) { + t.Parallel() + stub := &enterpriseStub{ + memberPages: [][]enterpriseStubOwner{{{id: 202, login: "plain-member"}}}, + batchErrorType: tc.errorType, + } + builder, _, ent := newTestEnterpriseRoleBuilder(t, stub) + + // Phase one reports no owners, then the invitation phase fails. + _, result, err := builder.Grants(context.Background(), ent.Resource, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + _, _, err = builder.Grants(context.Background(), ent.Resource, + resourceSdk.SyncOpAttrs{PageToken: pagination.Token{Token: result.NextPageToken}}) + require.Equal(t, tc.want, status.Code(err)) + }) + } +} + +// The members are paged, and every page gets its own batched lookup. +func TestEnterpriseRoleGrantsPageThroughMembers(t *testing.T) { + t.Parallel() + + stub := &enterpriseStub{ + memberPages: [][]enterpriseStubOwner{ + {{id: 202, login: "invited-member"}}, + {{id: 303, login: "second-page-invitee"}}, + }, + invitations: map[string]string{ + "invited-member": "EAI_one", + "second-page-invitee": "EAI_two", + }, + } + builder, _, ent := newTestEnterpriseRoleBuilder(t, stub) + + require.ElementsMatch(t, []string{"202", "303"}, principals(drainGrants(t, builder, ent.Resource))) + require.Equal(t, 2, stub.memberQueries) + require.Equal(t, 2, stub.invitationBatches) +} + +// The owners are read through the organization, so an organization that +// belongs to a different enterprise would report the wrong owners. +func TestEnterpriseRoleVerifyOrganization(t *testing.T) { + t.Parallel() + ctx := context.Background() + + stub := &enterpriseStub{} + builder, _, _ := newTestEnterpriseRoleBuilder(t, stub) + enterpriseClients, err := builder.clients(ctx) + require.NoError(t, err) + client := enterpriseClients[testEnterprise] + + require.NoError(t, client.verifyOrganization(ctx, testEnterprise)) + require.Equal(t, 1, stub.organizationChecks) + + client.org = "org-of-another-enterprise" + err = client.verifyOrganization(ctx, testEnterprise) + require.ErrorContains(t, err, "does not belong to enterprise") +} + +// A missing enterprise installation must reach the syncer as an error. The +// SDK fails the run on it, which is the point: C1 deletes every resource of a +// type that a completed sync did not report, so finishing the sync while +// reading no owners would drop the Owner role and every grant on it. +func TestEnterpriseRoleFailsClosedWithoutEnterpriseClients(t *testing.T) { + t.Parallel() + ctx := context.Background() + + clientsErr := status.Error(codes.FailedPrecondition, + "github-connector: GitHub App is not installed on enterprise") + builds := 0 + builder := EnterpriseRoleBuilder(nil, nil, nil, []string{testEnterprise}, + func(context.Context) (map[string]*githubEnterpriseAdministratorClient, error) { + builds++ + return nil, clientsErr + }, + ) + + _, _, err := builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.ErrorIs(t, err, clientsErr) + + roleResource, err := resourceSdk.NewRoleResource( + enterpriseRoleOwner, + resourceTypeEnterpriseRole, + testEnterprise+":"+enterpriseRoleOwner, + []resourceSdk.RoleTraitOption{}, + ) + require.NoError(t, err) + + _, _, err = builder.Grants(ctx, roleResource, resourceSdk.SyncOpAttrs{}) + require.ErrorIs(t, err, clientsErr) + + // Nothing is remembered from a failure: GitHub answers 404 for an + // uninstalled app, a revoked permission and a typo alike, so an operator + // who fixes it is picked up by the next call rather than by a restart. + require.Equal(t, 2, builds) +} + +// No build failure is remembered. GitHub answers 404 for an uninstalled app, +// a revoked permission and a typo alike, and errors from go-github or a +// cancelled context arrive wrapped with %w carrying no gRPC status at all, so +// nothing here can be classified as permanent. Remembering any of them would +// disable the resource type until the process restarts, and would leave an +// operator who fixed the configuration still reading the stale answer. +func TestEnterpriseRoleRetriesAClientBuildFailure(t *testing.T) { + t.Parallel() + ctx := context.Background() + + for _, tc := range []struct { + name string + err error + }{ + {"a misconfiguration", status.Error(codes.FailedPrecondition, "not installed on the enterprise")}, + {"a rate limit", status.Error(codes.Unavailable, "rate limited")}, + {"a go-github failure", fmt.Errorf("github-connector: failed to create installation token: %w", + &github.ErrorResponse{ + Response: &http.Response{StatusCode: http.StatusBadGateway, Request: &http.Request{}}, + Message: "Bad gateway", + })}, + {"a cancelled sync", fmt.Errorf("github-connector: discovering installations: %w", context.Canceled)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + builds := 0 + builder := EnterpriseRoleBuilder(nil, nil, nil, []string{testEnterprise}, + func(context.Context) (map[string]*githubEnterpriseAdministratorClient, error) { + builds++ + if builds == 1 { + return nil, tc.err + } + // List only checks the enterprise is present, so the + // client itself is never dereferenced here. + return map[string]*githubEnterpriseAdministratorClient{testEnterprise: nil}, nil + }, + ) + + _, _, err := builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.ErrorIs(t, err, tc.err) + + resources, _, err := builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, resources, 1) + require.Equal(t, 2, builds) + + // Once it succeeds the result is memoized. + _, _, err = builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Equal(t, 2, builds) + }) + } +} + +// Provisioning is only possible through an enterprise installation, so the PAT +// path must reject it rather than attempt a mutation it cannot make. +func TestEnterpriseRoleProvisioningTargetGuards(t *testing.T) { + t.Parallel() + ctx := context.Background() + + stub := &enterpriseStub{} + builder, principal, ent := newTestEnterpriseRoleBuilder(t, stub) + + t.Run("rejects a principal that is not a user", func(t *testing.T) { + t.Parallel() + notAUser := &v2.Resource{Id: &v2.ResourceId{ + ResourceType: resourceTypeTeam.Id, + Resource: "1", + }} + _, _, err := builder.Grant(ctx, notAUser, ent) + require.Equal(t, codes.InvalidArgument, status.Code(err)) + }) + + // Under a PAT there is no app to install, so naming one sends the operator + // after a fix that cannot apply. + t.Run("names the credential when the token cannot provision", func(t *testing.T) { + t.Parallel() + patBuilder := EnterpriseRoleProvisioningBuilder(nil, nil, nil, []string{testEnterprise}, nil) + _, _, err := patBuilder.Grant(ctx, principal, ent) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.Contains(t, err.Error(), "needs GitHub App authentication") + }) + + // Under app auth every other reason already failed the client build, so + // what reaches here is an entitlement naming an enterprise this connector + // was never configured for. + t.Run("names an enterprise that is not configured", func(t *testing.T) { + t.Parallel() + appBuilder := EnterpriseRoleProvisioningBuilder(nil, nil, nil, []string{testEnterprise}, + func(context.Context) (map[string]*githubEnterpriseAdministratorClient, error) { + return map[string]*githubEnterpriseAdministratorClient{}, nil + }, + ) + _, _, err := appBuilder.Grant(ctx, principal, ent) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.Contains(t, err.Error(), "not one of the configured enterprises") + }) +} + +// GitHub reports a GraphQL budget error as an HTTP 200 carrying errors[], so +// the status of the response says nothing. Reading the owners is the hottest +// GraphQL path in this role, and an unclassified budget error reaches the SDK +// as Unknown, which it does not retry: the sync aborts instead of backing off. +func TestEnterpriseRoleGrantsClassifyARateLimitedOwnersRead(t *testing.T) { + t.Parallel() + + builder, _, ent := newTestEnterpriseRoleBuilder(t, &enterpriseStub{ownersRateLimited: true}) + + _, _, err := builder.Grants(context.Background(), ent.Resource, resourceSdk.SyncOpAttrs{}) + require.Equal(t, codes.Unavailable, status.Code(err)) +} + +// An enterprise owner does not have to be an owner of the organization the app +// reads them through: observed live with a user whose organizationRole was +// DIRECT_MEMBER. Organization.enterpriseOwners returns them regardless, and the +// connector must emit their grant. +func TestEnterpriseRoleGrantsIncludeOwnerWhoIsNotAnOrgOwner(t *testing.T) { + t.Parallel() + + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{{ + {id: 158784853, login: "org-owner"}, + {id: 162376288, login: "enterprise-owner-only"}, + }}, + } + builder, _, ent := newTestEnterpriseRoleBuilder(t, stub) + + require.ElementsMatch(t, []string{"158784853", "162376288"}, + principals(drainGrants(t, builder, ent.Resource))) +} + +func TestEnterpriseRoleGrantsPagination(t *testing.T) { + t.Parallel() + ctx := context.Background() + + stub := &enterpriseStub{ + ownerPages: [][]enterpriseStubOwner{ + {{id: 101, login: "owner-page-one"}}, + {{id: 102, login: "owner-page-two"}}, + }, + } + builder, _, ent := newTestEnterpriseRoleBuilder(t, stub) + + resources, _, err := builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, resources, 1) + require.Equal(t, testEnterprise+":"+enterpriseRoleOwner, resources[0].GetId().GetResource()) + require.Zero(t, stub.ownerQueries) + + grants, result, err := builder.Grants(ctx, ent.Resource, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, grants, 1) + require.Equal(t, "101", grants[0].GetPrincipal().GetId().GetResource()) + require.NotEmpty(t, result.NextPageToken) + require.NotEmpty(t, result.Annotations) + + // The SDK drives the second page with the cursor from the first. + grants, result, err = builder.Grants(ctx, ent.Resource, resourceSdk.SyncOpAttrs{ + PageToken: pagination.Token{Token: result.NextPageToken}, + }) + require.NoError(t, err) + require.Len(t, grants, 1) + require.Equal(t, "102", grants[0].GetPrincipal().GetId().GetResource()) + require.Equal(t, 2, stub.ownerQueries) + + // The owners are exhausted, so the token moves on to the invitations + // rather than ending the sync. + require.NotEmpty(t, result.NextPageToken) + grants, result, err = builder.Grants(ctx, ent.Resource, resourceSdk.SyncOpAttrs{ + PageToken: pagination.Token{Token: result.NextPageToken}, + }) + require.NoError(t, err) + require.Empty(t, grants) + require.Empty(t, result.NextPageToken) + require.Equal(t, 2, stub.ownerQueries, "the invitation phase must not re-read the owners") + require.Equal(t, 1, stub.memberQueries) +} diff --git a/pkg/connector/graphql_transport.go b/pkg/connector/graphql_transport.go index d18fbf0c..0078de9c 100644 --- a/pkg/connector/graphql_transport.go +++ b/pkg/connector/graphql_transport.go @@ -1,12 +1,16 @@ package connector import ( + "bytes" + "encoding/json" "fmt" "io" "net/http" + "strings" "github.com/conductorone/baton-sdk/pkg/ratelimit" "github.com/conductorone/baton-sdk/pkg/uhttp" + "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -55,3 +59,130 @@ func (t *statusClassifyingTransport) RoundTrip(req *http.Request) (*http.Respons } return nil, st.Err() } + +// graphQLEnvelope is the body GraphQL answers with. Data and Errors can both +// be populated at once, which is how a partially resolved request comes back: +// the fields that resolved sit in Data and the rest report in Errors. +type graphQLEnvelope struct { + Data map[string]json.RawMessage `json:"data"` + Errors []graphQLError `json:"errors"` +} + +// graphQLError is one entry of the errors array GraphQL returns in the body. +type graphQLError struct { + Message string `json:"message"` + Type string `json:"type"` + Extensions struct { + Code string `json:"code"` + } `json:"extensions"` +} + +// The error types GitHub puts on a GraphQL error. They live here, next to the +// classifier, because callers that tolerate a specific type have to agree with +// it: the invitation batch drops NOT_FOUND entries before classifying, and a +// drifting spelling would silently stop dropping them. +const ( + graphQLErrorNotFound = "NOT_FOUND" + graphQLErrorForbidden = "FORBIDDEN" + graphQLErrorUnauthenticated = "UNAUTHENTICATED" + graphQLErrorUnprocessable = "UNPROCESSABLE" +) + +// graphQLErrorType normalizes where GitHub puts the classification, which +// differs between the legacy top-level field and the extensions object. +func graphQLErrorType(graphQLErr graphQLError) string { + if code := strings.ToUpper(graphQLErr.Extensions.Code); code != "" { + return code + } + + return strings.ToUpper(graphQLErr.Type) +} + +// enterpriseGraphQLTransport classifies the errors GitHub reports inside an +// HTTP 200 GraphQL body, so callers can branch on a gRPC code instead of +// matching error text. +// +// It wraps only the enterprise administration client, because the enterprise +// owner mutations rely on telling apart a rejected invitation (which Grant +// refuses rather than working around), a rate limit (retryable) and a missing +// invitation (already revoked). The shared GraphQL client keeps returning the library's +// own error untouched, because userResourceType.checkOrgSAML detects +// enterprise-level SAML by matching the text of that error. +type enterpriseGraphQLTransport struct { + base http.RoundTripper +} + +func (t *enterpriseGraphQLTransport) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := t.base.RoundTrip(req) + if err != nil || resp == nil { + return resp, err + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return resp, nil + } + + body, readErr := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if readErr != nil { + return nil, fmt.Errorf("read GraphQL response: %w", readErr) + } + resp.Body = io.NopCloser(bytes.NewReader(body)) + + var envelope graphQLEnvelope + if unmarshalErr := json.Unmarshal(body, &envelope); unmarshalErr != nil { + // A body this transport cannot parse is not a transport failure: hand + // it to the GraphQL library, which owns decoding the payload. + return resp, nil //nolint:nilerr // decoding the payload is the library's job + } + if len(envelope.Errors) == 0 { + return resp, nil + } + + messages := make([]string, 0, len(envelope.Errors)) + for _, graphQLErr := range envelope.Errors { + messages = append(messages, graphQLErr.Message) + } + st := status.New(graphQLErrorsCode(envelope.Errors), strings.Join(messages, "; ")) + if rlDesc, _ := ratelimit.ExtractRateLimitData(resp.StatusCode, &resp.Header); rlDesc != nil { + if withDetails, detailsErr := st.WithDetails(rlDesc); detailsErr == nil { + st = withDetails + } + } + + return nil, st.Err() +} + +// graphQLErrorsCode maps a GraphQL errors array onto a gRPC code. A rate limit +// wins over everything else so the SDK retries instead of failing the sync, and +// a credential or state error wins over NOT_FOUND because callers treat +// NOT_FOUND as a benign absence. Past that the first classified entry wins, so +// a later error cannot mask the code an earlier one already established. +func graphQLErrorsCode(graphQLErrors []graphQLError) codes.Code { + code := codes.Internal + for _, graphQLErr := range graphQLErrors { + errorType := graphQLErrorType(graphQLErr) + + switch { + case strings.Contains(errorType, "RATE_LIMIT"), strings.Contains(errorType, "RATELIMIT"): + return codes.Unavailable + // A credential problem outranks NOT_FOUND specifically. Callers read + // NOT_FOUND as "the thing is already gone" and report success, so a + // FORBIDDEN hidden behind one would turn a permission failure into a + // silent no-op. It does not outrank the rest, which are answers about + // the request rather than about the credential. + case errorType == graphQLErrorForbidden && (code == codes.Internal || code == codes.NotFound): + code = codes.PermissionDenied + case errorType == graphQLErrorUnauthenticated && (code == codes.Internal || code == codes.NotFound): + code = codes.Unauthenticated + // UNPROCESSABLE is how GitHub rejects a well-formed mutation that the + // current state does not allow, e.g. inviting someone who already + // administers the enterprise. + case errorType == graphQLErrorUnprocessable && (code == codes.Internal || code == codes.NotFound): + code = codes.FailedPrecondition + case errorType == graphQLErrorNotFound && code == codes.Internal: + code = codes.NotFound + } + } + + return code +} diff --git a/pkg/connector/graphql_transport_test.go b/pkg/connector/graphql_transport_test.go index f5384efc..f89e5b70 100644 --- a/pkg/connector/graphql_transport_test.go +++ b/pkg/connector/graphql_transport_test.go @@ -2,6 +2,7 @@ package connector import ( "context" + "io" "net/http" "net/http/httptest" "strconv" @@ -70,6 +71,77 @@ func TestStatusClassifyingTransport_PassesThrough2xx(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) } +func TestGraphQLErrorsCode(t *testing.T) { + cases := []struct { + name string + types []string + want codes.Code + }{ + {name: "unclassified", types: []string{"SOMETHING_NEW"}, want: codes.Internal}, + {name: "forbidden", types: []string{"FORBIDDEN"}, want: codes.PermissionDenied}, + {name: "not found", types: []string{"NOT_FOUND"}, want: codes.NotFound}, + {name: "unauthenticated", types: []string{"UNAUTHENTICATED"}, want: codes.Unauthenticated}, + {name: "unprocessable", types: []string{"UNPROCESSABLE"}, want: codes.FailedPrecondition}, + // A rate limit outranks everything: the SDK has to retry rather than + // fail the sync. + {name: "rate limit wins", types: []string{"FORBIDDEN", "RATE_LIMITED"}, want: codes.Unavailable}, + // Past a rate limit the first classified entry wins, so a later error + // cannot mask it. Grant must preserve UNPROCESSABLE as the actionable + // FailedPrecondition, and a trailing FORBIDDEN used to overwrite it. + {name: "first classified wins", types: []string{"UNPROCESSABLE", "FORBIDDEN"}, want: codes.FailedPrecondition}, + {name: "first classified entry wins", types: []string{"FORBIDDEN", "UNPROCESSABLE"}, want: codes.PermissionDenied}, + // NOT_FOUND is the one code a credential error may override. Callers + // read it as "already gone" and report success, so a batch whose + // missing invitations hide a FORBIDDEN must not look benign. + {name: "credential error beats not found", types: []string{"NOT_FOUND", "FORBIDDEN"}, want: codes.PermissionDenied}, + {name: "credential error beats not found, unauthenticated", types: []string{"NOT_FOUND", "UNAUTHENTICATED"}, want: codes.Unauthenticated}, + {name: "not found alone still maps to not found", types: []string{"NOT_FOUND", "NOT_FOUND"}, want: codes.NotFound}, + // Order must not hide Grant's unsafe-promotion rejection, so + // UNPROCESSABLE outranks NOT_FOUND from either position. + {name: "unprocessable beats not found", types: []string{"NOT_FOUND", "UNPROCESSABLE"}, want: codes.FailedPrecondition}, + {name: "unprocessable beats not found, reversed", types: []string{"UNPROCESSABLE", "NOT_FOUND"}, want: codes.FailedPrecondition}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + graphQLErrors := make([]graphQLError, 0, len(tc.types)) + for _, errorType := range tc.types { + graphQLErrors = append(graphQLErrors, graphQLError{Type: errorType}) + } + require.Equal(t, tc.want, graphQLErrorsCode(graphQLErrors)) + }) + } +} + +// extensions.code is preferred over the top-level type, because GitHub sets it +// on the errors that carry a machine-readable classification. +func TestGraphQLErrorsCodePrefersExtensionsCode(t *testing.T) { + graphQLErr := graphQLError{Type: "FORBIDDEN"} + graphQLErr.Extensions.Code = "RATE_LIMITED" + + require.Equal(t, codes.Unavailable, graphQLErrorsCode([]graphQLError{graphQLErr})) +} + +func TestEnterpriseGraphQLTransport_PassesThroughUnparseableBody(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`not json`)) + })) + t.Cleanup(srv.Close) + + client := &http.Client{ + Transport: &enterpriseGraphQLTransport{base: http.DefaultTransport}, + } + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, srv.URL+"/graphql", nil) + require.NoError(t, err) + + resp, err := client.Do(req) + require.NoError(t, err) + require.NotNil(t, resp) + t.Cleanup(func() { _ = resp.Body.Close() }) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, "not json", string(body)) +} + func TestStatusClassifyingTransport_ClassifiesClient4xx(t *testing.T) { cases := []struct { httpStatus int diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index ece84762..5d34d3a4 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -324,6 +324,35 @@ func isAuthError(resp *github.Response) bool { return resp.StatusCode == http.StatusUnauthorized } +// freshestRateLimit returns current with the rate limit of latest applied, +// keeping the earlier budget when latest reports none. +// +// It replaces rather than appends because Annotations.Pick returns the first +// match: a second descriptor added by Merge is never read, so the stale one +// would win over the fresh one. +func freshestRateLimit(current, latest annotations.Annotations) annotations.Annotations { + var rateLimit v2.RateLimitDescription + found, err := latest.Pick(&rateLimit) + if err != nil || !found { + return current + } + current.Update(&rateLimit) + + return current +} + +// isPermissionDenied reports the same condition as isPermissionError, for the +// paths that only have a typed error: the GraphQL clients return a gRPC status +// rather than a *github.Response. +func isPermissionDenied(err error) bool { + var grpcErr interface{ GRPCStatus() *status.Status } + if errors.As(err, &grpcErr) { + return grpcErr.GRPCStatus().Code() == codes.PermissionDenied + } + + return false +} + func isPermissionError(resp *github.Response) bool { if resp == nil { return false diff --git a/pkg/connector/token_refresh.go b/pkg/connector/token_refresh.go index e34db985..5827c528 100644 --- a/pkg/connector/token_refresh.go +++ b/pkg/connector/token_refresh.go @@ -4,7 +4,6 @@ import ( "context" "io" "net/http" - "net/url" "strings" "sync" @@ -158,16 +157,10 @@ func newGitHubAppClients(instanceURL string, httpClient *http.Client) (*github.C Transport: &statusClassifyingTransport{base: httpClient.Transport}, } - var gqlClient *githubv4.Client - if instanceURL != "" && instanceURL != githubDotCom { - gqlURL, err := url.Parse(instanceURL) - if err != nil { - return nil, nil, err - } - gqlURL.Path = "/api/graphql" - gqlClient = githubv4.NewEnterpriseClient(gqlURL.String(), gqlHTTPClient) - } else { - gqlClient = githubv4.NewClient(gqlHTTPClient) + endpoint, err := enterpriseGraphQLEndpoint(instanceURL) + if err != nil { + return nil, nil, err } - return gc, gqlClient, nil + + return gc, githubv4.NewEnterpriseClient(endpoint.String(), gqlHTTPClient), nil } diff --git a/pkg/customclient/client.go b/pkg/customclient/client.go index 85485447..e123a183 100644 --- a/pkg/customclient/client.go +++ b/pkg/customclient/client.go @@ -4,35 +4,121 @@ import ( "context" "fmt" "net/http" + "net/url" + "strconv" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/uhttp" "github.com/google/go-github/v69/github" ) +// githubMaxPageSize is the largest per_page the GitHub REST API accepts; the +// default is 30. +const githubMaxPageSize = 100 + +// Endpoint paths, one element per path segment, because endpoint() escapes +// each element it is given. +func enterpriseInstallationPath(enterprise string) []string { + return []string{"enterprises", enterprise, "installation"} +} + +func consumedLicensesPathParts(enterprise string) []string { + return []string{"enterprises", enterprise, "consumed-licenses"} +} + // used for endpoints not in the go-github library // example: https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/license?apiVersion=2022-11-28#list-enterprise-consumed-licenses type Client struct { *uhttp.BaseHttpClient + // baseURL is the go-github client's own base. Keeping it is what makes + // these endpoints follow --instance-url: on GitHub Enterprise Server + // WithEnterpriseURLs sets it to https://host/api/v3/, while a literal + // api.github.com would query GitHub.com instead of the instance. + baseURL *url.URL } func New(client *github.Client) *Client { return &Client{ BaseHttpClient: uhttp.NewBaseHttpClient(client.Client()), + baseURL: client.BaseURL, + } +} + +// endpoint resolves path segments against the base URL. Each element is one +// segment and is escaped, because url.JoinPath treats its arguments as +// already-escaped path: an unescaped value containing a slash would silently +// add segments. +// +// Escaping does not stop "." or ".." from being resolved away, which is fine +// here — every segment is either a constant or an operator-supplied config +// value, not caller input. +// +// github.NewClient always sets a base URL, so that error only fires on a +// hand-built Client. +func (c *Client) endpoint(segments ...string) (string, error) { + if c.baseURL == nil { + return "", fmt.Errorf("github client has no base URL") + } + + escaped := make([]string, 0, len(segments)) + for _, segment := range segments { + escaped = append(escaped, url.PathEscape(segment)) + } + + return url.JoinPath(c.baseURL.String(), escaped...) +} + +// GetEnterpriseInstallation returns this app's installation on one enterprise. +// +// Authenticates with the app's JWT, not with an installation token, and is +// decoded through this package's model because go-github's installation +// account is a *User, which carries no enterprise slug. +// https://docs.github.com/en/rest/apps/apps#get-an-enterprise-installation-for-the-authenticated-app +func (c *Client) GetEnterpriseInstallation(ctx context.Context, enterprise string) (*AppInstallation, *v2.RateLimitDescription, error) { + endpoint, err := c.endpoint(enterpriseInstallationPath(enterprise)...) + if err != nil { + return nil, nil, fmt.Errorf("error building the enterprise installation URL: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return nil, nil, fmt.Errorf("error creating request to get the enterprise installation: %w", err) } + + var target AppInstallation + var rateLimitData v2.RateLimitDescription + res, err := c.Do(req, + uhttp.WithJSONResponse(&target), + uhttp.WithRatelimitData(&rateLimitData), + ) + if err != nil { + if res != nil { + defer res.Body.Close() + logBody(ctx, res.Body) + } + return nil, &rateLimitData, fmt.Errorf("error getting the installation of enterprise %s: %w", enterprise, err) + } + + defer res.Body.Close() + + return &target, &rateLimitData, nil } // https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/license?apiVersion=2022-11-28#list-enterprise-consumed-licenses func (c *Client) ListEnterpriseConsumedLicenses(ctx context.Context, enterprise string, page int) (*EnterpriseConsumedLicense, *v2.RateLimitDescription, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("https://api.github.com/enterprises/%s/consumed-licenses", enterprise), nil) + endpoint, err := c.endpoint(consumedLicensesPathParts(enterprise)...) + if err != nil { + return nil, nil, fmt.Errorf("error building the consumed licenses URL: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { return nil, nil, fmt.Errorf("error creating request to list enterprise consumed licenses: %w", err) } q := req.URL.Query() - q.Add("page", fmt.Sprintf("%d", page)) - // GitHub REST API max per_page is 100, default is 30. - q.Add("per_page", "100") + q.Add("page", strconv.Itoa(page)) + q.Add("per_page", strconv.Itoa(githubMaxPageSize)) req.URL.RawQuery = q.Encode() var target EnterpriseConsumedLicense diff --git a/pkg/customclient/models.go b/pkg/customclient/models.go index c77da78d..ecf80e54 100644 --- a/pkg/customclient/models.go +++ b/pkg/customclient/models.go @@ -1,5 +1,9 @@ package customclient +type AppInstallation struct { + ID int64 `json:"id"` +} + // https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/license?apiVersion=2022-11-28#list-enterprise-consumed-licenses type EnterpriseConsumedLicense struct { TotalSeatsConsumed int `json:"total_seats_consumed"` diff --git a/test/mocks/endpointpattern.go b/test/mocks/endpointpattern.go index b7506972..368b08fa 100644 --- a/test/mocks/endpointpattern.go +++ b/test/mocks/endpointpattern.go @@ -2,9 +2,13 @@ package mocks import "github.com/migueleliasweb/go-github-mock/src/mock" +// methodGet is extracted so goconst stops flagging the repeated literal across +// the endpoint patterns below. +const methodGet = "GET" + var GetUserById = mock.EndpointPattern{ Pattern: "/user/{id}", - Method: "GET", + Method: methodGet, } var PutOrganizationsTeamsMembershipsByOrganizationByTeamIdByUsername = mock.EndpointPattern{ @@ -19,48 +23,48 @@ var DeleteOrganizationsTeamsMembershipsByOrganizationByTeamIdByUsername = mock.E var GetOrganizationById = mock.EndpointPattern{ Pattern: "/organizations/{org_id}", - Method: "GET", + Method: methodGet, } var GetOrgsByOrg = mock.EndpointPattern{ Pattern: "/orgs/{org}", - Method: "GET", + Method: methodGet, } var GetRepositoryById = mock.EndpointPattern{ Pattern: "/repositories/{repository_id}", - Method: "GET", + Method: methodGet, } var GetOrganizationsTeamByTeamId = mock.EndpointPattern{ Pattern: "/organizations/{org_id}/team/{team_id}", - Method: "GET", + Method: methodGet, } var GetOrganizationsTeamsMembersByTeamId = mock.EndpointPattern{ Pattern: "/organizations/{org_id}/team/{team_id}/members", - Method: "GET", + Method: methodGet, } var GetOrganizationsTeamsMembershipsByTeamIdByUsername = mock.EndpointPattern{ Pattern: "/organizations/{org_id}/team/{team_id}/memberships/{username}", - Method: "GET", + Method: methodGet, } // Organization role endpoints. var GetOrgsRolesByOrg = mock.EndpointPattern{ Pattern: "/orgs/{org}/organization-roles", - Method: "GET", + Method: methodGet, } var GetOrgsRolesTeamsByOrgByRoleId = mock.EndpointPattern{ Pattern: "/orgs/{org}/organization-roles/{role_id}/teams", - Method: "GET", + Method: methodGet, } var GetOrgsRolesUsersByOrgByRoleId = mock.EndpointPattern{ Pattern: "/orgs/{org}/organization-roles/{role_id}/users", - Method: "GET", + Method: methodGet, } var PutOrgsRolesUsersByOrgByRoleIdByUsername = mock.EndpointPattern{