From 8c02568d13cd6f5bebbad5c6914cf400c71d6f19 Mon Sep 17 00:00:00 2001 From: Lauren Leach Date: Fri, 24 Jul 2026 19:17:04 +0000 Subject: [PATCH 1/2] 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) +} From 37c9ed04249b0c735c19b4cd1f8be14cbfb23283 Mon Sep 17 00:00:00 2001 From: Lauren Leach Date: Fri, 14 Aug 2026 18:36:50 -0700 Subject: [PATCH 2/2] Drop the redundant syncRoles guard in userResourceType.Grants newUserBuilder already annotates the user resource type SkipEntitlementsAndGrants when roles are excluded from the sync filter, and the SDK honors that before ever calling Grants (shouldSkipGrants -> shouldSkipEntitlementsAndGrants in pkg/sync/syncer.go), so the early return was unreachable during a sync and duplicated the condition. With the guard gone the syncRoles struct field was write-only, so it is removed too; the constructor parameter stays, since it is what selects the annotation. Dropped the subtest that existed solely to pin the early return -- TestNewUserBuilder_ResourceTypeAnnotations already covers the annotation, which is the real mechanism. --- pkg/connector/users.go | 15 +++++---------- pkg/connector/users_test.go | 24 +++--------------------- 2 files changed, 8 insertions(+), 31 deletions(-) diff --git a/pkg/connector/users.go b/pkg/connector/users.go index a03e7bc2..a9c0276c 100644 --- a/pkg/connector/users.go +++ b/pkg/connector/users.go @@ -23,11 +23,6 @@ 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 { @@ -50,7 +45,6 @@ func newUserBuilder(client *client.Client, syncRoles bool) *userResourceType { client: client, managers: make(map[string]*jcapi1.Systemuserreturn), usersCache: newUsersCache(client), - syncRoles: syncRoles, } } @@ -58,11 +52,12 @@ func (o *userResourceType) Entitlements(_ context.Context, _ *v2.Resource, _ sdk return nil, nil, nil } +// Grants emits the cross-type role grant. There is no syncRoles guard here: +// when roles aren't being synced, newUserBuilder annotates the user resource +// type SkipEntitlementsAndGrants and the SDK never calls Grants() at all +// (shouldSkipGrants -> shouldSkipEntitlementsAndGrants in the SDK's +// pkg/sync/syncer.go), so a guard would be unreachable. 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 index 726dae87..37b43931 100644 --- a/pkg/connector/users_test.go +++ b/pkg/connector/users_test.go @@ -24,26 +24,9 @@ func TestUserGrants_RoleSyncFilter(t *testing.T) { 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") - }) - + // Grants() itself is unconditional: when roles are filtered out the SDK is + // stopped by the SkipEntitlementsAndGrants annotation before it ever calls + // Grants, which TestNewUserBuilder_ResourceTypeAnnotations pins. 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") @@ -58,7 +41,6 @@ func TestUserGrants_RoleSyncFilter(t *testing.T) { 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)