From d5102eac6369ad5b2eba3aa6a856b31f4e6606c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Sibiril?= <81782+aureliensibiril@users.noreply.github.com> Date: Sat, 11 Jul 2026 18:41:40 +0200 Subject: [PATCH] Add name resolver and Crisp verification tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover gaps the connectors shipped without: the Railway and Crisp name resolvers had no tests despite real branch logic (single vs multiple vs zero workspaces, best-effort on error, empty-website short-circuit, tier header), and the crispVerificationCode query was only unit-tested at the HMAC layer, never through the live schema and authorization stack. Add TestRailwayNameResolver and TestCrispNameResolver alongside the existing resolver tests, pin the Crisp driver's hardcoded MFA Unknown and nil Active, and add an e2e TestCrispVerificationCode asserting the code shape, determinism, organization binding, blank-input INVALID, and viewer FORBIDDEN. The verification-code query needs no Crisp credentials (only the always-set token secret and organization authorization), so it runs against the default e2e deployment. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com> --- e2e/console/connector_test.go | 81 ++++++++++ pkg/accessreview/drivers/crisp_test.go | 6 + .../drivers/name_resolver_test.go | 141 ++++++++++++++++++ 3 files changed, 228 insertions(+) diff --git a/e2e/console/connector_test.go b/e2e/console/connector_test.go index dc299b9ad..28cc32c04 100644 --- a/e2e/console/connector_test.go +++ b/e2e/console/connector_test.go @@ -365,3 +365,84 @@ func TestCreateAPIKeyConnector_RBAC(t *testing.T) { testutil.RequireForbiddenError(t, err, "viewer should not be able to create connector") }) } + +// TestCrispVerificationCode exercises the crispVerificationCode query end to end +// through the live schema and authorization stack. The code is a deterministic +// HMAC bound to (organization, website), so the query needs only the managed +// token secret (always set) and organization authorization — no Crisp +// credentials — and asserts the code's shape, determinism, org-binding, and the +// INVALID / FORBIDDEN error paths. +func TestCrispVerificationCode(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + orgID := owner.GetOrganizationID().String() + + const query = ` + query($organizationId: ID!, $websiteId: String!) { + crispVerificationCode(organizationId: $organizationId, websiteId: $websiteId) + } + ` + + getCode := func(t *testing.T, client *testutil.Client, org, website string) string { + t.Helper() + + var result struct { + CrispVerificationCode string `json:"crispVerificationCode"` + } + + err := client.Execute(query, map[string]any{ + "organizationId": org, + "websiteId": website, + }, &result) + require.NoError(t, err) + + return result.CrispVerificationCode + } + + const website = "1a2b3c4d-5e6f-7a8b-9c0d-1e2f3a4b5c6d" + + t.Run("owner gets a 12-char base32 code, deterministic per input", func(t *testing.T) { + t.Parallel() + + code := getCode(t, owner, orgID, website) + assert.Regexp(t, "^[A-Z2-7]{12}$", code) + + // The same inputs re-derive the same code; nothing is stored. + assert.Equal(t, code, getCode(t, owner, orgID, website)) + + // A different website under the same organization yields a different code. + assert.NotEqual(t, code, getCode(t, owner, orgID, "99999999-0000-0000-0000-000000000000")) + }) + + t.Run("code is organization-bound", func(t *testing.T) { + t.Parallel() + + otherOwner := testutil.NewClient(t, testutil.RoleOwner) + + mine := getCode(t, owner, orgID, website) + theirs := getCode(t, otherOwner, otherOwner.GetOrganizationID().String(), website) + assert.NotEqual(t, mine, theirs, "same website under different organizations must not share a code") + }) + + t.Run("blank websiteId is rejected as INVALID", func(t *testing.T) { + t.Parallel() + + _, err := owner.Do(query, map[string]any{ + "organizationId": orgID, + "websiteId": " ", + }) + testutil.RequireErrorCode(t, err, "INVALID", "blank websiteId must return INVALID not INTERNAL") + }) + + t.Run("viewer cannot read the verification code", func(t *testing.T) { + t.Parallel() + + viewer := testutil.NewClientInOrg(t, testutil.RoleViewer, owner) + + _, err := viewer.Do(query, map[string]any{ + "organizationId": viewer.GetOrganizationID().String(), + "websiteId": website, + }) + testutil.RequireForbiddenError(t, err, "viewer should not be able to read the crisp verification code") + }) +} diff --git a/pkg/accessreview/drivers/crisp_test.go b/pkg/accessreview/drivers/crisp_test.go index 2ae0c63db..d35659c5c 100644 --- a/pkg/accessreview/drivers/crisp_test.go +++ b/pkg/accessreview/drivers/crisp_test.go @@ -45,6 +45,10 @@ func TestCrispDriver(t *testing.T) { assert.True(t, owner.IsAdmin) assert.Equal(t, "Founder", owner.JobTitle) assert.Equal(t, coredata.AccessReviewEntryAccountTypeUser, owner.AccountType) + // Crisp's operators/list exposes no MFA or account-status signal, so the + // driver hardcodes MFA Unknown and leaves Active nil for every record. + assert.Equal(t, coredata.MFAStatusUnknown, owner.MFAStatus) + assert.Nil(t, owner.Active) member := records[1] assert.Equal(t, "9a1f3c2e-6b4d-4f8a-bc11-7d2e9f0a1b22", member.ExternalID) @@ -52,4 +56,6 @@ func TestCrispDriver(t *testing.T) { assert.Equal(t, []string{"Member"}, member.Roles) assert.False(t, member.IsAdmin) assert.Equal(t, "Support Agent", member.JobTitle) + assert.Equal(t, coredata.MFAStatusUnknown, member.MFAStatus) + assert.Nil(t, member.Active) } diff --git a/pkg/accessreview/drivers/name_resolver_test.go b/pkg/accessreview/drivers/name_resolver_test.go index 4111114c0..84100a0a6 100644 --- a/pkg/accessreview/drivers/name_resolver_test.go +++ b/pkg/accessreview/drivers/name_resolver_test.go @@ -580,6 +580,147 @@ func TestGitHubNameResolver(t *testing.T) { } } +func TestRailwayNameResolver(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + status int + body string + want string + }{ + { + name: "single workspace names the source", + status: http.StatusOK, + body: `{"data":{"me":{"name":"Jane Doe","workspaces":[{"id":"w1","name":"Acme Workspace"}]}}}`, + want: "Acme Workspace", + }, + { + name: "multiple workspaces fall back to account holder", + status: http.StatusOK, + body: `{"data":{"me":{"name":"Jane Doe","workspaces":[{"id":"w1","name":"Acme"},{"id":"w2","name":"Beta"}]}}}`, + want: "Jane Doe", + }, + { + name: "no workspaces fall back to account holder", + status: http.StatusOK, + body: `{"data":{"me":{"name":"Jane Doe","workspaces":[]}}}`, + want: "Jane Doe", + }, + { + name: "graphql error body is terminal (no name)", + status: http.StatusOK, + body: `{"data":{"me":null},"errors":[{"message":"unauthorized"}]}`, + want: "", + }, + { + name: "null me is terminal (no name)", + status: http.StatusOK, + body: `{"data":{"me":null}}`, + want: "", + }, + { + name: "non-2xx is terminal (no name)", + status: http.StatusInternalServerError, + body: `{"message":"boom"}`, + want: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/graphql/v2", r.URL.Path) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.body)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + got, err := NewRailwayNameResolver(client).ResolveInstanceName(context.Background()) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestCrispNameResolver(t *testing.T) { + t.Parallel() + + t.Run("empty website id returns nothing without HTTP call", func(t *testing.T) { + t.Parallel() + + client := &http.Client{Transport: roundTripperFunc(func(*http.Request) (*http.Response, error) { + t.Fatalf("resolver should not make an HTTP call for an empty website id") + return nil, nil + })} + + got, err := NewCrispNameResolver(client, "").ResolveInstanceName(context.Background()) + require.NoError(t, err) + assert.Empty(t, got) + }) + + cases := []struct { + name string + status int + body string + want string + }{ + { + name: "200 returns website name", + status: http.StatusOK, + body: `{"data":{"name":"Acme Support"}}`, + want: "Acme Support", + }, + { + name: "401 is terminal (no name)", + status: http.StatusUnauthorized, + body: `{"error":true,"reason":"not_allowed"}`, + want: "", + }, + { + name: "404 is terminal (no name)", + status: http.StatusNotFound, + body: `{"error":true,"reason":"website_not_found"}`, + want: "", + }, + { + name: "500 is terminal (no name)", + status: http.StatusInternalServerError, + body: `{"error":true,"reason":"boom"}`, + want: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(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, "/v1/website/1a2b3c4d", r.URL.Path) + assert.Equal(t, crispTierValue, r.Header.Get(crispTierHeader)) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.body)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + got, err := NewCrispNameResolver(client, "1a2b3c4d").ResolveInstanceName(context.Background()) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + // roundTripperFunc adapts a function into an http.RoundTripper, useful for // asserting that a resolver short-circuits before making any HTTP call. type roundTripperFunc func(*http.Request) (*http.Response, error)