Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ var (
field.WithDisplayName("Optimize sync for large organizations"),
field.WithDescription(
"Reduces API calls by using grant expansion for team-based repo access "+
"and skipping per-team detail fetches. Recommended for large orgs.",
"and skipping per-team detail fetches. Recommended for large orgs. "+
"Requires the credential to be able to read the org's default repository permission "+
"(admin:org scope or the Organization Administration read permission); "+
"validation and syncs fail without it.",
),
)
orgField = field.StringField(
Expand Down
48 changes: 45 additions & 3 deletions pkg/connector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error)
}
}

adminFound := false
firstAdminOrg := ""
for _, o := range orgLogins {
membership, _, err := gh.client.Organizations.GetOrgMembership(ctx, "", o)
if err != nil {
Expand All @@ -237,14 +237,20 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error)
continue
}

adminFound = true
if firstAdminOrg == "" {
firstAdminOrg = o
}
}

if !adminFound {
if firstAdminOrg == "" {
err := fmt.Errorf("access token must be an admin on at least one organization")
return nil, uhttp.WrapErrors(codes.PermissionDenied, "github-connector: credentials validation failed", err)
}

if err := gh.validateBasePermissionVisibility(ctx, firstAdminOrg); err != nil {
return nil, err
}

if len(gh.enterprises) > 0 {
l := ctxzap.Extract(ctx)
_, _, err := gh.customClient.ListEnterpriseConsumedLicenses(ctx, gh.enterprises[0], 1)
Expand All @@ -268,6 +274,10 @@ func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annot
return nil, err
}

if err := gh.validateBasePermissionVisibility(ctx, orgLogins[0]); err != nil {
return nil, err
}

if len(gh.enterprises) > 0 {
l := ctxzap.Extract(ctx)
_, _, err := gh.customClient.ListEnterpriseConsumedLicenses(ctx, gh.enterprises[0], 1)
Expand All @@ -282,6 +292,38 @@ func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annot
return nil, nil
}

// validateBasePermissionVisibility probes a single org to confirm the credential can
// read default_repository_permission, which direct-collaborators-only requires to sync
// org-member repo access (see getOrgBasePermission). GitHub returns the field only to
// org owners whose credential has admin:org scope / the GitHub App Organization
// Administration permission, and omits it silently otherwise.
//
// The scope/permission dimension is credential-wide, so probing one org where the
// credential is already confirmed as an admin is representative — no need to probe
// every org (there can be thousands). Per-org enforcement still happens at sync time.
func (gh *GitHub) validateBasePermissionVisibility(ctx context.Context, orgLogin string) error {
if !gh.directCollaboratorsOnly {
return nil
}

org, resp, err := gh.client.Organizations.Get(ctx, orgLogin)
if err != nil {
return wrapGitHubError(err, resp, fmt.Sprintf("github-connector: failed to get organization %s", orgLogin))
}

if org.GetDefaultRepoPermission() == "" {
err := fmt.Errorf(
"org %q did not return default_repository_permission; direct-collaborators-only requires it to sync org-member repo access correctly. "+
"Grant the credential org-owner visibility (admin:org scope, the GitHub App Organization Administration permission, "+
"or fine-grained PAT organization Administration read access), or disable direct-collaborators-only",
orgLogin,
)
return uhttp.WrapErrors(codes.PermissionDenied, "github-connector: credentials validation failed", err)
}

return nil
}

// newGitHubClient returns a new GitHub API client authenticated with an access token via oauth2.
func newGitHubClient(ctx context.Context, instanceURL string, ts oauth2.TokenSource) (*github.Client, error) {
httpClient, err := uhttp.NewClient(ctx, uhttp.WithLogger(true, ctxzap.Extract(ctx)))
Expand Down
53 changes: 53 additions & 0 deletions pkg/connector/connector_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package connector

import (
"context"
"testing"

"github.com/google/go-github/v69/github"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/conductorone/baton-github/test/mocks"
)

// TestValidateBasePermissionVisibility covers the up-front credential probe for
// direct-collaborators-only: default_repository_permission must be readable or
// validation fails with PermissionDenied. Seeded mock org ID is 12 (login
// "organization-12"), and the seeded org omits default_repository_permission,
// matching what GitHub returns to credentials without org-owner visibility.
func TestValidateBasePermissionVisibility(t *testing.T) {
ctx := context.Background()

setup := func(t *testing.T, directCollaboratorsOnly bool) (*mocks.MockGitHub, *GitHub) {
t.Helper()
mgh := mocks.NewMockGitHub()
_, _, _, _, _, err := mgh.Seed()
require.NoError(t, err)
return mgh, &GitHub{
client: github.NewClient(mgh.Server()),
directCollaboratorsOnly: directCollaboratorsOnly,
}
}

t.Run("missing default_repository_permission fails validation", func(t *testing.T) {
_, gh := setup(t, true)
err := gh.validateBasePermissionVisibility(ctx, "organization-12")
require.Error(t, err)
require.Equal(t, codes.PermissionDenied, status.Code(err))
require.ErrorContains(t, err, "default_repository_permission")
})

t.Run("visible default_repository_permission passes validation", func(t *testing.T) {
mgh, gh := setup(t, true)
mgh.SetOrgDefaultRepoPermission(12, "none")
require.NoError(t, gh.validateBasePermissionVisibility(ctx, "organization-12"))
})

t.Run("skipped when direct-collaborators-only is off", func(t *testing.T) {
_, gh := setup(t, false)
// No API call is made, so even the missing field is fine.
require.NoError(t, gh.validateBasePermissionVisibility(ctx, "organization-12"))
})
}
67 changes: 36 additions & 31 deletions pkg/connector/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,37 +197,40 @@ func (o *repositoryResourceType) Grants(
// When direct-collaborators-only is enabled, org members whose only repo access
// is via the org base permission won't appear in ListCollaborators. Add expandable
// grants so the SDK resolves org membership into repo access.
//
// If the base permission can't be determined, fail the sync: guessing in
// either direction produces wrong access data (inventing grants or
// silently dropping real ones).
if o.directCollaboratorsOnly {
basePerm, err := o.getOrgBasePermission(ctx, opts.Session, orgName, resource.ParentResourceId)
if err != nil {
l.Debug("failed to fetch org base permission, skipping org expansion", zap.Error(err))
} else {
orgResID := resource.ParentResourceId.Resource
// Org admins always have admin on all repos
adminEntitlementID := fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleAdmin)
for _, perm := range repoAccessLevels {
return nil, nil, err
}
orgResID := resource.ParentResourceId.Resource
// Org admins always have admin on all repos
adminEntitlementID := fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleAdmin)
for _, perm := range repoAccessLevels {
rv = append(rv, grant.NewGrant(resource, perm, resource.ParentResourceId,
grant.WithAnnotation(&v2.GrantExpandable{
EntitlementIds: []string{adminEntitlementID},
Shallow: true,
ResourceTypeIds: []string{resourceTypeUser.Id},
}),
))
}
// Org members get access based on the org's default repo permission
memberPerms := orgBasePermissionToRepoPermissions(basePerm)
if len(memberPerms) > 0 {
memberEntitlementID := fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleMember)
for _, perm := range memberPerms {
rv = append(rv, grant.NewGrant(resource, perm, resource.ParentResourceId,
grant.WithAnnotation(&v2.GrantExpandable{
EntitlementIds: []string{adminEntitlementID},
EntitlementIds: []string{memberEntitlementID},
Shallow: true,
ResourceTypeIds: []string{resourceTypeUser.Id},
}),
))
}
// Org members get access based on the org's default repo permission
memberPerms := orgBasePermissionToRepoPermissions(basePerm)
if len(memberPerms) > 0 {
memberEntitlementID := fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleMember)
for _, perm := range memberPerms {
rv = append(rv, grant.NewGrant(resource, perm, resource.ParentResourceId,
grant.WithAnnotation(&v2.GrantExpandable{
EntitlementIds: []string{memberEntitlementID},
Shallow: true,
ResourceTypeIds: []string{resourceTypeUser.Id},
}),
))
}
}
}
}

Expand Down Expand Up @@ -549,13 +552,14 @@ func orgBasePermissionSessionKey(orgID string) string {
// getOrgBasePermission fetches the org's default_repository_permission, caching in the session.
// Returns "read", "write", "admin", or "none".
//
// An empty/absent field is treated as "none" (fail closed). GitHub only returns
// An empty/absent field is an error. GitHub only returns
// default_repository_permission to org owners / tokens with admin:org (or the
// GitHub App Organization Administration permission). An omitted field means
// "unknown", not GitHub's create-org default of "read" — assuming "read" would
// invent pull grants for every org member on every repo.
// "unknown" — guessing "read" invents pull grants for every org member on
// every repo, and guessing "none" silently drops real org-base grants. With
// direct-collaborators-only enabled we cannot produce correct grants without
// this value, so fail the sync and tell the operator how to fix the credential.
func (o *repositoryResourceType) getOrgBasePermission(ctx context.Context, ss sessions.SessionStore, orgName string, orgResourceID *v2.ResourceId) (string, error) {
l := ctxzap.Extract(ctx)
key := orgBasePermissionSessionKey(orgResourceID.Resource)
cached, found, err := session.GetJSON[string](ctx, ss, key)
if err != nil {
Expand All @@ -572,13 +576,14 @@ func (o *repositoryResourceType) getOrgBasePermission(ctx context.Context, ss se

perm := org.GetDefaultRepoPermission()
if perm == "" {
l.Debug(
"baton-github: org default_repository_permission missing or empty; skipping org-member repo expansion (treating as none). "+
"Grant the credential org-owner visibility (admin:org / Organization Administration) to sync base-permission grants accurately.",
zap.String("org", orgName),
zap.String("org_id", orgResourceID.Resource),
err := fmt.Errorf(
"org %q did not return default_repository_permission; "+
"direct-collaborators-only requires it to sync org-member repo access correctly. "+
"Grant the credential org-owner visibility (admin:org scope, the GitHub App Organization Administration permission, "+
"or fine-grained PAT organization Administration read access), or disable direct-collaborators-only",
orgName,
)
perm = "none"
return "", uhttp.WrapErrors(codes.PermissionDenied, "baton-github: failed to determine org base permission", err)
}

if err := session.SetJSON(ctx, ss, key, perm); err != nil {
Expand Down
30 changes: 25 additions & 5 deletions pkg/connector/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource"
"github.com/google/go-github/v69/github"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/conductorone/baton-github/test"
"github.com/conductorone/baton-github/test/mocks"
Expand Down Expand Up @@ -87,7 +89,9 @@ func TestRepository(t *testing.T) {
// empty default_repository_permission bug: when GitHub omits the field (the
// credential lacks org-owner visibility), the connector used to assume "read"
// and emit expandable org-member pull grants on every repo — inventing access
// for every org member. It drives Grants() end-to-end with
// for every org member. With direct-collaborators-only enabled the base
// permission is required to produce correct grants, so an omitted field must
// hard-fail the sync instead of guessing. It drives Grants() end-to-end with
// direct-collaborators-only enabled.
//
// Seeded mock IDs: org 12, repo 34, user 56.
Expand Down Expand Up @@ -150,15 +154,31 @@ func TestRepositoryGrantsOrgBasePermissionExpansion(t *testing.T) {
return mgh, builder, repository
}

t.Run("missing default_repository_permission fails closed", func(t *testing.T) {
t.Run("missing default_repository_permission fails the sync", func(t *testing.T) {
_, builder, repository := setup(t)
// The seeded org omits default_repository_permission, like GitHub does
// for credentials without admin:org / Organization Administration.
// for credentials without admin:org / Organization Administration. We
// can't know org members' repo access without it, so Grants must error
// rather than guess in either direction.
grants, _, err := builder.Grants(ctx, repository, resourceSdk.SyncOpAttrs{
PageToken: pagination.Token{},
Session: &noOpSessionStore{},
})
require.ErrorContains(t, err, "default_repository_permission")
require.Equal(t, codes.PermissionDenied, status.Code(err),
"missing default_repository_permission is a credential permission problem, not a transient failure")
require.Empty(t, grants,
"an omitted default_repository_permission must not produce grants")
})

t.Run("none default_repository_permission emits no member expansion grants", func(t *testing.T) {
mgh, builder, repository := setup(t)
mgh.SetOrgDefaultRepoPermission(12, "none")
grants := listAllGrants(t, builder, repository)

require.NotEmpty(t, grants, "expected admin-expansion and direct-collaborator grants")
require.NotEmpty(t, grants, "admin expansion and direct collaborators are still emitted")
require.Empty(t, memberExpansionGrants(t, grants),
"an omitted default_repository_permission must not invent org-member repo grants")
"base permission none must not expand org members into repo access")
})

t.Run("read default_repository_permission still expands members to pull", func(t *testing.T) {
Expand Down
Loading