diff --git a/README.md b/README.md index 60911e7..9a6e0d7 100644 --- a/README.md +++ b/README.md @@ -286,9 +286,84 @@ On first startup, webhooker automatically generates a cryptographically secure session encryption key and stores it in the database. This key persists across restarts — no manual key management is needed. -On first startup, webhooker creates an `admin` user -with a randomly generated password and logs it to stdout. This password -is only displayed once. +#### The admin account + +On first startup — a `DATA_DIR` with no accounts in it — webhooker +creates an `admin` user with a randomly generated password and prints +it to standard output as a ruled banner: + +``` +======================================================================== +WEBHOOKER FIRST BOOT: an admin account has been created. + + username: admin + password: 3xamPl3-p4ssw0rd + +Save this password now: it is shown only here, and only once. +If it is lost, run `webhooker resetpw admin` on a stopped deployment. +======================================================================== +``` + +It is a banner rather than a log line because that is the only time it +is ever shown: as one `INFO` record it sat among the roughly 45 fx +`PROVIDE`/`RUN`/`HOOK` lines a boot writes, and under `docker run -d` +it is one line in a log subject to rotation. The database stores only +its Argon2id hash. There is no second account and no forgot-password +flow, so the banner and the reset command below are the only two ways +in. + +#### Recovering a lost admin password + +`webhooker resetpw` sets an existing account's password from the +command line: + +```bash +# Generate a new password and print it. +DATA_DIR=/var/lib/webhooker webhooker resetpw -generate admin + +# Or supply one on standard input (minimum 8 characters). +printf '%s' "$NEW_PASSWORD" | \ + DATA_DIR=/var/lib/webhooker webhooker resetpw admin +``` + +In a container it is the same binary, which the image sets as `CMD` +rather than `ENTRYPOINT`, so the whole command has to be given: + +```bash +docker run --rm -v webhooker-data:/var/lib/webhooker \ + webhooker /app/webhooker resetpw -generate admin +``` + +Stop the service first — with the volume still attached to a running +container, the command refuses. + +The password is never taken as a command-line argument: on Linux argv +is readable through `/proc` by every account on the host for as long as +the process lives. Standard input is echoed when it is a terminal — the +prompt says so — so `-generate` or a pipe is preferable on a shared +machine. + +What it will not do: + +- **Run against a live deployment.** It takes the same exclusive + `DATA_DIR` lock the server does (see + [Single-instance lock](#single-instance-lock)) and refuses while a + running instance holds it, naming the directory and exiting non-zero. + A running process keeps serving every session that authenticated with + the old password, so a reset underneath it would report a change the + service does not honour. +- **Create anything.** A `DATA_DIR` that does not exist, or that holds + no `webhooker.db`, is an error rather than a new empty deployment — + a mistyped path must not be built out and then reported as a success. +- **Create an account.** A username that does not exist is an error. + `resetpw` changes an existing account's password and nothing else. + +`DATA_DIR` selects the deployment exactly as it does for the server. A +password that changes on disk takes effect at the next login; sessions +that are already authenticated are unaffected either way. + +Changing a password you still know needs none of this — use +`POST /user/{username}/password` in the web UI. #### What `DEBUG=true` exposes @@ -325,11 +400,14 @@ What it does **not** put in the log: What is in the log regardless of `DEBUG`, and is not a debug-logging decision: -- **The initial `admin` password**, in the clear, once, at `INFO`, on - the first boot that creates the account. That line is the only place +- **The initial `admin` password**, in the clear, once, on the first + boot that creates the account — as the banner described under + [The admin account](#the-admin-account), written straight to standard + output rather than through the logger. That banner is the only place it is ever shown; the database stores the hash. A first boot's output is not safe to paste anywhere until that account's password has been - changed. + changed. The same applies to `webhooker resetpw -generate`, which + prints the password it generated in the same form. - **An authenticated operator's own configuration**, echoed back untruncated — webhook names, target hostnames. See the logging section under Security for the full list and for the per-line size @@ -842,7 +920,10 @@ A registered user of the webhooker service. Passwords are hashed with Argon2id using secure defaults (64 MB memory, 1 iteration, 4 threads, 32-byte key, 16-byte salt). On first startup, an `admin` user is created with a randomly generated 16-character -password logged to stdout. +password printed once to stdout; `webhooker resetpw` sets it again if +it is lost (see [The admin account](#the-admin-account)). Every one of +those paths hashes through the same `internal/database` code, so the +parameters cannot drift between them. #### Webhook @@ -1994,8 +2075,12 @@ imports. The entry point is `cmd/webhooker/main.go`. ``` webhooker/ ├── cmd/webhooker/ -│ └── main.go # Entry point: sets globals, locks DATA_DIR, wires fx +│ └── main.go # Entry point: subcommand dispatch; no args locks DATA_DIR and wires fx ├── internal/ +│ ├── banner/ +│ │ └── banner.go # Ruled block for the one credential shown in the clear +│ ├── resetpw/ +│ │ └── resetpw.go # `webhooker resetpw`: set an account's password, stopped deployments only │ ├── config/ │ │ └── config.go # Configuration loading from environment variables │ ├── database/ @@ -2191,6 +2276,9 @@ check, see [The login endpoint](#the-login-endpoint). header. API keys are stored per-user with usage tracking (`last_used_at`). - **Metrics:** Basic authentication protecting the `/metrics` endpoint. +- **Recovery:** `webhooker resetpw ` on a stopped deployment + is the only way back into an account whose password was lost (see + [Recovering a lost admin password](#recovering-a-lost-admin-password)). ### Security diff --git a/cmd/webhooker/main.go b/cmd/webhooker/main.go index 0bd5d32..1631256 100644 --- a/cmd/webhooker/main.go +++ b/cmd/webhooker/main.go @@ -17,6 +17,7 @@ import ( "sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/middleware" + "sneak.berlin/go/webhooker/internal/resetpw" "sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/session" ) @@ -48,6 +49,11 @@ import ( // and can still consume the whole budget on their own. const stopTimeout = 5 * time.Second +// exitUsage is the status for a command line this binary cannot make +// sense of, kept distinct from the 1 a refusal exits with so that a +// caller can tell "called wrong" from "declined". +const exitUsage = 2 + // Build-time variables set via -ldflags. // //nolint:gochecknoglobals // Build-time variables injected by the linker. @@ -60,7 +66,54 @@ func main() { globals.Appname = appname globals.Version = version - os.Exit(run(os.Stderr)) + os.Exit(dispatch(os.Args[1:], os.Stdin, os.Stdout, os.Stderr)) +} + +// dispatch routes the command line to a subcommand. +// +// No arguments runs the server, which is what the image's CMD and +// every existing deployment invoke; that path is unchanged, including +// where the DATA_DIR lock is taken relative to building the fx graph +// and how fx propagates a non-zero exit itself. +func dispatch( + args []string, + stdin io.Reader, + stdout, stderr io.Writer, +) int { + if len(args) == 0 { + return run(stderr) + } + + switch args[0] { + case resetpw.Name: + return resetpw.Run(args[1:], stdin, stdout, stderr) + case "help", "-h", "-help", "--help": + usage(stdout) + + return 0 + default: + _, _ = fmt.Fprintf( + stderr, "%s: unknown subcommand %q\n", appname, args[0], + ) + usage(stderr) + + return exitUsage + } +} + +// usage lists what the binary can be asked to do. +func usage(w io.Writer) { + _, _ = fmt.Fprintf(w, `usage: %s [subcommand] + +With no subcommand, runs the webhooker server. + +Subcommands: + %s [-generate] + Set an existing account's password on a stopped deployment. + Recovers an admin account whose bootstrap password was lost. + help + Print this message. +`, appname, resetpw.Name) } // run takes the exclusive DATA_DIR lock, then runs the application diff --git a/cmd/webhooker/main_test.go b/cmd/webhooker/main_test.go index 15bb53b..878ee24 100644 --- a/cmd/webhooker/main_test.go +++ b/cmd/webhooker/main_test.go @@ -2,12 +2,14 @@ package main import ( "bytes" + "strings" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sneak.berlin/go/webhooker/internal/datadir" + "sneak.berlin/go/webhooker/internal/resetpw" "sneak.berlin/go/webhooker/internal/server" ) @@ -68,6 +70,65 @@ func TestRunRefusesLockedDataDir(t *testing.T) { assert.Contains(t, stderr.String(), "another instance") } +// TestDispatch_NoArgumentsRunsTheServer pins the routing of a bare +// invocation, which is what the image's CMD and every deployment use. +// Adding subcommands must not move the server off the empty argument +// list, and must not move the DATA_DIR lock: this asserts the refusal +// arrives with no fx graph built, exactly as run does on its own. +func TestDispatch_NoArgumentsRunsTheServer(t *testing.T) { + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + lock, err := datadir.Acquire(dir) + require.NoError(t, err) + + defer func() { _ = lock.Release() }() + + var stdout, stderr bytes.Buffer + + code := dispatch(nil, strings.NewReader(""), &stdout, &stderr) + + require.Equal(t, 1, code) + assert.Contains(t, stderr.String(), "another instance") +} + +// TestDispatch_UnknownSubcommand keeps a mistyped subcommand from +// starting a server. Anything else would have `webhooker resetpww` +// silently take the DATA_DIR lock and serve. +func TestDispatch_UnknownSubcommand(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + + code := dispatch( + []string{"resetpww", "admin"}, + strings.NewReader(""), &stdout, &stderr, + ) + + require.Equal(t, 2, code) + assert.Contains(t, stderr.String(), "unknown subcommand") + assert.Contains( + t, stderr.String(), resetpw.Name, + "the usage must name the subcommand that does exist", + ) +} + +// TestDispatch_Help answers on standard output with a zero status, so +// `webhooker help` is usable in a pipe. +func TestDispatch_Help(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + + code := dispatch( + []string{"help"}, strings.NewReader(""), &stdout, &stderr, + ) + + require.Equal(t, 0, code) + assert.Empty(t, stderr.String()) + assert.Contains(t, stdout.String(), resetpw.Name) +} + // tailHeadroom is the slack the fx stop budget must keep beyond the // server stop hook. The hooks that run after the server — the // delivery engine, the healthcheck, the webhook DB manager and the diff --git a/internal/banner/banner.go b/internal/banner/banner.go new file mode 100644 index 0000000..33dc6af --- /dev/null +++ b/internal/banner/banner.go @@ -0,0 +1,47 @@ +// Package banner renders the operator-facing blocks that carry a +// plaintext credential. +// +// A generated password printed as one more structured log line is lost: +// a boot writes roughly 45 fx PROVIDE/RUN/HOOK lines around it, and +// under `docker run -d` it is one line in a log subject to rotation. A +// credential that is shown exactly once has to be findable by eye when +// an operator scrolls back, so it is written as a ruled block rather +// than as a log record. +// +// It is deliberately not a log line: it goes straight to the writer the +// caller names — standard output for both the first-boot account and +// the `resetpw` subcommand — so it is neither levelled, filtered, nor +// rendered as JSON by whichever handler internal/logger installed. +package banner + +import ( + "fmt" + "io" + "strings" +) + +// ruleWidth is the length of the horizontal rules, chosen to fit an +// 80-column terminal without wrapping. +const ruleWidth = 72 + +// Credentials writes a ruled block naming an account and its plaintext +// password. headline says which event produced it, and note says what +// the operator must do about it; both are written verbatim, so a +// multi-line note must already be wrapped. +func Credentials( + w io.Writer, + headline, username, password, note string, +) error { + rule := strings.Repeat("=", ruleWidth) + + _, err := fmt.Fprintf( + w, + "\n%s\n%s\n\n username: %s\n password: %s\n\n%s\n%s\n\n", + rule, headline, username, password, note, rule, + ) + if err != nil { + return fmt.Errorf("writing credentials banner: %w", err) + } + + return nil +} diff --git a/internal/banner/banner_test.go b/internal/banner/banner_test.go new file mode 100644 index 0000000..f91e7cb --- /dev/null +++ b/internal/banner/banner_test.go @@ -0,0 +1,59 @@ +package banner_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/banner" +) + +// TestCredentials_IsFindableByEye pins the properties that make the +// block worth having: rules above and below it, the two fields on +// their own lines, and blank lines separating it from whatever the +// surrounding log wrote. +func TestCredentials_IsFindableByEye(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + + require.NoError(t, banner.Credentials( + &out, "HEADLINE", "admin", "s3cret", "NOTE", + )) + + got := out.String() + lines := strings.Split(strings.Trim(got, "\n"), "\n") + + require.GreaterOrEqual(t, len(lines), 3) + assert.Equal(t, lines[0], lines[len(lines)-1], "rules must match") + assert.Greater( + t, len(lines[0]), 40, "the rule must be visible at a glance", + ) + assert.Equal(t, strings.Repeat("=", len(lines[0])), lines[0]) + + assert.Contains(t, got, "\n username: admin\n") + assert.Contains(t, got, "\n password: s3cret\n") + assert.Contains(t, got, "HEADLINE") + assert.Contains(t, got, "NOTE") + assert.True(t, strings.HasPrefix(got, "\n")) +} + +// failingWriter reports the write error a banner must not swallow: it +// is the one copy of a password that will never be shown again. +type failingWriter struct{} + +func (failingWriter) Write([]byte) (int, error) { + return 0, assert.AnError +} + +func TestCredentials_ReportsAWriteFailure(t *testing.T) { + t.Parallel() + + err := banner.Credentials( + failingWriter{}, "HEADLINE", "admin", "s3cret", "NOTE", + ) + + require.ErrorIs(t, err, assert.AnError) +} diff --git a/internal/database/bootstrap_banner_test.go b/internal/database/bootstrap_banner_test.go new file mode 100644 index 0000000..f56b467 --- /dev/null +++ b/internal/database/bootstrap_banner_test.go @@ -0,0 +1,85 @@ +package database_test + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/database" +) + +// passwordField is the banner line carrying the plaintext. +const passwordField = "password: " + +// bannerPassword returns the password the banner printed. +func bannerPassword(t *testing.T, out string) string { + t.Helper() + + for line := range strings.SplitSeq(out, "\n") { + _, value, found := strings.Cut(line, passwordField) + if found { + return strings.TrimSpace(value) + } + } + + t.Fatalf("no %q line in the banner:\n%s", passwordField, out) + + return "" +} + +// TestFirstBoot_PrintsTheAdminPasswordAsABanner is the bootstrap half +// of https://git.eeqj.de/sneak/webhooker/issues/208. +// +// The password is shown exactly once, and it used to be shown as one +// slog record among the roughly 45 fx PROVIDE/RUN/HOOK lines a boot +// writes — which is how deployments lost it and, with no reset path, +// locked themselves out. It must be emitted as a block an operator can +// find by eye, it must carry the plaintext that actually opens the +// account, and it must name the command that recovers it. +func TestFirstBoot_PrintsTheAdminPasswordAsABanner(t *testing.T) { + t.Parallel() + + db, lc := setupTestDB(t) + + var out bytes.Buffer + + db.ExportSetBannerOut(&out) + + ctx := context.Background() + require.NoError(t, lc.Start(ctx)) + + defer func() { require.NoError(t, lc.Stop(ctx)) }() + + printed := out.String() + + require.Contains( + t, printed, strings.Repeat("=", 20), + "the banner must be ruled off, not read as one more log line", + ) + require.Contains(t, printed, "username: admin") + assert.Contains( + t, printed, "resetpw", + "the banner must name the command that recovers the account", + ) + + password := bannerPassword(t, printed) + require.NotEmpty(t, password) + + // The printed plaintext must be the one that opens the account: + // a banner showing a different string would be worse than none. + var user database.User + + require.NoError( + t, + db.DB().Where("username = ?", "admin").First(&user).Error, + ) + + ok, err := database.VerifyPassword(password, user.Password) + require.NoError(t, err) + assert.True( + t, ok, "the printed password must open the seeded account", + ) +} diff --git a/internal/database/database.go b/internal/database/database.go index b6a1494..e92b440 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -8,6 +8,7 @@ import ( "encoding/base64" "errors" "fmt" + "io" "log/slog" "os" "path/filepath" @@ -16,6 +17,7 @@ import ( "gorm.io/driver/sqlite" "gorm.io/gorm" _ "modernc.org/sqlite" // Pure Go SQLite driver + "sneak.berlin/go/webhooker/internal/banner" "sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/gormlog" "sneak.berlin/go/webhooker/internal/logger" @@ -27,6 +29,20 @@ const ( sessionKeyLen = 32 ) +// MainDBFileName is the main application database inside DATA_DIR. It +// is exported so that an entry point acting on a data directory +// outside the fx graph can test for a deployment's existence without +// spelling the name a second time. +const MainDBFileName = "webhooker.db" + +// BootstrapPasswordNote is what the first-boot banner tells the +// operator to do about the password it just printed. It names the +// recovery command, because the moment that line scrolls away is +// exactly when the operator needs to know one exists. +const BootstrapPasswordNote = "Save this password now: it is shown " + + "only here, and only once.\nIf it is lost, run `webhooker " + + "resetpw admin` on a stopped deployment." + //nolint:revive // DatabaseParams is a standard fx naming convention. type DatabaseParams struct { fx.In @@ -40,6 +56,39 @@ type Database struct { db *gorm.DB log *slog.Logger params *DatabaseParams + + // bannerOut receives the first-boot credentials banner. Nil means + // os.Stdout, resolved at write time rather than at construction so + // that a caller which redirects the variable still captures it. + bannerOut io.Writer +} + +// Open connects to the main database in dataDir and migrates it, +// without the fx lifecycle and without seeding an admin account. +// +// It is for entry points that act on an existing deployment's data +// directory from outside the server graph — `webhooker resetpw`. Such a +// caller must already hold the DATA_DIR lock (see internal/datadir), +// and must Close the result. +// +// It does not create the admin account: seeding belongs to a server +// start, and a maintenance command that silently invented an account +// would answer "no such user" by creating one. +func Open(dataDir string, log *slog.Logger) (*Database, error) { + d := &Database{log: log} + + err := d.connectTo(dataDir) + if err != nil { + return nil, err + } + + return d, nil +} + +// Close closes the underlying connection. It is the exported form of +// the fx stop hook, for callers that built the Database with Open. +func (d *Database) Close() error { + return d.close() } // New creates a Database that connects on fx start and disconnects on stop. @@ -122,10 +171,22 @@ func (d *Database) GetOrCreateSessionKey() (string, error) { return encoded, nil } +// connect opens the configured data directory and, this being a +// server start, seeds the admin account when the deployment has none. func (d *Database) connect() error { - // Ensure the data directory exists before opening the database. - dataDir := d.params.Config.DataDir + err := d.connectTo(d.params.Config.DataDir) + if err != nil { + return err + } + return d.ensureAdminUser() +} + +// connectTo opens and migrates the main database in dataDir. It seeds +// nothing: whether an empty deployment gets an admin account is the +// caller's decision. +func (d *Database) connectTo(dataDir string) error { + // Ensure the data directory exists before opening the database. err := os.MkdirAll(dataDir, dataDirPerm) if err != nil { return fmt.Errorf( @@ -136,7 +197,7 @@ func (d *Database) connect() error { } // Construct the main application database path inside DATA_DIR. - dbPath := filepath.Join(dataDir, "webhooker.db") + dbPath := filepath.Join(dataDir, MainDBFileName) dbURL := fmt.Sprintf( "file:%s?cache=shared&mode=rwc", dbPath, @@ -190,10 +251,16 @@ func (d *Database) migrate() error { d.log.Info("database migrations completed") + return nil +} + +// ensureAdminUser creates the bootstrap admin account when the +// deployment has no users at all. +func (d *Database) ensureAdminUser() error { // Check if admin user exists var userCount int64 - err = d.db.Model(&User{}).Count(&userCount).Error + err := d.db.Model(&User{}).Count(&userCount).Error if err != nil { d.log.Error( "failed to count users", @@ -253,16 +320,46 @@ func (d *Database) createAdminUser() error { return err } - d.log.Info("admin user created", - "username", "admin", - "password", password, - "message", - "SAVE THIS PASSWORD - it will not be shown again!", + // The plaintext leaves this process here and nowhere else. It is + // deliberately not a log field: as one INFO record among the fx + // graph's own output it read as one more startup line, which is + // how deployments lost it. See internal/banner. + err = banner.Credentials( + d.banner(), + "WEBHOOKER FIRST BOOT: an admin account has been created.", + adminUser.Username, + password, + BootstrapPasswordNote, ) + if err != nil { + // Fail the start. The account is already committed, so the + // next boot seeds nothing and prints nothing: continuing here + // would hand the operator a running service whose only + // password was never shown. `webhooker resetpw` recovers it. + d.log.Error( + "failed to print the admin credentials banner", + "error", err, + ) + + return err + } + + d.log.Info("admin user created", "username", adminUser.Username) return nil } +// banner returns where the credentials banner is written. os.Stdout is +// resolved here rather than stored, so that a test which redirects the +// variable captures the banner. +func (d *Database) banner() io.Writer { + if d.bannerOut != nil { + return d.bannerOut + } + + return os.Stdout +} + func (d *Database) close() error { if d.db != nil { sqlDB, err := d.db.DB() diff --git a/internal/database/export_test.go b/internal/database/export_test.go index 8d38b86..5c10271 100644 --- a/internal/database/export_test.go +++ b/internal/database/export_test.go @@ -2,6 +2,7 @@ package database import ( "context" + "io" "log/slog" "os" "time" @@ -66,6 +67,13 @@ func (r *RetentionReaper) ExportSetInterval(d time.Duration) { r.interval = d } +// ExportSetBannerOut redirects the first-boot credentials banner, so a +// test can read what the operator would have seen. It must be called +// before the fx start hook runs, which is where the account is seeded. +func (d *Database) ExportSetBannerOut(w io.Writer) { + d.bannerOut = w +} + // DummyPasswordHashForTest exposes the encoded hash that unknown // usernames are verified against. func DummyPasswordHashForTest() string { diff --git a/internal/resetpw/resetpw.go b/internal/resetpw/resetpw.go new file mode 100644 index 0000000..10b57fa --- /dev/null +++ b/internal/resetpw/resetpw.go @@ -0,0 +1,472 @@ +// Package resetpw implements the `webhooker resetpw` subcommand, +// which sets an existing account's password from the command line. +// +// It exists because the bootstrap password is shown exactly once. If it +// is lost — the boot's output rotated away, the terminal closed — the +// deployment has no other way in: there is no second account, no +// forgot-password flow, and no environment override. The only recovery +// before this command was deleting the row from webhooker.db with a +// SQLite client so the next start would re-seed. +// +// It operates on a stopped deployment only. The password is read from +// standard input or generated, never taken from argv, and it reuses the +// service's own Argon2id hashing rather than reimplementing it. +package resetpw + +import ( + "bufio" + "errors" + "flag" + "fmt" + "io" + "io/fs" + "log/slog" + "os" + "path/filepath" + "strings" + + "gorm.io/gorm" + "sneak.berlin/go/webhooker/internal/banner" + "sneak.berlin/go/webhooker/internal/config" + "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/datadir" +) + +// Name is the subcommand's name on the command line. +const Name = "resetpw" + +const ( + // generatedPasswordLen is the length of a -generate password. It + // is longer than the 16 characters the first boot generates: this + // one is typed or pasted once by an operator recovering a + // deployment, not carried around. + generatedPasswordLen = 24 + + // minPasswordLen is the shortest password accepted on standard + // input. It is a floor against a stray keystroke or a truncated + // pipe silently becoming the account's credential, not a password + // policy. + minPasswordLen = 8 +) + +// Exit statuses. Usage errors are distinguished from operational ones +// so that a script can tell "you called it wrong" from "it refused". +const ( + exitOK = 0 + exitFailure = 1 + exitUsage = 2 +) + +// Sentinel errors, exported so a test can assert on the refusal rather +// than on the wording of a message. +var ( + // ErrNoDataDir reports that DATA_DIR names nothing, or names + // something that is not a directory. + ErrNoDataDir = errors.New("no data directory") + + // ErrNoDatabase reports that the data directory holds no main + // database, so there is no deployment to reset a password in. + ErrNoDatabase = errors.New("no webhooker database") + + // ErrLiveInstance reports that a running webhooker holds the data + // directory. + ErrLiveInstance = errors.New( + "data directory is held by a running webhooker", + ) + + // ErrNoSuchUser reports that the named account does not exist. + ErrNoSuchUser = errors.New("no such user") + + // ErrEmptyPassword reports that standard input carried nothing. + ErrEmptyPassword = errors.New("empty password") + + // ErrPasswordTooShort reports a password below minPasswordLen. + ErrPasswordTooShort = errors.New("password too short") + + // ErrNotUpdated reports that the update matched no row, which + // means the account disappeared between the lookup and the write. + ErrNotUpdated = errors.New("password was not updated") +) + +// usage describes the subcommand. It is written to the same stream as +// the error that provoked it. +func usage(w io.Writer, flags *flag.FlagSet) { + _, _ = fmt.Fprintf(w, `usage: webhooker %s [-generate] + +Set an existing account's password. The deployment must be stopped: +webhooker %s takes the same exclusive DATA_DIR lock the server does +and refuses to run while a live instance holds it. + +The password is read as one line from standard input, or generated +with -generate. It is never taken as a command-line argument, which +on Linux would publish it in /proc to every account on the host. + +DATA_DIR selects the deployment exactly as it does for the server +(default %s). The directory and its database must already exist; +nothing is created. + +Flags: +`, Name, Name, config.DefaultDataDir) + + flags.PrintDefaults() +} + +// Run executes the subcommand and returns the process exit status. +func Run( + args []string, + stdin io.Reader, + stdout, stderr io.Writer, +) int { + flags := flag.NewFlagSet("webhooker "+Name, flag.ContinueOnError) + flags.SetOutput(stderr) + + generate := flags.Bool( + "generate", false, + "generate a random password instead of reading one from "+ + "standard input, and print it", + ) + + flags.Usage = func() { usage(stderr, flags) } + + err := flags.Parse(args) + if err != nil { + // flag has already reported the error and printed the usage. + return exitUsage + } + + if flags.NArg() != 1 { + _, _ = fmt.Fprintf( + stderr, + "webhooker %s: exactly one username is required\n", + Name, + ) + usage(stderr, flags) + + return exitUsage + } + + err = reset(flags.Arg(0), *generate, stdin, stdout, stderr) + if err != nil { + _, _ = fmt.Fprintf(stderr, "webhooker %s: %v\n", Name, err) + + return exitFailure + } + + return exitOK +} + +// reset performs the whole operation against the configured data +// directory. +func reset( + username string, + generate bool, + stdin io.Reader, + stdout, stderr io.Writer, +) error { + dir := config.DataDir() + + err := checkDataDir(dir) + if err != nil { + return err + } + + lock, err := acquire(dir) + if err != nil { + return err + } + + // The kernel drops the lock when this process exits, whatever + // happens below; releasing explicitly is what makes a long-running + // caller — a test — see it freed. A release error tells the + // operator nothing they can act on. + defer func() { _ = lock.Release() }() + + db, err := database.Open(dir, cliLogger(stderr)) + if err != nil { + return err + } + + password, err := setPassword(db, username, generate, stdin, stderr) + + closeErr := db.Close() + + if err != nil { + return err + } + + if closeErr != nil { + return fmt.Errorf("closing the database: %w", closeErr) + } + + return report(stdout, stderr, dir, username, password, generate) +} + +// report tells the operator what happened. +// +// A generated password is printed in the same banner the first boot +// uses, because this is the only time it is ever shown. A password the +// operator supplied is not echoed back: they already have it, and +// writing it to standard output a second time would put it in another +// log for no gain. +func report( + stdout, stderr io.Writer, + dir, username, password string, + generate bool, +) error { + if generate { + write := func(w io.Writer) error { + return banner.Credentials( + w, + "WEBHOOKER PASSWORD RESET: this account's new "+ + "password is", + username, + password, + "Save this password now: it is shown only here.\n"+ + "The database stores only its Argon2id hash.", + ) + } + + err := write(stdout) + if err == nil { + return nil + } + + // The password is already stored. Standard output failing + // here is the difference between a recovered deployment and + // one locked out behind a password nobody has ever seen, so + // try the other stream before giving up. + if write(stderr) == nil { + return nil + } + + return err + } + + _, err := fmt.Fprintf( + stdout, + "password updated for user %q in %s\n", username, dir, + ) + if err != nil { + return fmt.Errorf("writing the result: %w", err) + } + + return nil +} + +// acquire takes the DATA_DIR lock, translating the contended case into +// the refusal this command owes the operator. +// +// Resetting a password underneath a live instance would not corrupt +// anything, but the running process keeps serving every session that +// authenticated with the old one, so the operator would be told the +// password changed while the deployment still behaved as though it had +// not. Refusing is also what the lock is for. +func acquire(dir string) (*datadir.Lock, error) { + lock, err := datadir.Acquire(dir) + if err == nil { + return lock, nil + } + + if errors.Is(err, datadir.ErrLocked) { + return nil, fmt.Errorf( + "%w: %s. Stop it and run this again: a password reset "+ + "does not reach a running process, whose existing "+ + "sessions stay authenticated", + ErrLiveInstance, dir, + ) + } + + return nil, err +} + +// checkDataDir refuses to act on a path that does not already hold a +// deployment. +// +// This runs before datadir.Acquire on purpose. Acquire calls +// os.MkdirAll, so a mistyped DATA_DIR would otherwise be built out — +// the directory tree, the lock file, and then an empty migrated +// database — and the command would report success against a deployment +// that does not exist while the real one stayed locked out. Nothing +// here creates anything. +func checkDataDir(dir string) error { + info, err := os.Stat(dir) + + switch { + case errors.Is(err, fs.ErrNotExist): + return fmt.Errorf( + "%w: %s (DATA_DIR). This acts on an existing "+ + "deployment and creates nothing", + ErrNoDataDir, dir, + ) + case err != nil: + return fmt.Errorf("checking data directory %s: %w", dir, err) + case !info.IsDir(): + return fmt.Errorf( + "%w: %s (DATA_DIR) is not a directory", ErrNoDataDir, dir, + ) + } + + dbPath := filepath.Join(dir, database.MainDBFileName) + + _, err = os.Stat(dbPath) + + switch { + case errors.Is(err, fs.ErrNotExist): + return fmt.Errorf( + "%w: %s does not exist. The admin account is created by "+ + "the first server start", + ErrNoDatabase, dbPath, + ) + case err != nil: + return fmt.Errorf("checking %s: %w", dbPath, err) + } + + return nil +} + +// setPassword looks the account up, obtains the new password, and +// writes its hash. +// +// The order matters: the account is resolved before an operator is +// asked to type anything, and the hash is computed in full before the +// single UPDATE that stores it. A failure at any step therefore leaves +// the stored credential exactly as it was — there is no half-written +// state to recover from. +func setPassword( + db *database.Database, + username string, + generate bool, + stdin io.Reader, + stderr io.Writer, +) (string, error) { + var user database.User + + err := db.DB().Where("username = ?", username).First(&user).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return "", fmt.Errorf( + "%w: %q. This changes an existing account's password "+ + "and never creates an account", + ErrNoSuchUser, username, + ) + } + + if err != nil { + return "", fmt.Errorf("looking up user %q: %w", username, err) + } + + password, err := newPassword(generate, stdin, stderr) + if err != nil { + return "", err + } + + hash, err := database.HashPassword(password) + if err != nil { + return "", fmt.Errorf("hashing the new password: %w", err) + } + + result := db.DB().Model(&database.User{}). + Where("id = ?", user.ID). + Update("password", hash) + if result.Error != nil { + return "", fmt.Errorf( + "updating user %q: %w", username, result.Error, + ) + } + + if result.RowsAffected != 1 { + return "", fmt.Errorf( + "%w: %q matched %d rows", + ErrNotUpdated, username, result.RowsAffected, + ) + } + + return password, nil +} + +// newPassword returns the password to store: generated, or read from +// standard input. +func newPassword( + generate bool, + stdin io.Reader, + stderr io.Writer, +) (string, error) { + if generate { + password, err := database.GenerateRandomPassword( + generatedPasswordLen, + ) + if err != nil { + return "", fmt.Errorf("generating a password: %w", err) + } + + return password, nil + } + + return readPassword(stdin, stderr) +} + +// readPassword reads the new password as one line from standard input, +// minus its line ending. +// +// Standard input rather than an argument: on Linux argv is readable +// through /proc by every account on the host for as long as the process +// lives, and a password typed as an argument lands in shell history +// besides. +// +// When standard input is a terminal the input is echoed — no attempt is +// made to put the terminal into no-echo mode — so the prompt says so +// rather than letting an operator assume otherwise. +func readPassword(stdin io.Reader, stderr io.Writer) (string, error) { + if f, ok := stdin.(*os.File); ok && isTerminal(f) { + _, _ = fmt.Fprintf( + stderr, + "New password (echoed as you type), then Enter: ", + ) + } + + line, err := bufio.NewReader(stdin).ReadString('\n') + if err != nil && !errors.Is(err, io.EOF) { + return "", fmt.Errorf( + "reading the password from standard input: %w", err, + ) + } + + password := strings.TrimRight(line, "\r\n") + + if password == "" { + return "", fmt.Errorf( + "%w: standard input carried no password. Pipe one in, "+ + "or pass -generate", + ErrEmptyPassword, + ) + } + + if len(password) < minPasswordLen { + return "", fmt.Errorf( + "%w: %d bytes, minimum %d", + ErrPasswordTooShort, len(password), minPasswordLen, + ) + } + + return password, nil +} + +// isTerminal reports whether f is a character device, which is as much +// as this needs to know to decide whether to prompt. +func isTerminal(f *os.File) bool { + info, err := f.Stat() + if err != nil { + return false + } + + return info.Mode()&os.ModeCharDevice != 0 +} + +// cliLogger builds the logger the database layer writes through while +// this command runs. It is deliberately quiet: connecting and migrating +// are steps the operator did not ask about, and the one thing they need +// to see is the outcome on standard output. +func cliLogger(stderr io.Writer) *slog.Logger { + return slog.New(slog.NewTextHandler(stderr, &slog.HandlerOptions{ + Level: slog.LevelWarn, + })) +} diff --git a/internal/resetpw/resetpw_test.go b/internal/resetpw/resetpw_test.go new file mode 100644 index 0000000..448ff1d --- /dev/null +++ b/internal/resetpw/resetpw_test.go @@ -0,0 +1,443 @@ +package resetpw_test + +import ( + "bytes" + "context" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/fx" + "go.uber.org/fx/fxtest" + "sneak.berlin/go/webhooker/internal/config" + "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/datadir" + "sneak.berlin/go/webhooker/internal/delivery" + "sneak.berlin/go/webhooker/internal/globals" + "sneak.berlin/go/webhooker/internal/handlers" + "sneak.berlin/go/webhooker/internal/healthcheck" + "sneak.berlin/go/webhooker/internal/logger" + "sneak.berlin/go/webhooker/internal/middleware" + "sneak.berlin/go/webhooker/internal/resetpw" + "sneak.berlin/go/webhooker/internal/session" +) + +const ( + // operatorUser is the account these tests recover. + operatorUser = "admin" + + // newPassword is what the operator sets it to. + newPassword = "correct horse battery staple" + + // placeholderHash stands in for the stored credential nobody + // knows any more — the lost bootstrap password. Nothing here + // verifies against it; what matters is whether it is still there + // after a refusal, or replaced after a reset. + placeholderHash = "$argon2id$lost" + + // exitOK and exitFailure are the statuses Run returns. + exitOK = 0 + exitFailure = 1 +) + +// testLogger is quiet unless something goes wrong. +func testLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{ + Level: slog.LevelWarn, + })) +} + +// newDeployment builds a data directory holding a migrated database +// with one account whose password is unknown, and points DATA_DIR at +// it. It deliberately does not boot the server graph: seeding through +// it would spend an Argon2id hash on a password no test can use. +func newDeployment(t *testing.T) string { + t.Helper() + + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + db, err := database.Open(dir, testLogger()) + require.NoError(t, err) + + require.NoError(t, db.DB().Create(&database.User{ + Username: operatorUser, + Password: placeholderHash, + }).Error) + + require.NoError(t, db.Close()) + + return dir +} + +// storedHash reads the account's stored credential back. +func storedHash(t *testing.T, dir string) string { + t.Helper() + + db, err := database.Open(dir, testLogger()) + require.NoError(t, err) + + defer func() { require.NoError(t, db.Close()) }() + + var user database.User + + require.NoError(t, db.DB(). + Where("username = ?", operatorUser). + First(&user).Error) + + return user.Password +} + +// bannerPassword returns the plaintext a credentials banner printed. +func bannerPassword(t *testing.T, out string) string { + t.Helper() + + for line := range strings.SplitSeq(out, "\n") { + _, value, found := strings.Cut(line, "password: ") + if found { + return strings.TrimSpace(value) + } + } + + t.Fatalf("no password line in:\n%s", out) + + return "" +} + +// run drives the subcommand with the given standard input and returns +// its status alongside what it wrote. +func run( + t *testing.T, stdin string, args ...string, +) (int, string, string) { + t.Helper() + + var stdout, stderr bytes.Buffer + + code := resetpw.Run( + args, strings.NewReader(stdin), &stdout, &stderr, + ) + + return code, stdout.String(), stderr.String() +} + +type noopNotifier struct{} + +func (n *noopNotifier) Notify([]delivery.Task) {} + +type noopEvictor struct{} + +func (n *noopEvictor) EvictWebhook(string) {} + +// newServerApp starts the real login path against dir: the handlers, +// the middleware that bounds password verification, the session store +// and the database, exactly as internal/handlers builds them. +// +// One application per test function, not per case: every start that +// finds no account seeds one at 64 MB of Argon2id, and this package's +// budget is not the place to spend that repeatedly. +func newServerApp( + t *testing.T, dir string, +) (*handlers.Handlers, *fxtest.App) { + t.Helper() + + var h *handlers.Handlers + + app := fxtest.New( + t, + fx.Provide( + globals.New, + logger.New, + func() *config.Config { + return &config.Config{DataDir: dir} + }, + database.New, + database.NewWebhookDBManager, + healthcheck.New, + session.New, + func() delivery.Notifier { return &noopNotifier{} }, + func() delivery.WebhookEvictor { return &noopEvictor{} }, + middleware.New, + handlers.New, + ), + fx.Populate(&h), + ) + + app.RequireStart() + + return h, app +} + +// submitLogin drives one login form POST through the real handler. +func submitLogin( + h *handlers.Handlers, username, password string, +) *httptest.ResponseRecorder { + form := url.Values{} + form.Set("username", username) + form.Set("password", password) + + req := httptest.NewRequestWithContext( + context.Background(), + http.MethodPost, + "/pages/login", + strings.NewReader(form.Encode()), + ) + req.Header.Set( + "Content-Type", "application/x-www-form-urlencoded", + ) + req.RemoteAddr = "10.0.0.1:44444" + + w := httptest.NewRecorder() + h.HandleLoginSubmit().ServeHTTP(w, req) + + return w +} + +// TestResetThenLogin is the definition of done of +// https://git.eeqj.de/sneak/webhooker/issues/208: an operator who lost +// the one-time bootstrap password sets a new one from the command line +// and logs in with it. +// +// The login is the real one — the form POST through +// handlers.HandleLoginSubmit, which looks the account up and verifies +// the stored Argon2id hash — so a reset that wrote a hash the login +// path cannot verify fails here rather than passing a re-implementation +// of the check. +// +//nolint:paralleltest // newDeployment sets DATA_DIR with t.Setenv. +func TestResetThenLogin(t *testing.T) { + dir := newDeployment(t) + + code, stdout, stderr := run(t, newPassword+"\n", operatorUser) + require.Equal(t, exitOK, code, "stderr: %s", stderr) + + assert.NotContains( + t, stdout, newPassword, + "a password the operator supplied must not be echoed back", + ) + assert.Contains(t, stdout, operatorUser) + + h, app := newServerApp(t, dir) + + defer app.RequireStop() + + got := submitLogin(h, operatorUser, newPassword) + require.Equal( + t, http.StatusSeeOther, got.Code, + "the new password must log in", + ) + + got = submitLogin(h, operatorUser, "not-"+newPassword) + require.NotEqual( + t, http.StatusSeeOther, got.Code, + "the reset must not make every password work", + ) +} + +// TestGeneratedPasswordIsPrintedAndWorks covers -generate, the mode an +// operator recovering a deployment actually reaches for. The generated +// password is shown once, in the banner, and must be the one stored. +// +//nolint:paralleltest // newDeployment sets DATA_DIR with t.Setenv. +func TestGeneratedPasswordIsPrintedAndWorks(t *testing.T) { + dir := newDeployment(t) + + code, stdout, stderr := run(t, "", "-generate", operatorUser) + require.Equal(t, exitOK, code, "stderr: %s", stderr) + + require.Contains( + t, stdout, strings.Repeat("=", 20), + "a generated password must be printed as a banner", + ) + + password := bannerPassword(t, stdout) + + ok, err := database.VerifyPassword(password, storedHash(t, dir)) + require.NoError(t, err) + assert.True( + t, ok, "the printed password must open the account", + ) +} + +// failingWriter is a standard output that cannot be written to. +type failingWriter struct{} + +func (failingWriter) Write([]byte) (int, error) { + return 0, assert.AnError +} + +// TestGeneratedPasswordSurvivesAFailedStdout covers the one outcome +// worse than an error: the password is already stored, so a banner +// that cannot be written to standard output must go to standard error +// rather than leaving the deployment behind a password nobody has seen. +// +//nolint:paralleltest // newDeployment sets DATA_DIR with t.Setenv. +func TestGeneratedPasswordSurvivesAFailedStdout(t *testing.T) { + dir := newDeployment(t) + + var stderr bytes.Buffer + + code := resetpw.Run( + []string{"-generate", operatorUser}, + strings.NewReader(""), failingWriter{}, &stderr, + ) + + require.Equal(t, exitOK, code) + + password := bannerPassword(t, stderr.String()) + + ok, err := database.VerifyPassword(password, storedHash(t, dir)) + require.NoError(t, err) + assert.True(t, ok) +} + +// TestRefusesLiveInstance pins the refusal the issue requires. The +// running deployment keeps serving the sessions that authenticated +// with the old password, so a reset underneath it would report a +// change the service does not honour. +// +//nolint:paralleltest // newDeployment sets DATA_DIR with t.Setenv. +func TestRefusesLiveInstance(t *testing.T) { + dir := newDeployment(t) + + lock, err := datadir.Acquire(dir) + require.NoError(t, err) + + defer func() { require.NoError(t, lock.Release()) }() + + code, _, stderr := run(t, newPassword+"\n", operatorUser) + + require.Equal(t, exitFailure, code) + assert.Contains(t, stderr, dir, "the refusal must name DATA_DIR") + assert.Contains(t, stderr, "running webhooker") + assert.Equal( + t, placeholderHash, storedHash(t, dir), + "a refused reset must not touch the stored credential", + ) +} + +// TestMissingDataDirCreatesNothing pins the side effect that must not +// happen. datadir.Acquire calls os.MkdirAll, so reaching it with a +// mistyped DATA_DIR would build the directory, take a lock in it and +// migrate an empty database there — reporting success against a +// deployment that does not exist. +func TestMissingDataDirCreatesNothing(t *testing.T) { + dir := filepath.Join(t.TempDir(), "typo", "webhooker") + t.Setenv("DATA_DIR", dir) + + code, _, stderr := run(t, newPassword+"\n", operatorUser) + + require.Equal(t, exitFailure, code) + assert.Contains(t, stderr, dir) + + _, err := os.Stat(dir) + assert.ErrorIs( + t, err, os.ErrNotExist, + "a mistyped DATA_DIR must not be created", + ) +} + +// TestMissingDatabaseCreatesNothing covers the directory that exists +// but holds no deployment: an empty volume, or the wrong one. Nothing +// may be written there either, lock file included. +func TestMissingDatabaseCreatesNothing(t *testing.T) { + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + code, _, stderr := run(t, newPassword+"\n", operatorUser) + + require.Equal(t, exitFailure, code) + assert.Contains(t, stderr, database.MainDBFileName) + + entries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Empty( + t, entries, + "nothing may be created in a directory holding no database", + ) +} + +// TestUnknownUserFails states the decision: resetpw changes an +// existing account's password and never creates an account. A typo in +// the username must say so rather than quietly adding a second user. +// +//nolint:paralleltest // newDeployment sets DATA_DIR with t.Setenv. +func TestUnknownUserFails(t *testing.T) { + dir := newDeployment(t) + + code, _, stderr := run(t, newPassword+"\n", "amdin") + + require.Equal(t, exitFailure, code) + assert.Contains(t, stderr, "amdin") + assert.Equal(t, placeholderHash, storedHash(t, dir)) + + var count int64 + + db, err := database.Open(dir, testLogger()) + require.NoError(t, err) + + defer func() { require.NoError(t, db.Close()) }() + + require.NoError(t, db.DB().Model(&database.User{}). + Count(&count).Error) + assert.EqualValues( + t, 1, count, "no account may have been created", + ) +} + +// TestRejectsUnusablePasswords covers what standard input can carry by +// accident: nothing at all, and a stray keystroke. Either would +// otherwise become the account's only credential. +// +//nolint:paralleltest // newDeployment sets DATA_DIR with t.Setenv. +func TestRejectsUnusablePasswords(t *testing.T) { + dir := newDeployment(t) + + for name, stdin := range map[string]string{ + "empty": "", + "newline": "\n", + "short": "hunter2\n", + } { + t.Run(name, func(t *testing.T) { + code, _, stderr := run(t, stdin, operatorUser) + + require.Equal(t, exitFailure, code) + assert.NotEmpty(t, stderr) + assert.Equal( + t, placeholderHash, storedHash(t, dir), + "a rejected password must not be stored", + ) + }) + } +} + +// TestUsageErrors pins the statuses a caller can script against: 2 for +// being called wrong, which is not the same as a refusal. +func TestUsageErrors(t *testing.T) { + t.Parallel() + + for name, args := range map[string][]string{ + "no username": {}, + "two usernames": {"admin", "root"}, + "unknown flag": {"-force", "admin"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + + code := resetpw.Run( + args, strings.NewReader(""), &stdout, &stderr, + ) + + assert.Equal(t, 2, code) + assert.NotEmpty(t, stderr) + }) + } +}