From 477a6a3ddff0c980dc4d08e21b9a6ab36b190e70 Mon Sep 17 00:00:00 2001 From: Ignacio Van Droogenbroeck <64545348+xe-nvdk@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:55:56 -0600 Subject: [PATCH] fix(db): tell apart Arc's 401, its three 403s and its 404 on the database listings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arc's three database listing endpoints carried no authentication at all. They now require read permission and, where the server restricts reads per database, a grant covering the database being listed — so `db list`, `db show` and `measurement list` can receive a 401 or a 403 where they previously could not. Those statuses are not one condition, and 403 is not even one condition by itself. Arc answers it for a token that carries no read permission, for a token whose grants do not cover the database asked for, and for its own permission data being unreadable, and distinguishes them only by message. The three need different responses from the operator, so arcli now reads the message and says which one happened: - Scoped: Arc refuses `GET /api/v1/databases` for a token restricted to particular databases rather than returning a filtered list, the same bar `SHOW DATABASES` applies. `db list` now says the token is scoped and names the way forward (`arcli db show `, `arcli measurement list --database `) instead of printing a bare `HTTP 403`. This is an expected state for a tenant-scoped token, so arcli does not retry it. - No read permission: says so and asks for a token that has it. Telling this operator to name a database would send them somewhere that cannot help. - Permission data unavailable: reported as a server-side fault, not as a statement about the token. A 404 stays a 404, so "you may not read it" and "it is not there" remain distinguishable, and a 401 says whether the connection has no token at all or carries one Arc rejected. Matching is by stable message fragment rather than whole string, and both the read middleware's wording and the handler gate's are recognised, so a change to the permission word cannot silently reclassify one refusal as another. An unrecognised 403 falls back to naming the token's permissions, which is safe to say about any refusal. `client.AccessDeniedError` carries the status, the database, whether the refusal was the per-database one and whether the connection sent a token; `client.Scoped(err)` reports the scoped case for callers embedding the package. Companion to the Arc server change on fix/rbac-measurement-where-and-databases-auth. --- README.md | 2 + docs/releases/v26.09.5.md | 7 +- internal/client/access.go | 164 ++++++++++++++ internal/client/access_test.go | 355 +++++++++++++++++++++++++++++++ internal/client/database.go | 24 ++- internal/commands/db.go | 14 +- internal/commands/measurement.go | 7 +- 7 files changed, 566 insertions(+), 7 deletions(-) create mode 100644 internal/client/access.go create mode 100644 internal/client/access_test.go diff --git a/README.md b/README.md index 7cdd545..3b4724e 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,8 @@ arcli logs --level warn --since 6h The command tree also covers API tokens, continuous queries, schedulers, predicate deletes, backups, cluster membership, and compaction. Run `arcli --help` or `arcli --help` for the complete command reference. +`db list`, `db show` and `measurement list` read Arc's database listing endpoints, which need a token carrying read permission. On a server that restricts reads per database, a token scoped to particular databases cannot list them all — Arc refuses rather than returning a filtered list, matching `SHOW DATABASES` — so `db list` reports that the token is scoped and asks you to name a database instead. Pass it to `arcli db show ` or `arcli measurement list --database `. + ## Output and automation Commands support the formats appropriate to their API: diff --git a/docs/releases/v26.09.5.md b/docs/releases/v26.09.5.md index a5fe8de..fa4941c 100644 --- a/docs/releases/v26.09.5.md +++ b/docs/releases/v26.09.5.md @@ -1,4 +1,4 @@ -arcli 26.09.5 shows what a backup lacks, now that Arc 26.09.3 reports it on every backup endpoint. +arcli 26.09.5 shows what a backup lacks, now that Arc 26.09.3 reports it on every backup endpoint, and tells you which of Arc's two read refusals turned a database listing down. ### Changed @@ -6,7 +6,12 @@ arcli 26.09.5 shows what a backup lacks, now that Arc 26.09.3 reports it on ever - `arcli backup show` names the files: its `INCOMPLETE` line now says how many files of the total and how many metadata files were skipped, how many of those were skipped for a key too long to store, how many files could not be listed, and how many files of an outside-root Iceberg warehouse were skipped, followed by one `skipped:` line per file the manifest names (up to 32) and one `unaddressable:` line per file in that sample. - `arcli backup status`, and the final status that `arcli backup create --wait` and `arcli backup restore --wait` print, list the skipped files the server names and report files a backup could not list (their names are in the manifest once the backup has completed, otherwise in the server log). For a restore, failed or completed, they say what the restored backup already lacked when it was taken and how many Iceberg warehouse files were not restored because the node has no outside-root warehouse. `create --wait` ends its success line with every gap the server reports, prints the names, and points at `arcli backup show` for the full breakdown. - `arcli backup restore` warns before restoring a backup that lacks files for any reason the manifest records (skipped data or metadata files, files that could not be listed, outside-root Iceberg warehouse files), not only skipped data files, and says how many of the skipped files had keys too long to store. +- `db list`, `db show` and `measurement list` report Arc's 401 and 403 answers as three separate conditions instead of one HTTP error. A 401 says whether the connection has no token at all or carries one Arc rejected; a 403 says whether the token lacks read permission outright or holds read but no grant for the database asked for. A 404 still says the database does not exist, so "you may not read it" and "it is not there" stay apart. +- A token scoped to particular databases cannot list all of them: Arc refuses `GET /api/v1/databases` rather than returning a filtered list, the same way `SHOW DATABASES` does. `arcli db list` now says the token is scoped and names the way forward — `arcli db show `, `arcli measurement list --database ` — instead of printing a bare `HTTP 403`. That is an expected state for a tenant-scoped token, so arcli does not retry it. +- `arcli db list --help`, `arcli db show --help` and `arcli measurement list --help` state the permission each endpoint needs. ### Notes - Against an Arc older than 26.09.3 every one of these reads as "none reported": `-` in the listing, no extra lines elsewhere. That is not a guarantee of completeness; it is what the server knew. +- Arc's three database listing endpoints previously carried no authentication at all. Nothing changes for an admin token, for a read token on a server that does not restrict reads per database, or for a server with authentication disabled. +- `client.AccessDeniedError` carries the status, the database asked for, whether the refusal was the per-database one, and whether the connection sent a token; `client.Scoped(err)` reports the scoped case for callers embedding the package. diff --git a/internal/client/access.go b/internal/client/access.go new file mode 100644 index 0000000..b10d810 --- /dev/null +++ b/internal/client/access.go @@ -0,0 +1,164 @@ +package client + +import ( + "errors" + "fmt" + "strings" +) + +// AccessDeniedError is Arc's refusal of a read on one of the database +// listing routes — GET /api/v1/databases, /api/v1/databases/:name and +// /api/v1/databases/:name/measurements. +// +// Those three routes used to carry no authentication at all: any valid +// token, and on some deployments no token, could enumerate every +// database and measurement name on the server. They now require the +// read permission and, where the server restricts reads per database, a +// read grant covering the database being listed. So a 401 or 403 from +// them is new, and the 403s do not all mean the same thing: +// +// - Arc refuses a token whose grants do not cover the database asked +// for. That is a NORMAL state for a tenant-scoped token, not a +// failure: the caller has to name a database it does hold a grant +// for. Arc will not answer the list-everything route with a +// filtered list — a scoped caller must name its database, the same +// way `SHOW DATABASES` behaves — so a 403 there is the expected +// answer and must not be retried. +// - Arc refuses a token that does not carry the "read" permission at +// all. That needs a different token; naming a database will not +// help. +// - Arc fails closed when it cannot read its own permission data. +// That is a server-side fault, not a statement about the token. +// +// Those three are told apart by the message Arc sends, because the +// status code is 403 for all of them. See classifyAccessError for the +// shapes and for what an unrecognised one is reported as. +type AccessDeniedError struct { + // Status is 401 or 403 as Arc returned it. + Status int + // Database is the database the caller asked about, or + // listAllDatabases for the list-everything route. + Database string + // Scoped is true when Arc refused because the token's grants do not + // cover the database asked for — "name a database you are granted". + // Only meaningful for Status 403. + Scoped bool + // Unavailable is true when Arc could not read its own permission + // data and failed closed. Nothing about the token or the database + // caused it. Only meaningful for Status 403. + Unavailable bool + // HasToken records whether the connection sent a bearer token at + // all, so a 401 on a token-less connection can say so instead of + // quoting a bare status code. + HasToken bool + // Message is Arc's own message, already control-scrubbed by + // decodeWriteError. + Message string +} + +// listAllDatabases is the pseudo-name the server uses for the +// list-everything route's permission check, and the value Database +// carries for it. +const listAllDatabases = "*" + +func (e *AccessDeniedError) Error() string { + switch { + case e.Status == 401 && !e.HasToken: + return "Arc requires a token to list databases but this connection has none " + + "(add one with `arcli config update NAME --token ...`)" + case e.Status == 401: + return fmt.Sprintf("Arc rejected this connection's token (%s); "+ + "it may be invalid, expired or revoked — issue a new one and "+ + "`arcli config update NAME --token ...`", e.Message) + case e.Unavailable: + return fmt.Sprintf("arc could not read its own permission data and refused the "+ + "request rather than guessing (%s); this is a server-side fault, not a "+ + "problem with this token — check the server log", e.Message) + case e.Scoped && e.Database == listAllDatabases: + return "this token is scoped to specific databases, so Arc will not list them all; " + + "name a database you are granted (`arcli db show `, " + + "`arcli measurement list --database `)" + case e.Scoped: + return fmt.Sprintf("this token has no read grant for database %q; "+ + "name a database it is granted, or ask an Arc administrator to grant it", + e.Database) + default: + return fmt.Sprintf("this token does not carry the read permission Arc requires "+ + "to list databases (%s); use a token with read permission", e.Message) + } +} + +// Scoped reports whether err is Arc's per-database RBAC denial — the +// "you are scoped, name your database" answer — rather than a missing +// read permission or a bad token. Exposed so a caller can treat that +// case as a normal, expected state. +func Scoped(err error) bool { + var ae *AccessDeniedError + return errors.As(err, &ae) && ae.Scoped +} + +// classifyAccessError maps the 401/403 the database listing routes can +// answer with onto *AccessDeniedError and leaves every other error +// untouched. database is the name asked about, or listAllDatabases for +// the list-everything route. +// +// Arc sends 403 for three different things and distinguishes them only +// by message, so the message is what this reads. The shapes, all from +// the read middleware and the handler gate on these routes: +// +// no permission for read on database 'x' -> scoped +// access denied: no read permission for database 'x' -> scoped +// permission data unavailable -> unavailable +// token does not have 'read' permission -> coarse +// Permission denied: read required -> coarse +// +// Matching is by stable fragment rather than whole string, so a change +// to the permission word or to surrounding punctuation does not +// silently reclassify one for another. An unrecognised 403 is reported +// as the coarse case on purpose: naming the token's permissions is a +// safe thing to say about any refusal, whereas telling an operator to +// pass a database they may already have passed is not. +func (c *Client) classifyAccessError(err error, database string) error { + var he *HTTPError + if !errors.As(err, &he) { + return err + } + if he.Status != 401 && he.Status != 403 { + return err + } + msg := he.Message + if msg == "" { + msg = he.Raw + } + return &AccessDeniedError{ + Status: he.Status, + Database: database, + Scoped: he.Status == 403 && isScopedDenial(msg), + Unavailable: he.Status == 403 && isPermissionDataUnavailable(msg), + HasToken: c.HasToken(), + Message: msg, + } +} + +// isScopedDenial recognises the two bodies that mean "your grants do not +// cover that database": the read middleware's +// "no permission for read on database 'x'" and the handler gate's +// "access denied: no read permission for database 'x'". +func isScopedDenial(msg string) bool { + m := strings.ToLower(strings.TrimSpace(msg)) + switch { + case strings.HasPrefix(m, "no permission for ") && strings.Contains(m, " on database "): + return true + case strings.HasPrefix(m, "access denied: no ") && strings.Contains(m, "permission for database"): + return true + default: + return false + } +} + +// isPermissionDataUnavailable recognises Arc's fail-closed answer when +// it could not load the token's grants. It is a 403 like the others but +// says nothing about the token, so it must not be reported as one. +func isPermissionDataUnavailable(msg string) bool { + return strings.Contains(strings.ToLower(msg), "permission data unavailable") +} diff --git a/internal/client/access_test.go b/internal/client/access_test.go new file mode 100644 index 0000000..7a52f50 --- /dev/null +++ b/internal/client/access_test.go @@ -0,0 +1,355 @@ +package client + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// Every body Arc's database listing routes answer a refusal with. All the +// 403s share a status code and differ only in message, so the message is +// the whole discriminator — and they do not mean the same thing: "no +// grant for that database" wants a different database, "no read +// permission" wants a different token, and "permission data unavailable" +// is a server fault that says nothing about either. +const ( + // The read middleware's RBAC denial, which is what these routes + // return in practice. extractDatabase supplies the name; it is empty + // for the list-everything route, which sends no database. + mwScopedBody = `{"success":false,"error":"no permission for read on database 'metrics'"}` + mwScopedAllBody = `{"success":false,"error":"no permission for read on database ''"}` + // The handler's own gate, reachable when it checks a different + // database/measurement pair than the middleware did. + handlerScopedBody = `{"error":"access denied: no read permission for database 'metrics'"}` + handlerScopedAllBody = `{"error":"access denied: no read permission for database '*'"}` + // Coarse refusals: the token has no read permission at all. + coarseForbiddenBody = `{"success":false,"error":"token does not have 'read' permission"}` + ossForbiddenBody = `{"success":false,"error":"Permission denied: read required"}` + // Arc could not load the token's grants and refused rather than guess. + unavailableBody = `{"success":false,"error":"permission data unavailable"}` + + noTokenBody = `{"success":false,"error":"Authentication required"}` + badTokenBody = `{"success":false,"error":"Invalid or expired token"}` +) + +// statusServer answers every request with one status and body. +func statusServer(t *testing.T, status int, body string) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = io.WriteString(w, body) + })) + t.Cleanup(srv.Close) + return srv +} + +// tokenlessClient is freshClient without a bearer token, for the 401 +// case where the connection itself has no credential. +func tokenlessClient(t *testing.T, srv *httptest.Server) *Client { + t.Helper() + c, err := New(Config{Endpoint: srv.URL}) + if err != nil { + t.Fatalf("New: %v", err) + } + return c +} + +func mustAccessDenied(t *testing.T, err error) *AccessDeniedError { + t.Helper() + if err == nil { + t.Fatal("expected an error, got nil") + } + var ae *AccessDeniedError + if !errors.As(err, &ae) { + t.Fatalf("error is %T (%v); expected *AccessDeniedError", err, err) + } + return ae +} + +// A scoped token asking for every database gets 403, and that is the +// expected answer rather than a failure to retry: Arc will not return a +// filtered list. The message has to say so and name the way forward. +func TestListDatabases_ScopedForbidden(t *testing.T) { + srv := statusServer(t, http.StatusForbidden, mwScopedAllBody) + c := freshClient(t, srv, "") + + _, err := c.ListDatabases(context.Background()) + ae := mustAccessDenied(t, err) + + if ae.Status != http.StatusForbidden { + t.Errorf("Status = %d; want 403", ae.Status) + } + if !ae.Scoped { + t.Error("Scoped = false; the RBAC denial body must classify as scoped") + } + if ae.Database != listAllDatabases { + t.Errorf("Database = %q; want %q for the list-everything route", ae.Database, listAllDatabases) + } + if !Scoped(err) { + t.Error("Scoped(err) = false; callers must be able to recognise this state") + } + msg := err.Error() + for _, want := range []string{"scoped to specific databases", "arcli db show"} { + if !strings.Contains(msg, want) { + t.Errorf("message %q does not mention %q", msg, want) + } + } +} + +// A token with no read permission at all is a different problem, and +// telling its holder to "name a database" would send them down the +// wrong path. It must say the token lacks read permission. +func TestListDatabases_CoarseForbidden(t *testing.T) { + srv := statusServer(t, http.StatusForbidden, coarseForbiddenBody) + c := freshClient(t, srv, "") + + _, err := c.ListDatabases(context.Background()) + ae := mustAccessDenied(t, err) + + if ae.Scoped { + t.Error("Scoped = true; the coarse permission denial is not a scoping state") + } + if Scoped(err) { + t.Error("Scoped(err) = true for a coarse denial") + } + msg := err.Error() + if !strings.Contains(msg, "read permission") { + t.Errorf("message %q does not name the missing permission", msg) + } + if strings.Contains(msg, "scoped to specific databases") { + t.Errorf("message %q misreports a missing permission as a scoping state", msg) + } +} + +// 401 on a connection that never had a token should say that, not quote +// a bare status code — the fix is to configure one. +func TestListDatabases_UnauthorizedWithoutToken(t *testing.T) { + srv := statusServer(t, http.StatusUnauthorized, noTokenBody) + c := tokenlessClient(t, srv) + + _, err := c.ListDatabases(context.Background()) + ae := mustAccessDenied(t, err) + + if ae.Status != http.StatusUnauthorized { + t.Errorf("Status = %d; want 401", ae.Status) + } + if ae.HasToken { + t.Error("HasToken = true on a connection with no token") + } + if msg := err.Error(); !strings.Contains(msg, "this connection has none") { + t.Errorf("message %q does not say the connection has no token", msg) + } +} + +// 401 with a token means the token is bad, which is a re-authentication +// problem and not a permissions one. +func TestListDatabases_UnauthorizedWithToken(t *testing.T) { + srv := statusServer(t, http.StatusUnauthorized, badTokenBody) + c := freshClient(t, srv, "") + + _, err := c.ListDatabases(context.Background()) + ae := mustAccessDenied(t, err) + + if !ae.HasToken { + t.Error("HasToken = false; the connection does carry a token") + } + if ae.Scoped { + t.Error("Scoped = true for a 401") + } + if msg := err.Error(); !strings.Contains(msg, "rejected this connection's token") { + t.Errorf("message %q does not point at re-authentication", msg) + } +} + +// The per-database routes carry the database they were refused for, so +// the message can name it. +func TestGetDatabase_ScopedForbidden(t *testing.T) { + srv := statusServer(t, http.StatusForbidden, mwScopedBody) + c := freshClient(t, srv, "") + + _, err := c.GetDatabase(context.Background(), "metrics") + ae := mustAccessDenied(t, err) + + if !ae.Scoped || ae.Database != "metrics" { + t.Errorf("got Scoped=%v Database=%q; want true/\"metrics\"", ae.Scoped, ae.Database) + } + if msg := err.Error(); !strings.Contains(msg, `"metrics"`) { + t.Errorf("message %q does not name the database", msg) + } +} + +func TestListMeasurements_ScopedForbidden(t *testing.T) { + srv := statusServer(t, http.StatusForbidden, mwScopedBody) + c := freshClient(t, srv, "") + + _, err := c.ListMeasurements(context.Background(), "metrics") + ae := mustAccessDenied(t, err) + + if !ae.Scoped || ae.Database != "metrics" { + t.Errorf("got Scoped=%v Database=%q; want true/\"metrics\"", ae.Scoped, ae.Database) + } +} + +// "No grant for this database" and "no such database" are different +// answers; 404 must stay an *HTTPError so the command layer keeps +// telling them apart. +func TestListingRoutes_NotFoundIsNotAccessDenied(t *testing.T) { + srv := statusServer(t, http.StatusNotFound, `{"error":"Database 'ghost' not found"}`) + c := freshClient(t, srv, "") + + for _, tc := range []struct { + name string + call func() error + }{ + {"get", func() error { _, err := c.GetDatabase(context.Background(), "ghost"); return err }}, + {"measurements", func() error { _, err := c.ListMeasurements(context.Background(), "ghost"); return err }}, + } { + t.Run(tc.name, func(t *testing.T) { + err := tc.call() + var ae *AccessDeniedError + if errors.As(err, &ae) { + t.Fatalf("404 classified as an access denial: %v", err) + } + var he *HTTPError + if !errors.As(err, &he) || he.Status != http.StatusNotFound { + t.Fatalf("error is %T (%v); expected *HTTPError with Status 404", err, err) + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("message %q does not say the database is missing", err.Error()) + } + }) + } +} + +// Everything that is not a 401 or 403 keeps its existing shape, so this +// change cannot swallow a server fault. +func TestListingRoutes_ServerErrorUnchanged(t *testing.T) { + srv := statusServer(t, http.StatusInternalServerError, `{"error":"Failed to list databases: boom"}`) + c := freshClient(t, srv, "") + + _, err := c.ListDatabases(context.Background()) + var ae *AccessDeniedError + if errors.As(err, &ae) { + t.Fatalf("500 classified as an access denial: %v", err) + } + var he *HTTPError + if !errors.As(err, &he) || he.Status != http.StatusInternalServerError { + t.Fatalf("error is %T (%v); expected *HTTPError with Status 500", err, err) + } +} + +// An unrecognised 403 body must not be reported as a scoping state — +// telling an operator to name a database they already named is worse +// than naming the permission. +func TestListingRoutes_UnknownForbiddenBodyIsNotScoped(t *testing.T) { + srv := statusServer(t, http.StatusForbidden, `{"error":"Delete operations are disabled."}`) + c := freshClient(t, srv, "") + + _, err := c.GetDatabase(context.Background(), "metrics") + ae := mustAccessDenied(t, err) + if ae.Scoped { + t.Errorf("Scoped = true for an unrecognised 403 body (%q)", ae.Message) + } +} + +// Arc answers these routes with 403 for three different reasons and +// distinguishes them only by message. Every shape the read middleware +// and the handler gate can produce has to land in the right bucket: a +// scoped token told to fix its permissions, or a permissionless token +// told to name a database, both send the operator the wrong way. +func TestClassifyAccessError_EveryForbiddenBody(t *testing.T) { + // perDatabase picks the route the row exercises: the advice depends on + // which route was called (name a database, or you have no grant for + // the one you named), not on which database Arc happened to echo. + tests := []struct { + name string + body string + perDatabase bool + wantScoped bool + wantUnavailable bool + wantInMessage string + }{ + { + name: "middleware RBAC denial, named database", body: mwScopedBody, + perDatabase: true, wantScoped: true, wantInMessage: "no read grant for database", + }, + { + name: "middleware RBAC denial, list-everything route sends no database", + body: mwScopedAllBody, wantScoped: true, wantInMessage: "scoped to specific databases", + }, + { + name: "handler gate, named database", body: handlerScopedBody, + perDatabase: true, wantScoped: true, wantInMessage: "no read grant for database", + }, + { + name: "handler gate, list-everything route", body: handlerScopedAllBody, + wantScoped: true, wantInMessage: "scoped to specific databases", + }, + { + name: "coarse: token has no read permission", body: coarseForbiddenBody, + wantInMessage: "does not carry the read permission", + }, + { + name: "coarse: RBAC not wired on this server", body: ossForbiddenBody, + wantInMessage: "does not carry the read permission", + }, + { + name: "server could not load the grants", body: unavailableBody, + wantUnavailable: true, wantInMessage: "server-side fault", + }, + { + name: "unrecognised body falls back to the coarse message", + body: `{"error":"something new"}`, wantInMessage: "does not carry the read permission", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := statusServer(t, http.StatusForbidden, tt.body) + c := freshClient(t, srv, "") + + var err error + if tt.perDatabase { + _, err = c.GetDatabase(context.Background(), "metrics") + } else { + _, err = c.ListDatabases(context.Background()) + } + ae := mustAccessDenied(t, err) + + if ae.Scoped != tt.wantScoped { + t.Errorf("Scoped = %v; want %v", ae.Scoped, tt.wantScoped) + } + if ae.Unavailable != tt.wantUnavailable { + t.Errorf("Unavailable = %v; want %v", ae.Unavailable, tt.wantUnavailable) + } + if Scoped(err) != tt.wantScoped { + t.Errorf("Scoped(err) = %v; want %v", Scoped(err), tt.wantScoped) + } + if msg := err.Error(); !strings.Contains(msg, tt.wantInMessage) { + t.Errorf("message %q does not contain %q", msg, tt.wantInMessage) + } + }) + } +} + +// The list-everything route sends no database name, so Arc's message +// carries an empty one. The advice still has to be "name a database", +// not "you have no grant for \"\"". +func TestListDatabases_EmptyDatabaseInDenialStillAsksForOne(t *testing.T) { + srv := statusServer(t, http.StatusForbidden, mwScopedAllBody) + c := freshClient(t, srv, "") + + _, err := c.ListDatabases(context.Background()) + msg := err.Error() + if !strings.Contains(msg, "scoped to specific databases") { + t.Errorf("message %q does not explain the scoping", msg) + } + if strings.Contains(msg, `""`) { + t.Errorf("message %q quotes an empty database name", msg) + } +} diff --git a/internal/client/database.go b/internal/client/database.go index cb7f29d..55c1d6e 100644 --- a/internal/client/database.go +++ b/internal/client/database.go @@ -46,6 +46,14 @@ type createDatabaseRequest struct { // ListDatabases returns every database the caller can see, with // measurement counts pre-computed by the server. +// +// The route requires the read permission, and a read grant covering +// every database when the server has RBAC configured. A token scoped to +// particular databases therefore gets HTTP 403 here rather than a +// filtered list — Arc will never filter this list, matching +// `SHOW DATABASES` — which arrives as an *AccessDeniedError with +// Scoped set. That is an expected answer for a scoped token, not a +// transient failure: name a database instead of retrying. func (c *Client) ListDatabases(ctx context.Context) (*DatabaseListResponse, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.cfg.Endpoint+"/api/v1/databases", nil) if err != nil { @@ -68,7 +76,7 @@ func (c *Client) ListDatabases(ctx context.Context) (*DatabaseListResponse, erro return nil, fmt.Errorf("read response: %w", err) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, decodeWriteError(resp.StatusCode, body) + return nil, c.classifyAccessError(decodeWriteError(resp.StatusCode, body), listAllDatabases) } var out DatabaseListResponse if err := json.Unmarshal(body, &out); err != nil { @@ -80,6 +88,11 @@ func (c *Client) ListDatabases(ctx context.Context) (*DatabaseListResponse, erro // GetDatabase returns metadata for one database. Returns a // recognisable "not found" error for HTTP 404 so the command layer can // distinguish "missing" from "broken." +// +// A 401, or a 403 from the read-permission or per-database RBAC gate, +// arrives as an *AccessDeniedError; a 404 stays an *HTTPError carrying +// Status 404, so "no grant for this database" and "no such database" +// stay distinguishable. func (c *Client) GetDatabase(ctx context.Context, name string) (*DatabaseInfo, error) { if name == "" { return nil, fmt.Errorf("database name is required") @@ -103,7 +116,7 @@ func (c *Client) GetDatabase(ctx context.Context, name string) (*DatabaseInfo, e return nil, fmt.Errorf("read response: %w", err) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, decodeWriteError(resp.StatusCode, body) + return nil, c.classifyAccessError(decodeWriteError(resp.StatusCode, body), name) } var out DatabaseInfo if err := json.Unmarshal(body, &out); err != nil { @@ -190,6 +203,11 @@ func (c *Client) DeleteDatabase(ctx context.Context, name string) error { // ListMeasurements returns measurements inside a single database via // GET /api/v1/databases/:name/measurements. +// +// Like GetDatabase, this needs the read permission plus a grant +// covering the named database; both refusals arrive as an +// *AccessDeniedError, while an unknown database stays a 404 +// *HTTPError. func (c *Client) ListMeasurements(ctx context.Context, database string) (*MeasurementListResponse, error) { if database == "" { return nil, fmt.Errorf("database name is required") @@ -213,7 +231,7 @@ func (c *Client) ListMeasurements(ctx context.Context, database string) (*Measur return nil, fmt.Errorf("read response: %w", err) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, decodeWriteError(resp.StatusCode, body) + return nil, c.classifyAccessError(decodeWriteError(resp.StatusCode, body), database) } var out MeasurementListResponse if err := json.Unmarshal(body, &out); err != nil { diff --git a/internal/commands/db.go b/internal/commands/db.go index 8b1be76..42f4dde 100644 --- a/internal/commands/db.go +++ b/internal/commands/db.go @@ -63,7 +63,13 @@ Output formats: table (default) | json | csv Each row shows the database name, its measurement count, and (when set -by the server) the creation timestamp.`, +by the server) the creation timestamp. + +The endpoint needs a token with read permission, and — where the server +restricts reads per database — a grant covering every database. A token +scoped to particular databases is refused here rather than given a +filtered list, the same way SHOW DATABASES behaves; name one of its +databases with ` + "`arcli db show `" + ` instead.`, RunE: func(cmd *cobra.Command, args []string) error { if timeout <= 0 { return fmt.Errorf("--timeout must be > 0 (got %s)", timeout) @@ -116,7 +122,11 @@ GET /api/v1/databases/:name/measurements so the operator sees Output formats: table (default — two stacked tables, db info then measurements) json (single object: {"database": {...}, "measurements": [...]}) - csv (measurements only — db metadata is one row, not table-shaped)`, + csv (measurements only — db metadata is one row, not table-shaped) + +Both endpoints need a token with read permission plus, where the server +restricts reads per database, a grant for this one. "No grant for that +database" and "no such database" are reported separately.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if timeout <= 0 { diff --git a/internal/commands/measurement.go b/internal/commands/measurement.go index e81511d..4f88b40 100644 --- a/internal/commands/measurement.go +++ b/internal/commands/measurement.go @@ -49,7 +49,12 @@ func newMeasurementListCmd() *cobra.Command { The database name comes from --database, or (when --database is omitted) from the active connection's default_database. If neither is set the -command errors before any network call.`, +command errors before any network call. + +The endpoint needs a token with read permission plus, where the server +restricts reads per database, a grant for the database named. A token +without that grant is refused with HTTP 403; pass --database with one +it does hold.`, Example: ` arcli measurement list --database metrics arcli measurement list -c prod --database logs -o json`, RunE: func(cmd *cobra.Command, args []string) error {