diff --git a/pkg/accessreview/drivers/name_resolver.go b/pkg/accessreview/drivers/name_resolver.go index 4a02c3420..37730bd2f 100644 --- a/pkg/accessreview/drivers/name_resolver.go +++ b/pkg/accessreview/drivers/name_resolver.go @@ -708,7 +708,8 @@ func (r *sentryNameResolver) ResolveInstanceName(ctx context.Context) (string, e return "", nil } - endpoint, err := url.JoinPath("https://sentry.io", "api", "0", "organizations", url.PathEscape(r.orgSlug)) + // Trailing slash required; see SentryDriver.ListAccounts. + endpoint, err := url.JoinPath("https://sentry.io", "api", "0", "organizations", url.PathEscape(r.orgSlug)+"/") if err != nil { return "", fmt.Errorf("cannot build sentry organization URL: %w", err) } diff --git a/pkg/accessreview/drivers/name_resolver_test.go b/pkg/accessreview/drivers/name_resolver_test.go index 0e2aa7d43..71c83b18a 100644 --- a/pkg/accessreview/drivers/name_resolver_test.go +++ b/pkg/accessreview/drivers/name_resolver_test.go @@ -170,7 +170,7 @@ func TestSentryNameResolver(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodGet, r.Method) - assert.Equal(t, "/api/0/organizations/acme", r.URL.Path) + assert.Equal(t, "/api/0/organizations/acme/", r.URL.Path) w.Header().Set("Content-Type", "application/json") w.WriteHeader(tc.status) _, _ = w.Write([]byte(tc.body)) diff --git a/pkg/accessreview/drivers/sentry.go b/pkg/accessreview/drivers/sentry.go index f45391112..d6a95ac1d 100644 --- a/pkg/accessreview/drivers/sentry.go +++ b/pkg/accessreview/drivers/sentry.go @@ -101,7 +101,10 @@ func (d *SentryDriver) ListAccounts(ctx context.Context) ([]AccountRecord, error var records []AccountRecord - nextURL, err := url.JoinPath("https://sentry.io", "api", "0", "organizations", url.PathEscape(orgSlug), "members") + // The trailing slash is required: Sentry's API only routes slashed + // paths and answers 404 (without redirecting) otherwise. url.JoinPath + // keeps it only when the last element carries it. + nextURL, err := url.JoinPath("https://sentry.io", "api", "0", "organizations", url.PathEscape(orgSlug), "members/") if err != nil { return nil, fmt.Errorf("cannot build sentry members URL: %w", err) } diff --git a/pkg/accessreview/drivers/sentry_test.go b/pkg/accessreview/drivers/sentry_test.go index 23ee89029..181005a8d 100644 --- a/pkg/accessreview/drivers/sentry_test.go +++ b/pkg/accessreview/drivers/sentry_test.go @@ -25,6 +25,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -54,12 +55,48 @@ func TestSentryDriver(t *testing.T) { assert.NotEmpty(t, r.Roles) } +// TestSentryDriverRequestsTrailingSlashPaths pins the exact request paths +// against Sentry's real routing: its API is Django-based and only matches +// paths ending in a slash, answering 404 (no redirect) otherwise. A 404 is +// indistinguishable from a revoked membership here, so dropping the slash +// silently turns every campaign fetch into a bogus "reconnect" error. +func TestSentryDriverRequestsTrailingSlashPaths(t *testing.T) { + t.Parallel() + + var gotPaths []string + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPaths = append(gotPaths, r.URL.Path) + + // Mimic Sentry: unslashed paths do not route. + if !strings.HasSuffix(r.URL.Path, "/") { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"detail":"The requested resource does not exist"}`)) + + return + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[{"id":"42","email":"alice@example.com","name":"Alice","orgRole":"member"}]`)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + records, err := NewSentryDriver(client, "acme-corp").ListAccounts(context.Background()) + require.NoError(t, err) + require.Len(t, records, 1) + assert.Equal(t, "alice@example.com", records[0].Email) + assert.Equal(t, []string{"/api/0/organizations/acme-corp/members/"}, gotPaths) +} + func TestSentryDriverListAccountsStaleSlug(t *testing.T) { t.Parallel() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodGet, r.Method) - assert.Equal(t, "/api/0/organizations/acme-old/members", r.URL.Path) + assert.Equal(t, "/api/0/organizations/acme-old/members/", r.URL.Path) w.WriteHeader(http.StatusNotFound) _, _ = w.Write([]byte(`{"detail":"The requested resource does not exist"}`)) })) @@ -88,7 +125,7 @@ func TestSentryDriverListAccountsAutoDiscoversSlug(t *testing.T) { assert.Equal(t, "true", r.URL.Query().Get("member")) w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`[{"slug":"` + discoveredSlug + `","name":"Discovered Org"}]`)) - case "/api/0/organizations/" + discoveredSlug + "/members": + case "/api/0/organizations/" + discoveredSlug + "/members/": w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`[{"id":"42","email":"alice@example.com","name":"Alice","orgRole":"member"}]`)) default: diff --git a/pkg/accessreview/drivers/testdata/sentry.yaml b/pkg/accessreview/drivers/testdata/sentry.yaml index 599b457b3..3e0e6e9eb 100644 --- a/pkg/accessreview/drivers/testdata/sentry.yaml +++ b/pkg/accessreview/drivers/testdata/sentry.yaml @@ -8,7 +8,7 @@ interactions: proto_minor: 1 content_length: 0 host: sentry.io - url: https://sentry.io/api/0/organizations/acme-corp/members + url: https://sentry.io/api/0/organizations/acme-corp/members/ method: GET response: proto: HTTP/2.0