diff --git a/pkg/accessreview/drivers/name_resolver.go b/pkg/accessreview/drivers/name_resolver.go index fedb649b8..455751de7 100644 --- a/pkg/accessreview/drivers/name_resolver.go +++ b/pkg/accessreview/drivers/name_resolver.go @@ -1760,3 +1760,52 @@ func (r *googleAnalyticsNameResolver) ResolveInstanceName(ctx context.Context) ( return resp.DisplayName, nil } + +// segmentNameResolver resolves the Segment workspace name. The Public API +// binds a token to exactly one workspace and exposes it at the API root, so +// the base URL (which already encodes the US/EU region) is the whole request. +type segmentNameResolver struct { + httpClient *http.Client + baseURL string +} + +func NewSegmentNameResolver(httpClient *http.Client, baseURL string) NameResolver { + return &segmentNameResolver{httpClient: httpClient, baseURL: baseURL} +} + +func (r *segmentNameResolver) ResolveInstanceName(ctx context.Context) (string, error) { + if r.baseURL == "" { + return "", nil + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, r.baseURL, nil) + if err != nil { + return "", fmt.Errorf("cannot create segment workspace request: %w", err) + } + + req.Header.Set("Accept", "application/json") + + httpResp, err := r.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("cannot execute segment workspace request: %w", err) + } + + defer func() { _ = httpResp.Body.Close() }() + + if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 { + return "", nameStatusError("segment workspace", httpResp.StatusCode) + } + + var resp struct { + Data struct { + Workspace struct { + Name string `json:"name"` + } `json:"workspace"` + } `json:"data"` + } + if err := json.NewDecoder(httpResp.Body).Decode(&resp); err != nil { + return "", fmt.Errorf("cannot decode segment workspace response: %w", err) + } + + return resp.Data.Workspace.Name, nil +} diff --git a/pkg/accessreview/drivers/name_resolver_test.go b/pkg/accessreview/drivers/name_resolver_test.go index b2df2e503..da8797d75 100644 --- a/pkg/accessreview/drivers/name_resolver_test.go +++ b/pkg/accessreview/drivers/name_resolver_test.go @@ -936,3 +936,109 @@ func TestGoogleAnalyticsNameResolver(t *testing.T) { }) } } + +func TestSegmentNameResolver(t *testing.T) { + t.Parallel() + + t.Run("reads the workspace name from the API root", func(t *testing.T) { + t.Parallel() + + var gotPath string + + srv := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + _, _ = w.Write([]byte(`{"data":{"workspace":{"id":"9aQ1Lj62S4bomZKLF4DPqW","name":"Acme Prod","slug":"acme-prod"}}}`)) + }), + ) + t.Cleanup(srv.Close) + + name, err := NewSegmentNameResolver(srv.Client(), srv.URL).ResolveInstanceName(context.Background()) + require.NoError(t, err) + assert.Equal(t, "Acme Prod", name) + // Get Workspace is the API root, not /workspace or /workspaces/{id}. + assert.Equal(t, "/", gotPath) + }) + + // A revoked token must not make the source-name worker retry forever. + t.Run("a client error is terminal", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }), + ) + t.Cleanup(srv.Close) + + _, err := NewSegmentNameResolver(srv.Client(), srv.URL).ResolveInstanceName(context.Background()) + require.Error(t, err) + assert.ErrorIs(t, err, ErrTerminalNameResolution) + }) + + t.Run("a server error stays retryable", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadGateway) + }), + ) + t.Cleanup(srv.Close) + + _, err := NewSegmentNameResolver(srv.Client(), srv.URL).ResolveInstanceName(context.Background()) + require.Error(t, err) + assert.False(t, errors.Is(err, ErrTerminalNameResolution)) + }) + + t.Run("an unset base URL resolves to nothing", func(t *testing.T) { + t.Parallel() + + name, err := NewSegmentNameResolver(http.DefaultClient, "").ResolveInstanceName(context.Background()) + require.NoError(t, err) + assert.Empty(t, name) + }) +} + +// The Segment Public API declares permissions on the shared UserV1 schema but +// today only populates it on the single-user read, so the driver falls back to +// GET /users/{id}. This pins the other branch: when the list does carry +// permissions, no per-user request is made. The httptest server fails the test +// if one is. +func TestSegmentDriverUsesInlinePermissions(t *testing.T) { + t.Parallel() + + var perUserCalls int + + srv := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/users": + _, _ = w.Write([]byte(`{"data":{"users":[{"id":"u1","name":"Ada","email":"ada@example.com","permissions":[{"roleName":"Workspace Owner"}]},{"id":"u2","name":"Bob","email":"bob@example.com","permissions":[]}],"pagination":{}}}`)) + case "/invites": + _, _ = w.Write([]byte(`{"data":{"invites":[],"pagination":{}}}`)) + default: + perUserCalls++ + + w.WriteHeader(http.StatusInternalServerError) + } + }), + ) + t.Cleanup(srv.Close) + + records, err := NewSegmentDriver(srv.Client(), srv.URL).ListAccounts(context.Background()) + require.NoError(t, err) + require.Len(t, records, 2) + + assert.Zero(t, perUserCalls, "inline permissions must not trigger the per-user fetch") + + assert.Equal(t, "ada@example.com", records[0].Email) + assert.True(t, records[0].IsAdmin) + assert.Equal(t, []string{"Workspace Owner"}, records[0].Roles) + + // An empty (but present) permissions array is authoritative: the user + // genuinely has no roles, so it must not be mistaken for "not populated". + assert.Equal(t, "bob@example.com", records[1].Email) + assert.False(t, records[1].IsAdmin) + assert.Empty(t, records[1].Roles) +} diff --git a/pkg/accessreview/drivers/segment.go b/pkg/accessreview/drivers/segment.go index 6fb7584b5..ea5044797 100644 --- a/pkg/accessreview/drivers/segment.go +++ b/pkg/accessreview/drivers/segment.go @@ -59,6 +59,13 @@ type segmentUser struct { ID string `json:"id"` Name string `json:"name"` Email string `json:"email"` + // Permissions is declared on the shared UserV1 schema that both /users + // and /users/{id} return, but is optional and today only populated by + // the single-user read. Decoding it here means the driver uses whatever + // the list gives it rather than assuming: nil (field absent) triggers the + // per-user fetch, non-nil — including an empty array for a user with no + // roles — is taken as authoritative. + Permissions []segmentPermission `json:"permissions"` } type segmentPermission struct { @@ -126,9 +133,12 @@ func (d *SegmentDriver) ListAccounts(ctx context.Context) ([]AccountRecord, erro seen[strings.ToLower(email)] = struct{}{} - perms, err := d.userPermissions(ctx, base, u.ID) - if err != nil { - return nil, fmt.Errorf("cannot list segment permissions for user %q: %w", u.ID, err) + perms := u.Permissions + if perms == nil { + perms, err = d.userPermissions(ctx, base, u.ID) + if err != nil { + return nil, fmt.Errorf("cannot list segment permissions for user %q: %w", u.ID, err) + } } roles, isAdmin := segmentRolesAndAdmin(perms) diff --git a/pkg/connector/provider/segment.go b/pkg/connector/provider/segment.go index 5ad243d54..bce9d773f 100644 --- a/pkg/connector/provider/segment.go +++ b/pkg/connector/provider/segment.go @@ -46,8 +46,6 @@ func segmentRegistration() *Registration { {Key: "region", Label: "Region", Required: true}, }, BuildProbeURL: buildSegmentProbeURL, - // No NewNameResolver: the Public API exposes no read-only workspace-name - // endpoint on the token's scope, so the source keeps its generic name. NewDriver: func(_ context.Context, c *http.Client, conn *coredata.Connector, _ *log.Logger) (drivers.Driver, error) { s, err := coredata.ConnectorSettings[coredata.SegmentConnectorSettings](conn) if err != nil { @@ -60,5 +58,15 @@ func segmentRegistration() *Registration { return drivers.NewSegmentDriver(c, s.BaseURL), nil }, + NewNameResolver: func(ctx context.Context, c *http.Client, conn *coredata.Connector, logger *log.Logger) drivers.NameResolver { + s, err := coredata.ConnectorSettings[coredata.SegmentConnectorSettings](conn) + if err != nil { + logger.ErrorCtx(ctx, "cannot read segment connector settings", log.Error(err)) + + return nil + } + + return drivers.NewSegmentNameResolver(c, s.BaseURL) + }, } }