From c0406b5b7e8c0877d483e8a46819d1afa73a1021 Mon Sep 17 00:00:00 2001 From: Alessandro Rinaldi Date: Sat, 5 Sep 2026 23:01:14 +0200 Subject: [PATCH 1/3] docs: design for multiple accounts Choosing between DWService accounts per command, the way the AWS CLI chooses a profile, under two constraints that shape the whole design: someone who does not want the feature must never learn it exists, and an existing configuration keeps working. Hence the email as the key, the first account registered becoming the default silently, and a flat configuration migrated in memory on load rather than rewritten underneath a read-only command. @ is not available as a selector: dwshell alice@myserver already means the remote OS user. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MvidAFW9a2r4hTgHPW9ywG --- .../specs/2026-09-05-multi-account-design.md | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-05-multi-account-design.md diff --git a/docs/superpowers/specs/2026-09-05-multi-account-design.md b/docs/superpowers/specs/2026-09-05-multi-account-design.md new file mode 100644 index 0000000..303c19f --- /dev/null +++ b/docs/superpowers/specs/2026-09-05-multi-account-design.md @@ -0,0 +1,170 @@ +# Multiple accounts (`--account`, `dwshell account …`) — design + +Date: 2026-09-05 +Status: approved for implementation + +## 1. Purpose + +Use dwshell with more than one DWService account — say a personal one and a +work one — choosing between them per command, the way the AWS CLI chooses a +profile. + +Two requirements shape everything below, and they are the reason this is not +simply "add a profiles list": + +- **Someone who does not want this must never learn it exists.** With one + account every command behaves exactly as it does today, prints exactly what it + prints today, and the word "default" appears nowhere. The feature only starts + meaning something when a second account is registered. +- **An existing configuration keeps working**, migrated rather than discarded. + +## 2. Selecting an account + +``` +--account one invocation +DWSHELL_ACCOUNT= a shell session, a script +``` + +The flag wins over the variable, the variable over the default. An unknown +account is an error naming the ones that are registered — the same shape as +`agent group` against an unknown group. + +`--account` is a value flag, so it must join `valueFlags` for the shell shortcut +to keep parsing (`dwshell --account a@b GHE -c "…"`). + +`@` is not available as a selector: `dwshell alice@myserver` already means the +remote OS user, and overloading it would make `dwshell ale@alerinaldi.it` mean +two different things depending on whether the left side happens to name an +account. + +## 3. Registering: the email is the key + +`dwshell login` keys the account by the email that was logged in. + +- an email not seen before → a new account is added +- an email already registered → its credentials are replaced, which is exactly + today's behaviour +- the **first** account registered becomes the default, so a single-account user + never meets the concept + +`login` ignores `--account` and `DWSHELL_ACCOUNT`: which account it touches is +decided by the email being logged in, not by a selector. Passing one anyway is +an error rather than a silent no-op, since it can only mean a misunderstanding. + +## 4. `dwshell account` + +``` +dwshell account list [--json] registered accounts, marking the default +dwshell account default change the default +dwshell account rm remove one, deregistering its trusted device +``` + +`account list` prints one account per line — the email, and `(default)` against +the default one — or the same as a JSON array under `--json`. It is the only +place the word "default" appears in output, and with one account it is +unremarkable. + +## 5. `logout` + +Acts on the **selected** account: deregisters its trusted device and removes it. +With one account that is precisely today's behaviour. `--all` clears every +account, for the person who wants a clean slate. + +`logout` and `account rm` do the same thing to a single account; `logout` acts +on the selected one, `account rm` on a named one. + +### Removing the default + +If exactly one account remains, it is promoted silently — there is nothing to +choose. If two or more remain, dwshell does **not** pick one: commands without +`--account` fail saying which accounts exist and to set one with +`dwshell account default`. Guessing here would silently point a later command at +the wrong account, which is worse than an error. + +## 6. On-disk format and migration + +```json +{ + "default": "ale@example.net", + "accounts": [ + { "user": "ale@example.net", "session": {…}, "trustedDevice": {…} }, + { "user": "info@example.com", "session": {…} } + ] +} +``` + +Today's file is flat — `{"user":…, "session":{…}, "trustedDevice":{…}}` — and is +recognised on load and converted **in memory** into a single account, which +becomes the default. The new shape reaches the disk only when something actually +saves. A read-only command never rewrites the file behind the user's back, and +an untouched old configuration keeps working indefinitely. + +A configuration holding a session but no user name (possible in principle, +though `login` always records one) migrates to an account listed as `(unnamed)`; +it stays usable as the default, and logging in again names it. + +The migration is tested against a real configuration file's shape, not an +invented one — session with `commandUrl`, `signKey` (name + JWK private key), +`customHeaders`, `cookies`, plus `trustedDevice` with `id`, `name`, `authKey`. + +`--config` is unchanged: it still names the one file. + +## 7. Structure + +The multi-account logic stays in `internal/config`, which is where the file +already lives. `Config` grows the accounts list and a notion of which one is +selected, and keeps exposing the selected account's fields, so its consumers +barely change: + +```go +type Account struct { + User string `json:"user,omitempty"` + Session *SessionState `json:"session,omitempty"` + TrustedDevice *auth.TrustedDevice `json:"trustedDevice,omitempty"` +} + +type Config struct { + Default string `json:"default,omitempty"` + Accounts []*Account `json:"accounts,omitempty"` + // path and the selected account are in-memory only +} + +func (c *Config) Select(email string) error // "" = env, then default +func (c *Config) Current() *Account // never nil once Select succeeded +func (c *Config) Add(user string) *Account +func (c *Config) Remove(email string) error +func (c *Config) SetDefault(email string) error +``` + +`internal/client` changes from `c.cfg.Session` to `c.cfg.Current().Session` and +takes the selected account through `client.New`. `cmd/dwshell/account.go` holds +the subcommand family, as `agent.go` and `files.go` do. + +## 8. Errors + +- unknown `--account` → refuse, listing registered accounts +- no accounts at all → the existing "run `dwshell login`" message, unchanged +- several accounts and no default → refuse, saying to set one +- `account rm` of an account that is not registered → refuse, listing them +- `account default` of an unregistered account → refuse, listing them + +## 9. Testing + +Unit tests in `internal/config`: migration from the flat shape (built from a +real file's structure), selection precedence (flag over environment over +default), first-account-becomes-default, removing the default with one and with +several remaining, and that saving a migrated config produces the new shape +without losing the trusted device. + +The invisibility requirement gets its own test: with exactly one account, a +round trip through load and save leaves behaviour and output unchanged. + +Live verification with two real accounts, both of the user's: registering the +second, running the same command against each, and confirming the first still +works with no flag. + +## 10. Open question + +The second account's credentials are the user's to supply. Registering it live +means `dwshell login --user info@futura.fm` with its password and whatever +second factor it has — which only they can provide. From 4deabe31347b8a5b5c22066a1be8f714050d7512 Mon Sep 17 00:00:00 2001 From: Alessandro Rinaldi Date: Sat, 5 Sep 2026 23:18:01 +0200 Subject: [PATCH 2/3] docs: implementation plan for multiple accounts Six tasks. The design keeps the multi-account logic inside internal/config, so the client changes from cfg.Session to cfg.Current().Session and little else, and the two constraints that motivated the feature are each pinned by a test rather than left as intentions: loading a flat configuration must not rewrite the file, and a single account must be usable whether or not it is marked default. The live check exploits a property of the two real accounts: the second owns the machines the first sees as shares, so a wrong selection shows up as own-versus-shared on the same machine instead of something subtle. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MvidAFW9a2r4hTgHPW9ywG --- .../plans/2026-09-05-multi-account.md | 883 ++++++++++++++++++ 1 file changed, 883 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-05-multi-account.md diff --git a/docs/superpowers/plans/2026-09-05-multi-account.md b/docs/superpowers/plans/2026-09-05-multi-account.md new file mode 100644 index 0000000..7f7638f --- /dev/null +++ b/docs/superpowers/plans/2026-09-05-multi-account.md @@ -0,0 +1,883 @@ +# Multiple accounts Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Register several DWService accounts and choose between them per command, without a single-account user ever noticing the feature exists. + +**Architecture:** All multi-account logic lives in `internal/config`, which already owns the file. `Config` gains an accounts list and a selected account, and keeps exposing the selected one, so `internal/client` changes from `c.cfg.Session` to `c.cfg.Current().Session` and little else. The CLI gains a global `--account` and an `account` subcommand family. + +**Tech Stack:** Go, standard library only. + +**Spec:** `docs/superpowers/specs/2026-09-05-multi-account-design.md` + +## Global Constraints + +- **Invisible with one account.** Every command behaves and prints exactly as today; `(default)` appears only in `account list`. This is a test, not an intention. +- **An existing flat configuration keeps working**, migrated **in memory** on load. The new shape reaches disk only when something saves — a read-only command never rewrites the user's file. +- Selection precedence: `--account` flag, then `DWSHELL_ACCOUNT`, then the default account. +- `login` ignores both selectors — the email being logged in decides the account. Passing one is an error. +- `@` is not a selector: `dwshell alice@myserver` already means the remote OS user. +- Removing the default promotes the survivor **only** when exactly one remains; with two or more, commands without `--account` fail asking for `account default`. + +## File Structure + +| File | Responsibility | +|---|---| +| `internal/config/config.go` (modify) | `Account`, accounts list, default, save/load | +| `internal/config/migrate.go` (new) | recognising the flat shape and converting it in memory | +| `internal/config/select.go` (new) | selection precedence, add/remove/set-default | +| `internal/client/client.go` (modify) | act on the selected account; login keyed by email; logout one or all | +| `cmd/dwshell/account.go` (new) | the `account` subcommand family | +| `cmd/dwshell/main.go` (modify) | `--account` on every command, `valueFlags`, dispatch, help | +| `README.md` (modify) | document it as an opt-in that single-account users can ignore | + +--- + +### Task 1: The accounts model and the migration + +**Files:** +- Modify: `internal/config/config.go` +- Create: `internal/config/migrate.go`, `internal/config/migrate_test.go` + +**Interfaces:** +- Produces: + ```go + type Account struct { + User string `json:"user,omitempty"` + Session *SessionState `json:"session,omitempty"` + TrustedDevice *auth.TrustedDevice `json:"trustedDevice,omitempty"` + } + type Config struct { + Default string `json:"default,omitempty"` + Accounts []*Account `json:"accounts,omitempty"` + // path, selected: in-memory + } + func (c *Config) Find(email string) *Account + ``` + +- [ ] **Step 1: Write the failing test** + +The fixture mirrors a real file's structure — session with `commandUrl`, `signKey`, `customHeaders`, `cookies`, plus a `trustedDevice` with its `authKey`. + +```go +package config + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +// flatConfig is the shape dwshell wrote before accounts existed. +const flatConfig = `{ + "user": "ale@example.net", + "session": { + "commandUrl": "https://node1.dwservice.net/ses/ND1/tok.dw", + "signKey": {"name":"k1","priv":{"crv":"P-256","d":"D","kty":"EC","x":"X","y":"Y"}}, + "customHeaders": true, + "cookies": [{"name":"DWSID","value":"abc"}] + }, + "trustedDevice": { + "id": "dev1", + "name": "dwshell on laptop", + "authKey": {"name":"k2","priv":{"crv":"P-256","d":"D2","kty":"EC","x":"X2","y":"Y2"}} + } +}` + +func writeConfig(t *testing.T, body string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + return p +} + +func TestLoadMigratesAFlatConfig(t *testing.T) { + c, err := Load(writeConfig(t, flatConfig)) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Accounts) != 1 { + t.Fatalf("expected one account, got %d", len(c.Accounts)) + } + a := c.Accounts[0] + if a.User != "ale@example.net" { + t.Errorf("user = %q", a.User) + } + if a.Session == nil || a.Session.CommandURL == "" || a.Session.SignKey == nil { + t.Error("the session must survive the migration whole") + } + if len(a.Session.Cookies) != 1 || a.Session.Cookies[0].Name != "DWSID" { + t.Error("the node cookie must survive: without it the session cannot be reused") + } + if a.TrustedDevice == nil || a.TrustedDevice.ID != "dev1" { + t.Error("the trusted device must survive, or passwordless refresh breaks") + } + if c.Default != "ale@example.net" { + t.Errorf("the migrated account becomes the default, got %q", c.Default) + } +} + +// Migrating must not touch the file: a read-only command should never rewrite +// the user's configuration behind their back. +func TestLoadDoesNotRewriteTheFile(t *testing.T) { + p := writeConfig(t, flatConfig) + before, _ := os.ReadFile(p) + if _, err := Load(p); err != nil { + t.Fatalf("Load: %v", err) + } + after, _ := os.ReadFile(p) + if string(before) != string(after) { + t.Fatal("Load rewrote the configuration file") + } +} + +// Saving a migrated config writes the new shape and drops the flat keys. +func TestSaveWritesTheAccountsShape(t *testing.T) { + p := writeConfig(t, flatConfig) + c, _ := Load(p) + if err := c.Save(); err != nil { + t.Fatalf("Save: %v", err) + } + var raw map[string]any + b, _ := os.ReadFile(p) + if err := json.Unmarshal(b, &raw); err != nil { + t.Fatal(err) + } + if _, ok := raw["accounts"]; !ok { + t.Error("the saved file must carry accounts") + } + for _, gone := range []string{"user", "session", "trustedDevice"} { + if _, ok := raw[gone]; ok { + t.Errorf("the flat key %q must not be written back", gone) + } + } +} + +func TestLoadOfANewShapeIsUnchanged(t *testing.T) { + body := `{"default":"a@b","accounts":[{"user":"a@b"},{"user":"c@d"}]}` + c, err := Load(writeConfig(t, body)) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Accounts) != 2 || c.Default != "a@b" { + t.Fatalf("got %d accounts, default %q", len(c.Accounts), c.Default) + } +} + +func TestLoadOfAMissingFileIsEmpty(t *testing.T) { + c, err := Load(filepath.Join(t.TempDir(), "absent.json")) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Accounts) != 0 { + t.Fatal("a missing file means no accounts, not an error") + } +} +``` + +- [ ] **Step 2: Run the tests and watch them fail** + +Run: `go test ./internal/config/ -v` +Expected: build failure — `c.Accounts` and `c.Default` do not exist + +- [ ] **Step 3: Write the implementation** + +In `config.go`, replace the flat fields with the accounts model: + +```go +// Account is one DWService account: its user, the reusable session, and the +// optional trusted device that refreshes that session without a password. +type Account struct { + User string `json:"user,omitempty"` + Session *SessionState `json:"session,omitempty"` + TrustedDevice *auth.TrustedDevice `json:"trustedDevice,omitempty"` +} + +// Config is the on-disk state: the accounts that have been logged in, and which +// of them commands use when none is named. +type Config struct { + Default string `json:"default,omitempty"` + Accounts []*Account `json:"accounts,omitempty"` + + path string + selected *Account +} + +// Find returns the account for an email, or nil. +func (c *Config) Find(email string) *Account { + for _, a := range c.Accounts { + if a.User == email { + return a + } + } + return nil +} +``` + +In `migrate.go`: + +```go +package config + +import "encoding/json" + +// flatConfig is the pre-accounts on-disk shape: a single account's fields at +// the top level. +type flatConfig struct { + User string `json:"user"` + Session *SessionState `json:"session"` + TrustedDevice *auth.TrustedDevice `json:"trustedDevice"` +} + +// migrateFlat converts a pre-accounts configuration into a single account, in +// memory. It is applied on every load and never writes: an untouched old file +// keeps working, and the new shape reaches the disk only when something saves +// for a reason of its own. +// +// A file with neither accounts nor flat fields is simply empty — a first run. +func migrateFlat(body []byte, c *Config) error { + if len(c.Accounts) > 0 { + return nil // already the new shape + } + var flat flatConfig + if err := json.Unmarshal(body, &flat); err != nil { + return err + } + if flat.Session == nil && flat.TrustedDevice == nil && flat.User == "" { + return nil + } + c.Accounts = []*Account{{ + User: flat.User, + Session: flat.Session, + TrustedDevice: flat.TrustedDevice, + }} + c.Default = flat.User + return nil +} +``` + +And in `Load`, after unmarshalling into `c`, call `migrateFlat(b, c)`. + +- [ ] **Step 4: Run the tests and watch them pass** + +Run: `go test ./internal/config/ -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add internal/config +git commit -m "config: hold several accounts, migrating a flat file in memory" +``` + +--- + +### Task 2: Selection, and the rules around the default + +**Files:** +- Create: `internal/config/select.go`, `internal/config/select_test.go` + +**Interfaces:** +- Consumes: `Account`, `Config`, `Find` (Task 1) +- Produces: + ```go + var ErrNoAccounts = errors.New(...) + func (c *Config) Select(email string) error // "" = DWSHELL_ACCOUNT, then Default + func (c *Config) Current() *Account // never nil after a successful Select + func (c *Config) Add(user string) *Account // first one becomes the default + func (c *Config) Remove(email string) error // promotes a lone survivor + func (c *Config) SetDefault(email string) error + func (c *Config) Emails() []string + ``` + +- [ ] **Step 1: Write the failing tests** + +```go +package config + +import ( + "strings" + "testing" +) + +func twoAccounts() *Config { + return &Config{ + Default: "a@b", + Accounts: []*Account{{User: "a@b"}, {User: "c@d"}}, + } +} + +func TestSelectPrefersTheFlagOverTheEnvironment(t *testing.T) { + t.Setenv("DWSHELL_ACCOUNT", "c@d") + c := twoAccounts() + if err := c.Select("a@b"); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "a@b" { + t.Fatalf("the flag must win, got %q", c.Current().User) + } +} + +func TestSelectFallsBackToTheEnvironmentThenTheDefault(t *testing.T) { + t.Setenv("DWSHELL_ACCOUNT", "c@d") + c := twoAccounts() + if err := c.Select(""); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "c@d" { + t.Fatalf("the environment must be used, got %q", c.Current().User) + } + + t.Setenv("DWSHELL_ACCOUNT", "") + c = twoAccounts() + if err := c.Select(""); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "a@b" { + t.Fatalf("the default must be used, got %q", c.Current().User) + } +} + +func TestSelectUnknownAccountListsTheKnownOnes(t *testing.T) { + c := twoAccounts() + err := c.Select("nope@x") + if err == nil { + t.Fatal("an unknown account must fail") + } + for _, want := range []string{"a@b", "c@d"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the error should list %q, got %q", want, err) + } + } +} + +// With one account there is nothing to choose, so a missing default is not an +// error: this is what keeps the feature invisible. +func TestSelectWithOneAccountAndNoDefault(t *testing.T) { + c := &Config{Accounts: []*Account{{User: "solo@x"}}} + if err := c.Select(""); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "solo@x" { + t.Fatal("the lone account is used whether or not it is marked default") + } +} + +func TestSelectWithSeveralAndNoDefaultAsksForOne(t *testing.T) { + c := &Config{Accounts: []*Account{{User: "a@b"}, {User: "c@d"}}} + err := c.Select("") + if err == nil { + t.Fatal("with no default and several accounts it must refuse rather than guess") + } + if !strings.Contains(err.Error(), "account default") { + t.Errorf("the error should say how to fix it, got %q", err) + } +} + +func TestAddMakesTheFirstAccountTheDefault(t *testing.T) { + c := &Config{} + c.Add("first@x") + if c.Default != "first@x" { + t.Fatalf("default = %q, want first@x", c.Default) + } + c.Add("second@x") + if c.Default != "first@x" { + t.Fatal("a later account must not steal the default") + } + if len(c.Accounts) != 2 { + t.Fatalf("got %d accounts", len(c.Accounts)) + } +} + +func TestAddIsIdempotentForTheSameEmail(t *testing.T) { + c := &Config{} + a1 := c.Add("same@x") + a2 := c.Add("same@x") + if a1 != a2 || len(c.Accounts) != 1 { + t.Fatal("logging in again with the same email updates that account") + } +} + +func TestRemovePromotesALoneSurvivor(t *testing.T) { + c := twoAccounts() + if err := c.Remove("a@b"); err != nil { + t.Fatalf("Remove: %v", err) + } + if c.Default != "c@d" { + t.Fatalf("with one account left it becomes the default, got %q", c.Default) + } +} + +func TestRemoveLeavesNoDefaultWhenSeveralRemain(t *testing.T) { + c := &Config{Default: "a@b", Accounts: []*Account{{User: "a@b"}, {User: "c@d"}, {User: "e@f"}}} + if err := c.Remove("a@b"); err != nil { + t.Fatalf("Remove: %v", err) + } + if c.Default != "" { + t.Fatalf("dwshell must not pick a default among several, got %q", c.Default) + } +} + +func TestRemoveUnknownAccountFails(t *testing.T) { + if err := twoAccounts().Remove("nope@x"); err == nil { + t.Fatal("removing an unregistered account must fail") + } +} + +func TestSetDefaultRejectsAnUnregisteredAccount(t *testing.T) { + if err := twoAccounts().SetDefault("nope@x"); err == nil { + t.Fatal("the default must name a registered account") + } +} + +func TestSelectWithNoAccountsSaysToLogIn(t *testing.T) { + c := &Config{} + if err := c.Select(""); err == nil { + t.Fatal("with no accounts at all it must fail") + } +} +``` + +- [ ] **Step 2: Run the tests and watch them fail** + +Run: `go test ./internal/config/ -run 'Select|Add|Remove|SetDefault' -v` +Expected: `undefined: Select`, `undefined: Add`, … + +- [ ] **Step 3: Write the implementation** + +```go +package config + +import ( + "errors" + "fmt" + "os" + "strings" +) + +// ErrNoAccounts means nothing has been logged in yet. +var ErrNoAccounts = errors.New("no account configured: run `dwshell login`") + +// Emails lists the registered accounts, in the order they were added. +func (c *Config) Emails() []string { + out := make([]string, 0, len(c.Accounts)) + for _, a := range c.Accounts { + out = append(out, a.User) + } + return out +} + +// Select picks the account commands will act on: the argument if given, else +// DWSHELL_ACCOUNT, else the default. With a single account there is nothing to +// choose and it is used whether or not it is marked default — which is what +// keeps this feature out of a single-account user's way. +// +// With several accounts and no default it refuses rather than guessing: picking +// one silently would point the next command at the wrong account. +func (c *Config) Select(email string) error { + if len(c.Accounts) == 0 { + return ErrNoAccounts + } + if email == "" { + email = os.Getenv("DWSHELL_ACCOUNT") + } + if email == "" { + if len(c.Accounts) == 1 { + c.selected = c.Accounts[0] + return nil + } + email = c.Default + } + if email == "" { + return fmt.Errorf("several accounts are configured and none is the default; "+ + "pick one with `dwshell account default ` or pass --account (%s)", + strings.Join(c.Emails(), ", ")) + } + a := c.Find(email) + if a == nil { + return fmt.Errorf("no account %q; registered accounts: %s", email, strings.Join(c.Emails(), ", ")) + } + c.selected = a + return nil +} + +// Current is the selected account. Select must have succeeded first; callers +// that never selected get the lone account, so single-account code paths that +// predate this feature keep working. +func (c *Config) Current() *Account { + if c.selected != nil { + return c.selected + } + if len(c.Accounts) == 1 { + return c.Accounts[0] + } + return &Account{} +} + +// Add returns the account for an email, creating it if it is new. The first +// account registered becomes the default, so someone who only ever logs in once +// never meets the concept. +func (c *Config) Add(user string) *Account { + if a := c.Find(user); a != nil { + return a + } + a := &Account{User: user} + c.Accounts = append(c.Accounts, a) + if len(c.Accounts) == 1 { + c.Default = user + } + c.selected = a + return a +} + +// Remove forgets an account. If exactly one remains it is promoted, there being +// nothing to choose; if several remain the default is left unset rather than +// guessed at. +func (c *Config) Remove(email string) error { + idx := -1 + for i, a := range c.Accounts { + if a.User == email { + idx = i + break + } + } + if idx < 0 { + return fmt.Errorf("no account %q; registered accounts: %s", email, strings.Join(c.Emails(), ", ")) + } + c.Accounts = append(c.Accounts[:idx], c.Accounts[idx+1:]...) + if c.selected != nil && c.selected.User == email { + c.selected = nil + } + if c.Default == email { + c.Default = "" + if len(c.Accounts) == 1 { + c.Default = c.Accounts[0].User + } + } + return nil +} + +// SetDefault marks a registered account as the one commands use by default. +func (c *Config) SetDefault(email string) error { + if c.Find(email) == nil { + return fmt.Errorf("no account %q; registered accounts: %s", email, strings.Join(c.Emails(), ", ")) + } + c.Default = email + return nil +} + +// Clear removes every account (logout --all). +func (c *Config) Clear() { + c.Accounts = nil + c.Default = "" + c.selected = nil +} +``` + +- [ ] **Step 4: Run the tests and watch them pass** + +Run: `go test ./internal/config/ -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add internal/config +git commit -m "config: select an account, and the rules around the default" +``` + +--- + +### Task 3: The client acts on the selected account + +**Files:** +- Modify: `internal/client/client.go` + +**Interfaces:** +- Produces: `func New(configPath, account string) (*Client, error)` — the account selector, empty for the default +- Consumes: `Select`, `Current`, `Add`, `Remove`, `Clear` (Task 2) + +- [ ] **Step 1: Adapt the call sites** + +`New` selects before use, so an unknown `--account` fails at the earliest point rather than midway through a command: + +```go +// New builds a Client from a config path (empty = default) and an account +// selector (empty = DWSHELL_ACCOUNT, else the default account), seeding the +// cookie jar with that account's persisted node cookie. +func New(configPath, account string) (*Client, error) { + cfg, err := config.Load(configPath) + if err != nil { + return nil, err + } + // An empty configuration is not an error here: `login` starts from one. + if len(cfg.Accounts) > 0 { + if err := cfg.Select(account); err != nil { + return nil, err + } + } else if account != "" { + return nil, fmt.Errorf("no account %q: nothing is configured yet", account) + } + jar, _ := cookiejar.New(nil) + if s := cfg.Current().Session; s != nil && len(s.Cookies) > 0 { + if u, e := neturl.Parse(s.CommandURL); e == nil { + var cks []*http.Cookie + for _, c := range s.Cookies { + cks = append(cks, &http.Cookie{Name: c.Name, Value: c.Value}) + } + jar.SetCookies(u, cks) + } + } + return &Client{cfg: cfg, http: &http.Client{Jar: jar, Timeout: 60 * time.Second}}, nil +} +``` + +Every `c.cfg.Session` becomes `c.cfg.Current().Session`, and likewise for `TrustedDevice`. + +`Login` keys the account by the email it just authenticated: + +```go + // The email is the account key: a new one is registered, a known one has its + // credentials replaced. + acct := c.cfg.Add(user) + if err := c.persistSession(ctx, boot); err != nil { + return err + } + if tdReq != nil && tdReq.Result != nil { + acct.TrustedDevice = tdReq.Result + } + return c.cfg.Save() +``` + +`persistSession` writes into `c.cfg.Current()`. + +`Logout` forgets the selected account; `LogoutAll` clears everything: + +```go +// Logout deregisters the selected account's trusted device (freeing its capped +// slot) and forgets it. With one account this is exactly what it always did. +func (c *Client) Logout(ctx context.Context) error { + c.deregister(ctx, c.cfg.Current()) + if u := c.cfg.Current().User; u != "" { + if err := c.cfg.Remove(u); err != nil { + return err + } + } else { + c.cfg.Clear() + } + return c.cfg.Save() +} + +// LogoutAll forgets every account, deregistering each trusted device. +func (c *Client) LogoutAll(ctx context.Context) error { + for _, a := range c.cfg.Accounts { + c.deregister(ctx, a) + } + c.cfg.Clear() + return c.cfg.Save() +} + +// deregister removes an account's trusted device server-side; failure is +// non-fatal, as it always was — the local credentials still go. +func (c *Client) deregister(ctx context.Context, a *config.Account) { + if a == nil || a.TrustedDevice == nil { + return + } + if cfg, err := auth.FetchLoginConfig(ctx, c.http); err == nil { + _ = auth.RemoveTrustedDevice(ctx, c.http, cfg, a.TrustedDevice) + } +} +``` + +- [ ] **Step 2: Build and run the suite** + +Run: `go build ./... && go test ./...` +Expected: compile errors at every `client.New(` call site in `cmd/dwshell`, fixed in Task 4. + +- [ ] **Step 3: Commit (with Task 4, which unbreaks the build)** + +--- + +### Task 4: `--account` on every command + +**Files:** +- Modify: `cmd/dwshell/main.go`, `cmd/dwshell/files.go`, `cmd/dwshell/agent.go` +- Test: `cmd/dwshell/main_test.go` + +- [ ] **Step 1: Write the failing test** + +```go +// --account takes a value, so the shell shortcut has to skip that value when +// looking for the agent name — otherwise `dwshell --account a@b GHE` would take +// the email for the agent. +func TestExtractAgentSkipsTheAccountValue(t *testing.T) { + agent, flags := extractAgent([]string{"--account", "a@b", "GHE", "-c", "ls"}) + if agent != "GHE" { + t.Fatalf("agent = %q, want GHE", agent) + } + joined := strings.Join(flags, " ") + if !strings.Contains(joined, "--account a@b") || !strings.Contains(joined, "-c ls") { + t.Fatalf("flags = %v", flags) + } +} +``` + +- [ ] **Step 2: Run the test and watch it fail** + +Run: `go test ./cmd/dwshell/ -run ExtractAgentSkips -v` +Expected: FAIL — `agent = "a@b"` + +- [ ] **Step 3: Write the implementation** + +```go +var valueFlags = map[string]bool{"c": true, "term": true, "config": true, "timeout": true, "account": true} +``` + +Every command that builds a client gains: + +```go + fs.StringVar(&account, "account", "", "account to use (default: the default account)") +``` + +and passes it: `client.New(configPath, account)`. Help gains, under Global: + +``` + --account email Account to use when several are logged in +``` + +- [ ] **Step 4: Run the suite** + +Run: `go build ./... && go test ./...` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add internal/client cmd/dwshell +git commit -m "account: act on the selected account, chosen with --account" +``` + +--- + +### Task 5: `dwshell account` and `logout --all` + +**Files:** +- Create: `cmd/dwshell/account.go` +- Modify: `cmd/dwshell/main.go` (dispatch, help, `logout --all`) + +- [ ] **Step 1: Write the implementation** + +```go +// cmdAccountList prints the registered accounts. It is the only output in +// dwshell that mentions a default, and with one account it says nothing +// remarkable. +func cmdAccountList(ctx context.Context, args []string) int { + // … --config, --json + for _, a := range cfg.Accounts { + mark := "" + if a.User == cfg.Default { + mark = " (default)" + } + fmt.Printf("%s%s\n", a.User, mark) + } + return 0 +} +``` + +`account default ` calls `SetDefault` then `Save`; `account rm ` deregisters that account's trusted device, calls `Remove`, then `Save`, behind the same confirmation `agent rm` uses. + +`logout` gains `--all`, routing to `LogoutAll`. + +- [ ] **Step 2: Run the suite** + +Run: `go test ./...` +Expected: PASS + +- [ ] **Step 3: Verify with a scratch configuration, not the real one** + +```bash +export DWSHELL_CONFIG=$(mktemp -d)/config.json +go run ./cmd/dwshell account list # no accounts → says to log in +``` + +- [ ] **Step 4: Commit** + +```bash +git add cmd/dwshell +git commit -m "account: list, set the default, and remove" +``` + +--- + +### Task 6: Live verification with two real accounts, then docs + +**Files:** +- Modify: `README.md` + +The two accounts see the same machines from opposite sides — the second owns +what the first sees as shares — which makes a wrong selection obvious rather +than subtle. + +- [ ] **Step 1: Prove the existing configuration still works untouched** + +```bash +cp ~/.config/dwshell/config.json /tmp/dwshell-config.bak +go run ./cmd/dwshell list | head -3 # unchanged, no flag, no migration written +diff <(cat ~/.config/dwshell/config.json) /tmp/dwshell-config.bak && echo "file untouched" +``` + +- [ ] **Step 2: Register the second account and check both** + +```bash +DWSHELL_PASSWORD=… go run ./cmd/dwshell login --user info@futura.fm +go run ./cmd/dwshell account list # two, the first marked default +go run ./cmd/dwshell list | grep Regia # shared +go run ./cmd/dwshell --account info@futura.fm list | grep Regia # own +DWSHELL_ACCOUNT=info@futura.fm go run ./cmd/dwshell list | grep Regia # own +go run ./cmd/dwshell list | grep Regia # shared again: the default is untouched +``` + +Expected: the same machine reads `shared` from one account and `own` from the +other, and the unflagged command keeps using the default. + +- [ ] **Step 3: Check a command that is not `list`** + +```bash +go run ./cmd/dwshell --account info@futura.fm Regia -c "echo ok" +``` + +- [ ] **Step 4: Restore** + +```bash +go run ./cmd/dwshell account rm info@futura.fm --yes +go run ./cmd/dwshell account list # one account again +``` + +- [ ] **Step 5: Document it** + +`README.md` gains a short section presenting accounts as opt-in: log in twice +and the second appears, choose with `--account` or `DWSHELL_ACCOUNT`, manage +with `dwshell account`. It says plainly that with one account nothing changes, +and that an existing configuration is migrated on first use. + +- [ ] **Step 6: Full verification and commit** + +```bash +gofmt -l cmd internal && go vet ./... && go test -race ./... +git add README.md && git commit -m "docs: using more than one account" +``` + +--- + +## Self-review + +**Spec coverage.** §2 selection → Task 2 (precedence) and Task 4 (`--account`, `valueFlags`). §3 registering by email → Task 2 `Add` and Task 3 `Login`. §4 `account` subcommand → Task 5. §5 logout and the default-removal rules → Tasks 2 and 3. §6 format and migration → Task 1. §7 structure → the file table. §8 errors → Task 2's tests, one per case. §9 testing → each task, plus Task 6 live. + +**Placeholders.** Task 5's list function is shown in outline rather than in full, because it is a print loop over an interface fixed in Task 2; every other step carries its code. + +**Type consistency.** `Account`, `Config`, `Find` from Task 1 are used unchanged. `Select`/`Current`/`Add`/`Remove`/`SetDefault`/`Emails` from Task 2 are what Tasks 3–5 call. `client.New` gains its second parameter in Task 3 and every call site is updated in Task 4 — the build is deliberately broken between them, which is why they share a commit. + +**Gap found while reviewing:** `login` must ignore `--account` (spec §3). Task 4 adds the flag to every command generically, so `login` needs the explicit refusal — folded into Task 4's implementation step as an error when both `--user` and `--account` disagree, rather than a silent no-op. From 139ca69a540370aee559f8b3c9aba086f65aafe0 Mon Sep 17 00:00:00 2001 From: Alessandro Rinaldi Date: Sat, 5 Sep 2026 23:27:58 +0200 Subject: [PATCH 3/3] account: use dwshell with more than one DWService account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Log in twice with different emails and both are kept; choose between them with --account or DWSHELL_ACCOUNT. The email is the account's identity, so logging in again with a known one refreshes it, exactly as before. The feature is built to be invisible until it is wanted. With a single account there is no default to think about: Select uses the lone account whether or not it is marked default, no output changes, and the word appears only in `account list`. That is a test, not an intention. An existing configuration is migrated in memory on load and written in the new shape only when something saves for a reason of its own — a read-only command must not rewrite the user's file behind their back. Verified against a real configuration: after `list` the file still had its flat keys, and only a command that saves turned them into accounts. Removing the default promotes the survivor when exactly one remains and otherwise leaves it unset, refusing later commands with an explanation rather than silently pointing them at the wrong account. login refuses --account: which account it touches is decided by the email being logged in, and it deliberately does not select an account, since "several accounts and no default" is a state logging in resolves. Verified live with two real accounts, which see the same machines from opposite sides: Regia reads shared from one and own from the other, so a wrong selection would be obvious rather than subtle. Checked through the flag, the environment variable, the default, and back. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MvidAFW9a2r4hTgHPW9ywG --- README.md | 35 ++++++- cmd/dwshell/account.go | 172 ++++++++++++++++++++++++++++++++ cmd/dwshell/agent.go | 68 +++---------- cmd/dwshell/files.go | 20 +++- cmd/dwshell/files_test.go | 19 +++- cmd/dwshell/main.go | 45 +++++++-- internal/client/client.go | 84 +++++++++++++--- internal/config/config.go | 34 +++++-- internal/config/migrate.go | 42 ++++++++ internal/config/migrate_test.go | 149 +++++++++++++++++++++++++++ internal/config/select.go | 128 ++++++++++++++++++++++++ internal/config/select_test.go | 148 +++++++++++++++++++++++++++ 12 files changed, 852 insertions(+), 92 deletions(-) create mode 100644 cmd/dwshell/account.go create mode 100644 internal/config/migrate.go create mode 100644 internal/config/migrate_test.go create mode 100644 internal/config/select.go create mode 100644 internal/config/select_test.go diff --git a/README.md b/README.md index 28d9d99..6149709 100644 --- a/README.md +++ b/README.md @@ -160,7 +160,8 @@ the session without any of this. To supply a code non-interactively, see | Command | Description | |---|---| | `dwshell login [--user U] [--no-trusted]` | Authenticate and persist the session (and, by default, a trusted device). | -| `dwshell logout` | Deregister the trusted device and forget local credentials. | +| `dwshell logout [--all]` | Deregister the trusted device and forget local credentials. | +| `dwshell account list\|default\|rm` | Manage accounts, once you log in with more than one. | | `dwshell list [--json]` | List machines with OS, online state, and owned/shared. | | `dwshell ` | Open an interactive shell. | | `dwshell -c "cmd"` | Run a command non-interactively; exit code is propagated. | @@ -247,6 +248,38 @@ the download page is where the licence is accepted. The agent's unattended mode it — an install attempted that way answers *"Silent installation forbidden. Please contact the support."* +### More than one account + +Log in twice with different emails and dwshell keeps both. Until you do, nothing +changes: with a single account there is no default to think about and no flag to +pass, and an existing configuration is carried over the first time dwshell saves +anything. + +```sh +$ dwshell login --user info@example.com # the email is the account's identity +$ dwshell account list +ale@example.net (default) +info@example.com + +$ dwshell list # the default account +$ dwshell list --account info@example.com # the other one +$ DWSHELL_ACCOUNT=info@example.com dwshell list # for a whole session +``` + +The first account you log in with becomes the default; change it with +`dwshell account default `. Logging in again with an email already +registered just refreshes that account. + +`dwshell logout` forgets the selected account, `--all` forgets every one, and +`dwshell account rm ` forgets a named one — each deregistering that +account's trusted device, and each asking for confirmation unless you pass +`--yes`. Remove the default and, if a single account is left, it takes over; if +several remain, dwshell asks you to pick one rather than choosing for you. + +Flags follow their subcommand — `dwshell list --account …`, not +`dwshell --account … list` — except in the shell shortcut, where +`dwshell --account … myserver` works too. + #### Agent name vs subcommand `dwshell ` is a convenience shortcut: the first argument is treated as an diff --git a/cmd/dwshell/account.go b/cmd/dwshell/account.go new file mode 100644 index 0000000..897d4d5 --- /dev/null +++ b/cmd/dwshell/account.go @@ -0,0 +1,172 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "os" + + "github.com/porech/dwshell/internal/client" + "github.com/porech/dwshell/internal/config" + "github.com/porech/dwshell/internal/term" +) + +// cmdAccount dispatches `dwshell account `, which exists only for people +// who log in with more than one account. With a single account there is nothing +// here worth running. +func cmdAccount(ctx context.Context, args []string) int { + if len(args) == 0 { + return fail("usage: dwshell account ") + } + switch args[0] { + case "list": + return cmdAccountList(args[1:]) + case "default": + return cmdAccountDefault(args[1:]) + case "rm": + return cmdAccountRemove(ctx, args[1:]) + default: + return fail("unknown account subcommand %q", args[0]) + } +} + +// accountJSON is the --json shape of one registered account. +type accountJSON struct { + User string `json:"user"` + Default bool `json:"default"` +} + +func cmdAccountList(args []string) int { + fs := newFlags("account list") + var configPath string + asJSON := false + fs.StringVar(&configPath, "config", "", "config file path") + fs.BoolVar(&asJSON, "json", false, "machine-readable output") + if err := fs.Parse(args); err != nil { + return 2 + } + cfg, err := config.Load(configPath) + if err != nil { + return fail("%v", err) + } + if len(cfg.Accounts) == 0 { + return fail("%v", config.ErrNoAccounts) + } + if asJSON { + out := make([]accountJSON, 0, len(cfg.Accounts)) + for _, a := range cfg.Accounts { + out = append(out, accountJSON{User: a.User, Default: a.User == cfg.Default}) + } + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + _ = enc.Encode(out) + return 0 + } + for _, a := range cfg.Accounts { + name := a.User + if name == "" { + name = "(unnamed)" + } + if a.User == cfg.Default { + fmt.Printf("%s (default)\n", name) + continue + } + fmt.Println(name) + } + return 0 +} + +func cmdAccountDefault(args []string) int { + fs := newFlags("account default") + var configPath string + fs.StringVar(&configPath, "config", "", "config file path") + if err := fs.Parse(args); err != nil { + return 2 + } + if fs.NArg() != 1 { + return fail("usage: dwshell account default ") + } + cfg, err := config.Load(configPath) + if err != nil { + return fail("%v", err) + } + if err := cfg.SetDefault(fs.Arg(0)); err != nil { + return fail("%v", err) + } + if err := cfg.Save(); err != nil { + return fail("%v", err) + } + fmt.Fprintf(os.Stderr, "default account is now %s\n", fs.Arg(0)) + return 0 +} + +func cmdAccountRemove(ctx context.Context, args []string) int { + fs := newFlags("account rm") + var configPath string + assumeYes := false + fs.StringVar(&configPath, "config", "", "config file path") + fs.BoolVar(&assumeYes, "yes", false, "do not ask for confirmation") + email, flagArgs := extractPositional(args) + if err := fs.Parse(flagArgs); err != nil { + return 2 + } + if email == "" { + return fail("usage: dwshell account rm [--yes]") + } + if err := confirm(fmt.Sprintf("remove account %q and deregister its trusted device?", email), + assumeYes, term.IsTTY()); err != nil { + return fail("%v", err) + } + // Selecting the account being removed is what lets the client deregister its + // trusted device server-side before forgetting it locally. + c, err := client.New(configPath, email) + if err != nil { + return fail("%v", err) + } + if err := c.Logout(ctx); err != nil { + return fail("%v", err) + } + fmt.Fprintf(os.Stderr, "account %s removed\n", email) + return 0 +} + +// extractPositional pulls the first non-flag token out of a subcommand's args, +// returning it with the flags that surrounded it. Go's flag package stops at +// the first positional, so `account rm a@b --yes` would otherwise drop --yes; +// this mirrors what extractAgent does for the shell shortcut. +func extractPositional(args []string) (pos string, flagArgs []string) { + i := 0 + for i < len(args) { + a := args[i] + if len(a) > 0 && a[0] == '-' { + flagArgs = append(flagArgs, a) + if !containsEq(a) && valueFlags[trimDashes(a)] && i+1 < len(args) { + i++ + flagArgs = append(flagArgs, args[i]) + } + i++ + continue + } + return a, append(flagArgs, args[i+1:]...) + } + return "", flagArgs +} + +// confirm gates a change that cannot be undone. With a terminal it asks; with +// none — a script, a CI job — it refuses unless the caller passed --yes, so +// automation cannot drop an account by accident. +func confirm(prompt string, assumeYes, interactive bool) error { + if assumeYes { + return nil + } + if !interactive { + return fmt.Errorf("%s refusing without a terminal; pass --yes to proceed", prompt) + } + fmt.Fprintf(os.Stderr, "%s [y/N] ", prompt) + var answer string + _, _ = fmt.Fscanln(os.Stdin, &answer) + if answer != "y" && answer != "Y" { + return fmt.Errorf("cancelled") + } + return nil +} diff --git a/cmd/dwshell/agent.go b/cmd/dwshell/agent.go index 9b02531..990293b 100644 --- a/cmd/dwshell/agent.go +++ b/cmd/dwshell/agent.go @@ -62,32 +62,6 @@ func printAgentJSON(a agentJSON) { _ = enc.Encode(a) } -// agentValueFlags are the `agent` subcommand flags that consume a following -// value token, so a positional can be pulled out from among them. -var agentValueFlags = map[string]bool{"config": true, "description": true, "group": true} - -// extractPositional pulls the first non-flag token out of a subcommand's args, -// returning it with the flags that surrounded it. Go's flag package stops at -// the first positional, so `agent create NAME --json` would otherwise silently -// drop --json; this mirrors what extractAgent does for the shell shortcut. -func extractPositional(args []string) (pos string, flagArgs []string) { - i := 0 - for i < len(args) { - a := args[i] - if len(a) > 0 && a[0] == '-' { - flagArgs = append(flagArgs, a) - if !containsEq(a) && agentValueFlags[trimDashes(a)] && i+1 < len(args) { - i++ - flagArgs = append(flagArgs, args[i]) - } - i++ - continue - } - return a, append(flagArgs, args[i+1:]...) - } - return "", flagArgs -} - // cmdAgentManage dispatches the `dwshell agent ` family. It is separate // from cmdAgent, which is the shell shortcut for `dwshell `. func cmdAgentManage(ctx context.Context, args []string) int { @@ -112,9 +86,11 @@ func cmdAgentManage(ctx context.Context, args []string) int { func cmdAgentCreate(ctx context.Context, args []string) int { fs := newFlags("agent create") + var account string var configPath, description, group string asJSON := false fs.StringVar(&configPath, "config", "", "config file path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.StringVar(&description, "description", "", "free-text description") fs.StringVar(&group, "group", "", "existing group to place the agent in") fs.BoolVar(&asJSON, "json", false, "machine-readable output") @@ -126,7 +102,7 @@ func cmdAgentCreate(ctx context.Context, args []string) int { return fail("usage: dwshell agent create [--group G] [--description D] [--json]") } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -163,25 +139,6 @@ func cmdAgentCreate(ctx context.Context, args []string) int { return 0 } -// confirm gates a change that cannot be undone. With a terminal it asks; with -// none — a script, a CI job — it refuses unless the caller passed --yes, so -// automation cannot delete a machine or invalidate a code by accident. -func confirm(prompt string, assumeYes, interactive bool) error { - if assumeYes { - return nil - } - if !interactive { - return fmt.Errorf("%s refusing without a terminal; pass --yes to proceed", prompt) - } - fmt.Fprintf(os.Stderr, "%s [y/N] ", prompt) - var answer string - _, _ = fmt.Fscanln(os.Stdin, &answer) - if answer != "y" && answer != "Y" { - return fmt.Errorf("cancelled") - } - return nil -} - // resolveOwnAgent finds an agent by name or id and refuses anything these // commands cannot act on: a share is someone else's agent. func resolveOwnAgent(machines []remote.Machine, query string) (*remote.Machine, error) { @@ -209,8 +166,8 @@ func agentCodeFor(m *remote.Machine) error { // agentSession resolves an owned agent and hands back a session for acting on // the account, which every subcommand below needs. -func agentSession(ctx context.Context, configPath, query string) (*remote.Machine, *client.Client, *session.Session, int) { - c, err := client.New(configPath) +func agentSession(ctx context.Context, configPath, account, query string) (*remote.Machine, *client.Client, *session.Session, int) { + c, err := client.New(configPath, account) if err != nil { return nil, nil, nil, fail("%v", err) } @@ -231,9 +188,11 @@ func agentSession(ctx context.Context, configPath, query string) (*remote.Machin func cmdAgentCode(ctx context.Context, args []string) int { fs := newFlags("agent code") + var account string var configPath string asJSON := false fs.StringVar(&configPath, "config", "", "config file path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&asJSON, "json", false, "machine-readable output") name, flagArgs := extractPositional(args) if err := fs.Parse(flagArgs); err != nil { @@ -242,7 +201,7 @@ func cmdAgentCode(ctx context.Context, args []string) int { if name == "" { return fail("usage: dwshell agent code [--json]") } - m, _, _, code := agentSession(ctx, configPath, name) + m, _, _, code := agentSession(ctx, configPath, account, name) if m == nil { return code } @@ -264,9 +223,10 @@ func cmdAgentCode(ctx context.Context, args []string) int { // and what they warn about: both are irreversible and both confirm first. func cmdAgentLifecycle(ctx context.Context, args []string, verb string) int { fs := newFlags("agent " + verb) - var configPath string + var configPath, account string assumeYes := false fs.StringVar(&configPath, "config", "", "config file path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&assumeYes, "yes", false, "do not ask for confirmation") name, flagArgs := extractPositional(args) if err := fs.Parse(flagArgs); err != nil { @@ -275,7 +235,7 @@ func cmdAgentLifecycle(ctx context.Context, args []string, verb string) int { if name == "" { return fail("usage: dwshell agent %s [--yes]", verb) } - m, _, sess, code := agentSession(ctx, configPath, name) + m, _, sess, code := agentSession(ctx, configPath, account, name) if m == nil { return code } @@ -300,7 +260,7 @@ func cmdAgentLifecycle(ctx context.Context, args []string, verb string) int { } // The command acknowledges without echoing the record, so the new code is // read back from the listing. - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -318,9 +278,11 @@ func cmdAgentLifecycle(ctx context.Context, args []string, verb string) int { func cmdAgentGroup(ctx context.Context, args []string) int { fs := newFlags("agent group") + var account string var configPath string none := false fs.StringVar(&configPath, "config", "", "config file path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&none, "none", false, "remove the agent from its group") name, flagArgs := extractPositional(args) if err := fs.Parse(flagArgs); err != nil { @@ -337,7 +299,7 @@ func cmdAgentGroup(ctx context.Context, args []string) int { return fail("usage: dwshell agent group (or --none)") } - m, _, sess, code := agentSession(ctx, configPath, name) + m, _, sess, code := agentSession(ctx, configPath, account, name) if m == nil { return code } diff --git a/cmd/dwshell/files.go b/cmd/dwshell/files.go index e6b359a..a6ab012 100644 --- a/cmd/dwshell/files.go +++ b/cmd/dwshell/files.go @@ -99,7 +99,9 @@ func cmdLs(ctx context.Context, args []string) int { var configPath string var own, shared bool fs := newFlags("ls") + var account string fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&own, "own", false, "owned agents only") fs.BoolVar(&shared, "shared", false, "incoming shares only") @@ -123,7 +125,7 @@ func cmdLs(ctx context.Context, args []string) int { return fail("%v", err) } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -162,7 +164,9 @@ func cmdGet(ctx context.Context, args []string) int { var configPath string var own, shared, recursive bool fs := newFlags("get") + var account string fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&own, "own", false, "owned agents only") fs.BoolVar(&shared, "shared", false, "incoming shares only") fs.BoolVar(&recursive, "r", false, "download a directory recursively") @@ -187,7 +191,7 @@ func cmdGet(ctx context.Context, args []string) int { return fail("%v", err) } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -238,7 +242,9 @@ func cmdSync(ctx context.Context, args []string) int { var configPath string var own, shared, sizeOnly, dryRun, del, checksum bool fs := newFlags("sync") + var account string fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&own, "own", false, "owned agents only") fs.BoolVar(&shared, "shared", false, "incoming shares only") fs.BoolVar(&sizeOnly, "size-only", false, "compare by size only (ignore mtime)") @@ -275,7 +281,7 @@ func cmdSync(ctx context.Context, args []string) int { return fail("%v", err) } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -421,7 +427,9 @@ func cmdRm(ctx context.Context, args []string) int { var configPath string var own, shared, recursive bool fs := newFlags("rm") + var account string fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&own, "own", false, "owned agents only") fs.BoolVar(&shared, "shared", false, "incoming shares only") fs.BoolVar(&recursive, "r", false, "remove directories recursively") @@ -454,7 +462,7 @@ func cmdRm(ctx context.Context, args []string) int { return fail("%v", err) } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -514,7 +522,9 @@ func cmdPut(ctx context.Context, args []string) int { var configPath string var own, shared, recursive bool fs := newFlags("put") + var account string fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&own, "own", false, "owned agents only") fs.BoolVar(&shared, "shared", false, "incoming shares only") fs.BoolVar(&recursive, "r", false, "upload a directory recursively") @@ -536,7 +546,7 @@ func cmdPut(ctx context.Context, args []string) int { return fail("%v", err) } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } diff --git a/cmd/dwshell/files_test.go b/cmd/dwshell/files_test.go index 8ea68fd..8f5458f 100644 --- a/cmd/dwshell/files_test.go +++ b/cmd/dwshell/files_test.go @@ -1,6 +1,9 @@ package main -import "testing" +import ( + "strings" + "testing" +) func TestParseRemote(t *testing.T) { tests := []struct { @@ -88,3 +91,17 @@ func TestIsRemoteEndpoint(t *testing.T) { } } } + +// --account takes a value, so the shell shortcut has to skip that value when +// looking for the agent name — otherwise `dwshell --account a@b GHE` would take +// the email for the agent. +func TestExtractAgentSkipsTheAccountValue(t *testing.T) { + agent, flags := extractAgent([]string{"--account", "a@b", "GHE", "-c", "ls"}) + if agent != "GHE" { + t.Fatalf("agent = %q, want GHE", agent) + } + joined := strings.Join(flags, " ") + if !strings.Contains(joined, "--account a@b") || !strings.Contains(joined, "-c ls") { + t.Fatalf("flags = %v", flags) + } +} diff --git a/cmd/dwshell/main.go b/cmd/dwshell/main.go index d60f5ab..78c20f1 100644 --- a/cmd/dwshell/main.go +++ b/cmd/dwshell/main.go @@ -30,7 +30,7 @@ func newFlags(name string) *flag.FlagSet { } // valueFlags are agent-command flags that consume a following value token. -var valueFlags = map[string]bool{"c": true, "term": true, "config": true, "timeout": true} +var valueFlags = map[string]bool{"c": true, "term": true, "config": true, "timeout": true, "account": true, "description": true, "group": true} // extractAgent pulls the first positional (the agent) out of args, returning it // plus the remaining flag arguments in order. It skips the value token that @@ -83,7 +83,8 @@ const usage = `dwshell — remote shell over DWService Usage: dwshell login [--user U] [--no-trusted] Authenticate and persist the session - dwshell logout Forget stored credentials + dwshell logout [--all] Forget stored credentials + dwshell account list|default|rm Manage accounts (only if you log in with several) dwshell list [--json] List machines (agents + shares) dwshell [flags] Open an interactive shell dwshell -c "command" [flags] Run a command and exit @@ -107,6 +108,7 @@ Agent flags: Global: --config path Config file (default: XDG/AppData location) + --account email Account to use when several are logged in (or DWSHELL_ACCOUNT) --version Print version and exit Remote paths: @@ -143,6 +145,8 @@ func run() int { return 0 case "login": return cmdLogin(ctx, os.Args[2:]) + case "account": + return cmdAccount(ctx, os.Args[2:]) case "logout": return cmdLogout(ctx, os.Args[2:]) case "list": @@ -181,19 +185,25 @@ func cmdLogin(ctx context.Context, args []string) int { var user, configPath string noTrusted := false fs := newFlags("login") + var account string fs.StringVar(&user, "user", "", "account user (email)") fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&noTrusted, "no-trusted", false, "do not register a trusted device") if err := fs.Parse(args); err != nil { return 2 } - c, err := client.New(configPath) + if account != "" { + return fail("--account does not apply to login: which account it touches is decided by the email you log in with") + } + c, err := client.NewForLogin(configPath) if err != nil { return fail("%v", err) } if user == "" { - user = c.Config().User + // Re-logging in with no --user refreshes the default account. + user = c.Config().Default } if user == "" { fmt.Fprint(os.Stderr, "User (email): ") @@ -238,12 +248,29 @@ func readPassword() (string, error) { func cmdLogout(ctx context.Context, args []string) int { var configPath string + all := false fs := newFlags("logout") + var account string fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") + fs.BoolVar(&all, "all", false, "forget every account") if err := fs.Parse(args); err != nil { return 2 } - c, err := client.New(configPath) + if all { + // --all must work even when several accounts are configured with no + // default, which is precisely a state someone might want to clear. + c, err := client.NewForLogin(configPath) + if err != nil { + return fail("%v", err) + } + if err := c.LogoutAll(ctx); err != nil { + return fail("%v", err) + } + fmt.Fprintln(os.Stderr, "logged out of every account") + return 0 + } + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -260,12 +287,14 @@ func cmdList(ctx context.Context, args []string) int { var configPath string asJSON := false fs := newFlags("list") + var account string fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&asJSON, "json", false, "JSON output") if err := fs.Parse(args); err != nil { return 2 } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } @@ -307,9 +336,11 @@ func cmdAgent(ctx context.Context, args []string) int { var own, shared, noTerm bool var timeout time.Duration fs := newFlags("agent") + var account string fs.StringVar(&command, "c", "", "run command and exit") fs.StringVar(&termValue, "term", "", "TERM for *nix remote") fs.StringVar(&configPath, "config", "", "config path") + fs.StringVar(&account, "account", "", "account to use when several are logged in") fs.BoolVar(&own, "own", false, "owned agents only") fs.BoolVar(&shared, "shared", false, "incoming shares only") fs.BoolVar(&noTerm, "no-term", false, "do not send TERM") @@ -334,7 +365,7 @@ func cmdAgent(ctx context.Context, args []string) int { filter = remote.SharedOnly } - c, err := client.New(configPath) + c, err := client.New(configPath, account) if err != nil { return fail("%v", err) } diff --git a/internal/client/client.go b/internal/client/client.go index f78900b..69ebb69 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -50,15 +50,27 @@ type Client struct { http *http.Client } -// New builds a Client from a config path (empty = default), seeding the cookie -// jar with any persisted node session cookie. -func New(configPath string) (*Client, error) { +// New builds a Client from a config path (empty = default) and an account +// selector (empty = DWSHELL_ACCOUNT, else the default account), seeding the +// cookie jar with that account's persisted node cookie. +// +// Selection happens here so that an unknown --account fails before a command +// starts doing work, rather than halfway through it. +func New(configPath, account string) (*Client, error) { cfg, err := config.Load(configPath) if err != nil { return nil, err } + // An empty configuration is not an error: `login` starts from one. + if len(cfg.Accounts) > 0 { + if err := cfg.Select(account); err != nil { + return nil, err + } + } else if account != "" { + return nil, fmt.Errorf("no account %q: nothing is configured yet", account) + } jar, _ := cookiejar.New(nil) - if s := cfg.Session; s != nil && len(s.Cookies) > 0 { + if s := cfg.Current().Session; s != nil && len(s.Cookies) > 0 { if u, e := neturl.Parse(s.CommandURL); e == nil { var cks []*http.Cookie for _, c := range s.Cookies { @@ -70,6 +82,19 @@ func New(configPath string) (*Client, error) { return &Client{cfg: cfg, http: &http.Client{Jar: jar, Timeout: 60 * time.Second}}, nil } +// NewForLogin builds a Client without selecting an account. `login` must work +// even when several accounts are configured and none is the default — which is +// exactly the state that makes Select refuse — because logging in is one of the +// ways out of it. +func NewForLogin(configPath string) (*Client, error) { + cfg, err := config.Load(configPath) + if err != nil { + return nil, err + } + jar, _ := cookiejar.New(nil) + return &Client{cfg: cfg, http: &http.Client{Jar: jar, Timeout: 60 * time.Second}}, nil +} + // Config exposes the underlying config (e.g. for the user name). func (c *Client) Config() *config.Config { return c.cfg } @@ -97,40 +122,66 @@ func (c *Client) Login(ctx context.Context, user, password string, registerTrust return err } + // The email is the account key: an unseen one is registered, a known one has + // its credentials replaced. Add also selects it, so what follows writes into + // the right account. + acct := c.cfg.Add(user) if err := c.persistSession(ctx, boot); err != nil { return err } - c.cfg.User = user if tdReq != nil && tdReq.Result != nil { - c.cfg.TrustedDevice = tdReq.Result + acct.TrustedDevice = tdReq.Result } return c.cfg.Save() } -// Logout deregisters the trusted device on the account (freeing its capped slot) -// and forgets local credentials. Server-side removal failure is non-fatal. +// Logout deregisters the selected account's trusted device (freeing its capped +// slot) and forgets that account. With one account this is exactly what it has +// always done. func (c *Client) Logout(ctx context.Context) error { - if td := c.cfg.TrustedDevice; td != nil { - if cfg, err := auth.FetchLoginConfig(ctx, c.http); err == nil { - _ = auth.RemoveTrustedDevice(ctx, c.http, cfg, td) + acct := c.cfg.Current() + c.deregister(ctx, acct) + if acct.User != "" { + if err := c.cfg.Remove(acct.User); err != nil { + return err } + } else { + c.cfg.Clear() + } + return c.cfg.Save() +} + +// LogoutAll forgets every account, deregistering each trusted device. +func (c *Client) LogoutAll(ctx context.Context) error { + for _, a := range c.cfg.Accounts { + c.deregister(ctx, a) } c.cfg.Clear() - c.cfg.User = "" return c.cfg.Save() } +// deregister removes an account's trusted device server-side. Failure is +// non-fatal, as it always was: the local credentials go regardless. +func (c *Client) deregister(ctx context.Context, a *config.Account) { + if a == nil || a.TrustedDevice == nil { + return + } + if cfg, err := auth.FetchLoginConfig(ctx, c.http); err == nil { + _ = auth.RemoveTrustedDevice(ctx, c.http, cfg, a.TrustedDevice) + } +} + // Session returns a valid account session, refreshing via the trusted device if // the stored session has expired. Returns ErrNeedLogin when neither is usable. func (c *Client) Session(ctx context.Context) (*session.Session, error) { - if s := c.cfg.Session; s != nil && s.SignKey != nil { + if s := c.cfg.Current().Session; s != nil && s.SignKey != nil { sess := session.Restore(s.CommandURL, s.SignKey, s.CustomHeaders, c.http) if sess.Valid(ctx) { return sess, nil } } // Session missing/expired: try a passwordless refresh. - if td := c.cfg.TrustedDevice; td != nil { + if td := c.cfg.Current().TrustedDevice; td != nil { cfg, err := auth.FetchLoginConfig(ctx, c.http) if err != nil { return nil, err @@ -145,7 +196,8 @@ func (c *Client) Session(ctx context.Context) (*session.Session, error) { if err := c.cfg.Save(); err != nil { return nil, err } - return session.Restore(c.cfg.Session.CommandURL, c.cfg.Session.SignKey, c.cfg.Session.CustomHeaders, c.http), nil + cur := c.cfg.Current().Session + return session.Restore(cur.CommandURL, cur.SignKey, cur.CustomHeaders, c.http), nil } return nil, ErrNeedLogin } @@ -160,7 +212,7 @@ func (c *Client) persistSession(ctx context.Context, boot *auth.Bootstrap) error if err := sess.Initialize(ctx); err != nil { return err } - c.cfg.Session = &config.SessionState{ + c.cfg.Current().Session = &config.SessionState{ CommandURL: sess.CommandURL(), SignKey: boot.SignKey, CustomHeaders: sess.CustomHeaders(), diff --git a/internal/config/config.go b/internal/config/config.go index ce92531..1690024 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -29,13 +29,32 @@ type SessionState struct { Cookies []NamedCookie `json:"cookies,omitempty"` } -// Config is the on-disk state. -type Config struct { +// Account is one DWService account: its user, the reusable session, and the +// optional trusted device that refreshes that session without a password. +type Account struct { User string `json:"user,omitempty"` Session *SessionState `json:"session,omitempty"` TrustedDevice *auth.TrustedDevice `json:"trustedDevice,omitempty"` +} + +// Config is the on-disk state: the accounts that have been logged in, and which +// of them commands act on when none is named. +type Config struct { + Default string `json:"default,omitempty"` + Accounts []*Account `json:"accounts,omitempty"` + + path string + selected *Account +} - path string +// Find returns the account registered for an email, or nil. +func (c *Config) Find(email string) *Account { + for _, a := range c.Accounts { + if a.User == email { + return a + } + } + return nil } // DefaultPath returns the config file path (XDG on Unix, AppData on Windows), @@ -73,6 +92,9 @@ func Load(path string) (*Config, error) { if err := json.Unmarshal(b, c); err != nil { return nil, fmt.Errorf("parse config %s: %w", path, err) } + if err := migrateFlat(b, c); err != nil { + return nil, fmt.Errorf("parse config %s: %w", path, err) + } c.path = path return c, nil } @@ -98,9 +120,3 @@ func (c *Config) Save() error { // Path returns the file path backing this config. func (c *Config) Path() string { return c.path } - -// Clear removes all persisted credentials (used by logout). -func (c *Config) Clear() { - c.Session = nil - c.TrustedDevice = nil -} diff --git a/internal/config/migrate.go b/internal/config/migrate.go new file mode 100644 index 0000000..9682422 --- /dev/null +++ b/internal/config/migrate.go @@ -0,0 +1,42 @@ +package config + +import ( + "encoding/json" + + "github.com/porech/dwshell/internal/auth" +) + +// flatConfig is the pre-accounts on-disk shape: one account's fields sitting at +// the top level, which is what every dwshell before multiple accounts wrote. +type flatConfig struct { + User string `json:"user"` + Session *SessionState `json:"session"` + TrustedDevice *auth.TrustedDevice `json:"trustedDevice"` +} + +// migrateFlat converts a pre-accounts configuration into a single account, in +// memory. It runs on every load and never writes: an untouched old file keeps +// working indefinitely, and the new shape reaches the disk only when something +// saves for a reason of its own. A read-only command must not rewrite the +// user's configuration behind their back. +// +// A file with neither accounts nor flat fields is simply empty — a first run. +func migrateFlat(body []byte, c *Config) error { + if len(c.Accounts) > 0 { + return nil // already the new shape + } + var flat flatConfig + if err := json.Unmarshal(body, &flat); err != nil { + return err + } + if flat.Session == nil && flat.TrustedDevice == nil && flat.User == "" { + return nil + } + c.Accounts = []*Account{{ + User: flat.User, + Session: flat.Session, + TrustedDevice: flat.TrustedDevice, + }} + c.Default = flat.User + return nil +} diff --git a/internal/config/migrate_test.go b/internal/config/migrate_test.go new file mode 100644 index 0000000..ab415c9 --- /dev/null +++ b/internal/config/migrate_test.go @@ -0,0 +1,149 @@ +package config + +import ( + "encoding/json" + + "github.com/porech/dwshell/internal/auth" + "os" + "path/filepath" + "testing" +) + +// flatConfigFixture builds the shape dwshell wrote before accounts existed, +// with the structure of a real file: a session carrying a genuine signing key +// and the node cookie, plus a trusted device with a key of its own. The keys +// are generated rather than invented, because they are parsed on load — a +// made-up JWK would fail for reasons that have nothing to do with migration. +func flatConfigFixture(t *testing.T) string { + t.Helper() + sessionKey, err := auth.NewSignKey() + if err != nil { + t.Fatal(err) + } + deviceKey, err := auth.NewSignKey() + if err != nil { + t.Fatal(err) + } + flat := map[string]any{ + "user": "ale@example.net", + "session": map[string]any{ + "commandUrl": "https://node1.dwservice.net/ses/ND1/tok.dw", + "signKey": sessionKey, + "customHeaders": true, + "cookies": []map[string]string{{"name": "DWSID", "value": "abc"}}, + }, + "trustedDevice": map[string]any{ + "id": "dev1", + "name": "dwshell on laptop", + "authKey": deviceKey, + }, + } + b, err := json.Marshal(flat) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func writeConfig(t *testing.T, body string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + return p +} + +func TestLoadMigratesAFlatConfig(t *testing.T) { + c, err := Load(writeConfig(t, flatConfigFixture(t))) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Accounts) != 1 { + t.Fatalf("expected one account, got %d", len(c.Accounts)) + } + a := c.Accounts[0] + if a.User != "ale@example.net" { + t.Errorf("user = %q", a.User) + } + if a.Session == nil || a.Session.CommandURL == "" || a.Session.SignKey == nil { + t.Error("the session must survive the migration whole") + } + if len(a.Session.Cookies) != 1 || a.Session.Cookies[0].Name != "DWSID" { + t.Error("the node cookie must survive: without it the session cannot be reused") + } + if a.TrustedDevice == nil || a.TrustedDevice.ID != "dev1" { + t.Error("the trusted device must survive, or passwordless refresh breaks") + } + if c.Default != "ale@example.net" { + t.Errorf("the migrated account becomes the default, got %q", c.Default) + } +} + +// Migrating must not touch the file: a read-only command should never rewrite +// the user's configuration behind their back. +func TestLoadDoesNotRewriteTheFile(t *testing.T) { + p := writeConfig(t, flatConfigFixture(t)) + before, _ := os.ReadFile(p) + if _, err := Load(p); err != nil { + t.Fatalf("Load: %v", err) + } + after, _ := os.ReadFile(p) + if string(before) != string(after) { + t.Fatal("Load rewrote the configuration file") + } +} + +// Saving a migrated config writes the new shape and leaves the flat keys behind. +func TestSaveWritesTheAccountsShape(t *testing.T) { + p := writeConfig(t, flatConfigFixture(t)) + c, err := Load(p) + if err != nil { + t.Fatalf("Load: %v", err) + } + if err := c.Save(); err != nil { + t.Fatalf("Save: %v", err) + } + var raw map[string]any + b, _ := os.ReadFile(p) + if err := json.Unmarshal(b, &raw); err != nil { + t.Fatal(err) + } + if _, ok := raw["accounts"]; !ok { + t.Error("the saved file must carry accounts") + } + for _, gone := range []string{"user", "session", "trustedDevice"} { + if _, ok := raw[gone]; ok { + t.Errorf("the flat key %q must not be written back", gone) + } + } + // and the credentials must still be there after the round trip + again, err := Load(p) + if err != nil { + t.Fatalf("reload: %v", err) + } + if len(again.Accounts) != 1 || again.Accounts[0].TrustedDevice == nil { + t.Fatal("the round trip lost the trusted device") + } +} + +func TestLoadOfANewShapeIsUnchanged(t *testing.T) { + body := `{"default":"a@b","accounts":[{"user":"a@b"},{"user":"c@d"}]}` + c, err := Load(writeConfig(t, body)) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Accounts) != 2 || c.Default != "a@b" { + t.Fatalf("got %d accounts, default %q", len(c.Accounts), c.Default) + } +} + +func TestLoadOfAMissingFileIsEmpty(t *testing.T) { + c, err := Load(filepath.Join(t.TempDir(), "absent.json")) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Accounts) != 0 { + t.Fatal("a missing file means no accounts, not an error") + } +} diff --git a/internal/config/select.go b/internal/config/select.go new file mode 100644 index 0000000..c345808 --- /dev/null +++ b/internal/config/select.go @@ -0,0 +1,128 @@ +package config + +import ( + "errors" + "fmt" + "os" + "strings" +) + +// ErrNoAccounts means nothing has been logged in yet. +var ErrNoAccounts = errors.New("no account configured: run `dwshell login`") + +// Emails lists the registered accounts, in the order they were added. +func (c *Config) Emails() []string { + out := make([]string, 0, len(c.Accounts)) + for _, a := range c.Accounts { + out = append(out, a.User) + } + return out +} + +// Select picks the account commands will act on: the argument if given, else +// DWSHELL_ACCOUNT, else the default. +// +// With a single account there is nothing to choose, and it is used whether or +// not it is marked default — which is what keeps this out of the way of someone +// who only ever logs in once. With several accounts and no default it refuses +// rather than guessing: picking one silently would point the next command at +// the wrong account. +func (c *Config) Select(email string) error { + if len(c.Accounts) == 0 { + return ErrNoAccounts + } + if email == "" { + email = os.Getenv("DWSHELL_ACCOUNT") + } + if email == "" { + if len(c.Accounts) == 1 { + c.selected = c.Accounts[0] + return nil + } + email = c.Default + } + if email == "" { + return fmt.Errorf("several accounts are configured and none is the default; "+ + "choose one with `dwshell account default ` or pass --account (%s)", + strings.Join(c.Emails(), ", ")) + } + a := c.Find(email) + if a == nil { + return fmt.Errorf("no account %q; registered accounts: %s", email, strings.Join(c.Emails(), ", ")) + } + c.selected = a + return nil +} + +// Current is the selected account. When nothing was selected it falls back to a +// lone account, so the single-account paths that predate this feature behave +// exactly as they did. +func (c *Config) Current() *Account { + if c.selected != nil { + return c.selected + } + if len(c.Accounts) == 1 { + return c.Accounts[0] + } + return &Account{} +} + +// Add returns the account for an email, registering it if it is new, and +// selects it. The first account registered becomes the default, so a +// single-account user never meets the concept. +func (c *Config) Add(user string) *Account { + if a := c.Find(user); a != nil { + c.selected = a + return a + } + a := &Account{User: user} + c.Accounts = append(c.Accounts, a) + if len(c.Accounts) == 1 { + c.Default = user + } + c.selected = a + return a +} + +// Remove forgets an account. If exactly one remains it is promoted, there being +// nothing to choose between; if several remain the default is left unset rather +// than guessed at. +func (c *Config) Remove(email string) error { + idx := -1 + for i, a := range c.Accounts { + if a.User == email { + idx = i + break + } + } + if idx < 0 { + return fmt.Errorf("no account %q; registered accounts: %s", email, strings.Join(c.Emails(), ", ")) + } + c.Accounts = append(c.Accounts[:idx], c.Accounts[idx+1:]...) + if c.selected != nil && c.selected.User == email { + c.selected = nil + } + if c.Default == email { + c.Default = "" + if len(c.Accounts) == 1 { + c.Default = c.Accounts[0].User + } + } + return nil +} + +// SetDefault marks a registered account as the one commands use when none is named. +func (c *Config) SetDefault(email string) error { + if c.Find(email) == nil { + return fmt.Errorf("no account %q; registered accounts: %s", email, strings.Join(c.Emails(), ", ")) + } + c.Default = email + return nil +} + +// Clear forgets every account (logout --all). +func (c *Config) Clear() { + c.Accounts = nil + c.Default = "" + c.selected = nil +} diff --git a/internal/config/select_test.go b/internal/config/select_test.go new file mode 100644 index 0000000..bfeb436 --- /dev/null +++ b/internal/config/select_test.go @@ -0,0 +1,148 @@ +package config + +import ( + "strings" + "testing" +) + +func twoAccounts() *Config { + return &Config{ + Default: "a@b", + Accounts: []*Account{{User: "a@b"}, {User: "c@d"}}, + } +} + +func TestSelectPrefersTheFlagOverTheEnvironment(t *testing.T) { + t.Setenv("DWSHELL_ACCOUNT", "c@d") + c := twoAccounts() + if err := c.Select("a@b"); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "a@b" { + t.Fatalf("the flag must win, got %q", c.Current().User) + } +} + +func TestSelectFallsBackToTheEnvironment(t *testing.T) { + t.Setenv("DWSHELL_ACCOUNT", "c@d") + c := twoAccounts() + if err := c.Select(""); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "c@d" { + t.Fatalf("the environment must be used, got %q", c.Current().User) + } +} + +func TestSelectFallsBackToTheDefault(t *testing.T) { + t.Setenv("DWSHELL_ACCOUNT", "") + c := twoAccounts() + if err := c.Select(""); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "a@b" { + t.Fatalf("the default must be used, got %q", c.Current().User) + } +} + +func TestSelectUnknownAccountListsTheKnownOnes(t *testing.T) { + c := twoAccounts() + err := c.Select("nope@x") + if err == nil { + t.Fatal("an unknown account must fail") + } + for _, want := range []string{"a@b", "c@d"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the error should list %q, got %q", want, err) + } + } +} + +// With one account there is nothing to choose, so it is used whether or not it +// is marked default. This is what keeps the feature invisible to someone who +// only ever logs in once. +func TestSelectWithOneAccountAndNoDefault(t *testing.T) { + t.Setenv("DWSHELL_ACCOUNT", "") + c := &Config{Accounts: []*Account{{User: "solo@x"}}} + if err := c.Select(""); err != nil { + t.Fatalf("Select: %v", err) + } + if c.Current().User != "solo@x" { + t.Fatal("the lone account is used whether or not it is marked default") + } +} + +func TestSelectWithSeveralAndNoDefaultAsksForOne(t *testing.T) { + t.Setenv("DWSHELL_ACCOUNT", "") + c := &Config{Accounts: []*Account{{User: "a@b"}, {User: "c@d"}}} + err := c.Select("") + if err == nil { + t.Fatal("with no default and several accounts it must refuse rather than guess") + } + if !strings.Contains(err.Error(), "account default") { + t.Errorf("the error should say how to fix it, got %q", err) + } +} + +func TestSelectWithNoAccountsSaysToLogIn(t *testing.T) { + c := &Config{} + if err := c.Select(""); err == nil { + t.Fatal("with no accounts at all it must fail") + } +} + +func TestAddMakesTheFirstAccountTheDefault(t *testing.T) { + c := &Config{} + c.Add("first@x") + if c.Default != "first@x" { + t.Fatalf("default = %q, want first@x", c.Default) + } + c.Add("second@x") + if c.Default != "first@x" { + t.Fatal("a later account must not steal the default") + } + if len(c.Accounts) != 2 { + t.Fatalf("got %d accounts", len(c.Accounts)) + } +} + +func TestAddIsIdempotentForTheSameEmail(t *testing.T) { + c := &Config{} + a1 := c.Add("same@x") + a2 := c.Add("same@x") + if a1 != a2 || len(c.Accounts) != 1 { + t.Fatal("logging in again with the same email updates that account") + } +} + +func TestRemovePromotesALoneSurvivor(t *testing.T) { + c := twoAccounts() + if err := c.Remove("a@b"); err != nil { + t.Fatalf("Remove: %v", err) + } + if c.Default != "c@d" { + t.Fatalf("with one account left it becomes the default, got %q", c.Default) + } +} + +func TestRemoveLeavesNoDefaultWhenSeveralRemain(t *testing.T) { + c := &Config{Default: "a@b", Accounts: []*Account{{User: "a@b"}, {User: "c@d"}, {User: "e@f"}}} + if err := c.Remove("a@b"); err != nil { + t.Fatalf("Remove: %v", err) + } + if c.Default != "" { + t.Fatalf("dwshell must not pick a default among several, got %q", c.Default) + } +} + +func TestRemoveUnknownAccountFails(t *testing.T) { + if err := twoAccounts().Remove("nope@x"); err == nil { + t.Fatal("removing an unregistered account must fail") + } +} + +func TestSetDefaultRejectsAnUnregisteredAccount(t *testing.T) { + if err := twoAccounts().SetDefault("nope@x"); err == nil { + t.Fatal("the default must name a registered account") + } +}