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) +}