From eee8b7b7f484fca22bbed48ea78e6b75fdb52d15 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Fri, 2 Oct 2026 13:37:25 +0200 Subject: [PATCH 1/3] Act as another user with --as, and show admin status in whoami An admin can now run any command as another user: --as USER asks the server for a token acting as them, through reva's admin HTTP service, and uses it in place of the signed-in credential. Nothing else changes. The paths the CLI builds and the shares it lists follow from whom the server says the token belongs to, so no command needs to know. The token is asked for once per command and never cached, so each command has its own entry in reva's audit log and no token for someone else's account is left in /tmp. A completion never impersonates. whoami says whether the user is an admin, and under --as who is impersonating them. A server without admin features is not an error there. The dev revad builds from reva's admin-http branch and makes einstein an admin, so the integration tests cover both sides against a real server. --- dev/Dockerfile | 9 ++-- dev/revad/cernbox.toml | 13 +++++ docs/design.md | 13 ++++- integration/admin_test.go | 97 +++++++++++++++++++++++++++++++++ internal/cli/admin_test.go | 90 +++++++++++++++++++++++++++++++ internal/cli/auth_cmd.go | 48 ++++++++++++++--- internal/cli/command_test.go | 34 ++++++++++++ internal/cli/root.go | 30 ++++++++++- pkg/client/admin.go | 100 +++++++++++++++++++++++++++++++++++ pkg/client/admin_test.go | 100 +++++++++++++++++++++++++++++++++++ 10 files changed, 519 insertions(+), 15 deletions(-) create mode 100644 integration/admin_test.go create mode 100644 internal/cli/admin_test.go create mode 100644 pkg/client/admin.go create mode 100644 pkg/client/admin_test.go diff --git a/dev/Dockerfile b/dev/Dockerfile index ed011bd..82e53bd 100644 --- a/dev/Dockerfile +++ b/dev/Dockerfile @@ -7,10 +7,11 @@ RUN apt-get update \ && apt-get install -y --no-install-recommends musl-tools \ && rm -rf /var/lib/apt/lists/* -# The reva branch carrying the Kerberos auth manager and the SPNEGO credential -# strategy. gaia resolves a branch name through the GitHub API, so this follows -# the branch head; point it at a released version once the branch is merged. -ARG REVA_VERSION=kerberos-auth +# The reva branch carrying the Kerberos auth manager, the SPNEGO credential +# strategy, and the admin HTTP service --as needs (it is built on +# kerberos-auth). gaia resolves a branch name through the GitHub API, so this +# follows the branch head; point it at a released version once it is merged. +ARG REVA_VERSION=admin-http # cgo, rather than CGO_ENABLED=0: the sql share driver runs on sqlite, which # needs cgo, and it is the only share driver whose "not found" error the diff --git a/dev/revad/cernbox.toml b/dev/revad/cernbox.toml index 557634e..0ab9782 100644 --- a/dev/revad/cernbox.toml +++ b/dev/revad/cernbox.toml @@ -348,6 +348,15 @@ driver = "json" [grpc.services.groupprovider.drivers.json] groups = "/etc/revad/groups.demo.json" +# The Admin API, for --as. einstein is in sailing-lovers and so is an admin; +# marie and richard are not, which is what the refusal tests need. The socket is +# off because the image is scratch and has no /run to bind it in. +[grpc.services.admin] +address = ":19600" +admin_group = "sailing-lovers" +machine_auth_apikey = "{{ vars.machine_api_key }}" +socket = "off" + ### HTTP ENDPOINTS ### @@ -505,6 +514,10 @@ insecure = true [http.services.ocm] address = ":443" +# The HTTP face of the Admin API: /admin/status and /admin/impersonate. +[http.services.admin] +address = ":443" + [http.services.sciencemesh] address = ":443" provider_domain = "revad" diff --git a/docs/design.md b/docs/design.md index 1029918..d06a3cc 100644 --- a/docs/design.md +++ b/docs/design.md @@ -50,6 +50,7 @@ Everything the CLI needs is already exposed over HTTPS by the CERNBox frontend. | Identity | `GET /graph/v1.0/me` | | | Recursive download | archiver service, URL and formats from capabilities | One request for a whole tree | | App tokens | OCS connected-clients API | Backed by [appauth](https://github.com/cs3org/reva/blob/master/pkg/auth/manager/appauth/appauth.go) | +| Admin status, impersonation | `GET /admin/status`, `POST /admin/impersonate` | reva's `admin` HTTP service, the HTTPS face of the gRPC Admin API (§3.6) | Locks (`LOCK`/`UNLOCK`) are available on ocdav but are lower priority for a CLI. @@ -169,6 +170,14 @@ Each cache entry is keyed by `(endpoint, principal-or-subject)`. A user who does - **No ticket available**: create a scoped app token, `cernbox token create --path /eos/project/x --permission read --expiry 2026-12-31`, and expose it via `$CERNBOX_APP_TOKEN`. Reva already supports path- and share-scoped app tokens — see `getPathScope` in [app-tokens-create.go:199](https://github.com/cs3org/reva/blob/master/cmd/reva/app-tokens-create.go#L199) — so these can be least-privilege rather than full-account credentials, and the CLI should make the scoped form the documented default. - **Service accounts**: a keytab plus `kinit -kt` before invoking, or an app token. Both work unchanged. +### 3.6 Acting as another user + +An admin of the deployment — a member of the Admin API's `admin_group` — can run any command as another user with `--as USER`. reva has no "admin may touch other users' data" logic: the only power is impersonation, which hands out an ordinary user token for the target. So `--as` changes one thing, the credential, and everything else follows from whom the server says the token belongs to: `/me` answers as the target, path-addressed URLs are built under their name, `share list` lists their shares. No command knows about it. + +The token comes from `POST /admin/impersonate` with the admin's own credential. The server steps the caller up and impersonates in one request, so the admin token never reaches the client. Both steps are in reva's audit log. The impersonation token is never cached: every command asks again, so each has its own audit entry, and no token for someone else's account is left in `/tmp`. It lives as long as a token from signing in (the Admin API's `impersonation_ttl`, or the token manager's own lifetime, a day by default), so a long transfer made as the user finishes; the short `admin_ttl` applies only to the admin token, which stays on the server. + +`whoami` shows whether the user is an admin, from `GET /admin/status`, which only checks and is not audited. With `--as` it shows the impersonated user and who is impersonating them. A completion never impersonates. + ## 4. Path and namespace model Absolute CS3 paths are canonical, matching what the dav files root already exposes and what users already type for `eos`: @@ -209,7 +218,8 @@ On lxplus, `/eos/user/g/gdelmont` is *both* a valid CERNBox remote path and a re cernbox login [--method kerberos|device|app-token] cernbox logout cernbox status # provider, identity, expiry, endpoint -cernbox whoami [--output json] +cernbox whoami [--output json] # identity, and whether an admin +cernbox --as USER CMD ... # any command, as another user (admins) cernbox ls [-l] [-r] [--all] PATH cernbox stat PATH @@ -326,6 +336,7 @@ Errors are mapped from CS3 status codes and HTTP status to human sentences, with - App tokens are created scoped by default; the unscoped form requires an explicit `--all`. - `--insecure` and `--skip-verify` exist for dev instances, print a warning to stderr on every use, and are refused when the endpoint is a `cern.ch` host. - No credential is ever passed as a command line argument in a way that would appear in `ps` output or shell history; `--password` prompts rather than accepting a value. +- Every `--as` command is recorded in reva's audit log as an impersonation by the signed-in admin. Its token is never written to the cache (§3.6). - Phase 2 caveats — no mutual authentication, per-process replay cache — are documented, not silently assumed away (§3.3). ## 11. Testing diff --git a/integration/admin_test.go b/integration/admin_test.go new file mode 100644 index 0000000..a82efed --- /dev/null +++ b/integration/admin_test.go @@ -0,0 +1,97 @@ +//go:build integration + +package integration_test + +import ( + "strings" + "testing" +) + +// The dev revad makes einstein an admin (he is in sailing-lovers, the +// admin_group) and marie not. + +func TestWhoamiSaysWhetherAdmin(t *testing.T) { + e := setup(t) + + for _, tc := range []struct { + who account + want bool + }{ + {e.self(), true}, + {e.other(), false}, + } { + var me struct { + Username string `json:"username"` + Admin *bool `json:"admin"` + } + e.runJSONAs(tc.who, &me, "whoami") + if me.Admin == nil || *me.Admin != tc.want { + t.Errorf("whoami as %s: admin = %v, want %v", me.Username, me.Admin, tc.want) + } + } +} + +func TestAsShowsWhoActs(t *testing.T) { + e := setup(t) + + var me struct { + Username string `json:"username"` + Admin *bool `json:"admin"` + ImpersonatedBy string `json:"impersonated_by"` + } + e.runJSON(&me, "--as", otherUser, "whoami") + if me.Username != otherUser || me.ImpersonatedBy != username { + t.Errorf("whoami --as = %+v, want %s impersonated by %s", me, otherUser, username) + } + if me.Admin == nil || *me.Admin { + t.Errorf("admin = %v: whoami --as reports the impersonated user, who is not one", me.Admin) + } +} + +// TestAsListsAnotherUsersShares is the first reason for --as: seeing what +// someone else has shared, as they see it. +func TestAsListsAnotherUsersShares(t *testing.T) { + e := setup(t) + dir := e.recipientDir() + e.mustRunAs(e.other(), "share", "create", dir, "--with", "richard", "--role", "viewer") + + var shares []struct { + Path string `json:"path"` + } + e.runJSON(&shares, "--as", otherUser, "share", "list") + for _, s := range shares { + if s.Path == dir { + return + } + } + t.Errorf("%s's share of %s is not in the listing: %+v", otherUser, dir, shares) +} + +// TestAsWritesIntoAnotherUsersHome is the second: putting a file where someone +// else can find it, which the admin's own identity is not allowed to do. +func TestAsWritesIntoAnotherUsersHome(t *testing.T) { + e := setup(t) + dir := e.recipientDir() + local := e.writeLocal("fix.txt", []byte("from the admin")) + + if _, _, code := e.run("put", local, "cb:"+dir+"/"); code != 4 { + t.Fatalf("put into %s without --as exited %d, want 4: the test proves nothing", dir, code) + } + + e.mustRun("--as", otherUser, "put", local, "cb:"+dir+"/") + if got := e.mustRunAs(e.other(), "cat", dir+"/fix.txt"); got != "from the admin" { + t.Errorf("%s reads %q", otherUser, got) + } +} + +func TestAsRefusedForNonAdmin(t *testing.T) { + e := setup(t) + + _, stderr, code := e.runAs(e.other(), "--as", username, "ls") + if code != 4 { + t.Errorf("exit %d, want 4 (permission); stderr: %s", code, stderr) + } + if !strings.Contains(stderr, "not an administrator") { + t.Errorf("stderr does not say why: %s", stderr) + } +} diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go new file mode 100644 index 0000000..6557064 --- /dev/null +++ b/internal/cli/admin_test.go @@ -0,0 +1,90 @@ +package cli + +import ( + "encoding/json" + "net/http" + "slices" + "strings" + "testing" + + "github.com/cernbox/cernbox-cli/pkg/cberr" +) + +// TestAsActsAsUser checks that --as changes whose identity every request is +// made with, and that it asks the server once per command. +func TestAsActsAsUser(t *testing.T) { + box := newTestBox(t) + box.admin = true + + _, stderr, err := run(t, box, "--as", "marie", "share", "list") + if err != nil { + t.Fatal(err) + } + if !slices.Equal(box.impersonations, []string{"marie"}) { + t.Errorf("impersonations = %v, want one, for marie", box.impersonations) + } + if got := box.auths[http.MethodGet+" /graph/v1beta1/me/drive/sharedByMe"]; got != "Bearer as-marie" { + t.Errorf("share list was made with %q, want marie's token", got) + } + // Asking for the token is the admin's own request. + if got := box.auths[http.MethodPost+" /admin/impersonate"]; got != "Bearer test-token" { + t.Errorf("impersonate was made with %q, want the admin's token", got) + } + if !strings.Contains(stderr, "Acting as marie.") { + t.Errorf("stderr does not say whom the command acts as: %q", stderr) + } +} + +func TestAsDeniedForNonAdmin(t *testing.T) { + box := newTestBox(t) + + _, _, err := run(t, box, "--as", "marie", "ls") + if cberr.ExitCode(err) != cberr.ExitPermission { + t.Fatalf("err = %v (exit %d), want a permission error", err, cberr.ExitCode(err)) + } + if !strings.Contains(err.Error(), "not an administrator") { + t.Errorf("error does not say why: %v", err) + } + for k := range box.auths { + if strings.HasPrefix(k, "PROPFIND") { + t.Errorf("listed files after being refused: %s", k) + } + } +} + +func TestWhoamiShowsAdmin(t *testing.T) { + for _, admin := range []bool{true, false} { + box := newTestBox(t) + box.admin = admin + + stdout, _, err := run(t, box, "-o", "json", "whoami") + if err != nil { + t.Fatal(err) + } + var got whoamiResult + if err := json.Unmarshal([]byte(stdout), &got); err != nil { + t.Fatalf("decoding %q: %v", stdout, err) + } + if got.Username != "einstein" || got.Admin == nil || *got.Admin != admin { + t.Errorf("whoami = %+v, want einstein with admin %v", got, admin) + } + if got.ImpersonatedBy != "" { + t.Errorf("impersonated_by = %q without --as", got.ImpersonatedBy) + } + } +} + +func TestWhoamiAs(t *testing.T) { + box := newTestBox(t) + box.admin = true + + stdout, _, err := run(t, box, "--as", "marie", "whoami") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"marie", "Admin", "no", "Impersonated by", "einstein"} { + if !strings.Contains(stdout, want) { + t.Errorf("whoami output lacks %q:\n%s", want, stdout) + } + } +} diff --git a/internal/cli/auth_cmd.go b/internal/cli/auth_cmd.go index 605e444..7bb25a8 100644 --- a/internal/cli/auth_cmd.go +++ b/internal/cli/auth_cmd.go @@ -2,10 +2,12 @@ package cli import ( "errors" + "slices" "time" "github.com/cernbox/cernbox-cli/pkg/auth" "github.com/cernbox/cernbox-cli/pkg/cberr" + "github.com/cernbox/cernbox-cli/pkg/client" "github.com/cernbox/cernbox-cli/pkg/output" "github.com/spf13/cobra" ) @@ -27,7 +29,7 @@ func newLoginCmd(app *App) *cobra.Command { if err != nil { return err } - me, err := app.client.Me(ctx) + me, err := app.signedIn.Me(ctx) if err != nil { return err } @@ -106,10 +108,11 @@ func newStatusCmd(app *App) *cobra.Command { if tokErr == nil { st.Provider = tok.Provider st.Expires = expiryString(tok) - if me, err := app.client.Me(ctx); err == nil { + if me, err := app.signedIn.Me(ctx); err == nil { st.User = me.Username st.DisplayName = me.DisplayName } + st.ActingAs = app.flags.as } else { st.Error = tokErr.Error() } @@ -131,6 +134,9 @@ func newStatusCmd(app *App) *cobra.Command { {Name: "Server", Value: orDash(st.ServerVersion)}, {Name: "Client", Value: Version}, } + if st.ActingAs != "" { + fields = slices.Insert(fields, 2, output.Field{Name: "Acting as", Value: st.ActingAs}) + } if st.Error != "" { fields = append(fields, output.Field{Name: "Problem", Value: st.Error}) } @@ -143,6 +149,7 @@ type statusResult struct { Endpoint string `json:"endpoint"` User string `json:"user,omitempty"` DisplayName string `json:"display_name,omitempty"` + ActingAs string `json:"acting_as,omitempty"` Provider string `json:"provider,omitempty"` Expires string `json:"expires,omitempty"` Available []string `json:"available_methods,omitempty"` @@ -168,16 +175,41 @@ func newWhoamiCmd(app *App) *cobra.Command { if err != nil { return err } - return app.out.Object(me, - output.Field{Name: "Username", Value: me.Username}, - output.Field{Name: "Display name", Value: me.DisplayName}, - output.Field{Name: "Mail", Value: me.Mail}, - output.Field{Name: "ID", Value: me.ID}, - ) + res := whoamiResult{User: *me} + fields := []output.Field{ + {Name: "Username", Value: me.Username}, + {Name: "Display name", Value: me.DisplayName}, + {Name: "Mail", Value: me.Mail}, + {Name: "ID", Value: me.ID}, + } + + // A server without admin features is not an error here: whoami + // answers who you are, and whether you are an admin is only known + // when the server can say. + if admin, err := app.client.IsAdmin(ctx); err == nil { + res.Admin = &admin + fields = append(fields, output.Field{Name: "Admin", Value: yesNo(admin)}) + } + + if app.flags.as != "" { + signedIn, err := app.signedIn.Me(ctx) + if err != nil { + return err + } + res.ImpersonatedBy = signedIn.Username + fields = append(fields, output.Field{Name: "Impersonated by", Value: signedIn.Username}) + } + return app.out.Object(res, fields...) }, } } +type whoamiResult struct { + client.User + Admin *bool `json:"admin,omitempty"` + ImpersonatedBy string `json:"impersonated_by,omitempty"` +} + func expiryString(tok *auth.Token) string { if tok == nil || tok.Expiry.IsZero() { return "" diff --git a/internal/cli/command_test.go b/internal/cli/command_test.go index 0206871..25157f9 100644 --- a/internal/cli/command_test.go +++ b/internal/cli/command_test.go @@ -143,6 +143,16 @@ type testBox struct { // assumed: deleting a twice-written probe from EOS left two entries, so // anything that clears up after itself has to clear all of them. trashOnDelete bool + + // admin makes the signed-in user an admin, who may impersonate others. An + // impersonated user's token is "as-", and the box answers /me for it + // as that user. + admin bool + // impersonations records each user an impersonation was asked for. + impersonations []string + // auths records the credential each request carried, keyed by method and + // path, so a test can check whose identity a request was made with. + auths map[string]string } // trashFakeLayout is the layout ocdav parses the trash range with. A value it @@ -227,6 +237,10 @@ func (b *testBox) route(w http.ResponseWriter, r *http.Request) { } b.requests = append(b.requests, r.Method+" "+r.URL.Path) + if b.auths == nil { + b.auths = map[string]string{} + } + b.auths[r.Method+" "+r.URL.Path] = r.Header.Get("Authorization") switch { case strings.HasPrefix(r.URL.Path, "/ocs/v1.php/cloud/capabilities"): @@ -271,6 +285,26 @@ func (b *testBox) route(w http.ResponseWriter, r *http.Request) { `"allow_creation":true,`+ `"app_providers":[{"name":"Collabora"},{"name":"OnlyOffice"}]}]}`) + case r.URL.Path == "/admin/status": + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"admin":%t}`, b.admin && r.Header.Get("Authorization") == "Bearer test-token") + + case r.URL.Path == "/admin/impersonate": + w.Header().Set("Content-Type", "application/json") + if !b.admin { + w.WriteHeader(http.StatusForbidden) + fmt.Fprint(w, `{"message":"You are not an administrator of this server."}`) + return + } + var req struct{ User string } + _ = json.NewDecoder(r.Body).Decode(&req) + b.impersonations = append(b.impersonations, req.User) + fmt.Fprintf(w, `{"token":%q}`, "as-"+req.User) + + case r.URL.Path == "/graph/v1.0/me" && strings.HasPrefix(r.Header.Get("Authorization"), "Bearer as-"): + user := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer as-") + fmt.Fprintf(w, `{"id":%q,"displayName":%q,"onPremisesSamAccountName":%q}`, user, user, user) + case r.URL.Path == "/graph/v1.0/me": fmt.Fprint(w, `{"id":"u1","displayName":"Albert Einstein","mail":"einstein@cern.ch","onPremisesSamAccountName":"einstein"}`) diff --git a/internal/cli/root.go b/internal/cli/root.go index c3269ab..4e7d47d 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -43,6 +43,8 @@ type globalFlags struct { appTokenFile string kerberosSPN string + as string + insecure bool skipVerify bool timeout time.Duration @@ -64,6 +66,9 @@ type App struct { client *client.Client chain *auth.Chain + // signedIn is the client for the signed-in user. It is client itself + // unless --as is acting as someone else. + signedIn *client.Client // completing records that this process was started by a shell to complete a // command line rather than to run one. It changes what the CLI is allowed to @@ -154,6 +159,8 @@ func newRootCmd(app *App) *cobra.Command { pf.StringVar(&f.kerberosSPN, "kerberos-spn", "", "Kerberos service name to ask for, if it differs from the server host") + pf.StringVar(&f.as, "as", "", "act as this user (admins only)") + pf.BoolVar(&f.insecure, "insecure", false, "allow plain HTTP (development instances only)") pf.BoolVar(&f.skipVerify, "skip-verify", false, "do not verify the server certificate (development instances only)") pf.DurationVar(&f.timeout, "timeout", 0, "overall timeout for the command, 0 for none") @@ -312,8 +319,27 @@ func (a *App) connect() error { if err != nil { return err } - a.client = c - return nil + a.client, a.signedIn = c, c + // A completion does not act as anyone: every impersonation is recorded in + // the server's audit log, and a TAB press is not worth a line there. + if f.as == "" || a.completing { + return nil + } + + // --as swaps the credential for one acting as that user, obtained from the + // server with the signed-in user's own. Everything the CLI then does — the + // paths it builds, the shares it lists — follows from whom the server says + // the token belongs to, so no command needs to know. + tok, err := c.Impersonate(context.Background(), f.as) + if err != nil { + return err + } + a.out.Msg("Acting as %s.", f.as) + acting := client.CredentialFunc(func(context.Context) (client.Credential, error) { + return client.Credential{Header: "Authorization", Value: "Bearer " + tok}, nil + }) + a.client, err = client.New(cfg.Endpoint, append(opts, client.WithCredentials(acting))...) + return err } // warnInsecure refuses to disable certificate checks against a CERN host. A diff --git a/pkg/client/admin.go b/pkg/client/admin.go new file mode 100644 index 0000000..3cddd0f --- /dev/null +++ b/pkg/client/admin.go @@ -0,0 +1,100 @@ +package client + +import ( + "context" + "encoding/json" + "net/http" + "strings" + + "github.com/cernbox/cernbox-cli/pkg/cberr" +) + +const ( + adminStatusPath = "/admin/status" + adminImpersonatePath = "/admin/impersonate" +) + +type adminStatusResponse struct { + Admin bool `json:"admin"` +} + +// IsAdmin reports whether the signed-in user is an admin of the server, that +// is, whether they may impersonate other users. Asking does not elevate and +// leaves no audit trail. +func (c *Client) IsAdmin(ctx context.Context) (bool, error) { + var res adminStatusResponse + err := c.adminCall(ctx, request{ + method: http.MethodGet, + url: c.URL(adminStatusPath), + op: "check admin status", + }, &res) + return res.Admin, err +} + +type impersonateRequest struct { + User string `json:"user"` +} + +type impersonateResponse struct { + Token string `json:"token"` +} + +// Impersonate obtains a token acting as user. It is an ordinary user token: the +// server treats every request made with it as made by that user. The server +// checks that the signed-in user is an admin and records the impersonation in +// its audit log. +func (c *Client) Impersonate(ctx context.Context, user string) (string, error) { + const op = "act as" + payload, err := json.Marshal(impersonateRequest{User: user}) + if err != nil { + return "", cberr.Wrap(cberr.KindOther, op, user, err) + } + var res impersonateResponse + err = c.adminCall(ctx, request{ + method: http.MethodPost, + url: c.URL(adminImpersonatePath), + header: http.Header{"Content-Type": []string{"application/json"}}, + body: bytesBody(payload), + op: op, + path: user, + }, &res) + if err != nil { + return "", err + } + if res.Token == "" { + return "", cberr.New(cberr.KindOther, op, user, "the server returned no token") + } + return res.Token, nil +} + +// adminCall makes a request to the admin endpoints and decodes the answer. +// +// It tells apart the two 404s those endpoints can produce. The admin service +// answers in JSON, so a JSON 404 is its own — "no such user" — while any other +// 404 means the server has no admin service at all. The generic "no such file +// or directory" would be wrong for both. +func (c *Client) adminCall(ctx context.Context, r request, out any) error { + r.header = cloneHeader(r.header) + r.header.Set("Accept", "application/json") + r.expects = []int{http.StatusOK, http.StatusNotFound} + resp, err := c.do(ctx, r) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNotFound { + if strings.HasPrefix(resp.Header.Get("Content-Type"), "application/json") { + return cberr.New(cberr.KindNotFound, r.op, r.path, "no such user") + } + return cberr.New(cberr.KindOther, r.op, r.path, "this server does not offer admin features") + } + return decodeJSON(resp.Body, out, r.op, r.path) +} + +func cloneHeader(h http.Header) http.Header { + if h == nil { + return http.Header{} + } + return h.Clone() +} diff --git a/pkg/client/admin_test.go b/pkg/client/admin_test.go new file mode 100644 index 0000000..64bc181 --- /dev/null +++ b/pkg/client/admin_test.go @@ -0,0 +1,100 @@ +package client + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "testing" + + "github.com/cernbox/cernbox-cli/pkg/cberr" +) + +func adminJSON(w http.ResponseWriter, status int, body string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + fmt.Fprint(w, body) +} + +func TestIsAdmin(t *testing.T) { + for _, want := range []bool{true, false} { + f := newFakeServer(t) + f.on(http.MethodGet, adminStatusPath, func(w http.ResponseWriter, r *http.Request) { + adminJSON(w, http.StatusOK, fmt.Sprintf(`{"admin":%v}`, want)) + }) + got, err := f.client().IsAdmin(context.Background()) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Errorf("IsAdmin = %v, want %v", got, want) + } + } +} + +func TestImpersonate(t *testing.T) { + f := newFakeServer(t) + f.on(http.MethodPost, adminImpersonatePath, func(w http.ResponseWriter, r *http.Request) { + adminJSON(w, http.StatusOK, `{"token":"marie-token"}`) + }) + + tok, err := f.client().Impersonate(context.Background(), "marie") + if err != nil { + t.Fatal(err) + } + if tok != "marie-token" { + t.Errorf("token = %q", tok) + } + + req := f.lastRequest(http.MethodPost) + var body impersonateRequest + if err := json.Unmarshal([]byte(req.Body), &body); err != nil { + t.Fatalf("request body %q: %v", req.Body, err) + } + if body.User != "marie" { + t.Errorf("request body = %+v", body) + } + // The request is the admin's own: it carries the signed-in credential. + if got := req.Header.Get("Authorization"); got != "Bearer test-token" { + t.Errorf("Authorization = %q", got) + } +} + +// TestImpersonateErrors pins the exit codes scripts branch on, and the two +// meanings of a 404. +func TestImpersonateErrors(t *testing.T) { + cases := []struct { + name string + handler http.HandlerFunc + kind cberr.Kind + msg string + }{ + {"not an admin", func(w http.ResponseWriter, r *http.Request) { + adminJSON(w, http.StatusForbidden, `{"message":"You are not an administrator of this server."}`) + }, cberr.KindPermission, "You are not an administrator of this server."}, + {"not identified", func(w http.ResponseWriter, r *http.Request) { + adminJSON(w, http.StatusUnauthorized, `{"message":"The server could not identify you."}`) + }, cberr.KindAuth, "The server could not identify you."}, + {"no such user", func(w http.ResponseWriter, r *http.Request) { + adminJSON(w, http.StatusNotFound, `{"message":"No such user."}`) + }, cberr.KindNotFound, "no such user"}, + {"no admin service", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "404 page not found", http.StatusNotFound) + }, cberr.KindOther, "this server does not offer admin features"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := newFakeServer(t) + f.on(http.MethodPost, adminImpersonatePath, tc.handler) + _, err := f.client().Impersonate(context.Background(), "marie") + ce, ok := errors.AsType[*cberr.Error](err) + if !ok { + t.Fatalf("err = %v, want a *cberr.Error", err) + } + if ce.Kind != tc.kind || ce.Msg != tc.msg { + t.Errorf("got kind %v msg %q, want kind %v msg %q", ce.Kind, ce.Msg, tc.kind, tc.msg) + } + }) + } +} From 8ed0120bc1a5c05be7a6965631e6abb074330d67 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Fri, 2 Oct 2026 15:19:21 +0200 Subject: [PATCH 2/3] Name the user a path is for with USER@cb:, and copy between users A path can now carry the identity it is reached as: marie@cb:~/Documents, marie@cb:project/cernbox:data, marie@cb:/eos/user/m/marie/x. It is scp's user@host:path, so it reads as what it is, and pkg/pathspec is still the only place that decides it. A command acts as one user. A USER@cb: path makes the whole command act as USER, as --as does, and paths for two different users are refused before anyone is impersonated. cp and sync are the exception, because their two sides may belong to two people: each side keeps its own identity. When the two differ the server cannot copy, since a COPY carries one credential, so the bytes are relayed through the client, read as one user and written as the other, without touching local disk. copy and paste refuse such a path: the clipboard is the signed-in user's. Each user named is impersonated once per command, and naming yourself is not an impersonation at all. Acting as someone is transparent: nothing in the output says it happened, and whoami answers as the user acted as. reva's audit log is the record. In cp and sync only the cb: marker makes a path remote now. A bare alias such as home:x is a local name there, so a name with a colon in it can no longer turn into a remote path; cb:~/x is the home space, and cb: on its own is the home space too. --- README.md | 24 ++++- docs/design.md | 21 +++- integration/admin_test.go | 56 ++++++++-- internal/cli/admin_test.go | 136 ++++++++++++++++++++++-- internal/cli/auth_cmd.go | 18 +--- internal/cli/command_test.go | 14 ++- internal/cli/completion.go | 13 +-- internal/cli/edit_cmd.go | 2 +- internal/cli/root.go | 151 ++++++++++++++++++++++----- internal/cli/sync_cmd.go | 6 +- internal/cli/transfer_cmd.go | 69 +++++++++--- pkg/pathspec/pathspec.go | 190 +++++++++++++++++++++------------- pkg/pathspec/pathspec_test.go | 93 +++++++++++++---- pkg/transfer/relay.go | 136 ++++++++++++++++++++++++ 14 files changed, 751 insertions(+), 178 deletions(-) create mode 100644 pkg/transfer/relay.go diff --git a/README.md b/README.md index ed67fd3..3993879 100644 --- a/README.md +++ b/README.md @@ -58,16 +58,32 @@ Paths look like the ones you already use: ```bash cernbox ls /eos/user/g/gdelmont/Documents cernbox ls /eos/project/c/cernbox/data -cernbox ls home:Documents # your own space, by name +cernbox ls Documents # relative to your home +cernbox ls project/cernbox:data # a space, by its alias ``` -Commands that copy between your computer and CERNBox are the exception: there you mark the CERNBox side with `cb:`, because the same path can exist on both. +`cp` and `sync` are the exception: there you mark the CERNBox side with `cb:`, because the same path can exist on both. After the marker, `~` is your home. ```bash -cernbox cp ./report.pdf cb:/eos/user/g/gdelmont/Documents/ -cernbox cp cb:/eos/user/g/gdelmont/Documents/report.pdf . +cernbox cp ./report.pdf cb:~/Documents/ +cernbox cp cb:~/Documents/report.pdf . +cernbox cp -r cb:/eos/project/c/cernbox/data ./data ``` +### Acting as another user + +An admin of the CERNBox deployment can do anything another user can, as them — look at their shares, put a file where they will find it. `--as` acts as them for the whole command; `USER@cb:` in front of a path reaches that one path as them, the way `scp` names a user and a host: + +```bash +cernbox --as marie share list # marie's shares, as she sees them +cernbox ls marie@cb:~/Documents # same as --as marie ls Documents +cernbox cp ./fix.txt marie@cb:~/ # into marie's home, written as marie +cernbox cp cb:~/report.pdf marie@cb:~/Documents/ # from your space to hers +cernbox ls marie@cb:project/cernbox:data # a project, as marie sees it +``` + +A command acts as one user, except `cp` and `sync`, whose two sides may differ. A copy between two users goes through your computer, because the server cannot copy across them: the bytes are read as one and written as the other. `cernbox whoami` says whether you are an admin. Every command run as someone else is recorded in the server's audit log under your name. + ## Browsing ```bash diff --git a/docs/design.md b/docs/design.md index d06a3cc..7a8c62b 100644 --- a/docs/design.md +++ b/docs/design.md @@ -176,7 +176,7 @@ An admin of the deployment — a member of the Admin API's `admin_group` — can The token comes from `POST /admin/impersonate` with the admin's own credential. The server steps the caller up and impersonates in one request, so the admin token never reaches the client. Both steps are in reva's audit log. The impersonation token is never cached: every command asks again, so each has its own audit entry, and no token for someone else's account is left in `/tmp`. It lives as long as a token from signing in (the Admin API's `impersonation_ttl`, or the token manager's own lifetime, a day by default), so a long transfer made as the user finishes; the short `admin_ttl` applies only to the admin token, which stays on the server. -`whoami` shows whether the user is an admin, from `GET /admin/status`, which only checks and is not audited. With `--as` it shows the impersonated user and who is impersonating them. A completion never impersonates. +`whoami` shows whether the user is an admin, from `GET /admin/status`, which only checks and is not audited. Acting as someone is transparent: with `--as`, `whoami` answers as that user and nothing in the output says who is behind it; the record of that is reva's audit log. A completion never impersonates. ## 4. Path and namespace model @@ -194,6 +194,7 @@ Space-qualified aliases are accepted and resolve through the locally cached `me/ home:Documents project/cernbox:data :relative/path # for scripts holding an ID +Documents # a bare relative path is in the home space ``` Resolution strategy: path-addressed operations go to `/remote.php/dav/files/{user}/{path}`, which accepts absolute CS3 paths at CERN directly. Id-addressed operations — anything where the CLI already holds a drive ID from ocgraph, which is all of sharing — go to `/remote.php/dav/spaces/{space-id}/{rel}`. The spaces listing is cached locally with a short TTL and refreshed on a resolution miss. @@ -203,15 +204,29 @@ Resolution strategy: path-addressed operations go to `/remote.php/dav/files/{use On lxplus, `/eos/user/g/gdelmont` is *both* a valid CERNBox remote path and a real local FUSE mount point. `cernbox cp /eos/user/g/gdelmont/a.txt /eos/user/g/gdelmont/b.txt` is genuinely ambiguous, and guessing would be worse than either answer. So: - **Namespace commands** — `ls`, `stat`, `find`, `du`, `mkdir`, `rm`, `mv`, `touch`, `cat`, `share`, `link`, `trash`, `versions` — take bare remote paths. There is no local side, so there is no ambiguity. -- **Transfer commands** — `cp`, `sync` — require the remote side to carry a `cb:` prefix: +- **Transfer commands** — `cp`, `sync` — take a path as remote only when it carries the `cb:` marker. Anything else is local, a bare alias such as `home:x` included: an alias is not a marker, and a local name with a colon in it must not turn into a remote path. After the marker, `~` is the home space, which spares `cb:home:`: ```bash - cernbox cp ./report.pdf cb:/eos/user/g/gdelmont/Documents/ + cernbox cp ./report.pdf cb:~/Documents/ cernbox cp -r cb:/eos/project/c/cernbox/data ./data ``` + `~` only works after the marker. Written first, the shell expands it to the local home before the CLI sees it. + `cb:` is accepted everywhere a remote path is, so a script can be explicit throughout if it prefers. +### Whose view a path is in + +The marker can name a user: `marie@cb:~/Documents`, `marie@cb:project/cernbox:data`, `marie@cb:/eos/user/m/marie/x`. The path is then marie's view of CERNBox — her home, the spaces she sees — and reaching it means acting as her (§3.6), which only an admin can. It is the shape of scp's `user@host:path`, so it reads as what it is, and it is the only place an identity is written into a path; `pkg/pathspec` carries it on the parsed spec and nothing else interprets it. + +A command acts as one user. A `USER@cb:` path makes the whole command act as USER, as `--as` would, and two paths naming different users are refused. `cp` and `sync` are the exception, because their two sides are two places that may belong to two people: each side is acted on as its own user. When the two differ, the server cannot copy — a `COPY` carries one credential — so the bytes are relayed through the client, read as one user and written as the other, file by file, never touching local disk. `copy` and `paste` refuse a user's path, because the clipboard belongs to the signed-in user and a path in someone else's view would quietly make it theirs. + +Naming yourself is not an impersonation: `gdelmont@cb:` from gdelmont is just `cb:`, and leaves nothing in the audit log. + +### A local mount as a faster way to the same place + +Not built, but kept possible. On lxplus a `cb:` path is often also mounted locally. `cb:` says *what* an argument is — a CERNBox resource — and the same command means the same thing on every machine. *How* it is reached is the transfer engine's business: having resolved the spec, it could notice that the path is mounted and use the mount instead of HTTPS. That only holds when acting as yourself, the local Unix user is the CERNBox user, and the mount is the same storage; any `USER@cb:` path goes through the server. Nothing in the path grammar needs to change for it. + ## 5. Command surface ``` diff --git a/integration/admin_test.go b/integration/admin_test.go index a82efed..d7bf1ca 100644 --- a/integration/admin_test.go +++ b/integration/admin_test.go @@ -35,13 +35,12 @@ func TestAsShowsWhoActs(t *testing.T) { e := setup(t) var me struct { - Username string `json:"username"` - Admin *bool `json:"admin"` - ImpersonatedBy string `json:"impersonated_by"` + Username string `json:"username"` + Admin *bool `json:"admin"` } e.runJSON(&me, "--as", otherUser, "whoami") - if me.Username != otherUser || me.ImpersonatedBy != username { - t.Errorf("whoami --as = %+v, want %s impersonated by %s", me, otherUser, username) + if me.Username != otherUser { + t.Errorf("whoami --as = %+v, want %s", me, otherUser) } if me.Admin == nil || *me.Admin { t.Errorf("admin = %v: whoami --as reports the impersonated user, who is not one", me.Admin) @@ -74,11 +73,11 @@ func TestAsWritesIntoAnotherUsersHome(t *testing.T) { dir := e.recipientDir() local := e.writeLocal("fix.txt", []byte("from the admin")) - if _, _, code := e.run("put", local, "cb:"+dir+"/"); code != 4 { + if _, _, code := e.run("cp", local, "cb:"+dir+"/"); code != 4 { t.Fatalf("put into %s without --as exited %d, want 4: the test proves nothing", dir, code) } - e.mustRun("--as", otherUser, "put", local, "cb:"+dir+"/") + e.mustRun("--as", otherUser, "cp", local, "cb:"+dir+"/") if got := e.mustRunAs(e.other(), "cat", dir+"/fix.txt"); got != "from the admin" { t.Errorf("%s reads %q", otherUser, got) } @@ -95,3 +94,46 @@ func TestAsRefusedForNonAdmin(t *testing.T) { t.Errorf("stderr does not say why: %s", stderr) } } + +// TestIdentityMarkerReachesAnotherUsersHome reads marie's home through +// "marie@cb:~", which the admin's own identity could not list. +func TestIdentityMarkerReachesAnotherUsersHome(t *testing.T) { + e := setup(t) + dir := e.recipientDir() + e.mustRunAs(e.other(), "cp", e.writeLocal("hers.txt", []byte("marie's")), "cb:"+dir+"/") + rel := strings.TrimPrefix(dir, otherHomeRoot+"/") + + if _, _, code := e.run("ls", dir); code != 4 { + t.Fatalf("listing %s as %s exited %d, want 4: the test proves nothing", dir, username, code) + } + out := e.mustRun("ls", otherUser+"@cb:~/"+rel) + if !strings.Contains(out, "hers.txt") { + t.Errorf("ls %s@cb:~/%s = %q, want hers.txt", otherUser, rel, out) + } +} + +// TestCpBetweenUsers copies from the admin's own space into marie's: read as +// one user, written as the other, which no server-side copy can do. +func TestCpBetweenUsers(t *testing.T) { + e := setup(t) + e.mustRun("cp", e.writeLocal("report.txt", []byte("for marie")), "cb:"+e.remotePath("report.txt")) + dir := e.recipientDir() + + e.mustRun("cp", "--verify", "cb:"+e.remotePath("report.txt"), otherUser+"@cb:"+dir+"/") + if got := e.mustRunAs(e.other(), "cat", dir+"/report.txt"); got != "for marie" { + t.Errorf("%s reads %q", otherUser, got) + } +} + +func TestCpBetweenUsersCopiesATree(t *testing.T) { + e := setup(t) + e.mustRun("mkdir", "-p", e.remotePath("tree/sub")) + e.mustRun("cp", e.writeLocal("a.txt", []byte("alpha")), "cb:"+e.remotePath("tree/a.txt")) + e.mustRun("cp", e.writeLocal("b.txt", []byte("beta")), "cb:"+e.remotePath("tree/sub/b.txt")) + dir := e.recipientDir() + + e.mustRun("cp", "-r", "cb:"+e.remotePath("tree"), otherUser+"@cb:"+dir+"/") + if got := e.mustRunAs(e.other(), "cat", dir+"/tree/sub/b.txt"); got != "beta" { + t.Errorf("%s reads %q for the nested file", otherUser, got) + } +} diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index 6557064..e3d16e8 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -16,7 +16,7 @@ func TestAsActsAsUser(t *testing.T) { box := newTestBox(t) box.admin = true - _, stderr, err := run(t, box, "--as", "marie", "share", "list") + _, _, err := run(t, box, "--as", "marie", "share", "list") if err != nil { t.Fatal(err) } @@ -30,9 +30,6 @@ func TestAsActsAsUser(t *testing.T) { if got := box.auths[http.MethodPost+" /admin/impersonate"]; got != "Bearer test-token" { t.Errorf("impersonate was made with %q, want the admin's token", got) } - if !strings.Contains(stderr, "Acting as marie.") { - t.Errorf("stderr does not say whom the command acts as: %q", stderr) - } } func TestAsDeniedForNonAdmin(t *testing.T) { @@ -68,9 +65,6 @@ func TestWhoamiShowsAdmin(t *testing.T) { if got.Username != "einstein" || got.Admin == nil || *got.Admin != admin { t.Errorf("whoami = %+v, want einstein with admin %v", got, admin) } - if got.ImpersonatedBy != "" { - t.Errorf("impersonated_by = %q without --as", got.ImpersonatedBy) - } } } @@ -82,9 +76,135 @@ func TestWhoamiAs(t *testing.T) { if err != nil { t.Fatal(err) } - for _, want := range []string{"marie", "Admin", "no", "Impersonated by", "einstein"} { + // Acting as someone is transparent: whoami answers as them, and says + // nothing about who is behind it. + for _, want := range []string{"marie", "Admin", "no"} { if !strings.Contains(stdout, want) { t.Errorf("whoami output lacks %q:\n%s", want, stdout) } } + if strings.Contains(stdout, "einstein") { + t.Errorf("whoami --as mentions the signed-in user:\n%s", stdout) + } +} + +// TestIdentityMarkerActsAsUser checks that USER@cb: on a path makes the command +// act as that user, the way --as does. +func TestIdentityMarkerActsAsUser(t *testing.T) { + box := newTestBox(t) + box.admin = true + box.putFile("/eos/user/e/einstein/shared/a.txt", "alpha") + + if _, _, err := run(t, box, "stat", "marie@cb:/eos/user/e/einstein/shared/a.txt"); err != nil { + t.Fatal(err) + } + if !slices.Equal(box.impersonations, []string{"marie"}) { + t.Errorf("impersonations = %v, want one, for marie", box.impersonations) + } + key := "PROPFIND /remote.php/dav/files/marie/eos/user/e/einstein/shared/a.txt" + if got := box.auths[key]; got != "Bearer as-marie" { + t.Errorf("%s was made with %q, want marie's token", key, got) + } +} + +// TestNamingYourselfIsNotAnImpersonation keeps the audit log free of entries +// for a path the signed-in user wrote their own name on. +func TestNamingYourselfIsNotAnImpersonation(t *testing.T) { + box := newTestBox(t) + box.admin = true + box.putFile("/eos/user/e/einstein/a.txt", "alpha") + + if _, _, err := run(t, box, "stat", "einstein@cb:~/a.txt"); err != nil { + t.Fatal(err) + } + if len(box.impersonations) != 0 { + t.Errorf("impersonated for the signed-in user: %v", box.impersonations) + } +} + +// TestOneUserPerCommand refuses paths for two different users outside cp and +// sync, before anyone is impersonated. +func TestOneUserPerCommand(t *testing.T) { + for _, args := range [][]string{ + {"mv", "marie@cb:~/a", "richard@cb:~/b"}, + {"--as", "marie", "ls", "richard@cb:~"}, + {"copy", "marie@cb:~/a.txt"}, + {"paste", "marie@cb:~/"}, + } { + box := newTestBox(t) + box.admin = true + _, _, err := run(t, box, args...) + if cberr.ExitCode(err) != cberr.ExitUsage { + t.Errorf("%v: err = %v (exit %d), want a usage error", args, err, cberr.ExitCode(err)) + } + if len(box.impersonations) != 0 { + t.Errorf("%v: impersonated before refusing: %v", args, box.impersonations) + } + } +} + +// TestCpBetweenUsersRelays checks that a copy whose two sides are acted on as +// different users reads as one and writes as the other, since no server-side +// COPY can carry both. +func TestCpBetweenUsersRelays(t *testing.T) { + box := newTestBox(t) + box.admin = true + box.putFile("/eos/user/e/einstein/a.txt", "alpha") + box.mkdir("/eos/user/e/einstein/shared") + + if _, _, err := run(t, box, "cp", "cb:~/a.txt", "marie@cb:/eos/user/e/einstein/shared/"); err != nil { + t.Fatal(err) + } + if got := box.files["/eos/user/e/einstein/shared/a.txt"]; got != "alpha" { + t.Errorf("copied = %q", got) + } + for k := range box.auths { + if strings.HasPrefix(k, "COPY ") { + t.Errorf("used a server-side copy across users: %s", k) + } + } + if got := box.auths["GET /remote.php/dav/files/einstein/eos/user/e/einstein/a.txt"]; got != "Bearer test-token" { + t.Errorf("read with %q, want the signed-in user's token", got) + } + if got := box.auths["PUT /remote.php/dav/files/marie/eos/user/e/einstein/shared/a.txt"]; got != "Bearer as-marie" { + t.Errorf("written with %q, want marie's token", got) + } +} + +// TestCpSameUserCopiesOnTheServer keeps a copy within one user's view a COPY, +// even when that user is named on both sides. +func TestCpSameUserCopiesOnTheServer(t *testing.T) { + box := newTestBox(t) + box.admin = true + box.putFile("/eos/user/e/einstein/a.txt", "alpha") + + if _, _, err := run(t, box, "cp", "marie@cb:/eos/user/e/einstein/a.txt", "marie@cb:/eos/user/e/einstein/b.txt"); err != nil { + t.Fatal(err) + } + if !slices.Equal(box.impersonations, []string{"marie"}) { + t.Errorf("impersonations = %v, want marie once", box.impersonations) + } + if got := box.auths["COPY /remote.php/dav/files/marie/eos/user/e/einstein/a.txt"]; got != "Bearer as-marie" { + t.Errorf("COPY made with %q, want a server-side copy as marie", got) + } +} + +func TestCpBetweenUsersRelaysATree(t *testing.T) { + box := newTestBox(t) + box.admin = true + box.putFile("/eos/user/e/einstein/data/a.txt", "alpha") + box.putFile("/eos/user/e/einstein/data/sub/b.txt", "beta") + box.mkdir("/eos/user/e/einstein/shared") + + if _, _, err := run(t, box, "cp", "-r", "--verify", "cb:~/data", "marie@cb:/eos/user/e/einstein/shared/"); err != nil { + t.Fatal(err) + } + for p, want := range map[string]string{ + "/eos/user/e/einstein/shared/data/a.txt": "alpha", + "/eos/user/e/einstein/shared/data/sub/b.txt": "beta", + } { + if got := box.files[p]; got != want { + t.Errorf("%s = %q, want %q", p, got, want) + } + } } diff --git a/internal/cli/auth_cmd.go b/internal/cli/auth_cmd.go index 7bb25a8..e281069 100644 --- a/internal/cli/auth_cmd.go +++ b/internal/cli/auth_cmd.go @@ -2,7 +2,6 @@ package cli import ( "errors" - "slices" "time" "github.com/cernbox/cernbox-cli/pkg/auth" @@ -112,7 +111,6 @@ func newStatusCmd(app *App) *cobra.Command { st.User = me.Username st.DisplayName = me.DisplayName } - st.ActingAs = app.flags.as } else { st.Error = tokErr.Error() } @@ -134,9 +132,6 @@ func newStatusCmd(app *App) *cobra.Command { {Name: "Server", Value: orDash(st.ServerVersion)}, {Name: "Client", Value: Version}, } - if st.ActingAs != "" { - fields = slices.Insert(fields, 2, output.Field{Name: "Acting as", Value: st.ActingAs}) - } if st.Error != "" { fields = append(fields, output.Field{Name: "Problem", Value: st.Error}) } @@ -149,7 +144,6 @@ type statusResult struct { Endpoint string `json:"endpoint"` User string `json:"user,omitempty"` DisplayName string `json:"display_name,omitempty"` - ActingAs string `json:"acting_as,omitempty"` Provider string `json:"provider,omitempty"` Expires string `json:"expires,omitempty"` Available []string `json:"available_methods,omitempty"` @@ -190,15 +184,6 @@ func newWhoamiCmd(app *App) *cobra.Command { res.Admin = &admin fields = append(fields, output.Field{Name: "Admin", Value: yesNo(admin)}) } - - if app.flags.as != "" { - signedIn, err := app.signedIn.Me(ctx) - if err != nil { - return err - } - res.ImpersonatedBy = signedIn.Username - fields = append(fields, output.Field{Name: "Impersonated by", Value: signedIn.Username}) - } return app.out.Object(res, fields...) }, } @@ -206,8 +191,7 @@ func newWhoamiCmd(app *App) *cobra.Command { type whoamiResult struct { client.User - Admin *bool `json:"admin,omitempty"` - ImpersonatedBy string `json:"impersonated_by,omitempty"` + Admin *bool `json:"admin,omitempty"` } func expiryString(tok *auth.Token) string { diff --git a/internal/cli/command_test.go b/internal/cli/command_test.go index 25157f9..528ac32 100644 --- a/internal/cli/command_test.go +++ b/internal/cli/command_test.go @@ -241,6 +241,13 @@ func (b *testBox) route(w http.ResponseWriter, r *http.Request) { b.auths = map[string]string{} } b.auths[r.Method+" "+r.URL.Path] = r.Header.Get("Authorization") + // The box keeps one tree. A user acted as through an impersonation token sees + // it under their own DAV prefix, which is where the client addresses them. + if user, ok := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer as-"); ok { + if rest, ok := strings.CutPrefix(r.URL.Path, "/remote.php/dav/files/"+user); ok { + r.URL.Path = testDavPrefix + rest + } + } switch { case strings.HasPrefix(r.URL.Path, "/ocs/v1.php/cloud/capabilities"): @@ -672,12 +679,15 @@ func (b *testBox) serveDav(w http.ResponseWriter, r *http.Request) { // destPath extracts the path a MOVE or COPY targets from its Destination header. func destPath(r *http.Request) (string, bool) { + // Any user's prefix: an impersonated user addresses the same tree under + // their own name. dst := r.Header.Get("Destination") - _, after, ok := strings.Cut(dst, testDavPrefix) + _, after, ok := strings.Cut(dst, "/remote.php/dav/files/") if !ok { return "", false } - return path.Clean(after), true + _, rest, _ := strings.Cut(after, "/") + return path.Clean("/" + rest), true } // copyTree duplicates a file or a whole collection, reporting whether the source diff --git a/internal/cli/completion.go b/internal/cli/completion.go index 26edc2a..a9848ba 100644 --- a/internal/cli/completion.go +++ b/internal/cli/completion.go @@ -77,13 +77,9 @@ func (a *App) completeTransfer(_ *cobra.Command, _ []string, toComplete string) } // looksRemote reports whether a partially typed word has already been marked as -// a CERNBox path, by the cb: prefix or by a space alias. +// a CERNBox path. func looksRemote(arg string) bool { - if strings.HasPrefix(arg, pathspec.RemotePrefix) { - return true - } - spec, err := pathspec.ParseTransfer(arg) - return err == nil && spec.IsRemote() + return pathspec.IsMarkedRemote(arg) } // remoteCandidates lists the children of the directory the user is part way @@ -92,6 +88,11 @@ func (a *App) remoteCandidates(toComplete string) ([]string, cobra.ShellCompDire if !a.ready() { return nil, cobra.ShellCompDirectiveNoFileComp } + // Another user's view would take an impersonation, which a completion never + // does: each one is a line in the server's audit log. + if _, ok := pathspec.Identity(toComplete); ok { + return nil, cobra.ShellCompDirectiveNoFileComp + } ctx, cancel := completionContext() defer cancel() diff --git a/internal/cli/edit_cmd.go b/internal/cli/edit_cmd.go index f6a6528..033d77e 100644 --- a/internal/cli/edit_cmd.go +++ b/internal/cli/edit_cmd.go @@ -162,7 +162,7 @@ func (a *App) resolveEditTarget(ctx context.Context, arg, in string) (editTarget } switch { - case strings.HasPrefix(arg, pathspec.RemotePrefix): + case pathspec.IsMarkedRemote(arg): remote, err := a.resolve(ctx, arg) return editTarget{remote: remote}, err case isBareEditName(arg): diff --git a/internal/cli/root.go b/internal/cli/root.go index 4e7d47d..928c27d 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -67,8 +67,13 @@ type App struct { client *client.Client chain *auth.Chain // signedIn is the client for the signed-in user. It is client itself - // unless --as is acting as someone else. + // unless the command acts as someone else. signedIn *client.Client + // acting holds a client for each user the command has acted as, so that a + // user named twice is impersonated once. clientOpts and endpoint are what + // they are built from. + acting map[string]*client.Client + clientOpts []client.Option // completing records that this process was started by a shell to complete a // command line rather than to run one. It changes what the CLI is allowed to @@ -133,14 +138,16 @@ func newRootCmd(app *App) *cobra.Command { Use: "cernbox", Short: "Command-line client for CERNBox", Long: "Work with your CERNBox files from the command line.\n\n" + - "Paths look like /eos/user/g/gdelmont, or home:Documents to use a space\n" + - "alias. Commands that copy between your computer and CERNBox mark the\n" + - "CERNBox side with cb:, because a path like /eos/... can exist on your\n" + - "computer too.\n\n" + + "Paths look like /eos/user/g/gdelmont, Documents for a path in your home,\n" + + "or project/cernbox:data to use a space alias. cp and sync mark the CERNBox\n" + + "side with cb:, as in cb:~/Documents, because a path like /eos/... can exist\n" + + "on your computer too.\n\n" + + "An admin can act as another user with --as USER, or for one path by writing\n" + + "USER@cb: in front of it.\n\n" + "Run 'cernbox status' to see how you are signed in.", SilenceUsage: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - return app.setup(cmd) + return app.setup(cmd, args) }, } @@ -217,7 +224,7 @@ func newRootCmd(app *App) *cobra.Command { // setup builds the configuration, the output writer, the credential chain and // the client. It runs before every command. -func (a *App) setup(cmd *cobra.Command) error { +func (a *App) setup(cmd *cobra.Command, args []string) error { f := a.flags if a.stdout == nil { @@ -252,7 +259,47 @@ func (a *App) setup(cmd *cobra.Command) error { if noServerNeeded(cmd) { return nil } - return a.connect() + // The paths are checked first, so that a command refused for naming two + // users has impersonated neither. + user, err := identityFromArgs(cmd, args, f.as) + if err != nil { + return err + } + if err := a.connect(); err != nil { + return err + } + return a.actAs(user) +} + +// identityFromArgs works out whom a command acts as from the paths it was given: +// a "USER@cb:" path makes the whole command act as USER, the way --as does. +// +// cp and sync are the exception. They move data from one place to another, and +// the two places may belong to different users, so each path keeps its own +// identity and nothing here applies. copy and paste refuse one outright: the +// clipboard is the signed-in user's, and a path in someone else's view would +// silently make it theirs. +func identityFromArgs(cmd *cobra.Command, args []string, as string) (string, error) { + switch cmd.Name() { + case "cp", "sync": + return as, nil + } + for _, arg := range args { + user, ok := pathspec.Identity(arg) + if !ok { + continue + } + switch { + case cmd.Name() == "copy" || cmd.Name() == "paste": + return "", cberr.Usagef("%s works with your own clipboard, so %q cannot name another user: "+ + "run it with --as %s to use theirs", cmd.Name(), arg, user) + case as != "" && user != as: + return "", cberr.Usagef("%q is written for %s, but this command acts as %s: "+ + "one command acts as one user (only cp and sync mix them)", arg, user, as) + } + as = user + } + return as, nil } // connect builds the credential chain and the client from configuration and the @@ -320,26 +367,67 @@ func (a *App) connect() error { return err } a.client, a.signedIn = c, c - // A completion does not act as anyone: every impersonation is recorded in - // the server's audit log, and a TAB press is not worth a line there. - if f.as == "" || a.completing { + a.acting, a.clientOpts = map[string]*client.Client{}, opts + return a.actAs(f.as) +} + +// actAs makes user the identity the command runs as. Everything the CLI then +// does — the paths it builds, the shares it lists — follows from whom the server +// says the token belongs to, so no command needs to know. +// +// A completion does not act as anyone: every impersonation is recorded in the +// server's audit log, and a TAB press is not worth a line there. +func (a *App) actAs(user string) error { + if user == "" || a.completing { return nil } - - // --as swaps the credential for one acting as that user, obtained from the - // server with the signed-in user's own. Everything the CLI then does — the - // paths it builds, the shares it lists — follows from whom the server says - // the token belongs to, so no command needs to know. - tok, err := c.Impersonate(context.Background(), f.as) + c, err := a.clientAs(context.Background(), user) if err != nil { return err } - a.out.Msg("Acting as %s.", f.as) - acting := client.CredentialFunc(func(context.Context) (client.Credential, error) { + a.client = c + return nil +} + +// clientAs returns a client acting as user. Naming the signed-in user is not an +// impersonation: it returns their own client, and leaves nothing in the audit +// log. Anyone else's token comes from the server, asked for with the signed-in +// user's own credential, once per user per command. +func (a *App) clientAs(ctx context.Context, user string) (*client.Client, error) { + if c, ok := a.acting[user]; ok { + return c, nil + } + me, err := a.signedIn.Me(ctx) + if err != nil { + return nil, err + } + if me.Username == user { + a.acting[user] = a.signedIn + return a.signedIn, nil + } + + tok, err := a.signedIn.Impersonate(ctx, user) + if err != nil { + return nil, err + } + creds := client.CredentialFunc(func(context.Context) (client.Credential, error) { return client.Credential{Header: "Authorization", Value: "Bearer " + tok}, nil }) - a.client, err = client.New(cfg.Endpoint, append(opts, client.WithCredentials(acting))...) - return err + c, err := client.New(a.cfg.Endpoint, append(a.clientOpts, client.WithCredentials(creds))...) + if err != nil { + return nil, err + } + a.acting[user] = c + return c, nil +} + +// clientFor returns the client a remote spec is acted on with: the user it was +// written for, or the identity the command runs as. +func (a *App) clientFor(ctx context.Context, spec pathspec.Spec) (*client.Client, error) { + if spec.As == "" { + return a.client, nil + } + return a.clientAs(ctx, spec.As) } // warnInsecure refuses to disable certificate checks against a CERN host. A @@ -452,12 +540,27 @@ func (a *App) resolve(ctx context.Context, arg string) (string, error) { if err != nil { return "", cberr.Usagef("%v", err) } - return spec.Resolve(ctx, a.client) + return a.resolveSpec(ctx, spec) } -// resolveSpec turns a parsed spec into an absolute CERNBox path. +// resolveSpec turns a parsed spec into an absolute CERNBox path, resolving any +// space alias in the view of the user the spec is acted on as. func (a *App) resolveSpec(ctx context.Context, spec pathspec.Spec) (string, error) { - return spec.Resolve(ctx, a.client) + c, err := a.clientFor(ctx, spec) + if err != nil { + return "", err + } + return spec.Resolve(ctx, c) +} + +// transferEngineFor builds the transfer engine acting with c rather than the +// command's own client. +func (a *App) transferEngineFor(c *client.Client, o transferFlags) (*transfer.Engine, error) { + e, err := a.transferEngine(o) + if err != nil { + return nil, err + } + return transfer.New(c, e.Options()), nil } // transferEngineWith builds the transfer engine, reporting progress through pf diff --git a/internal/cli/sync_cmd.go b/internal/cli/sync_cmd.go index 6b96bef..1bbd735 100644 --- a/internal/cli/sync_cmd.go +++ b/internal/cli/sync_cmd.go @@ -59,7 +59,11 @@ func newSyncCmd(app *App) *cobra.Command { return err } - engine, err := app.transferEngine(*flags) + c, err := app.clientFor(ctx, remoteSpec) + if err != nil { + return err + } + engine, err := app.transferEngineFor(c, *flags) if err != nil { return err } diff --git a/internal/cli/transfer_cmd.go b/internal/cli/transfer_cmd.go index f61b585..8aad40c 100644 --- a/internal/cli/transfer_cmd.go +++ b/internal/cli/transfer_cmd.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/cernbox/cernbox-cli/pkg/cberr" + "github.com/cernbox/cernbox-cli/pkg/client" "github.com/cernbox/cernbox-cli/pkg/output" "github.com/cernbox/cernbox-cli/pkg/pathspec" "github.com/cernbox/cernbox-cli/pkg/transfer" @@ -43,17 +44,22 @@ func newCpCmd(app *App) *cobra.Command { cmd := &cobra.Command{ Use: "cp SOURCE DEST", Short: "Copy between your computer and CERNBox", - Long: "Copy files between your computer and CERNBox.\n" + + Long: "Copy files between your computer and CERNBox, or within CERNBox.\n" + "\n" + "Mark the CERNBox side with cb:, because a path like /eos/... can exist on your\n" + "computer too.\n" + "\n" + "Large uploads go up in chunks and an interrupted transfer carries on where it\n" + "stopped. A whole directory comes down as one archive when the server can\n" + - "build one, which is much faster for many small files.", - Example: " cernbox cp ./report.pdf cb:/eos/user/g/gdelmont/Documents/\n" + + "build one, which is much faster for many small files.\n" + + "\n" + + "An admin can write USER@cb: to reach a path as another user sees it. A copy\n" + + "between two users streams through this computer, since the server cannot\n" + + "copy across them itself.", + Example: " cernbox cp ./report.pdf cb:~/Documents/\n" + " cernbox cp -r cb:/eos/project/c/cernbox/data ./data\n" + - " cernbox cp cb:/eos/user/g/gdelmont/a.txt cb:/eos/user/g/gdelmont/b.txt", + " cernbox cp cb:~/a.txt cb:~/b.txt\n" + + " cernbox cp cb:~/report.pdf marie@cb:~/Documents/", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { ctx, cancel := app.ctx(cmd) @@ -66,7 +72,7 @@ func newCpCmd(app *App) *cobra.Command { switch { case src.IsRemote() && dst.IsRemote(): - return app.serverSideCopy(ctx, src, dst, flags) + return app.remoteCopy(ctx, src, dst, flags) case src.IsLocal(): return app.runUpload(ctx, cmd, src.Path, dst, flags) default: @@ -81,7 +87,11 @@ func newCpCmd(app *App) *cobra.Command { // runUpload copies a local path to a remote spec. func (a *App) runUpload(ctx context.Context, cmd *cobra.Command, local string, dst pathspec.Spec, flags *transferFlags) error { - engine, err := a.transferEngine(*flags) + c, err := a.clientFor(ctx, dst) + if err != nil { + return err + } + engine, err := a.transferEngineFor(c, *flags) if err != nil { return err } @@ -101,7 +111,7 @@ func (a *App) runUpload(ctx context.Context, cmd *cobra.Command, local string, d // A destination that is an existing directory, or was written with a // trailing slash, means "into it" — the same rule cp(1) uses. - if a.destIsDirectory(ctx, remote, dst) { + if destIsDirectory(ctx, c, remote, dst) { remote = path.Join(remote, filepath.Base(strings.TrimRight(local, string(os.PathSeparator)))) } @@ -114,7 +124,11 @@ func (a *App) runUpload(ctx context.Context, cmd *cobra.Command, local string, d // runDownload copies a remote spec to a local path. func (a *App) runDownload(ctx context.Context, cmd *cobra.Command, src pathspec.Spec, local string, flags *transferFlags) error { - engine, err := a.transferEngine(*flags) + c, err := a.clientFor(ctx, src) + if err != nil { + return err + } + engine, err := a.transferEngineFor(c, *flags) if err != nil { return err } @@ -124,7 +138,7 @@ func (a *App) runDownload(ctx context.Context, cmd *cobra.Command, src pathspec. return err } - info, err := a.client.Stat(ctx, remote) + info, err := c.Stat(ctx, remote) if err != nil { return err } @@ -144,9 +158,11 @@ func (a *App) runDownload(ctx context.Context, cmd *cobra.Command, src pathspec. return a.reportTransfer(stats, "Downloaded") } -// serverSideCopy duplicates within CERNBox without moving the bytes through -// the client. -func (a *App) serverSideCopy(ctx context.Context, src, dst pathspec.Spec, flags *transferFlags) error { +// remoteCopy duplicates within CERNBox. When both sides are acted on as the same +// user the server copies it without the bytes moving through the client. When +// they are not, it cannot — a COPY carries one credential — so the bytes are +// relayed: read as one user, written as the other. +func (a *App) remoteCopy(ctx context.Context, src, dst pathspec.Spec, flags *transferFlags) error { from, err := a.resolveSpec(ctx, src) if err != nil { return err @@ -155,15 +171,36 @@ func (a *App) serverSideCopy(ctx context.Context, src, dst pathspec.Spec, flags if err != nil { return err } + srcClient, err := a.clientFor(ctx, src) + if err != nil { + return err + } + dstClient, err := a.clientFor(ctx, dst) + if err != nil { + return err + } - if info, err := a.client.Stat(ctx, to); err == nil && info.IsDir { + if destIsDirectory(ctx, dstClient, to, dst) { to = path.Join(to, path.Base(from)) } + + if srcClient != dstClient { + engine, err := a.transferEngineFor(srcClient, *flags) + if err != nil { + return err + } + stats, err := engine.Relay(ctx, from, dstClient, to) + if err != nil { + return err + } + return a.reportTransfer(stats, "Copied") + } + if flags.dryRun { a.out.Msg("Would copy %s to %s", from, to) return nil } - if err := a.client.Copy(ctx, from, to, flags.force); err != nil { + if err := srcClient.Copy(ctx, from, to, flags.force); err != nil { return err } a.out.Msg("Copied %s to %s", from, to) @@ -172,11 +209,11 @@ func (a *App) serverSideCopy(ctx context.Context, src, dst pathspec.Spec, flags // destIsDirectory reports whether a remote destination should be treated as a // container rather than the target name. -func (a *App) destIsDirectory(ctx context.Context, remote string, spec pathspec.Spec) bool { +func destIsDirectory(ctx context.Context, c *client.Client, remote string, spec pathspec.Spec) bool { if spec.TrailingSlash { return true } - info, err := a.client.Stat(ctx, remote) + info, err := c.Stat(ctx, remote) return err == nil && info.IsDir } diff --git a/pkg/pathspec/pathspec.go b/pkg/pathspec/pathspec.go index 76baf7e..59874bc 100644 --- a/pkg/pathspec/pathspec.go +++ b/pkg/pathspec/pathspec.go @@ -9,11 +9,15 @@ // // - Namespace commands (ls, rm, share, ...) have no local side at all, so // every path they take is remote. Use ParseRemote. -// - Transfer commands (cp, sync) require the remote side to be marked with a -// "cb:" prefix or written as a space alias. Use ParseTransfer. +// - Transfer commands (cp, sync) take a path as remote only when it is marked +// with "cb:". Use ParseTransfer. // -// Nothing outside this package may infer local-versus-remote by any other -// means. +// The marker can also name a user, "marie@cb:", which makes the path that user's +// view of CERNBox: the CLI acts as them to reach it. That is the same shape as +// scp's user@host:path, and it is the only place an identity is written. +// +// Nothing outside this package may infer local-versus-remote, or whom a path +// is acted on as, by any other means. package pathspec import ( @@ -47,12 +51,20 @@ func (k Kind) String() string { const ( // RemotePrefix forces an argument to be interpreted as a CERNBox path. RemotePrefix = "cb:" - // LocalPrefix forces an argument to be interpreted as a local path. It - // exists so that a local directory whose name collides with a space alias - // is still addressable. + // LocalPrefix forces an argument to be interpreted as a local path. A + // transfer never needs it, since anything without cb: is local there; it is + // for edit, which looks for an unmarked path in CERNBox first. LocalPrefix = "file:" ) +// HomeShorthand stands for the home space after the marker: "cb:~/Documents". +// It only works there. Written first on the command line, the shell would expand +// it to the local home before the CLI saw it. +const HomeShorthand = "~" + +// userRe matches the username in an identity marker, "marie@cb:". +var userRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) + // HomeAlias is the space alias for the caller's own home space. A relative // remote path is resolved against it. const HomeAlias = "home" @@ -81,6 +93,11 @@ type Spec struct { // local path. Resolve turns it into an absolute path. Space string + // As is the user the path was written for, "marie" in "marie@cb:~/x", or + // empty for the identity the command runs as. The path, its space alias + // included, is resolved as that user sees CERNBox. + As string + // TrailingSlash records whether the argument ended in a separator. Transfer // commands use it the way cp does: "put a.txt cb:/eos/x/dir/" means "into // dir", whereas "put a.txt cb:/eos/x/dir" means "as dir". @@ -97,18 +114,29 @@ func (s Spec) IsRemote() bool { return s.Kind == Remote } // IsLocal reports whether s names a local file. func (s Spec) IsLocal() bool { return s.Kind == Local } -// String renders the spec back into the syntax a user would type. +// String renders the spec back into the syntax a user would type. A remote +// spec always carries the marker, so that what comes out parses back to the same +// thing in any command. func (s Spec) String() string { var b strings.Builder - switch { - case s.Kind == Local: - b.WriteString(s.Path) - case s.Space != "": - b.WriteString(s.Space) - b.WriteString(":") - b.WriteString(s.Path) - default: + if s.Kind == Local { b.WriteString(s.Path) + } else { + if s.As != "" { + b.WriteString(s.As + "@") + } + b.WriteString(RemotePrefix) + switch { + case s.Space == HomeAlias: + b.WriteString(HomeShorthand) + if s.Path != "" { + b.WriteString("/" + s.Path) + } + case s.Space != "": + b.WriteString(s.Space + ":" + s.Path) + default: + b.WriteString(s.Path) + } } if s.TrailingSlash && !strings.HasSuffix(b.String(), "/") { b.WriteString("/") @@ -146,31 +174,66 @@ func (s Spec) Resolve(ctx context.Context, r SpaceResolver) (string, error) { } // ParseRemote parses an argument for a command that only ever operates on -// CERNBox. Every form is accepted: an absolute path, a space alias, a "cb:" -// prefixed path, and a relative path, which is taken relative to the home -// space. +// CERNBox. The marker is optional: an absolute path, a space alias, "~/" for the +// home space, and a relative path, which is taken relative to the home space, +// are all remote. "marie@cb:" in front makes it marie's view. func ParseRemote(arg string) (Spec, error) { if arg == "" { return Spec{}, fmt.Errorf("empty path") } - raw := arg - if rest, ok := strings.CutPrefix(arg, LocalPrefix); ok { return Spec{}, fmt.Errorf("%q names a local path, but this command only works on CERNBox paths", rest) } - arg = strings.TrimPrefix(arg, RemotePrefix) + as, rest, _ := splitMarker(arg) + return parseLocation(rest, as, arg) +} + +// ParseTransfer parses an argument for a command that moves data between the +// local filesystem and CERNBox. The argument is remote only when it says so, +// with "cb:" or "USER@cb:". Everything else is local, including an absolute +// /eos path, which on lxplus is a real local mount. +func ParseTransfer(arg string) (Spec, error) { if arg == "" { - return Spec{}, fmt.Errorf("%q has no path after %q", raw, RemotePrefix) + return Spec{}, fmt.Errorf("empty path") + } + if rest, ok := strings.CutPrefix(arg, LocalPrefix); ok { + if rest == "" { + return Spec{}, fmt.Errorf("%q has no path after %q", arg, LocalPrefix) + } + return localSpec(rest, arg), nil + } + if as, rest, ok := splitMarker(arg); ok { + return parseLocation(rest, as, arg) } + return localSpec(arg, arg), nil +} +// parseLocation parses what follows the marker: a CERNBox location. An empty +// one is the home space, so that "cb:" and "marie@cb:" name a home the way "~" +// does in a shell. +func parseLocation(arg, as, raw string) (Spec, error) { trailing := hasTrailingSlash(arg) + remote := func(space, p string) Spec { + return Spec{Kind: Remote, Space: space, Path: p, As: as, TrailingSlash: trailing, Raw: raw} + } + + if arg == "" || arg == HomeShorthand { + return remote(HomeAlias, ""), nil + } + if rest, ok := strings.CutPrefix(arg, HomeShorthand+"/"); ok { + cleaned, err := cleanRel(rest) + if err != nil { + return Spec{}, fmt.Errorf("%q: %w", raw, err) + } + return remote(HomeAlias, cleaned), nil + } if alias, rest, ok := splitAlias(arg); ok { cleaned, err := cleanRel(rest) if err != nil { return Spec{}, fmt.Errorf("%q: %w", raw, err) } - return Spec{Kind: Remote, Space: alias, Path: cleaned, TrailingSlash: trailing, Raw: raw}, nil + return remote(alias, cleaned), nil } if strings.HasPrefix(arg, "/") { @@ -178,58 +241,43 @@ func ParseRemote(arg string) (Spec, error) { if err != nil { return Spec{}, fmt.Errorf("%q: %w", raw, err) } - return Spec{Kind: Remote, Path: cleaned, TrailingSlash: trailing, Raw: raw}, nil + return remote("", cleaned), nil } - // A bare relative path is relative to the caller's home space, so that + // A bare relative path is relative to the home space, so that // "cernbox ls Documents" does what it looks like it does. cleaned, err := cleanRel(arg) if err != nil { return Spec{}, fmt.Errorf("%q: %w", raw, err) } - return Spec{Kind: Remote, Space: HomeAlias, Path: cleaned, TrailingSlash: trailing, Raw: raw}, nil + return remote(HomeAlias, cleaned), nil } -// ParseTransfer parses an argument for a command that moves data between the -// local filesystem and CERNBox. The argument is remote only when it says so: -// either a "cb:" prefix or a space alias. Everything else is local, including -// an absolute /eos path, which on lxplus is a real local mount. -func ParseTransfer(arg string) (Spec, error) { - if arg == "" { - return Spec{}, fmt.Errorf("empty path") - } - raw := arg - trailing := hasTrailingSlash(arg) - - if rest, ok := strings.CutPrefix(arg, LocalPrefix); ok { - if rest == "" { - return Spec{}, fmt.Errorf("%q has no path after %q", raw, LocalPrefix) - } - return localSpec(rest, raw), nil - } - +// splitMarker recognises the remote marker at the start of arg, "cb:" or +// "USER@cb:", and returns the user it names, if any, and what follows it. An +// argument without the marker comes back whole, with ok false. +func splitMarker(arg string) (as, rest string, ok bool) { if rest, ok := strings.CutPrefix(arg, RemotePrefix); ok { - if rest == "" { - return Spec{}, fmt.Errorf("%q has no path after %q", raw, RemotePrefix) - } - s, err := ParseRemote(rest) - if err != nil { - return Spec{}, err - } - s.Raw = raw - s.TrailingSlash = trailing - return s, nil + return "", rest, true } - - if alias, rest, ok := splitAlias(arg); ok { - cleaned, err := cleanRel(rest) - if err != nil { - return Spec{}, fmt.Errorf("%q: %w", raw, err) - } - return Spec{Kind: Remote, Space: alias, Path: cleaned, TrailingSlash: trailing, Raw: raw}, nil + user, rest, found := strings.Cut(arg, "@"+RemotePrefix) + if !found || !userRe.MatchString(user) { + return "", arg, false } + return user, rest, true +} - return localSpec(arg, raw), nil +// IsMarkedRemote reports whether arg carries the remote marker, "cb:" or +// "USER@cb:". +func IsMarkedRemote(arg string) bool { + _, _, ok := splitMarker(arg) + return ok +} + +// Identity returns the user arg is written for, "marie" for "marie@cb:~/x". +func Identity(arg string) (string, bool) { + as, _, _ := splitMarker(arg) + return as, as != "" } // localSpec builds the spec for a local path. It is cleaned by the platform's @@ -267,19 +315,21 @@ func ParseTransferPair(src, dst string) (Spec, Spec, error) { // join a candidate name onto it and get something the user could have typed. // // "cb:/eos/user/g/gdelmont/Doc" -> "cb:/eos/user/g/gdelmont/", "Doc" -// "home:notes/dr" -> "home:notes/", "dr" +// "cb:~/notes/dr" -> "cb:~/notes/", "dr" +// "marie@cb:~/Do" -> "marie@cb:~/", "Do" // "home:" -> "home:", "" // "Doc" -> "", "Doc" // -// The directory part is not itself a valid argument in every case: "cb:" and "" -// name no path at all. Resolve those to the home space, which is what a bare -// relative path means. +// The directory part names the home space when it is empty or only the marker, +// which is what ParseRemote makes of it too. func SplitForCompletion(arg string) (dir, frag string) { prefix := "" - if rest, ok := strings.CutPrefix(arg, RemotePrefix); ok { - prefix, arg = RemotePrefix, rest + if _, rest, ok := splitMarker(arg); ok { + prefix, arg = arg[:len(arg)-len(rest)], rest } - if alias, rest, ok := splitAlias(arg); ok { + if rest, ok := strings.CutPrefix(arg, HomeShorthand+"/"); ok { + prefix, arg = prefix+HomeShorthand+"/", rest + } else if alias, rest, ok := splitAlias(arg); ok { prefix, arg = prefix+alias+":", rest } if i := strings.LastIndex(arg, "/"); i >= 0 { diff --git a/pkg/pathspec/pathspec_test.go b/pkg/pathspec/pathspec_test.go index 5eba5b4..241c46a 100644 --- a/pkg/pathspec/pathspec_test.go +++ b/pkg/pathspec/pathspec_test.go @@ -63,7 +63,6 @@ func TestParseRemoteErrors(t *testing.T) { wantSub string }{ {"empty", "", "empty path"}, - {"cb prefix with nothing after it", "cb:", "no path after"}, {"file prefix rejected", "file:./local.txt", "only works on CERNBox paths"}, {"escaping absolute path", "/eos/../../etc/passwd", "escapes the namespace root"}, {"escaping alias path", "home:../../etc/passwd", "escapes the space root"}, @@ -105,9 +104,15 @@ func TestParseTransferDisambiguation(t *testing.T) { {"relative local path", "./report.pdf", Local, "report.pdf", ""}, {"plain local path", "report.pdf", Local, "report.pdf", ""}, {"local absolute path", "/tmp/report.pdf", Local, local("/tmp/report.pdf"), ""}, - {"space alias is remote without cb", "home:Documents", Remote, "Documents", "home"}, - {"project alias is remote", "project/cernbox:data", Remote, "data", "project/cernbox"}, + // Only the marker makes a transfer path remote. An alias on its own is a + // local name with a colon in it. + {"bare space alias is local", "home:Documents", Local, local("home:Documents"), ""}, + {"marked space alias is remote", "cb:home:Documents", Remote, "Documents", "home"}, + {"marked project alias is remote", "cb:project/cernbox:data", Remote, "data", "project/cernbox"}, + {"home shorthand after the marker", "cb:~/Documents", Remote, "Documents", "home"}, + {"marker alone is the home space", "cb:", Remote, "", "home"}, {"file prefix forces local", "file:home:weird", Local, "home:weird", ""}, + {"scp-style host stays local", "gdelmont@lxplus:notes.txt", Local, local("gdelmont@lxplus:notes.txt"), ""}, {"windows drive letter stays local", `C:\Users\gdelmont\a.txt`, Local, `C:\Users\gdelmont\a.txt`, ""}, {"colon inside a relative file name stays local", "./weird:name.txt", Local, weird, ""}, } @@ -131,25 +136,75 @@ func TestParseTransferDisambiguation(t *testing.T) { } } -// TestParseTransferAliasAmbiguity pins a known, deliberate false positive: -// "data/archive:2024" is shaped exactly like a project space alias, so it is -// read as one. There is no way to tell the two apart from the string alone, -// which is why the file: prefix exists. -func TestParseTransferAliasAmbiguity(t *testing.T) { - got, err := ParseTransfer("data/archive:2024") - if err != nil { - t.Fatal(err) +// TestIdentityMarker covers "USER@cb:", which reads a path as another user +// sees CERNBox. The user is carried on the spec and nowhere else. +func TestIdentityMarker(t *testing.T) { + tests := []struct { + arg string + wantAs string + wantSpace string + wantPath string + }{ + {"marie@cb:~/Documents", "marie", "home", "Documents"}, + {"marie@cb:", "marie", "home", ""}, + {"marie@cb:project/cernbox:data", "marie", "project/cernbox", "data"}, + {"marie@cb:/eos/user/m/marie/x", "marie", "", "/eos/user/m/marie/x"}, + {"marie@cb:notes", "marie", "home", "notes"}, + {"cb:~/notes", "", "home", "notes"}, } - if !got.IsRemote() || got.Space != "data/archive" { - t.Errorf("got %+v, want it read as the space alias data/archive", got) + for _, tt := range tests { + for name, parse := range map[string]func(string) (Spec, error){"remote": ParseRemote, "transfer": ParseTransfer} { + got, err := parse(tt.arg) + if err != nil { + t.Fatalf("%s(%q): %v", name, tt.arg, err) + } + if !got.IsRemote() || got.As != tt.wantAs || got.Space != tt.wantSpace || got.Path != tt.wantPath { + t.Errorf("%s(%q) = %+v, want remote as %q in space %q at %q", + name, tt.arg, got, tt.wantAs, tt.wantSpace, tt.wantPath) + } + if user, ok := Identity(tt.arg); user != tt.wantAs || ok != (tt.wantAs != "") { + t.Errorf("Identity(%q) = %q, %v", tt.arg, user, ok) + } + } } +} - escaped, err := ParseTransfer("file:data/archive:2024") - if err != nil { - t.Fatal(err) +// TestIdentityMarkerNeedsAUsername keeps text that only looks like the marker +// from naming an identity. +func TestIdentityMarkerNeedsAUsername(t *testing.T) { + for _, arg := range []string{"@cb:x", "dir/marie@cb:x", "a b@cb:x", "marie@cbx"} { + if user, ok := Identity(arg); ok { + t.Errorf("Identity(%q) = %q, want none", arg, user) + } + got, err := ParseTransfer(arg) + if err != nil { + t.Fatalf("ParseTransfer(%q): %v", arg, err) + } + if !got.IsLocal() { + t.Errorf("ParseTransfer(%q) is remote, want local: it carries no marker", arg) + } } - if !escaped.IsLocal() || escaped.Path != filepath.FromSlash("data/archive:2024") { - t.Errorf("file: prefix did not force a local path, got %+v", escaped) +} + +// TestStringRoundTrips checks that a remote spec renders to something every +// parser reads back the same way, marker and identity included. +func TestStringRoundTrips(t *testing.T) { + for _, arg := range []string{ + "cb:~/Documents", "cb:~", "marie@cb:~/a/b", "cb:/eos/user/g/x", + "marie@cb:project/cernbox:data", "cb:project/cernbox:", + } { + spec, err := ParseTransfer(arg) + if err != nil { + t.Fatal(err) + } + again, err := ParseTransfer(spec.String()) + if err != nil { + t.Fatal(err) + } + again.Raw, spec.Raw = "", "" + if again != spec { + t.Errorf("%q rendered as %q, which parses to %+v, not %+v", arg, spec.String(), again, spec) + } } } @@ -207,7 +262,7 @@ func TestParseTransferPair(t *testing.T) { }) t.Run("two remote paths are allowed", func(t *testing.T) { - src, dst, err := ParseTransferPair("cb:/eos/a", "home:b") + src, dst, err := ParseTransferPair("cb:/eos/a", "marie@cb:~/b") if err != nil { t.Fatal(err) } diff --git a/pkg/transfer/relay.go b/pkg/transfer/relay.go new file mode 100644 index 0000000..7bd020a --- /dev/null +++ b/pkg/transfer/relay.go @@ -0,0 +1,136 @@ +package transfer + +import ( + "context" + "errors" + "fmt" + "path" + "strings" + "sync" + "time" + + "github.com/cernbox/cernbox-cli/pkg/cberr" + "github.com/cernbox/cernbox-cli/pkg/client" +) + +// Relay copies a CERNBox file or tree, read through this engine's client, to +// dst as another client sees it. The two clients act as different users, so +// the server cannot copy it itself: a COPY carries one credential. The bytes +// stream through this process instead, one file at a time per job, and never +// touch local disk. +func (e *Engine) Relay(ctx context.Context, src string, to *client.Client, dst string) (*Stats, error) { + start := time.Now() + stats := &Stats{} + + info, err := e.c.Stat(ctx, src) + if err != nil { + return nil, err + } + if !info.IsDir { + n, err := e.relayFile(ctx, *info, to, dst) + stats.Duration = time.Since(start) + switch { + case errors.Is(err, errSkipped): + stats.Skipped++ + return stats, nil + case err != nil: + return nil, err + } + stats.Files, stats.Bytes = 1, n + return stats, nil + } + + // Walk visits a directory before its children, so creating directories in + // the order seen gives every file a parent before it arrives. + var files []client.ResourceInfo + err = e.c.Walk(ctx, src, func(ri client.ResourceInfo) error { + target := path.Join(dst, strings.TrimPrefix(ri.Path, src)) + if !ri.IsDir { + files = append(files, ri) + return nil + } + stats.Dirs++ + if e.opts.DryRun { + return nil + } + return to.Mkdir(ctx, target, true) + }) + if err != nil { + return nil, err + } + + var mu sync.Mutex + err = e.eachParallel(ctx, len(files), func(ctx context.Context, i int) error { + ri := files[i] + n, err := e.relayFile(ctx, ri, to, path.Join(dst, strings.TrimPrefix(ri.Path, src))) + mu.Lock() + defer mu.Unlock() + switch { + case errors.Is(err, errSkipped): + stats.Skipped++ + return nil + case err != nil: + return err + default: + stats.Files++ + stats.Bytes += n + return nil + } + }) + stats.Duration = time.Since(start) + return stats, err +} + +// relayFile streams one file from this engine's client to dst through to. +func (e *Engine) relayFile(ctx context.Context, src client.ResourceInfo, to *client.Client, dst string) (int64, error) { + if !e.opts.Overwrite { + if _, err := to.Stat(ctx, dst); err == nil { + e.emit(Event{Path: dst, Total: src.Size, Done: true}) + return 0, errSkipped + } else if cberr.KindOf(err) != cberr.KindNotFound { + return 0, err + } + } + if e.opts.DryRun { + e.emit(Event{Path: dst, Transferred: src.Size, Total: src.Size, Done: true}) + return src.Size, nil + } + + body, size, err := e.c.Download(ctx, src.Path, 0) + if err != nil { + return 0, err + } + defer body.Close() + if err := to.UploadStream(ctx, dst, body, size); err != nil { + return 0, err + } + + if e.opts.Verify { + if err := verifyRelayed(ctx, src, to, dst); err != nil { + return 0, err + } + } + e.emit(Event{Path: dst, Transferred: size, Total: size, Done: true}) + return size, nil +} + +// verifyRelayed compares what arrived with what was sent, using the checksums +// the server keeps for both. Nothing was hashed on the way through, so this is +// the server's word on each side, which is the word that matters. +func verifyRelayed(ctx context.Context, src client.ResourceInfo, to *client.Client, dst string) error { + got, err := to.Stat(ctx, dst) + if err != nil { + return err + } + if got.Size != src.Size { + return cberr.New(cberr.KindOther, "verify", dst, + fmt.Sprintf("arrived with %d bytes, %d were sent", got.Size, src.Size)) + } + for alg, want := range src.Checksums { + if have := got.Checksums[alg]; have != "" && !strings.EqualFold(have, want) { + return cberr.New(cberr.KindOther, "verify", dst, + fmt.Sprintf("%s checksum %s does not match the source's %s", alg, have, want)) + } + } + return nil +} From 8c1d8dddc75789c4a05e476f37ae48507f7b8df5 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Fri, 2 Oct 2026 15:34:27 +0200 Subject: [PATCH 3/3] dev: build reva from the cernbox-cli-dev integration branch The admin HTTP service is now its own reva pull request on master (cs3org/reva#5863), independent of Kerberos authentication (#5827). This environment needs both until they are merged, so it builds a branch that joins them. --- dev/Dockerfile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dev/Dockerfile b/dev/Dockerfile index 82e53bd..4012ae4 100644 --- a/dev/Dockerfile +++ b/dev/Dockerfile @@ -7,11 +7,13 @@ RUN apt-get update \ && apt-get install -y --no-install-recommends musl-tools \ && rm -rf /var/lib/apt/lists/* -# The reva branch carrying the Kerberos auth manager, the SPNEGO credential -# strategy, and the admin HTTP service --as needs (it is built on -# kerberos-auth). gaia resolves a branch name through the GitHub API, so this -# follows the branch head; point it at a released version once it is merged. -ARG REVA_VERSION=admin-http +# A reva integration branch, cernbox-cli-dev, that joins the two pieces this +# environment needs and that are not in master yet: Kerberos authentication +# (cs3org/reva#5827: the auth manager and the SPNEGO credential strategy) and +# the admin HTTP service --as uses (cs3org/reva#5863). It has no pull request of +# its own. gaia resolves a branch name through the GitHub API, so this follows +# its head; point it back at master, or a release, once both are merged. +ARG REVA_VERSION=cernbox-cli-dev # cgo, rather than CGO_ENABLED=0: the sql share driver runs on sqlite, which # needs cgo, and it is the only share driver whose "not found" error the