Add sentryctl dashboards permissions list|grant|revoke
PUT/DELETE /dashboards/{id}/permissions/{userId} (per-resource dashboard
grants, built earlier this phase) had no caller but Go tests and curl --
named as a real, disclosed gap in docs/phase-4-runbook.md. Adds a CLI
surface: sentryctl dashboards permissions list/grant/revoke, following
the existing dashboards subcommand pattern.
grant/revoke needed a new httpclient.go helper (httpMutateNoBody) since
both endpoints respond 204 No Content -- the existing helpers all expect
a JSON body to pretty-print. grant validates the role client-side
(viewer/editor only, mirroring api/dashboards.validGrantRole) before
making a request, since Admin/Owner already have tenant-wide dashboard
access and a resource-level grant can never raise someone past Editor.
Verified with real httptest.Server round trips (method, path, request
body, and error-body parsing on a 501 from a deployment with no
enterprise permission service wired in) -- the same pattern every other
sentryctl subcommand's tests already use, no fake/mock client needed
since sentryctl itself is just an HTTP client with no store of its own.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -31,11 +31,24 @@ sentryctl dashboards list
|
||||
sentryctl dashboards get <id>
|
||||
sentryctl dashboards apply dashboard.json # imports a dashboard exported via the web UI's "Export JSON" button
|
||||
|
||||
sentryctl dashboards permissions list <dashboard-id>
|
||||
sentryctl dashboards permissions grant <dashboard-id> <user-id> viewer|editor
|
||||
sentryctl dashboards permissions revoke <dashboard-id> <user-id>
|
||||
|
||||
sentryctl alerts list
|
||||
sentryctl alerts get <id>
|
||||
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
|
||||
|
||||
@@ -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 <dashboard-id> <user-id> <viewer|editor>")
|
||||
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 <dashboard-id> <user-id>")
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -55,6 +55,9 @@ Usage:
|
||||
sentryctl ping [--api <url>]
|
||||
sentryctl query "<query>" [--api <url>] [--language sql|spl] [--json]
|
||||
sentryctl dashboards list|get <id>|apply <file> [--api <url>]
|
||||
sentryctl dashboards permissions list <dashboard-id> [--api <url>]
|
||||
sentryctl dashboards permissions grant <dashboard-id> <user-id> viewer|editor [--api <url>]
|
||||
sentryctl dashboards permissions revoke <dashboard-id> <user-id> [--api <url>]
|
||||
sentryctl alerts list|get <id>|apply <file> [--alerting-api <url>]
|
||||
|
||||
Commands:
|
||||
@@ -67,6 +70,12 @@ Commands:
|
||||
"apply <file>" 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 <file>" creates a rule from a JSON file with the
|
||||
same shape POST /rules accepts.
|
||||
|
||||
+22
-7
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user