Fix the Segment region input and invite duplicates
The region is a two-value allow-list the server resolves to an API host, but it rendered as a free-text field: only PostHog is special-cased in the API-key dialog, everything else falls through to a generic Field. Typing "EU1" — the region Segment's own UI shows for the EU workspace — passed the non-empty check, then failed the mutation, and the dialog's generic error blamed the API key. It is a select now, so the label no longer has to spell the accepted values out. An invite that has already been accepted can still be listed, and the member and the invite were keyed differently (user ID vs email), so the same person surfaced as two rows — one active with roles, one inactive without. Invites for an email already seen among members are dropped. Per-user permission errors now name the user, and the probe URL builds its query with url.Values rather than a hand-written string. The region-to-host mapping is the only API-key setting that derives a value instead of storing input verbatim, and it had no test; a typo in either host would only have surfaced as a live 404. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
This commit is contained in:
@@ -26,6 +26,8 @@ import {
|
||||
DialogContent,
|
||||
DialogFooter,
|
||||
Field,
|
||||
Option,
|
||||
Select,
|
||||
useDialogRef,
|
||||
useToast,
|
||||
} from "@probo/ui";
|
||||
@@ -248,7 +250,8 @@ export function APIKeyConnectorDialog({
|
||||
};
|
||||
|
||||
// PostHog renders a dedicated deployment selector (Cloud region or
|
||||
// self-hosted URL); every other provider falls back to generic fields.
|
||||
// self-hosted URL) and Segment a region selector; every other provider falls
|
||||
// back to generic fields.
|
||||
const renderAPIKeyExtraSettings = () => {
|
||||
if (!provider) {
|
||||
return null;
|
||||
@@ -263,6 +266,25 @@ export function APIKeyConnectorDialog({
|
||||
);
|
||||
}
|
||||
|
||||
// The server maps this to a regional API host and rejects anything outside
|
||||
// the allow-list, so it must not be typed by hand.
|
||||
if (provider.provider === "SEGMENT") {
|
||||
return (
|
||||
<div className="space-y-1.5">
|
||||
<label className="text-sm font-medium">{__("Region")}</label>
|
||||
<Select
|
||||
value={extraSettingValues.region ?? ""}
|
||||
onValueChange={(val: string) =>
|
||||
setExtraSettingValues(prev => ({ ...prev, region: val }))}
|
||||
placeholder={__("Select a region")}
|
||||
>
|
||||
<Option value="US">{__("United States")}</Option>
|
||||
<Option value="EU">{__("Europe")}</Option>
|
||||
</Select>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return provider.extraSettings.map((setting) => {
|
||||
const value = extraSettingValues[setting.key] ?? "";
|
||||
return (
|
||||
|
||||
@@ -116,6 +116,7 @@ func (d *SegmentDriver) ListAccounts(ctx context.Context) ([]AccountRecord, erro
|
||||
}
|
||||
|
||||
records := make([]AccountRecord, 0, len(users))
|
||||
seen := make(map[string]struct{}, len(users))
|
||||
|
||||
for _, u := range users {
|
||||
email := strings.TrimSpace(u.Email)
|
||||
@@ -123,9 +124,11 @@ func (d *SegmentDriver) ListAccounts(ctx context.Context) ([]AccountRecord, erro
|
||||
continue
|
||||
}
|
||||
|
||||
seen[strings.ToLower(email)] = struct{}{}
|
||||
|
||||
perms, err := d.userPermissions(ctx, base, u.ID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, fmt.Errorf("cannot list segment permissions for user %q: %w", u.ID, err)
|
||||
}
|
||||
|
||||
roles, isAdmin := segmentRolesAndAdmin(perms)
|
||||
@@ -157,15 +160,22 @@ func (d *SegmentDriver) ListAccounts(ctx context.Context) ([]AccountRecord, erro
|
||||
continue
|
||||
}
|
||||
|
||||
// An invite that has already been accepted can still be listed; the
|
||||
// member record above is the authoritative one, so skip the duplicate
|
||||
// rather than emit two rows for the same person.
|
||||
if _, ok := seen[strings.ToLower(email)]; ok {
|
||||
continue
|
||||
}
|
||||
|
||||
// A pending invite carries no role and no stable id at the workspace
|
||||
// level, so it is surfaced as an inactive member keyed by email.
|
||||
inactive := false
|
||||
active := false
|
||||
|
||||
records = append(records, AccountRecord{
|
||||
Email: email,
|
||||
FullName: email,
|
||||
Roles: []string{},
|
||||
Active: &inactive,
|
||||
Active: &active,
|
||||
MFAStatus: coredata.MFAStatusUnknown,
|
||||
AuthMethod: coredata.AccessReviewEntryAuthMethodUnknown,
|
||||
AccountType: coredata.AccessReviewEntryAccountTypeUser,
|
||||
|
||||
@@ -636,8 +636,11 @@ func buildSegmentProbeURL(conn *coredata.Connector) (string, error) {
|
||||
return "", fmt.Errorf("cannot parse segment base URL: %w", err)
|
||||
}
|
||||
|
||||
q := url.Values{}
|
||||
q.Set("pagination.count", "1")
|
||||
|
||||
u.Path = "/users"
|
||||
u.RawQuery = "pagination.count=1"
|
||||
u.RawQuery = q.Encode()
|
||||
|
||||
return u.String(), nil
|
||||
}
|
||||
|
||||
@@ -43,7 +43,7 @@ func segmentRegistration() *Registration {
|
||||
// as an extra setting and resolved to a base URL (Pattern 3 + region);
|
||||
// there is nothing to pick.
|
||||
ExtraSettings: []ExtraSetting{
|
||||
{Key: "region", Label: "Region (US or EU)", Required: true},
|
||||
{Key: "region", Label: "Region", Required: true},
|
||||
},
|
||||
BuildProbeURL: buildSegmentProbeURL,
|
||||
// No NewNameResolver: the Public API exposes no read-only workspace-name
|
||||
|
||||
99
pkg/server/api/console/v1/segment_test.go
Normal file
99
pkg/server/api/console/v1/segment_test.go
Normal file
@@ -0,0 +1,99 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// Permission is hereby granted, free of charge, to any person obtaining a copy
|
||||
// of this software and associated documentation files (the "Software"), to deal
|
||||
// in the Software without restriction, including without limitation the rights
|
||||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
|
||||
// copies of the Software, and to permit persons to whom the Software is
|
||||
// furnished to do so, subject to the following conditions:
|
||||
//
|
||||
// The above copyright notice and this permission notice shall be included in
|
||||
// all copies or substantial portions of the Software.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
|
||||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
|
||||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
|
||||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
|
||||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
|
||||
// SOFTWARE.
|
||||
|
||||
package console_v1
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"go.probo.inc/probo/pkg/coredata"
|
||||
"go.probo.inc/probo/pkg/server/api/console/v1/types"
|
||||
)
|
||||
|
||||
// The Segment region is the only API-key setting resolved to a derived value
|
||||
// rather than stored verbatim, so the region -> host mapping is pinned here:
|
||||
// a typo in either host would otherwise only surface as a live 404.
|
||||
func TestApiKeyConnectorSettings_SegmentRegion(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
region string
|
||||
baseURL string
|
||||
}{
|
||||
{name: "US", region: "US", baseURL: "https://api.segmentapis.com"},
|
||||
{name: "EU", region: "EU", baseURL: "https://eu1.api.segmentapis.com"},
|
||||
{name: "lowercase", region: "eu", baseURL: "https://eu1.api.segmentapis.com"},
|
||||
{name: "padded", region: " us ", baseURL: "https://api.segmentapis.com"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
raw, err := apiKeyConnectorSettings(types.CreateAPIKeyConnectorInput{
|
||||
Provider: coredata.ConnectorProviderSegment,
|
||||
SegmentRegion: &tc.region,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
var settings coredata.SegmentConnectorSettings
|
||||
require.NoError(t, json.Unmarshal(raw, &settings))
|
||||
assert.Equal(t, tc.baseURL, settings.BaseURL)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestApiKeyConnectorSettings_SegmentRejectsUnknownRegion(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, name := range []string{"empty", "whitespace", "apac", "eu1", "host"} {
|
||||
region := map[string]string{
|
||||
"empty": "",
|
||||
"whitespace": " ",
|
||||
"apac": "APAC",
|
||||
// The region Segment's own UI shows for the EU workspace; it must
|
||||
// not silently fall through to a wrong host.
|
||||
"eu1": "EU1",
|
||||
"host": "https://eu1.api.segmentapis.com",
|
||||
}[name]
|
||||
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
_, err := apiKeyConnectorSettings(types.CreateAPIKeyConnectorInput{
|
||||
Provider: coredata.ConnectorProviderSegment,
|
||||
SegmentRegion: ®ion,
|
||||
})
|
||||
require.Error(t, err)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestApiKeyConnectorSettings_SegmentRequiresRegion(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
_, err := apiKeyConnectorSettings(types.CreateAPIKeyConnectorInput{
|
||||
Provider: coredata.ConnectorProviderSegment,
|
||||
})
|
||||
require.Error(t, err)
|
||||
}
|
||||
Reference in New Issue
Block a user