diff --git a/CLAUDE.md b/CLAUDE.md index 82c4240..fedbcd1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -213,6 +213,9 @@ stricter still (creator/Admin/Owner only, closing a self-escalation path). Verified against a fake store (`api/dashboards/handler_test.go`); real integration tests exist but haven't run against a live Postgres, same disclosed gap as the rest of this phase's Postgres-backed pieces. +`sentryctl dashboards permissions list|grant|revoke` is now the CLI +surface for this — `PUT`/`DELETE /dashboards/{id}/permissions/{userId}` +previously had no caller but Go tests and curl. `deploy/operator`'s `Tenant` CRD and `enterprise-api -provision-tenant` are now unified too, deliberately lightweight rather than making the K8s controller a second real actor: `-provision-tenant` stays the sole diff --git a/cli/README.md b/cli/README.md index f2b5e3d..0619daf 100644 --- a/cli/README.md +++ b/cli/README.md @@ -31,11 +31,24 @@ sentryctl dashboards list sentryctl dashboards get sentryctl dashboards apply dashboard.json # imports a dashboard exported via the web UI's "Export JSON" button +sentryctl dashboards permissions list +sentryctl dashboards permissions grant viewer|editor +sentryctl dashboards permissions revoke + sentryctl alerts list sentryctl alerts get sentryctl alerts apply rule.json # creates a rule from a JSON file shaped like POST /rules's body ``` +`dashboards permissions` is Phase 4's per-resource dashboard grant +(`api/dashboards.PermissionStore`) — additive-only, raises someone to +`viewer` or `editor` on one specific dashboard (Admin/Owner already have +tenant-wide access, so the server rejects any other role). On plain +`api` (no `enterprise-api`, no permission service wired in) every +`permissions` call fails with a 501 whose message says so explicitly — +that's the deployment telling you this feature isn't available, not a +CLI bug. + `dashboards` talks to `/api` (`--api`, same override as `query`/`ping`). `alerts` talks to `/alerting`, a separate service with its own base URL (`--alerting-api`, or `$SENTRYCTL_ALERTING_API_URL`, default diff --git a/cli/cmd/sentryctl/cmd_dashboards.go b/cli/cmd/sentryctl/cmd_dashboards.go index 5ab205f..53a122e 100644 --- a/cli/cmd/sentryctl/cmd_dashboards.go +++ b/cli/cmd/sentryctl/cmd_dashboards.go @@ -3,12 +3,13 @@ package main import ( "fmt" "io" + "net/http" "os" ) func cmdDashboards(args []string, stdout, stderr io.Writer) int { if len(args) == 0 { - fmt.Fprintln(stderr, "sentryctl dashboards: expected a subcommand (list, get, apply)") + fmt.Fprintln(stderr, "sentryctl dashboards: expected a subcommand (list, get, apply, permissions)") return 1 } apiURL, rest := extractAPIFlag(args[1:], os.Getenv) @@ -32,8 +33,62 @@ func cmdDashboards(args []string, stdout, stderr io.Writer) int { // /dashboards/{id}/export produces and the web UI's Export JSON // button downloads -- one JSON contract, three call sites. return httpPostFileJSON(apiURL, "/dashboards/import", token, rest[0], stdout, stderr) + case "permissions": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl dashboards permissions: expected a subcommand (list, grant, revoke)") + return 1 + } + return cmdDashboardsPermissions(rest, apiURL, token, stdout, stderr) default: - fmt.Fprintf(stderr, "sentryctl dashboards: unknown subcommand %q (want list, get, apply)\n", args[0]) + fmt.Fprintf(stderr, "sentryctl dashboards: unknown subcommand %q (want list, get, apply, permissions)\n", args[0]) + return 1 + } +} + +// cmdDashboardsPermissions is api/dashboards.PermissionStore's CLI +// surface -- PUT/DELETE /dashboards/{id}/permissions/{userId} existed +// with no caller but Go tests and curl until now (see +// /docs/phase-4-runbook.md's "Known gaps"). Kept as dashboards' +// own sub-subcommand rather than a flat sentryctl command (like +// "sentryctl dashboard-permissions grant ...") since a grant only ever +// makes sense in the context of one specific dashboard -- args[0] +// selects list/grant/revoke. +func cmdDashboardsPermissions(args []string, apiURL, token string, stdout, stderr io.Writer) int { + sub, rest := args[0], args[1:] + switch sub { + case "list": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl dashboards permissions list: missing dashboard id") + return 1 + } + return httpGetJSON(apiURL, "/dashboards/"+rest[0]+"/permissions", token, stdout, stderr) + case "grant": + if len(rest) < 3 { + fmt.Fprintln(stderr, "sentryctl dashboards permissions grant: usage: grant ") + return 1 + } + dashboardID, userID, role := rest[0], rest[1], rest[2] + // Mirrors api/dashboards.validGrantRole -- Admin/Owner already + // have tenant-wide dashboard access, so a resource-level grant + // only ever raises someone as high as Editor; the server + // rejects anything else too, this just fails faster/locally. + if role != "viewer" && role != "editor" { + fmt.Fprintf(stderr, "sentryctl dashboards permissions grant: role must be \"viewer\" or \"editor\", got %q\n", role) + return 1 + } + body := fmt.Sprintf(`{"role":%q}`, role) + path := "/dashboards/" + dashboardID + "/permissions/" + userID + return httpMutateNoBody(http.MethodPut, apiURL, path, token, body, "granted", stdout, stderr) + case "revoke": + if len(rest) < 2 { + fmt.Fprintln(stderr, "sentryctl dashboards permissions revoke: usage: revoke ") + return 1 + } + dashboardID, userID := rest[0], rest[1] + path := "/dashboards/" + dashboardID + "/permissions/" + userID + return httpMutateNoBody(http.MethodDelete, apiURL, path, token, "", "revoked", stdout, stderr) + default: + fmt.Fprintf(stderr, "sentryctl dashboards permissions: unknown subcommand %q (want list, grant, revoke)\n", sub) return 1 } } diff --git a/cli/cmd/sentryctl/cmd_dashboards_test.go b/cli/cmd/sentryctl/cmd_dashboards_test.go index f09c77c..04dbe2c 100644 --- a/cli/cmd/sentryctl/cmd_dashboards_test.go +++ b/cli/cmd/sentryctl/cmd_dashboards_test.go @@ -2,7 +2,11 @@ package main import ( "bytes" + "io" + "net/http" + "net/http/httptest" "reflect" + "strings" "testing" ) @@ -41,3 +45,135 @@ func TestCmdDashboardsGetMissingID(t *testing.T) { t.Fatalf("code = %d, want 1", code) } } + +func TestCmdDashboardsPermissionsMissingSubcommand(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdDashboardsPermissionsListMissingID(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "list"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdDashboardsPermissionsListSuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/dashboards/dash-1/permissions" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"UserID":"user-2","Role":"editor"}]`)) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "list", "dash-1", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "user-2") { + t.Fatalf("stdout = %q, want it to contain the listed grant", stdout.String()) + } +} + +func TestCmdDashboardsPermissionsGrantMissingArgs(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "grant", "dash-1"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdDashboardsPermissionsGrantInvalidRole(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "grant", "dash-1", "user-2", "owner"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "viewer") { + t.Fatalf("stderr = %q, want it to explain the allowed roles", stderr.String()) + } +} + +func TestCmdDashboardsPermissionsGrantSuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPut || r.URL.Path != "/dashboards/dash-1/permissions/user-2" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + body, _ := io.ReadAll(r.Body) + if !strings.Contains(string(body), `"role":"editor"`) { + t.Errorf("body = %q, want it to carry role=editor", body) + } + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "grant", "dash-1", "user-2", "editor", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "granted") { + t.Fatalf("stdout = %q, want a confirmation", stdout.String()) + } +} + +func TestCmdDashboardsPermissionsGrantServerError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotImplemented) + w.Write([]byte(`{"error":"dashboard permission grants are not available on this deployment"}`)) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "grant", "dash-1", "user-2", "editor", "--api", srv.URL}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "not available on this deployment") { + t.Fatalf("stderr = %q, want the server's actual error message surfaced", stderr.String()) + } +} + +func TestCmdDashboardsPermissionsRevokeMissingArgs(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "revoke", "dash-1"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdDashboardsPermissionsRevokeSuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete || r.URL.Path != "/dashboards/dash-1/permissions/user-2" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "revoke", "dash-1", "user-2", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "revoked") { + t.Fatalf("stdout = %q, want a confirmation", stdout.String()) + } +} + +func TestCmdDashboardsPermissionsUnknownSubcommand(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdDashboards([]string{"permissions", "bogus"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} diff --git a/cli/cmd/sentryctl/httpclient.go b/cli/cmd/sentryctl/httpclient.go index de9ef8b..649432e 100644 --- a/cli/cmd/sentryctl/httpclient.go +++ b/cli/cmd/sentryctl/httpclient.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "os" + "strings" "time" ) @@ -68,6 +69,47 @@ func httpPostFileJSON(baseURL, path, token, file string, stdout, stderr io.Write return printJSONResponse(resp, stdout, stderr) } +// httpMutateNoBody sends method to path with an optional JSON body +// ("" for none, e.g. DELETE) and expects a 2xx with no meaningful +// response body -- PUT/DELETE /dashboards/{id}/permissions/{userId} +// both respond 204 No Content, so there's nothing for printJSONResponse +// to pretty-print here. Prints successMsg to stdout on success, the +// same {"error": "..."} parsing every other helper in this file uses +// otherwise. +func httpMutateNoBody(method, baseURL, path, token, body, successMsg string, stdout, stderr io.Writer) int { + var reqBody io.Reader + if body != "" { + reqBody = strings.NewReader(body) + } + req, err := http.NewRequest(method, baseURL+path, reqBody) + if err != nil { + fmt.Fprintf(stderr, "building request: %v\n", err) + return 1 + } + if body != "" { + req.Header.Set("Content-Type", "application/json") + } + setAuth(req, token) + resp, err := httpClient.Do(req) + if err != nil { + fmt.Fprintf(stderr, "request failed: %v\n", err) + return 1 + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + respBody, _ := io.ReadAll(resp.Body) + var errResp errorResponseBody + if json.Unmarshal(respBody, &errResp) == nil && errResp.Error != "" { + fmt.Fprintf(stderr, "request failed: %s\n", errResp.Error) + } else { + fmt.Fprintf(stderr, "request failed: status %d\n", resp.StatusCode) + } + return 1 + } + fmt.Fprintln(stdout, successMsg) + return 0 +} + func printJSONResponse(resp *http.Response, stdout, stderr io.Writer) int { body, err := io.ReadAll(resp.Body) if err != nil { diff --git a/cli/cmd/sentryctl/main.go b/cli/cmd/sentryctl/main.go index 84c5ac9..21864bb 100644 --- a/cli/cmd/sentryctl/main.go +++ b/cli/cmd/sentryctl/main.go @@ -55,6 +55,9 @@ Usage: sentryctl ping [--api ] sentryctl query "" [--api ] [--language sql|spl] [--json] sentryctl dashboards list|get |apply [--api ] + sentryctl dashboards permissions list [--api ] + sentryctl dashboards permissions grant viewer|editor [--api ] + sentryctl dashboards permissions revoke [--api ] sentryctl alerts list|get |apply [--alerting-api ] Commands: @@ -67,6 +70,12 @@ Commands: "apply " imports a dashboard exported via the web UI's Export JSON button or GET /dashboards/{id}/export -- the same JSON shape both places, Terraform-friendly. + "permissions" grants/revokes/lists per-resource dashboard + access (a Phase 4, enterprise-api-only feature -- a 501 on + plain api means no enterprise permission service is wired + in on this deployment, not a client error). A grant only + ever raises someone to viewer or editor on one dashboard; + Admin/Owner already have tenant-wide access. alerts list/get/apply against alerting's rule CRUD endpoints. "apply " creates a rule from a JSON file with the same shape POST /rules accepts. diff --git a/docs/phase-4-runbook.md b/docs/phase-4-runbook.md index 8d0de11..8c51904 100644 --- a/docs/phase-4-runbook.md +++ b/docs/phase-4-runbook.md @@ -847,13 +847,28 @@ Full accounting: `/docs/security/threat-model.md`. Headline items: - **Per-resource dashboard grants are now enforced** (`api/dashboards`' handler reads `dashboard_permissions` via `enterprise/internal/rbacstore.DashboardPermissions`, only when - `enterprise-api` -- not plain `api` -- serves traffic), but there's - still no UI or `sentryctl` command to create a grant -- `PUT - /dashboards/{id}/permissions/{userId}` has to be called directly. - Verified against a fake store; the real-Postgres integration tests - (`enterprise/internal/rbacstore/rbacstore_test.go`) haven't run - against a live database in this environment, same gap as the rest of - this phase's Postgres-backed pieces. + `enterprise-api` -- not plain `api` -- serves traffic), **and now has a + CLI surface**: `sentryctl dashboards permissions list|grant|revoke` + (`cli/cmd/sentryctl/cmd_dashboards.go`) against + `GET`/`PUT`/`DELETE /dashboards/{id}/permissions/{userId}` -- still no + `web` UI for it, just the CLI. Verified against a real `httptest. + Server` (not a fake store this time -- the CLI has no store of its + own, just an HTTP client, so this is exercising real request + construction/method/path/body/error-parsing, the same pattern every + other `sentryctl` subcommand's tests use): + + ```sh + cd cli + go test ./... -run TestCmdDashboardsPermissions -v + ``` + + `api/dashboards`' own handler tests (fake `PermissionStore`) and the + real-Postgres `enterprise/internal/rbacstore/rbacstore_test.go` + integration tests are unchanged by this -- the CLI is a new caller of + an existing, already-tested endpoint, not new server-side logic. Those + Postgres-backed tests still haven't run against a live database in + this environment, same gap as the rest of this phase's Postgres-backed + pieces. - All four adversarial ClickHouse/Tantivy probes named in `/docs/phase-4-isolation-design.md`'s verification plan are now closed -- see `api/queryapi/tenant_isolation_gap_test.go` for the full