From 8c02568d13cd6f5bebbad5c6914cf400c71d6f19 Mon Sep 17 00:00:00 2001 From: Lauren Leach Date: Fri, 24 Jul 2026 19:17:04 +0000 Subject: [PATCH] Guard role grant emission behind WillSyncResourceType userResourceType.Grants() emits role grants as a sync optimization (the user API response already includes role membership), but did so unconditionally. When a customer's sync filter excludes roles, the connector still emitted grants referencing an unsynced resource type. Gate the emission on cli.ConnectorOpts.WillSyncResourceType("role"), threaded from NewLambdaConnector through the Connector struct into the user builder. Also mark the user resource type with SkipEntitlementsAndGrants when roles aren't synced, since the user builder has no entitlements/grants of its own otherwise. Follows the pattern from ConductorOne/baton-linear#55. Co-authored-by: c1-squire-dev[bot] --- pkg/connector/connector.go | 22 ++++++-- pkg/connector/resource_types.go | 8 ++- pkg/connector/users.go | 25 ++++++++- pkg/connector/users_test.go | 96 +++++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 pkg/connector/users_test.go diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index a415021f..a058bedc 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -17,7 +17,8 @@ import ( ) type Connector struct { - client *client.Client + client *client.Client + syncRoles bool } // Option is a function that configures a Connector. @@ -37,9 +38,24 @@ func WithAPIKey(ctx context.Context, apiKey string, orgId string, baseURL string } } +// WithSyncRoles configures whether the connector should emit role grants +// discovered as a side effect of syncing users. This should reflect whether +// the role resource type is actually included in the sync filter. +func WithSyncRoles(syncRoles bool) Option { + return func(c *Connector) error { + c.syncRoles = syncRoles + return nil + } +} + func NewLambdaConnector(ctx context.Context, jumpcloudCfg *cfg.Jumpcloud, cliOpts *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { l := ctxzap.Extract(ctx) + syncRoles := true + if cliOpts != nil { + syncRoles = cliOpts.WillSyncResourceType(RoleResourceTypeID) + } + opts := WithAPIKey( ctx, jumpcloudCfg.ApiKey, @@ -47,7 +63,7 @@ func NewLambdaConnector(ctx context.Context, jumpcloudCfg *cfg.Jumpcloud, cliOpt jumpcloudCfg.BaseUrl, ) - cb, err := New(ctx, opts) + cb, err := New(ctx, opts, WithSyncRoles(syncRoles)) if err != nil { l.Error("error creating connector", zap.Error(err)) return nil, nil, err @@ -79,7 +95,7 @@ func New(ctx context.Context, opts ...Option) (*Connector, error) { // ResourceSyncers returns a ResourceSyncer for each resource type that should be synced from the upstream service. func (c *Connector) ResourceSyncers(ctx context.Context) []connectorbuilder.ResourceSyncerV2 { return []connectorbuilder.ResourceSyncerV2{ - newUserBuilder(c.client), + newUserBuilder(c.client, c.syncRoles), newGroupBuilder(c.client), newRoleBuilder(), newAppBuilder(c.client), diff --git a/pkg/connector/resource_types.go b/pkg/connector/resource_types.go index 0b4e4ec1..80c0a498 100644 --- a/pkg/connector/resource_types.go +++ b/pkg/connector/resource_types.go @@ -5,6 +5,12 @@ import ( "github.com/conductorone/baton-sdk/pkg/annotations" ) +// RoleResourceTypeID is the resource type ID for roles, exported so callers +// (e.g. cmd/main.go) can gate cross-type grant emission on +// cli.ConnectorOpts.WillSyncResourceType(RoleResourceTypeID) without +// duplicating the string literal. +const RoleResourceTypeID = "role" + var ( resourceTypeUser = &v2.ResourceType{ Id: "user", @@ -23,7 +29,7 @@ var ( Traits: []v2.ResourceType_Trait{v2.ResourceType_TRAIT_APP}, } resourceTypeRole = &v2.ResourceType{ - Id: "role", + Id: RoleResourceTypeID, DisplayName: "Role", Traits: []v2.ResourceType_Trait{v2.ResourceType_TRAIT_ROLE}, Annotations: annotations.New(&v2.SkipGrants{}), diff --git a/pkg/connector/users.go b/pkg/connector/users.go index 5bc55e5a..a03e7bc2 100644 --- a/pkg/connector/users.go +++ b/pkg/connector/users.go @@ -14,6 +14,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "go.uber.org/zap" "google.golang.org/grpc/codes" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" ) @@ -22,18 +23,34 @@ type userResourceType struct { client *client.Client usersCache *usersCache managers map[string]*jcapi1.Systemuserreturn + // syncRoles reports whether the role resource type is included in the + // customer's sync filter. The user builder emits role grants as a sync + // optimization (see Grants below); when roles aren't being synced those + // grants must be suppressed so they don't reference an unsynced type. + syncRoles bool } func (o *userResourceType) ResourceType(_ context.Context) *v2.ResourceType { return o.resourceType } -func newUserBuilder(client *client.Client) *userResourceType { +func newUserBuilder(client *client.Client, syncRoles bool) *userResourceType { + resourceType := resourceTypeUser + if !syncRoles { + // The user builder has no entitlements or grants of its own -- its only + // Grants() output is the cross-type role grant gated below. When roles + // aren't being synced, skip entitlement/grant discovery for users entirely. + rt := proto.Clone(resourceTypeUser).(*v2.ResourceType) + rt.Annotations = annotations.New(&v2.SkipEntitlementsAndGrants{}) + resourceType = rt + } + return &userResourceType{ - resourceType: resourceTypeUser, + resourceType: resourceType, client: client, managers: make(map[string]*jcapi1.Systemuserreturn), usersCache: newUsersCache(client), + syncRoles: syncRoles, } } @@ -42,6 +59,10 @@ func (o *userResourceType) Entitlements(_ context.Context, _ *v2.Resource, _ sdk } func (o *userResourceType) Grants(ctx context.Context, resource *v2.Resource, _ sdkResources.SyncOpAttrs) ([]*v2.Grant, *sdkResources.SyncOpResults, error) { + if !o.syncRoles { + return nil, nil, nil + } + userID := resource.Id.Resource // Only admin users have role grants. System users won't be found in the admin users endpoint. adminUser, err := o.client.GetUserByID(ctx, userID) diff --git a/pkg/connector/users_test.go b/pkg/connector/users_test.go new file mode 100644 index 00000000..726dae87 --- /dev/null +++ b/pkg/connector/users_test.go @@ -0,0 +1,96 @@ +package connector + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/conductorone/baton-jumpcloud/pkg/client" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + sdkResources "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" +) + +// TestUserGrants_RoleSyncFilter covers ConductorOne/baton-linear#55: the user +// builder emits role grants as a sync optimization, but must not do so when +// the customer's sync filter excludes the role resource type. +func TestUserGrants_RoleSyncFilter(t *testing.T) { + userResource := &v2.Resource{ + Id: fmtResourceId(resourceTypeUser.Id, "user-1"), + } + + t.Run("role type filtered out -> no grants, no API call", func(t *testing.T) { + called := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + c, err := client.NewClient(context.Background(), "api-key", "", srv.URL) + require.NoError(t, err) + + builder := newUserBuilder(c, false) + require.False(t, builder.syncRoles) + + grants, _, err := builder.Grants(context.Background(), userResource, sdkResources.SyncOpAttrs{}) + require.NoError(t, err) + require.Empty(t, grants) + require.False(t, called, "role grant discovery must not call the API when roles are filtered out") + }) + + t.Run("role type synced -> role grant emitted", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{ + "id": "user-1", + "roleName": "Administrator", + }) + })) + defer srv.Close() + + c, err := client.NewClient(context.Background(), "api-key", "", srv.URL) + require.NoError(t, err) + + builder := newUserBuilder(c, true) + require.True(t, builder.syncRoles) + + grants, _, err := builder.Grants(context.Background(), userResource, sdkResources.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, grants, 1) + require.Equal(t, fmtRoleNameAsID("Administrator"), grants[0].Entitlement.Resource.Id.Resource) + require.Equal(t, resourceTypeRole.Id, grants[0].Entitlement.Resource.Id.ResourceType) + }) +} + +// TestNewUserBuilder_ResourceTypeAnnotations covers Step 4 of the +// WillSyncResourceType gating pattern: when the user builder has no +// entitlements/grants of its own to offer, mark the emitted resource type +// with SkipEntitlementsAndGrants so the SDK doesn't bother syncing them. +func TestNewUserBuilder_ResourceTypeAnnotations(t *testing.T) { + t.Run("role type filtered out -> SkipEntitlementsAndGrants set", func(t *testing.T) { + builder := newUserBuilder(nil, false) + var skip v2.SkipEntitlementsAndGrants + ok, err := annotationsContain(builder.resourceType.GetAnnotations(), &skip) + require.NoError(t, err) + require.True(t, ok) + }) + + t.Run("role type synced -> no SkipEntitlementsAndGrants", func(t *testing.T) { + builder := newUserBuilder(nil, true) + var skip v2.SkipEntitlementsAndGrants + ok, err := annotationsContain(builder.resourceType.GetAnnotations(), &skip) + require.NoError(t, err) + require.False(t, ok) + }) +} + +func annotationsContain(annos []*anypb.Any, msg proto.Message) (bool, error) { + as := annotations.Annotations(annos) + return as.Pick(msg) +}