Resolve the Segment workspace name and reuse listed permissions

Two corrections, both settled from Segment's published OpenAPI document
and their own client code rather than guessed.

The registration claimed the Public API exposes no workspace-name
endpoint on the token's scope. That is wrong: Get Workspace is the API
root, GET /, returning data.workspace.name for the workspace the token
is bound to — the base URL already encodes the US/EU region, so the URL
is the whole request. Without a resolver an organization running a prod
and a staging workspace saw two rows both named "Segment".

The per-user GET /users/{id} is what makes a large workspace exceed the
per-source budget, and it exists only to read permissions[].roleName.
Both endpoints return the same UserV1 schema, on which permissions is
declared but optional, so whether the list populates it is a server
behaviour no specification settles. Rather than assume, the list
response is now decoded for permissions and the per-user request is
issued only when the field is absent. Today Segment omits it — their own
Terraform provider's mock returns /users without permissions and
/users/{id} with them — so behaviour is unchanged; if that ever changes
the extra round trip disappears on its own. An empty-but-present array
is authoritative, meaning a user with no roles, not a missing field.

Page size stays at 200: the 1-1000 range is prose in the pagination
guide, the schema sets no maximum, and the migration guide says 200.

Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>

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

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

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)

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