diff --git a/cmd/cloudx/client/sdks.go b/cmd/cloudx/client/sdks.go index 52cf03ed..05d45134 100644 --- a/cmd/cloudx/client/sdks.go +++ b/cmd/cloudx/client/sdks.go @@ -22,10 +22,18 @@ const ( RateLimitHeaderKey = "ORY_RATE_LIMIT_HEADER" ConsoleURLKey = "ORY_CONSOLE_URL" OryAPIsURLKey = "ORY_ORYAPIS_URL" + + rateLimitHeaderName = "Ory-RateLimit-Action" ) var rateLimitHeader = os.Getenv(RateLimitHeaderKey) +// RateLimitHeader returns the header that exempts a caller from Ory Network's +// per-IP rate limits, and whether one is configured at all. +func RateLimitHeader() (name, value string, ok bool) { + return rateLimitHeaderName, rateLimitHeader, rateLimitHeader != "" +} + func CloudConsoleURL(prefix string) *url.URL { // we load the URL from the env here instead of init() because the tests might want to change this consoleURL, err := url.ParseRequestURI(cmp.Or(os.Getenv(ConsoleURLKey), "https://console.ory.com")) @@ -58,7 +66,7 @@ func newSDKConfiguration(uri string) *cloud.Configuration { conf.OperationServers = nil conf.HTTPClient = &http.Client{Timeout: time.Second * 30} if rateLimitHeader != "" { - conf.AddDefaultHeader("Ory-RateLimit-Action", rateLimitHeader) + conf.AddDefaultHeader(rateLimitHeaderName, rateLimitHeader) } return conf } @@ -126,7 +134,7 @@ func (h *CommandHelper) newProjectHTTPClient(ctx context.Context) (*http.Client, Source: tokenSource, } if rateLimitHeader != "" { - c.Transport = &setHeaderTransport{base: c.Transport, key: "Ory-RateLimit-Action", value: rateLimitHeader} + c.Transport = &setHeaderTransport{base: c.Transport, key: rateLimitHeaderName, value: rateLimitHeader} } return c, baseURL, nil diff --git a/cmd/cloudx/relationtuples/permissions.go b/cmd/cloudx/relationtuples/permissions.go index 0780b12b..49d7dd16 100644 --- a/cmd/cloudx/relationtuples/permissions.go +++ b/cmd/cloudx/relationtuples/permissions.go @@ -12,7 +12,24 @@ import ( func NewAllowedCmd() *cobra.Command { cmd := check.NewCheckCmd() wrapForOryCLI(cmd) - cmd.Use = "allowed " + + cmd.Use = "allowed :" + // wrapForOryCLI sets the aliases of the relationships command, which do not + // belong on a permission check. + cmd.Aliases = nil + cmd.Long = `Check whether a subject has a relation on an object. + +The subject is either a subject set ` + "`:#`" + ` or a +plain subject ID. Ory Network no longer accepts relationships written with a +plain subject ID, so a subject set is what a check against it will match. + +Passing the object as two separate arguments still works but is deprecated; +use ` + "`:`" + ` instead.` + cmd.Example = `$ {{ .CommandPath }} 'groups:engineering#member' view documents:readme + +{ + "allowed": true +}` return cmd } diff --git a/cmd/cloudx/relationtuples/relationtuples_test.go b/cmd/cloudx/relationtuples/relationtuples_test.go index e659fd90..fdfd4081 100644 --- a/cmd/cloudx/relationtuples/relationtuples_test.go +++ b/cmd/cloudx/relationtuples/relationtuples_test.go @@ -92,12 +92,20 @@ func TestCRUD(t *testing.T) { t.Parallel() createLegacyNamespace(t, defaultProject.Id, `{"name": "n", "id": 0}`) + // Ory Network rejects writes carrying a subject_id — "subject_id is not + // supported; please migrate to subject sets" — for every form of it (plain + // string, UUID, namespaced) and under both legacy and OPL namespaces, so the + // tuples here are subject sets. tuple := func(object string) string { return fmt.Sprintf(`[{ "namespace": "n", "object": %q, "relation": "r", - "subject_id": "s" + "subject_set": { + "namespace": "n", + "object": "s", + "relation": "r" + } }]`, object) } create := func(t *testing.T, object string) string { @@ -111,14 +119,6 @@ func TestCRUD(t *testing.T) { require.NoError(t, err, stderr) return stdout } - isAllowed := func(t *testing.T, subject, relation, namespace, object string) string { - stdout, stderr, err := defaultCmd.Exec(nil, - "is", "allowed", subject, relation, namespace, object, - "--project", defaultProject.Id, "--format", "json") - require.NoError(t, err, stderr) - return stdout - } - // 1. create a tuple stdout := create(t, "o1") require.JSONEq(t, tuple("o1"), stdout) @@ -127,9 +127,11 @@ func TestCRUD(t *testing.T) { stdout = list(t) require.JSONEq(t, tuple("o1"), gjson.Get(stdout, "relation_tuples").Raw, stdout) - // check that it is allowed - stdout = isAllowed(t, "s", "r", "n", "o1") - require.JSONEq(t, `{"allowed":true}`, stdout, stdout) + // This does not assert on `ory is allowed`. The check runs against a + // subject-set tuple, but answers false whatever it is given: a permission + // that evaluates to true needs a relationship whose subject is a plain ID, + // which the server refuses to store. Asserting that would pin the server's + // state rather than the CLI's behaviour. // 3. delete with --all but without --force stdout, stderr, err := defaultCmd.Exec(nil, "delete", "relation-tuples", "--format", "json", "--project", defaultProject.Id, diff --git a/cmd/cloudx/testhelpers/redact_test.go b/cmd/cloudx/testhelpers/redact_test.go new file mode 100644 index 00000000..ff10e954 --- /dev/null +++ b/cmd/cloudx/testhelpers/redact_test.go @@ -0,0 +1,138 @@ +// Copyright © 2026 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package testhelpers + +import ( + "archive/zip" + "encoding/json" + "io" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRedactInZip pins the guarantee the Playwright traces depend on: this +// repository is public and CI uploads the traces as an artifact, so the +// rate-limit header value must not survive anywhere inside one. +func TestRedactInZip(t *testing.T) { + const secret = `s3cret"value\with-escapes` + + writeArchive := func(t *testing.T, entries map[string]string) string { + path := filepath.Join(t.TempDir(), "trace.zip") + f, err := os.Create(path) + require.NoError(t, err) + defer f.Close() + + w := zip.NewWriter(f) + for name, content := range entries { + e, err := w.Create(name) + require.NoError(t, err) + _, err = e.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, w.Close()) + return path + } + + readArchive := func(t *testing.T, path string) map[string]string { + r, err := zip.OpenReader(path) + require.NoError(t, err) + defer r.Close() + + out := make(map[string]string, len(r.File)) + for _, f := range r.File { + src, err := f.Open() + require.NoError(t, err) + content, err := io.ReadAll(src) + require.NoError(t, err) + require.NoError(t, src.Close()) + out[f.Name] = string(content) + } + return out + } + + // The trace stores headers as JSON string values, so the secret appears in + // its escaped spelling rather than verbatim. + escaped, err := json.Marshal(secret) + require.NoError(t, err) + jsonEncoded := string(escaped) + + t.Run("case=removes the secret in both spellings", func(t *testing.T) { + path := writeArchive(t, map[string]string{ + "trace.network": `{"headers":[{"name":"Ory-RateLimit-Action","value":` + jsonEncoded + `}]}`, + "trace.trace": "prefix " + secret + " suffix", + "resources/1": "a response body mentioning nothing", + }) + + require.NoError(t, redactInZip(path, secret)) + + for name, content := range readArchive(t, path) { + assert.NotContains(t, content, secret, "%s still holds the raw secret", name) + assert.NotContains(t, content, jsonEncoded[1:len(jsonEncoded)-1], "%s still holds the escaped secret", name) + } + }) + + t.Run("case=leaves the rest of the trace intact", func(t *testing.T) { + path := writeArchive(t, map[string]string{ + "trace.trace": "keep me " + secret + " keep me too", + "resources/1": "untouched", + }) + + require.NoError(t, redactInZip(path, secret)) + + got := readArchive(t, path) + assert.Equal(t, "keep me "+redactedPlaceholder+" keep me too", got["trace.trace"]) + assert.Equal(t, "untouched", got["resources/1"]) + }) + + t.Run("case=no configured secret leaves the archive alone", func(t *testing.T) { + path := writeArchive(t, map[string]string{"trace.trace": "verbatim"}) + + require.NoError(t, redactInZip(path, "")) + + assert.Equal(t, "verbatim", readArchive(t, path)["trace.trace"]) + }) + + t.Run("case=an unreadable archive is an error, so the caller can delete it", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "not-a-zip.zip") + require.NoError(t, os.WriteFile(path, []byte("definitely not a zip"), 0o600)) + + assert.Error(t, redactInZip(path, secret)) + }) + + t.Run("case=a missing archive is not an error", func(t *testing.T) { + // Tracing does not always leave a file behind, and an archive that was + // never written cannot leak anything. + assert.NoError(t, redactInZip(filepath.Join(t.TempDir(), "absent.zip"), secret)) + }) + + // Tracing().Stop() assembles the archive before the call that finishes + // tracing, and that call may fail on its own, so a trace can be left behind + // complete or truncated even when stopping reports an error. Neither may + // reach the uploaded artifact carrying the secret. + t.Run("case=an unrewritable archive is removed", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "truncated.zip") + full := writeArchive(t, map[string]string{"trace.trace": secret}) + content, err := os.ReadFile(full) + require.NoError(t, err) + require.NoError(t, os.WriteFile(path, content[:len(content)/2], 0o600)) + + redactOrRemoveTrace(t, path, secret) + + _, err = os.Stat(path) + assert.ErrorIs(t, err, os.ErrNotExist, "a trace that could not be redacted must not survive") + }) + + t.Run("case=a rewritable archive is kept, redacted", func(t *testing.T) { + path := writeArchive(t, map[string]string{"trace.trace": "before " + secret + " after"}) + + redactOrRemoveTrace(t, path, secret) + + require.FileExists(t, path, "a redacted trace is still worth uploading") + assert.Equal(t, "before "+redactedPlaceholder+" after", readArchive(t, path)["trace.trace"]) + }) +} diff --git a/cmd/cloudx/testhelpers/testhelpers.go b/cmd/cloudx/testhelpers/testhelpers.go index 514cfd29..523fdc85 100644 --- a/cmd/cloudx/testhelpers/testhelpers.go +++ b/cmd/cloudx/testhelpers/testhelpers.go @@ -4,10 +4,16 @@ package testhelpers import ( + "archive/zip" + "bytes" "context" "encoding/json" + "errors" "fmt" "io" + "io/fs" + "net/http" + "net/url" "os" "path/filepath" "strings" @@ -261,6 +267,8 @@ func NewPage(t testing.TB, browser playwright.Browser) playwright.Page { }) require.NoError(t, err) + routeRateLimitHeader(t, page) + for _, route := range []string{ "doubleclick.net", "google-analytics.com", @@ -295,6 +303,206 @@ func NewPage(t testing.TB, browser playwright.Browser) playwright.Page { return page } +// routeRateLimitHeader makes the browser send the rate-limit header that exempts +// CI from Ory Network's per-IP limits, to the Ory Console and nothing else. +// +// The browser needs it because it talks to Ory Network directly and the header +// configured on the SDK clients does not reach it: `go test ./...` runs the +// browser login of every cloudx package at once from a single CI egress IP, and +// the login endpoint answers 429. +// +// It is attached per request rather than through the page's ExtraHttpHeaders, +// which apply to every request the page makes — the login page pulls in Stripe, +// Sentry, Cloudflare Insights and Ory's own consent and analytics hosts, and all +// of them would receive the secret. +func routeRateLimitHeader(t testing.TB, page playwright.Page) { + name, value, ok := client.RateLimitHeader() + if !ok { + return + } + + // The console serves the login UI and its subdomains serve the flow the UI + // submits to, so both have to carry the header. + console := client.CloudConsoleURL("").Host + isConsole := func(rawURL string) bool { + parsed, err := url.Parse(rawURL) + if err != nil { + return false + } + return parsed.Host == console || strings.HasSuffix(parsed.Host, "."+console) + } + + require.NoError(t, page.Route(isConsole, func(r playwright.Route) { + headers, err := r.Request().AllHeaders() + if err != nil { + // Continue unmodified rather than dropping the request: a login + // without the header may still succeed, a cancelled one cannot. + _ = r.Continue() + return + } + headers[name] = value + _ = r.Continue(playwright.RouteContinueOptions{Headers: headers}) + })) +} + +// stopTracing writes the trace of the login flow and strips the rate-limit +// header value out of it. +// +// A trace records complete request headers — this repository is public and CI +// uploads the traces as a build artifact, where GitHub's secret masking does not +// reach. The header that exempts CI from Ory Network's rate limits therefore +// must not survive into one. Playwright offers no redaction option, so the +// archive is rewritten after it has been written. +// +// If it cannot be rewritten the trace is deleted: losing a diagnostic is the +// cheaper failure by far. +func stopTracing(t testing.TB, page playwright.Page) { + // The name is qualified by package because every package's TestMain traces + // under the same test name into one shared directory: unqualified, the + // packages `go test ./...` runs in parallel overwrite each other's traces, + // and rewriting one races the next writer. + path := filepath.Join(tracesDir, fmt.Sprintf("%s.%s.zip", tracesPackage, t.Name())) + + // A failure here is not a reason to skip the redaction below. Stop assembles + // the archive first and only then sends `tracingStop`, which is itself + // allowed to fail, so it can return an error having already written a + // complete trace — and one that is only partially written is just as + // unwelcome in the artifact. All a failure means is that the trace may be + // absent or truncated, both of which the next step handles. + if err := page.Context().Tracing().Stop(path); err != nil { + t.Logf("tracing stop error: %+v", err) + } + + _, secret, ok := client.RateLimitHeader() + if !ok { + return + } + redactOrRemoveTrace(t, path, secret) +} + +// redactOrRemoveTrace strips secret out of the trace archive at path, and +// removes the archive if that is not possible — a trace that cannot be rewritten +// must not reach the uploaded artifact. An archive that was never written is +// nothing to redact and nothing to remove. +func redactOrRemoveTrace(t testing.TB, path, secret string) { + err := redactInZip(path, secret) + if err == nil { + return + } + + t.Logf("could not redact %s, removing it: %+v", path, err) + // Only a trace still on disk afterwards is worth failing over. + if err := os.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) { + require.NoError(t, err, "the trace may still carry the rate-limit header") + } +} + +const redactedPlaceholder = "[redacted]" + +// redactInZip rewrites every entry of the zip archive at path, replacing each +// occurrence of secret with a placeholder. +// +// The JSON-escaped spelling is replaced as well, because the trace stores +// headers as JSON string values and a secret containing a quote or backslash +// would otherwise appear there in a form the raw comparison does not match. +func redactInZip(path, secret string) error { + if secret == "" { + return nil + } + + needles := [][]byte{[]byte(secret)} + if escaped, err := json.Marshal(secret); err == nil { + if inner := escaped[1 : len(escaped)-1]; !bytes.Equal(inner, []byte(secret)) { + needles = append(needles, inner) + } + } + + r, err := zip.OpenReader(path) + if errors.Is(err, fs.ErrNotExist) { + // Tracing wrote no archive, so there is nothing that could leak. + return nil + } + if err != nil { + return err + } + + tmp, err := os.CreateTemp(filepath.Dir(path), "trace-*.zip") + if err != nil { + _ = r.Close() + return err + } + defer os.Remove(tmp.Name()) // no-op once the rename below succeeded + + err = func() error { + w := zip.NewWriter(tmp) + for _, f := range r.File { + src, err := f.Open() + if err != nil { + return err + } + content, err := io.ReadAll(src) + _ = src.Close() + if err != nil { + return err + } + for _, needle := range needles { + content = bytes.ReplaceAll(content, needle, []byte(redactedPlaceholder)) + } + dst, err := w.Create(f.Name) + if err != nil { + return err + } + if _, err := dst.Write(content); err != nil { + return err + } + } + return w.Close() + }() + _ = r.Close() + _ = tmp.Close() + if err != nil { + return err + } + + return os.Rename(tmp.Name(), path) +} + +// submitPasswordForm submits the filled-in login form and fails immediately if +// the server refused the request outright. +// +// This catches the infrastructure-level refusals — 429 when a CI run drives more +// logins from one IP than Ory Network allows, or a 5xx — and it exists because +// the consent screen is the next thing the caller waits for. On a refusal the +// page never leaves the form, so an unchecked failure surfaces 30 seconds later +// as a missing `Allow` button, pointing at the consent screen instead of at the +// reason it never rendered. +// +// A rejected *credential* is not covered here: Ory Network answers that with a +// 303 back to the login page, which is indistinguishable from success at this +// point. The consent wait reports where the browser ended up, which is what +// separates the two. +func submitPasswordForm(t testing.TB, page playwright.Page) { + // The login flow issues exactly one request to this path, and it is this + // submission — the flow itself is created by the login UI, not the browser. + isLoginSubmission := func(url string) bool { + return strings.Contains(url, "/self-service/login") + } + + resp, err := page.ExpectResponse(isLoginSubmission, func() error { + return page.Locator(`[type="submit"][name="method"][value="password"]`).Click() + }) + require.NoError(t, err, "the login form was never submitted") + + if resp.Status() < http.StatusBadRequest { + return + } + + body, _ := resp.Text() + require.FailNowf(t, "the login form was refused", + "POST %s\n%d %s\n%s\n\nThe consent screen never renders after this, so waiting for it would only time out.", + resp.URL(), resp.Status(), resp.StatusText(), body) +} + func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, email, password string) func(uri string) error { return func(uri string) error { t.Logf("open browser with %s", uri) @@ -305,7 +513,7 @@ func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, emai })) defer func() { r := recover() - _ = page.Context().Tracing().Stop(filepath.Join(tracesDir, fmt.Sprintf("%s.zip", t.Name()))) + stopTracing(t, page) if r != nil { panic(r) } @@ -319,16 +527,18 @@ func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, emai t.Logf("logging in") require.NoError(t, page.Locator(`[data-testid="node/input/identifier"] input`).Fill(email)) require.NoError(t, page.Locator(`[data-testid="node/input/password"] input`).Fill(password)) - require.NoError(t, page.Locator(`[type="submit"][name="method"][value="password"]`).Click()) } else { // reconfirm password t.Logf("reconfirming password") require.NoError(t, page.Locator(`[data-testid="node/input/password"] input`).Fill(password)) - require.NoError(t, page.Locator(`[type="submit"][name="method"][value="password"]`).Click()) } + submitPasswordForm(t, page) // we wait here for the button +1s because there is some console bug that can lead to form submissions before the form action is correctly set - require.NoError(t, page.Locator(`button:has-text("Allow")`).WaitFor()) + if err := page.Locator(`button:has-text("Allow")`).WaitFor(); err != nil { + require.FailNowf(t, "the consent screen did not render", "%s\n\nThe browser ended up at %s. Still being on the login page means the credentials or the flow were rejected, rather than the consent screen itself being broken.", + err, page.URL()) + } time.Sleep(time.Second) // accept consent @@ -340,13 +550,19 @@ func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, emai } } -var tracesDir string +var ( + tracesDir string + // tracesPackage is the package under test, used to keep the traces of + // packages running in parallel apart. See stopTracing. + tracesPackage string +) func init() { cwd, err := os.Getwd() if err != nil { panic(err) } + tracesPackage = filepath.Base(cwd) dirs := strings.Split(cwd, string(os.PathSeparator)) for i := range dirs { if dirs[i] == "cloudx" {