diff --git a/internal/config/config.go b/internal/config/config.go index c90fdcf..1ed4a90 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -82,6 +82,11 @@ var defaults = []struct { {"rp_display_name", "mailgoserver", ""}, {"", "", `Full origin (scheme+host+port) the dashboard is served at, e.g. "https://mail.example.com"`}, {"rp_origin", "http://localhost:5000", ""}, + {"", "", "Require every admin account (global or domain-scoped) to set up TOTP/passkey MFA"}, + {"enforce_admin_mfa", "false", ""}, + {"", "", "Require every mailbox's self-service webmail login to have TOTP/passkey MFA"}, + {"", "", "(overridable per-domain or per-mailbox — see the Domains/Mailboxes edit pages)"}, + {"enforce_mailbox_mfa", "false", ""}, }}, {"IMAP", []defaultKV{ {"", "", "IMAP server configuration for mailbox retrieval (Thunderbird, etc.)"}, diff --git a/internal/db/admin_models.go b/internal/db/admin_models.go index 25d0d37..5f98652 100644 --- a/internal/db/admin_models.go +++ b/internal/db/admin_models.go @@ -3,10 +3,17 @@ package db import "time" type AdminUser struct { - ID int64 - Username string - PasswordHash string + ID int64 + Username string + PasswordHash string + // MustChangePassword forces a password change before this admin can use the rest + // of the dashboard — true for the seeded default account and every newly-created + // admin (delegated or not). MustChangeUsername additionally forces choosing a new + // username too — true only for the seeded default account (username "admin"), + // never for admins created via the delegation flow, who pick their own username + // up front (see addAdmin/CreateScopedAdminUser). MustChangePassword bool + MustChangeUsername bool TOTPSecret string TOTPEnabled bool IsGlobalAdmin bool diff --git a/internal/db/crud_admin.go b/internal/db/crud_admin.go index b02d76e..143cf92 100644 --- a/internal/db/crud_admin.go +++ b/internal/db/crud_admin.go @@ -8,13 +8,13 @@ import ( "time" ) -const adminUserColumns = `id, username, password_hash, must_change_password, totp_secret, totp_enabled, is_global_admin, created_by, created_at` +const adminUserColumns = `id, username, password_hash, must_change_password, must_change_username, totp_secret, totp_enabled, is_global_admin, created_by, created_at` func scanAdminUser(row *sql.Row) (*AdminUser, error) { var u AdminUser var createdAt string var createdBy sql.NullInt64 - if err := row.Scan(&u.ID, &u.Username, &u.PasswordHash, &u.MustChangePassword, &u.TOTPSecret, &u.TOTPEnabled, &u.IsGlobalAdmin, &createdBy, &createdAt); err != nil { + if err := row.Scan(&u.ID, &u.Username, &u.PasswordHash, &u.MustChangePassword, &u.MustChangeUsername, &u.TOTPSecret, &u.TOTPEnabled, &u.IsGlobalAdmin, &createdBy, &createdAt); err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -42,8 +42,11 @@ const ( ) // SeedDefaultAdminIfEmpty creates the default admin account on a brand-new install -// (no admin users yet at all) with must_change_password set, so the default -// credentials can never be left in place silently. +// (no admin users yet at all) with must_change_password AND must_change_username set, +// so the well-known default credentials (username "admin") can never be left in place +// silently. This is the one and only place must_change_username is ever set — every +// other admin (delegated, or a global admin created via the delegation flow) picks +// their own username up front and only needs to set their own password. func (d *DB) SeedDefaultAdminIfEmpty() error { n, err := d.CountAdminUsers() if err != nil { @@ -56,15 +59,19 @@ func (d *DB) SeedDefaultAdminIfEmpty() error { if err != nil { return err } - _, err = d.CreateAdminUser(DefaultAdminUsername, hash, true) + _, err = d.Exec(`INSERT INTO esrv_admin_users (username, password_hash, must_change_password, must_change_username, is_global_admin) VALUES (?, ?, 1, 1, 1)`, + DefaultAdminUsername, hash) return err } // CreateAdminUser inserts a new global-admin account (full access, no domain -// restriction). mustChangePassword should be true for the seeded default account so -// it can't keep running on default credentials. +// restriction) — used by the delegation flow when a global admin grants another user +// global access. mustChangePassword should be true so the admin who set the initial +// password isn't the only one who knows it; must_change_username is always false +// here, since the account was created with the username the new admin will actually +// use (see SeedDefaultAdminIfEmpty for the one exception). func (d *DB) CreateAdminUser(username, passwordHash string, mustChangePassword bool) (int64, error) { - res, err := d.Exec(`INSERT INTO esrv_admin_users (username, password_hash, must_change_password, is_global_admin) VALUES (?, ?, ?, 1)`, + res, err := d.Exec(`INSERT INTO esrv_admin_users (username, password_hash, must_change_password, must_change_username, is_global_admin) VALUES (?, ?, ?, 0, 1)`, username, passwordHash, mustChangePassword) if err != nil { return 0, err @@ -82,7 +89,7 @@ func (d *DB) CreateScopedAdminUser(username, passwordHash string, createdBy int6 } defer tx.Rollback() - res, err := tx.Exec(`INSERT INTO esrv_admin_users (username, password_hash, must_change_password, is_global_admin, created_by) VALUES (?, ?, 1, 0, ?)`, + res, err := tx.Exec(`INSERT INTO esrv_admin_users (username, password_hash, must_change_password, must_change_username, is_global_admin, created_by) VALUES (?, ?, 1, 0, 0, ?)`, username, passwordHash, createdBy) if err != nil { return 0, err @@ -128,7 +135,7 @@ func scanAdminUsers(rows *sql.Rows) ([]AdminUser, error) { var u AdminUser var createdAt string var createdBy sql.NullInt64 - if err := rows.Scan(&u.ID, &u.Username, &u.PasswordHash, &u.MustChangePassword, &u.TOTPSecret, &u.TOTPEnabled, &u.IsGlobalAdmin, &createdBy, &createdAt); err != nil { + if err := rows.Scan(&u.ID, &u.Username, &u.PasswordHash, &u.MustChangePassword, &u.MustChangeUsername, &u.TOTPSecret, &u.TOTPEnabled, &u.IsGlobalAdmin, &createdBy, &createdAt); err != nil { return nil, err } u.CreatedAt, _ = parseTime(createdAt) @@ -214,10 +221,19 @@ func (d *DB) GetAdminUserByID(id int64) (*AdminUser, error) { return scanAdminUser(row) } -// UpdateAdminCredentials mirrors the forced first-login change: new username, -// password hash, and clears must_change_password in one step. +// UpdateAdminCredentials mirrors the forced first-login change for the seeded default +// admin: new username, password hash, and clears must_change_password/ +// must_change_username in one step. func (d *DB) UpdateAdminCredentials(id int64, username, passwordHash string) error { - _, err := d.Exec(`UPDATE esrv_admin_users SET username = ?, password_hash = ?, must_change_password = 0 WHERE id = ?`, username, passwordHash, id) + _, err := d.Exec(`UPDATE esrv_admin_users SET username = ?, password_hash = ?, must_change_password = 0, must_change_username = 0 WHERE id = ?`, username, passwordHash, id) + return err +} + +// UpdateAdminPasswordClearMustChange mirrors the forced first-login change for a +// delegated admin: password hash only (the username was already chosen when the +// account was created), clearing must_change_password. +func (d *DB) UpdateAdminPasswordClearMustChange(id int64, passwordHash string) error { + _, err := d.Exec(`UPDATE esrv_admin_users SET password_hash = ?, must_change_password = 0, must_change_username = 0 WHERE id = ?`, passwordHash, id) return err } @@ -236,6 +252,26 @@ func (d *DB) DisableAdminTOTP(id int64) error { return err } +// ResetAdminMFA clears every second factor an admin has enrolled — TOTP and every +// registered passkey — e.g. after a lost device, so they can re-enroll from scratch. +// Distinct from DisableAdminTOTP (TOTP only, self-service from /account): this is the +// admin-management action a manager takes on someone else's account (see +// adminWithManageAccess's delegation rule for who's allowed to). +func (d *DB) ResetAdminMFA(id int64) error { + tx, err := d.Begin() + if err != nil { + return err + } + defer tx.Rollback() + if _, err := tx.Exec(`UPDATE esrv_admin_users SET totp_secret = '', totp_enabled = 0 WHERE id = ?`, id); err != nil { + return err + } + if _, err := tx.Exec(`DELETE FROM esrv_webauthn_credentials WHERE user_id = ?`, id); err != nil { + return err + } + return tx.Commit() +} + // --- Sessions --- func newSessionToken() string { diff --git a/internal/db/crud_domains.go b/internal/db/crud_domains.go index ae2d88c..ca22edd 100644 --- a/internal/db/crud_domains.go +++ b/internal/db/crud_domains.go @@ -17,7 +17,7 @@ func (d *DB) ListDomains() ([]Domain, error) { var dm Domain var createdAt string var verifiedAt *string - if err := rows.Scan(&dm.ID, &dm.DomainName, &dm.IsActive, &createdAt, &dm.VerificationToken, &dm.IsVerified, &verifiedAt); err != nil { + if err := rows.Scan(&dm.ID, &dm.DomainName, &dm.IsActive, &createdAt, &dm.VerificationToken, &dm.IsVerified, &verifiedAt, &dm.MFAExempt); err != nil { return nil, err } dm.CreatedAt, _ = parseTime(createdAt) @@ -81,6 +81,13 @@ func (d *DB) SetDomainActive(id int64, active bool) error { return err } +// SetDomainMFAExempt overrides [Auth] enforce_mailbox_mfa off for every mailbox under +// this domain (regardless of each mailbox's own MFAExempt). +func (d *DB) SetDomainMFAExempt(id int64, exempt bool) error { + _, err := d.Exec(`UPDATE esrv_domains SET mfa_exempt = ? WHERE id = ?`, exempt, id) + return err +} + // SetDomainVerified mirrors marking a domain as DNS-ownership-verified (or reverting // it, e.g. if an admin wants to force re-verification). func (d *DB) SetDomainVerified(id int64, verified bool) error { diff --git a/internal/db/crud_mailboxes.go b/internal/db/crud_mailboxes.go index 7619986..d120e8c 100644 --- a/internal/db/crud_mailboxes.go +++ b/internal/db/crud_mailboxes.go @@ -5,13 +5,13 @@ import ( "errors" ) -const mailboxColumns = `id, email, domain_id, password_hash, is_active, quota_bytes, used_bytes, dek_wrapped, dek_nonce, created_at, created_by, totp_secret, totp_enabled` +const mailboxColumns = `id, email, domain_id, password_hash, is_active, quota_bytes, used_bytes, dek_wrapped, dek_nonce, created_at, created_by, totp_secret, totp_enabled, mfa_exempt` func scanMailbox(row *sql.Row) (*Mailbox, error) { var m Mailbox var createdAt string var createdBy sql.NullInt64 - if err := row.Scan(&m.ID, &m.Email, &m.DomainID, &m.PasswordHash, &m.IsActive, &m.QuotaBytes, &m.UsedBytes, &m.DEKWrapped, &m.DEKNonce, &createdAt, &createdBy, &m.TOTPSecret, &m.TOTPEnabled); err != nil { + if err := row.Scan(&m.ID, &m.Email, &m.DomainID, &m.PasswordHash, &m.IsActive, &m.QuotaBytes, &m.UsedBytes, &m.DEKWrapped, &m.DEKNonce, &createdAt, &createdBy, &m.TOTPSecret, &m.TOTPEnabled, &m.MFAExempt); err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -31,7 +31,7 @@ type MailboxWithDomain struct { } func (d *DB) ListMailboxes() ([]MailboxWithDomain, error) { - rows, err := d.Query(`SELECT m.id, m.email, m.domain_id, m.password_hash, m.is_active, m.quota_bytes, m.used_bytes, m.dek_wrapped, m.dek_nonce, m.created_at, m.created_by, m.totp_secret, m.totp_enabled, dm.domain_name + rows, err := d.Query(`SELECT m.id, m.email, m.domain_id, m.password_hash, m.is_active, m.quota_bytes, m.used_bytes, m.dek_wrapped, m.dek_nonce, m.created_at, m.created_by, m.totp_secret, m.totp_enabled, m.mfa_exempt, dm.domain_name FROM esrv_mailboxes m JOIN esrv_domains dm ON dm.id = m.domain_id ORDER BY m.email`) if err != nil { return nil, err @@ -42,7 +42,7 @@ func (d *DB) ListMailboxes() ([]MailboxWithDomain, error) { var m MailboxWithDomain var createdAt string var createdBy sql.NullInt64 - if err := rows.Scan(&m.ID, &m.Email, &m.DomainID, &m.PasswordHash, &m.IsActive, &m.QuotaBytes, &m.UsedBytes, &m.DEKWrapped, &m.DEKNonce, &createdAt, &createdBy, &m.TOTPSecret, &m.TOTPEnabled, &m.DomainName); err != nil { + if err := rows.Scan(&m.ID, &m.Email, &m.DomainID, &m.PasswordHash, &m.IsActive, &m.QuotaBytes, &m.UsedBytes, &m.DEKWrapped, &m.DEKNonce, &createdAt, &createdBy, &m.TOTPSecret, &m.TOTPEnabled, &m.MFAExempt, &m.DomainName); err != nil { return nil, err } m.CreatedAt, _ = parseTime(createdAt) @@ -65,7 +65,7 @@ func (d *DB) ListMailboxesForDomain(domainID int64) ([]Mailbox, error) { var m Mailbox var createdAt string var createdBy sql.NullInt64 - if err := rows.Scan(&m.ID, &m.Email, &m.DomainID, &m.PasswordHash, &m.IsActive, &m.QuotaBytes, &m.UsedBytes, &m.DEKWrapped, &m.DEKNonce, &createdAt, &createdBy, &m.TOTPSecret, &m.TOTPEnabled); err != nil { + if err := rows.Scan(&m.ID, &m.Email, &m.DomainID, &m.PasswordHash, &m.IsActive, &m.QuotaBytes, &m.UsedBytes, &m.DEKWrapped, &m.DEKNonce, &createdAt, &createdBy, &m.TOTPSecret, &m.TOTPEnabled, &m.MFAExempt); err != nil { return nil, err } m.CreatedAt, _ = parseTime(createdAt) @@ -113,6 +113,13 @@ func (d *DB) SetMailboxActive(id int64, active bool) error { return err } +// SetMailboxMFAExempt overrides [Auth] enforce_mailbox_mfa off for this mailbox +// specifically, even if its domain isn't exempt. +func (d *DB) SetMailboxMFAExempt(id int64, exempt bool) error { + _, err := d.Exec(`UPDATE esrv_mailboxes SET mfa_exempt = ? WHERE id = ?`, exempt, id) + return err +} + func (d *DB) SetMailboxQuota(id int64, quotaBytes int64) error { _, err := d.Exec(`UPDATE esrv_mailboxes SET quota_bytes = ? WHERE id = ?`, quotaBytes, id) return err @@ -133,6 +140,26 @@ func (d *DB) DisableMailboxTOTP(id int64) error { return err } +// ResetMailboxMFA clears every second factor a mailbox owner has enrolled — TOTP and +// every registered passkey — e.g. after a lost device, or (under enforce_mailbox_mfa) +// to let an admin get them unstuck without needing a domain/mailbox exemption. Distinct +// from DisableMailboxTOTP (TOTP only, self-service from the webmail portal): this is +// the admin-management action taken from the Mailboxes list on someone else's account. +func (d *DB) ResetMailboxMFA(id int64) error { + tx, err := d.Begin() + if err != nil { + return err + } + defer tx.Rollback() + if _, err := tx.Exec(`UPDATE esrv_mailboxes SET totp_secret = '', totp_enabled = 0 WHERE id = ?`, id); err != nil { + return err + } + if _, err := tx.Exec(`DELETE FROM esrv_mailbox_webauthn_credentials WHERE mailbox_id = ?`, id); err != nil { + return err + } + return tx.Commit() +} + // AddMailboxUsedBytes adjusts the cached running total by delta (positive on store, // negative on delete) in a single statement, avoiding a read-modify-write race. func (d *DB) AddMailboxUsedBytes(id int64, delta int64) error { diff --git a/internal/db/mailbox_models.go b/internal/db/mailbox_models.go index 73dd8ad..3d3305a 100644 --- a/internal/db/mailbox_models.go +++ b/internal/db/mailbox_models.go @@ -19,6 +19,9 @@ type Mailbox struct { CreatedBy *int64 TOTPSecret string TOTPEnabled bool + // MFAExempt overrides [Auth] enforce_mailbox_mfa off for this mailbox specifically, + // even if its domain isn't exempt. + MFAExempt bool } // MailboxSession is a self-service webmail portal login — a parallel schema to diff --git a/internal/db/models.go b/internal/db/models.go index 92c3eea..90c4c78 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -10,6 +10,9 @@ type Domain struct { VerificationToken string IsVerified bool VerifiedAt *time.Time + // MFAExempt overrides [Auth] enforce_mailbox_mfa off for every mailbox under this + // domain, regardless of that mailbox's own MFAExempt. + MFAExempt bool } type Sender struct { diff --git a/internal/db/queries.go b/internal/db/queries.go index f71abbd..c338bbd 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -54,14 +54,14 @@ func (d *DB) GetSenderByEmail(email string) (*Sender, error) { return &s, nil } -const domainColumns = `id, domain_name, is_active, created_at, verification_token, is_verified, verified_at` +const domainColumns = `id, domain_name, is_active, created_at, verification_token, is_verified, verified_at, mfa_exempt` // scanDomain scans a row selected with domainColumns, in that order. func scanDomain(row *sql.Row) (*Domain, error) { var dom Domain var createdAt string var verifiedAt sql.NullString - if err := row.Scan(&dom.ID, &dom.DomainName, &dom.IsActive, &createdAt, &dom.VerificationToken, &dom.IsVerified, &verifiedAt); err != nil { + if err := row.Scan(&dom.ID, &dom.DomainName, &dom.IsActive, &createdAt, &dom.VerificationToken, &dom.IsVerified, &verifiedAt, &dom.MFAExempt); err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil } diff --git a/internal/db/schema.go b/internal/db/schema.go index 124dca3..e625b44 100644 --- a/internal/db/schema.go +++ b/internal/db/schema.go @@ -22,7 +22,8 @@ CREATE TABLE IF NOT EXISTS esrv_domains ( verification_token TEXT NOT NULL DEFAULT '', is_verified INTEGER NOT NULL DEFAULT 0, verified_at DATETIME, - default_mailbox_quota_bytes INTEGER NOT NULL DEFAULT 5368709120 + default_mailbox_quota_bytes INTEGER NOT NULL DEFAULT 5368709120, + mfa_exempt INTEGER NOT NULL DEFAULT 0 ); CREATE TABLE IF NOT EXISTS esrv_senders ( @@ -119,6 +120,7 @@ CREATE TABLE IF NOT EXISTS esrv_admin_users ( username TEXT NOT NULL UNIQUE, password_hash TEXT NOT NULL, must_change_password INTEGER NOT NULL DEFAULT 0, + must_change_username INTEGER NOT NULL DEFAULT 0, totp_secret TEXT NOT NULL DEFAULT '', totp_enabled INTEGER NOT NULL DEFAULT 0, is_global_admin INTEGER NOT NULL DEFAULT 0, @@ -171,7 +173,8 @@ CREATE TABLE IF NOT EXISTS esrv_mailboxes ( created_at DATETIME DEFAULT CURRENT_TIMESTAMP, created_by INTEGER REFERENCES esrv_admin_users(id), totp_secret TEXT NOT NULL DEFAULT '', - totp_enabled INTEGER NOT NULL DEFAULT 0 + totp_enabled INTEGER NOT NULL DEFAULT 0, + mfa_exempt INTEGER NOT NULL DEFAULT 0 ); -- Self-service webmail portal sessions — deliberately a parallel schema to @@ -286,10 +289,18 @@ func migrateAddedColumns(db *sql.DB) { `ALTER TABLE esrv_mailboxes ADD COLUMN totp_secret TEXT NOT NULL DEFAULT ''`, `ALTER TABLE esrv_mailboxes ADD COLUMN totp_enabled INTEGER NOT NULL DEFAULT 0`, `ALTER TABLE esrv_mailbox_app_passwords ADD COLUMN expires_at DATETIME`, + `ALTER TABLE esrv_admin_users ADD COLUMN must_change_username INTEGER NOT NULL DEFAULT 0`, + `ALTER TABLE esrv_domains ADD COLUMN mfa_exempt INTEGER NOT NULL DEFAULT 0`, + `ALTER TABLE esrv_mailboxes ADD COLUMN mfa_exempt INTEGER NOT NULL DEFAULT 0`, } for _, stmt := range stmts { db.Exec(stmt) } + // Backfill for installs that already have a still-pending default admin (username + // "admin", never completed the forced first-login yet): must_change_username + // defaults to 0 for every pre-existing row above, which would otherwise let that + // account skip its username change entirely once it re-hits /first-login next. + db.Exec(`UPDATE esrv_admin_users SET must_change_username = 1 WHERE username = ? AND must_change_password = 1`, DefaultAdminUsername) } // DB wraps *sql.DB with the query helpers below. diff --git a/internal/webui/account.go b/internal/webui/account.go index 9dcb852..92426a4 100644 --- a/internal/webui/account.go +++ b/internal/webui/account.go @@ -17,7 +17,9 @@ import ( func (a *App) accountPage(w http.ResponseWriter, r *http.Request) { user := userFromContext(r) creds, _ := a.DB.ListWebAuthnCredentials(user.ID) - a.render(w, r, "account.html", M{"active": "account", "user": user, "passkeys": creds}) + hasMFA := user.TOTPEnabled || len(creds) > 0 + mfaRequired := !hasMFA && a.Cfg.Section("Auth").Key("enforce_admin_mfa").MustBool(false) + a.render(w, r, "account.html", M{"active": "account", "user": user, "passkeys": creds, "mfa_required": mfaRequired}) } // changePassword mirrors a normal (not forced) password change from account settings. diff --git a/internal/webui/admin_mfa_reset_test.go b/internal/webui/admin_mfa_reset_test.go new file mode 100644 index 0000000..21aab7c --- /dev/null +++ b/internal/webui/admin_mfa_reset_test.go @@ -0,0 +1,183 @@ +package webui + +import ( + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "mailgoserver/internal/db" +) + +// TestResetAdminMFA confirms a manager can clear another admin's TOTP and passkeys +// (e.g. after a lost device), and that only an admin who could otherwise manage that +// target (per the existing delegation rule) is allowed to. +func TestResetAdminMFA(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + cookie := loginSession(t, app) + + targetID, err := app.DB.CreateAdminUser("has-mfa-admin", mustHash(t), false) + if err != nil { + t.Fatal(err) + } + if err := app.DB.SetAdminTOTPSecret(targetID, "JBSWY3DPEHPK3PXP", true); err != nil { + t.Fatal(err) + } + if err := app.DB.CreateWebAuthnCredential(targetID, "yubikey", "cred-id-1", "cred-data-1"); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, Prefix+"/admins/"+strconv.FormatInt(targetID, 10)+"/reset_mfa", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusFound { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + + target, err := app.DB.GetAdminUserByID(targetID) + if err != nil || target == nil { + t.Fatal(err) + } + if target.TOTPEnabled || target.TOTPSecret != "" { + t.Error("TOTP should be cleared") + } + creds, err := app.DB.ListWebAuthnCredentials(targetID) + if err != nil || len(creds) != 0 { + t.Errorf("expected no passkeys left, got %d (err=%v)", len(creds), err) + } +} + +// TestResetAdminMFADeniedOutsideDelegationScope confirms a scoped admin can't reset +// MFA for an admin outside their delegation scope (mirrors the existing remove/edit +// access checks — resetting someone's MFA is just as sensitive an action). +func TestResetAdminMFADeniedOutsideDelegationScope(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + domainA, domainB, _, _ := setupTwoTenants(t, app) + _ = domainA + + cookieA := scopedLogin(t, app, "tenant-a-admin", []int64{domainA.ID}) + targetID, err := app.DB.CreateScopedAdminUser("tenant-b-admin", mustHash(t), 0, []int64{domainB.ID}) + if err != nil { + t.Fatal(err) + } + if err := app.DB.SetAdminTOTPSecret(targetID, "JBSWY3DPEHPK3PXP", true); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, Prefix+"/admins/"+strconv.FormatInt(targetID, 10)+"/reset_mfa", nil) + req.AddCookie(cookieA) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusNotFound { + t.Fatalf("expected 404 for an out-of-scope target, got %d", rec.Code) + } + + target, err := app.DB.GetAdminUserByID(targetID) + if err != nil || target == nil { + t.Fatal(err) + } + if !target.TOTPEnabled { + t.Error("TOTP should NOT have been reset for an out-of-scope admin") + } +} + +// TestAdminCannotRemoveOrResetOwnAccount confirms the existing self-management +// blocks (canManageAdmin already rejects target.ID == user.ID) also apply to the new +// reset_mfa route, and that the admins.html list hides both actions for your own row. +func TestAdminCannotRemoveOrResetOwnAccount(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + cookie := loginSession(t, app) + + sess, err := app.DB.GetSession(cookie.Value) + if err != nil || sess == nil { + t.Fatal(err) + } + selfID := strconv.FormatInt(sess.UserID, 10) + + for _, action := range []string{"remove", "reset_mfa"} { + req := httptest.NewRequest(http.MethodPost, Prefix+"/admins/"+selfID+"/"+action, nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusNotFound { + t.Errorf("%s on own account: status=%d, want 404", action, rec.Code) + } + } + + stillThere, err := app.DB.GetAdminUserByID(sess.UserID) + if err != nil || stillThere == nil { + t.Fatal("own account should not have been removed") + } + + // The admins list must not render a Remove/Reset MFA button for your own row. + req := httptest.NewRequest(http.MethodGet, Prefix+"/admins", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("/admins: status=%d", rec.Code) + } + body := rec.Body.String() + if strings.Contains(body, "/admins/"+selfID+"/remove") { + t.Error("admins.html should not render a Remove action for the signed-in admin's own row") + } + if strings.Contains(body, "/admins/"+selfID+"/reset_mfa") { + t.Error("admins.html should not render a Reset MFA action for the signed-in admin's own row") + } + if !strings.Contains(body, "This is you") { + t.Error("expected the signed-in admin's own row to be marked, not just have its buttons hidden") + } +} + +// TestResetMailboxMFA confirms an admin can clear a mailbox owner's TOTP and passkeys. +func TestResetMailboxMFA(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + cookie := loginSession(t, app) + + mailboxes, err := app.DB.ListMailboxes() + if err != nil || len(mailboxes) == 0 { + t.Fatal("no seeded mailbox") + } + mboxID := mailboxes[0].ID + if err := app.DB.SetMailboxTOTPSecret(mboxID, "JBSWY3DPEHPK3PXP", true); err != nil { + t.Fatal(err) + } + if err := app.DB.CreateMailboxWebAuthnCredential(mboxID, "phone", "mcred-1", "mcred-data-1"); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, Prefix+"/mailboxes/"+strconv.FormatInt(mboxID, 10)+"/reset_mfa", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusFound { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + + updated, err := app.DB.GetMailboxByID(mboxID) + if err != nil || updated == nil { + t.Fatal(err) + } + if updated.TOTPEnabled || updated.TOTPSecret != "" { + t.Error("TOTP should be cleared") + } + creds, err := app.DB.ListMailboxWebAuthnCredentials(mboxID) + if err != nil || len(creds) != 0 { + t.Errorf("expected no passkeys left, got %d (err=%v)", len(creds), err) + } +} + +func mustHash(t *testing.T) string { + t.Helper() + hash, err := db.HashPassword("some-strong-password-1!") + if err != nil { + t.Fatal(err) + } + return hash +} diff --git a/internal/webui/admins.go b/internal/webui/admins.go index 1ced465..c728acd 100644 --- a/internal/webui/admins.go +++ b/internal/webui/admins.go @@ -87,7 +87,7 @@ func (a *App) adminsList(w http.ResponseWriter, r *http.Request) { } rows = append(rows, M{"user": u, "domain_names": domainNames}) } - a.render(w, r, "admins.html", M{"active": "admins", "rows": rows}) + a.render(w, r, "admins.html", M{"active": "admins", "rows": rows, "current_user_id": userFromContext(r).ID}) } func (a *App) addAdminForm(w http.ResponseWriter, r *http.Request) { @@ -221,6 +221,22 @@ func (a *App) adminWithManageAccess(w http.ResponseWriter, r *http.Request) (*db return target, true } +// resetAdminMFA clears a target admin's TOTP and passkeys — e.g. after a lost device +// — so they can sign back in without a second factor (or under enforce_admin_mfa, +// re-enroll from /account on their next login) without needing database access. +func (a *App) resetAdminMFA(w http.ResponseWriter, r *http.Request) { + target, ok := a.adminWithManageAccess(w, r) + if !ok { + return + } + if err := a.DB.ResetAdminMFA(target.ID); err != nil { + setFlash(w, "error", "Error resetting MFA") + } else { + setFlash(w, "success", "MFA reset for "+target.Username) + } + http.Redirect(w, r, Prefix+"/admins", http.StatusFound) +} + func (a *App) removeAdmin(w http.ResponseWriter, r *http.Request) { target, ok := a.adminWithManageAccess(w, r) if !ok { diff --git a/internal/webui/auth.go b/internal/webui/auth.go index cb2c77f..50abe23 100644 --- a/internal/webui/auth.go +++ b/internal/webui/auth.go @@ -3,6 +3,7 @@ package webui import ( "context" "net/http" + "strings" "time" "mailgoserver/internal/db" @@ -53,6 +54,21 @@ func scopeFromContext(r *http.Request) accessScope { return s } +// requireGlobalAdmin gates a handler behind the current admin's scope being global — +// used for server-wide settings (Server Settings, Let's Encrypt) that a domain-scoped +// admin has no business reading or changing, even if they can guess the URL. 404 (not +// 403) matches requireDomainAccess's reasoning: a scoped admin shouldn't be able to +// tell "doesn't exist" from "exists but isn't mine" by probing. +func (a *App) requireGlobalAdmin(next http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if !scopeFromContext(r).Global { + http.NotFound(w, r) + return + } + next(w, r) + } +} + // requireDomainAccess checks the current admin's scope covers domainID; if not, it // writes a 404 (not 403 — a scoped admin shouldn't be able to distinguish "doesn't // exist" from "exists but isn't mine" by probing IDs) and returns false, matching the @@ -138,13 +154,16 @@ func (a *App) requireAuth(next http.Handler) http.Handler { return } - needsMFA := user.TOTPEnabled - if !needsMFA { + // hasMFA: this account already has a second factor configured (TOTP or a + // passkey) — distinct from sess.MFAVerified, which is about *this session* + // having satisfied it. + hasMFA := user.TOTPEnabled + if !hasMFA { if n, _ := a.DB.CountWebAuthnCredentials(user.ID); n > 0 { - needsMFA = true + hasMFA = true } } - if needsMFA && !sess.MFAVerified { + if hasMFA && !sess.MFAVerified { http.Redirect(w, r, Prefix+"/login/mfa", http.StatusFound) return } @@ -154,6 +173,17 @@ func (a *App) requireAuth(next http.Handler) http.Handler { return } + // enforce_admin_mfa applies to every admin, global or scoped — force setup at + // /account (which has the TOTP/passkey enrollment forms) before anything else + // is reachable, mirroring the must_change_password gate above. Checked after + // must_change_password so a brand-new admin sets a real password first. + if !hasMFA && !user.MustChangePassword && a.Cfg.Section("Auth").Key("enforce_admin_mfa").MustBool(false) { + if r.URL.Path != Prefix+"/account" && !strings.HasPrefix(r.URL.Path, Prefix+"/account/") { + http.Redirect(w, r, Prefix+"/account", http.StatusFound) + return + } + } + scope, err := a.buildAccessScope(user) if err != nil { a.Logger.Error("build access scope: %v", err) diff --git a/internal/webui/domains.go b/internal/webui/domains.go index a3d63bb..b0e47e0 100644 --- a/internal/webui/domains.go +++ b/internal/webui/domains.go @@ -141,6 +141,11 @@ func (a *App) editDomain(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, Prefix+"/domains", http.StatusFound) return } + if err := a.DB.SetDomainMFAExempt(id, r.FormValue("mfa_exempt") == "on"); err != nil { + setFlash(w, "error", "Error updating domain") + http.Redirect(w, r, Prefix+"/domains", http.StatusFound) + return + } setFlash(w, "success", "Domain updated successfully") http.Redirect(w, r, Prefix+"/domains", http.StatusFound) } diff --git a/internal/webui/first_login_test.go b/internal/webui/first_login_test.go new file mode 100644 index 0000000..a788d2c --- /dev/null +++ b/internal/webui/first_login_test.go @@ -0,0 +1,134 @@ +package webui + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "mailgoserver/internal/db" +) + +// TestSeedDefaultAdminMustChangeUsername confirms the seeded "admin" account is still +// forced through the full username+password change on first login. +func TestSeedDefaultAdminMustChangeUsername(t *testing.T) { + app := newTestApp(t) + if err := app.DB.SeedDefaultAdminIfEmpty(); err != nil { + t.Fatal(err) + } + mux := app.Mux() + + form := url.Values{"username": {db.DefaultAdminUsername}, "password": {db.DefaultAdminPassword}} + req := httptest.NewRequest(http.MethodPost, Prefix+"/login", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + cookie := sessionCookieFrom(t, rec) + + // The forced-redirect page must show a username field. + req = httptest.NewRequest(http.MethodGet, Prefix+"/", nil) + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusFound || rec.Header().Get("Location") != Prefix+"/first-login" { + t.Fatalf("expected redirect to /first-login, got %d Location=%q", rec.Code, rec.Header().Get("Location")) + } + req = httptest.NewRequest(http.MethodGet, Prefix+"/first-login", nil) + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if !strings.Contains(rec.Body.String(), `name="username"`) { + t.Error("default admin's first-login page should still ask for a new username") + } + + // Submitting without a username must fail — it's still required for this account. + form = url.Values{"password": {"BrandNewPassw0rd!"}, "password_confirm": {"BrandNewPassw0rd!"}} + req = httptest.NewRequest(http.MethodPost, Prefix+"/first-login", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "Choose a username") { + t.Fatalf("expected a 'choose a username' validation error, got %d: %s", rec.Code, rec.Body.String()) + } + + // With a username, it succeeds and the account is fully usable. + form = url.Values{"username": {"realadmin"}, "password": {"BrandNewPassw0rd!"}, "password_confirm": {"BrandNewPassw0rd!"}} + req = httptest.NewRequest(http.MethodPost, Prefix+"/first-login", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusFound || rec.Header().Get("Location") != Prefix+"/" { + t.Fatalf("expected redirect to dashboard, got %d Location=%q", rec.Code, rec.Header().Get("Location")) + } +} + +// TestDelegatedAdminOnlyChangesPassword confirms an admin created through the +// delegation flow (addAdmin) — who already picked their own username at creation +// time — is only ever asked for a new password on first login, never a username. +func TestDelegatedAdminOnlyChangesPassword(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + + hash, err := db.HashPassword("initial-temp-password-1!") + if err != nil { + t.Fatal(err) + } + userID, err := app.DB.CreateScopedAdminUser("delegate-bob", hash, 0, nil) + if err != nil { + t.Fatal(err) + } + token, err := app.DB.CreateSession(userID, true, sessionTTL) + if err != nil { + t.Fatal(err) + } + cookie := &http.Cookie{Name: sessionCookieName, Value: token} + + req := httptest.NewRequest(http.MethodGet, Prefix+"/first-login", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("first-login page: status=%d", rec.Code) + } + if strings.Contains(rec.Body.String(), `name="username"`) { + t.Error("a delegated admin's first-login page should not ask for a new username") + } + + form := url.Values{"password": {"BrandNewPassw0rd!"}, "password_confirm": {"BrandNewPassw0rd!"}} + req = httptest.NewRequest(http.MethodPost, Prefix+"/first-login", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusFound || rec.Header().Get("Location") != Prefix+"/" { + t.Fatalf("expected redirect to dashboard, got %d Location=%q body=%s", rec.Code, rec.Header().Get("Location"), rec.Body.String()) + } + + updated, err := app.DB.GetAdminUserByID(userID) + if err != nil || updated == nil { + t.Fatal(err) + } + if updated.Username != "delegate-bob" { + t.Errorf("username changed unexpectedly to %q", updated.Username) + } + if updated.MustChangePassword { + t.Error("must_change_password should be cleared after first login") + } + if !db.CheckPassword("BrandNewPassw0rd!", updated.PasswordHash) { + t.Error("password was not actually updated") + } +} + +func sessionCookieFrom(t *testing.T, rec *httptest.ResponseRecorder) *http.Cookie { + t.Helper() + for _, c := range rec.Result().Cookies() { + if c.Name == sessionCookieName { + return c + } + } + t.Fatal("no session cookie set") + return nil +} diff --git a/internal/webui/login.go b/internal/webui/login.go index debe6b0..33fcd2a 100644 --- a/internal/webui/login.go +++ b/internal/webui/login.go @@ -155,25 +155,25 @@ func (a *App) logout(w http.ResponseWriter, r *http.Request) { func (a *App) firstLoginForm(w http.ResponseWriter, r *http.Request) { user := userFromContext(r) - a.render(w, r, "first_login.html", M{"username": user.Username}) + a.render(w, r, "first_login.html", M{"username": user.Username, "must_change_username": user.MustChangeUsername}) } -// firstLoginSubmit mirrors the forced "you can't keep the default credentials" flow: -// require a new username and password before must_change_password clears. +// firstLoginSubmit mirrors the forced credential-change flow: always require a new +// password before must_change_password clears; only the seeded default admin +// (MustChangeUsername) is additionally required to pick a new username — a delegated +// admin already chose their own username when the account was created, so re-asking +// for one here would just be busywork with no security purpose. func (a *App) firstLoginSubmit(w http.ResponseWriter, r *http.Request) { user := userFromContext(r) - newUsername := strings.TrimSpace(r.FormValue("username")) newPassword := r.FormValue("password") confirm := r.FormValue("password_confirm") fail := func(msg string) { - a.render(w, r, "first_login.html", M{"username": newUsername, "error": msg}) + a.render(w, r, "first_login.html", M{ + "username": r.FormValue("username"), "must_change_username": user.MustChangeUsername, "error": msg, + }) } - if newUsername == "" { - fail("Choose a username.") - return - } if !isStrongPassword(newPassword) { fail("Password must be at least 10 characters and include a letter, a number, and a symbol.") return @@ -182,16 +182,32 @@ func (a *App) firstLoginSubmit(w http.ResponseWriter, r *http.Request) { fail("Passwords don't match.") return } - if existing, _ := a.DB.GetAdminUserByUsername(newUsername); existing != nil && existing.ID != user.ID { - fail("That username is already taken.") - return - } hash, err := db.HashPassword(newPassword) if err != nil { fail("Something went wrong. Try again.") return } + + if !user.MustChangeUsername { + if err := a.DB.UpdateAdminPasswordClearMustChange(user.ID, hash); err != nil { + fail("Something went wrong. Try again.") + return + } + setFlash(w, "success", "Password updated. Welcome to your dashboard.") + http.Redirect(w, r, Prefix+"/", http.StatusFound) + return + } + + newUsername := strings.TrimSpace(r.FormValue("username")) + if newUsername == "" { + fail("Choose a username.") + return + } + if existing, _ := a.DB.GetAdminUserByUsername(newUsername); existing != nil && existing.ID != user.ID { + fail("That username is already taken.") + return + } if err := a.DB.UpdateAdminCredentials(user.ID, newUsername, hash); err != nil { fail("Something went wrong. Try again.") return diff --git a/internal/webui/mailboxes.go b/internal/webui/mailboxes.go index 8afb45f..1de7af3 100644 --- a/internal/webui/mailboxes.go +++ b/internal/webui/mailboxes.go @@ -129,6 +129,23 @@ func (a *App) mailboxWithAccess(w http.ResponseWriter, r *http.Request) (mailbox return mailbox, true } +// resetMailboxMFA clears a mailbox owner's TOTP and passkeys — e.g. after a lost +// device, or (under enforce_mailbox_mfa) to unblock their webmail login without +// needing a domain/mailbox exemption — so they can sign back in and, if MFA is +// enforced, re-enroll from the webmail portal on their next login. +func (a *App) resetMailboxMFA(w http.ResponseWriter, r *http.Request) { + mailbox, ok := a.mailboxWithAccess(w, r) + if !ok { + return + } + if err := a.DB.ResetMailboxMFA(mailbox.ID); err != nil { + setFlash(w, "error", "Error resetting MFA") + } else { + setFlash(w, "success", "MFA reset for "+mailbox.Email) + } + http.Redirect(w, r, Prefix+"/mailboxes", http.StatusFound) +} + func (a *App) disableMailbox(w http.ResponseWriter, r *http.Request) { mailbox, ok := a.mailboxWithAccess(w, r) if !ok { @@ -226,6 +243,11 @@ func (a *App) editMailbox(w http.ResponseWriter, r *http.Request) { return } } + if err := a.DB.SetMailboxMFAExempt(mailbox.ID, r.FormValue("mfa_exempt") == "on"); err != nil { + setFlash(w, "error", "Error updating mailbox") + http.Redirect(w, r, Prefix+"/mailboxes", http.StatusFound) + return + } setFlash(w, "success", "Mailbox updated successfully") http.Redirect(w, r, Prefix+"/mailboxes", http.StatusFound) } diff --git a/internal/webui/mfa_enforcement_test.go b/internal/webui/mfa_enforcement_test.go new file mode 100644 index 0000000..42dac87 --- /dev/null +++ b/internal/webui/mfa_enforcement_test.go @@ -0,0 +1,167 @@ +package webui + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "mailgoserver/internal/db" +) + +// TestAdminMFAEnforcementForcesSetupThenReleases confirms enforce_admin_mfa blocks +// every other admin page — redirecting to /account, which has the TOTP/passkey +// enrollment forms — until the admin actually sets up a second factor, after which +// normal access resumes. +func TestAdminMFAEnforcementForcesSetupThenReleases(t *testing.T) { + app := newTestApp(t) + app.Cfg.Section("Auth").Key("enforce_admin_mfa").SetValue("true") + mux := app.Mux() + + hash, err := db.HashPassword("no-mfa-yet-password-1!") + if err != nil { + t.Fatal(err) + } + userID, err := app.DB.CreateAdminUser("no-mfa-admin", hash, false) + if err != nil { + t.Fatal(err) + } + token, err := app.DB.CreateSession(userID, true, sessionTTL) + if err != nil { + t.Fatal(err) + } + cookie := &http.Cookie{Name: sessionCookieName, Value: token} + + // Blocked from an ordinary page, redirected to /account. + req := httptest.NewRequest(http.MethodGet, Prefix+"/domains", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusFound || rec.Header().Get("Location") != Prefix+"/account" { + t.Fatalf("expected redirect to /account, got %d Location=%q", rec.Code, rec.Header().Get("Location")) + } + + // /account itself must be reachable (that's where MFA setup happens). + req = httptest.NewRequest(http.MethodGet, Prefix+"/account", nil) + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("/account: status=%d body=%s", rec.Code, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "requires two-factor authentication") { + t.Error("expected the MFA-required banner on /account") + } + + // Once TOTP is enabled, other pages become reachable again. + if err := app.DB.SetAdminTOTPSecret(userID, "JBSWY3DPEHPK3PXP", true); err != nil { + t.Fatal(err) + } + req = httptest.NewRequest(http.MethodGet, Prefix+"/domains", nil) + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected /domains reachable after enabling MFA, got %d", rec.Code) + } +} + +// TestAdminMFAEnforcementOffByDefault confirms nothing changes for existing installs +// unless the admin explicitly turns enforcement on. +func TestAdminMFAEnforcementOffByDefault(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + cookie := loginSession(t, app) + + req := httptest.NewRequest(http.MethodGet, Prefix+"/domains", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected /domains reachable with enforcement off, got %d", rec.Code) + } +} + +// TestMailboxMFAEnforcementBlocksLogin confirms enforce_mailbox_mfa blocks the +// self-service webmail login outright (no session is ever created) for a mailbox with +// no MFA configured, and that a mailbox-level or domain-level exemption lets the login +// through instead — the bootstrap path for a mailbox to set up its own MFA under +// enforcement. App-password creation/use is deliberately untouched by any of this; +// see mailboxNeedsMFASetup's doc comment. +func TestMailboxMFAEnforcementBlocksLogin(t *testing.T) { + app := newTestApp(t) + app.Cfg.Section("Auth").Key("enforce_mailbox_mfa").SetValue("true") + mux := app.Mux() + + domainID, err := app.DB.CreateDomain("mfatest.example") + if err != nil { + t.Fatal(err) + } + mhash, err := db.HashPassword("mailbox-owner-password-1!") + if err != nil { + t.Fatal(err) + } + dek := make([]byte, 32) + mboxID, err := app.DB.CreateMailbox("owner@mfatest.example", mhash, domainID, 1<<30, dek, dek) + if err != nil { + t.Fatal(err) + } + + tryLogin := func() (status int, sessionCookieSet bool) { + form := url.Values{"email": {"owner@mfatest.example"}, "password": {"mailbox-owner-password-1!"}} + req := httptest.NewRequest(http.MethodPost, MailboxPrefix+"/login", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + for _, c := range rec.Result().Cookies() { + if c.Name == mailboxSessionCookieName && c.Value != "" { + sessionCookieSet = true + } + } + return rec.Code, sessionCookieSet + } + + status, gotSession := tryLogin() + if status != http.StatusOK || gotSession { + t.Fatalf("expected login rejected with no session, got status=%d session=%v", status, gotSession) + } + + // Mailbox-level exemption lets the login through. + if err := app.DB.SetMailboxMFAExempt(mboxID, true); err != nil { + t.Fatal(err) + } + status, gotSession = tryLogin() + if status != http.StatusFound || !gotSession { + t.Fatalf("expected login to succeed once mailbox-exempt, got status=%d session=%v", status, gotSession) + } + + // Un-exempt the mailbox but exempt its domain instead — still overrides. + if err := app.DB.SetMailboxMFAExempt(mboxID, false); err != nil { + t.Fatal(err) + } + if err := app.DB.SetDomainMFAExempt(domainID, true); err != nil { + t.Fatal(err) + } + status, gotSession = tryLogin() + if status != http.StatusFound || !gotSession { + t.Fatalf("expected login to succeed once domain-exempt, got status=%d session=%v", status, gotSession) + } + + // Un-exempt everything, but set up TOTP MFA on the mailbox directly — login + // succeeds (goes to the pending-MFA step) without needing any exemption at all. + if err := app.DB.SetDomainMFAExempt(domainID, false); err != nil { + t.Fatal(err) + } + if err := app.DB.SetMailboxTOTPSecret(mboxID, "JBSWY3DPEHPK3PXP", true); err != nil { + t.Fatal(err) + } + form := url.Values{"email": {"owner@mfatest.example"}, "password": {"mailbox-owner-password-1!"}} + req := httptest.NewRequest(http.MethodPost, MailboxPrefix+"/login", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusFound || rec.Header().Get("Location") != MailboxPrefix+"/login/mfa" { + t.Fatalf("expected redirect to MFA step once TOTP is configured, got %d Location=%q", rec.Code, rec.Header().Get("Location")) + } +} diff --git a/internal/webui/render.go b/internal/webui/render.go index d693f58..91c41f6 100644 --- a/internal/webui/render.go +++ b/internal/webui/render.go @@ -196,6 +196,12 @@ func (a *App) render(w http.ResponseWriter, r *http.Request, page string, data M } data["flashes"] = popFlashes(w, r) data["health"] = a.checkHealth() + // Drives the sidebar hiding Server Settings/Let's Encrypt for scoped admins (see + // requireGlobalAdmin, which is the actual enforcement — this only controls the + // link's visibility). + if u := userFromContext(r); u != nil { + data["is_global_admin"] = u.IsGlobalAdmin + } // Sidebar badge counts (Domains/Senders/Mailboxes/IPs/DKIM Keys) — computed here, // centrally, so every authenticated page shows them, not just the dashboard (which // used to compute these itself and nowhere else did). diff --git a/internal/webui/settings.go b/internal/webui/settings.go index 17c56ae..38dde01 100644 --- a/internal/webui/settings.go +++ b/internal/webui/settings.go @@ -42,6 +42,25 @@ func (a *App) settingsUpdate(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, Prefix+"/settings", http.StatusFound) return } + // Turning on enforce_admin_mfa immediately blocks every admin route except + // /account for any admin without MFA configured — including /settings itself. + // Without this precondition, an admin who enables enforcement before setting up + // their own MFA would lock themselves out with no way back in (short of editing + // the database directly), since requireAuth's gate applies to this very handler. + if r.FormValue("Auth.enforce_admin_mfa") == "true" && a.Cfg.Section("Auth").Key("enforce_admin_mfa").Value() != "true" { + user := userFromContext(r) + hasMFA := user.TOTPEnabled + if !hasMFA { + if n, _ := a.DB.CountWebAuthnCredentials(user.ID); n > 0 { + hasMFA = true + } + } + if !hasMFA { + setFlash(w, "error", "Set up your own two-factor authentication (see Account) before enforcing it for all admins — otherwise you'd lock yourself out.") + http.Redirect(w, r, Prefix+"/settings", http.StatusFound) + return + } + } changed := false for _, name := range a.Cfg.SectionStrings() { sec := a.Cfg.Section(name) diff --git a/internal/webui/settings_mfa_test.go b/internal/webui/settings_mfa_test.go new file mode 100644 index 0000000..4423d6e --- /dev/null +++ b/internal/webui/settings_mfa_test.go @@ -0,0 +1,98 @@ +package webui + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +// TestSettingsRejectsEnablingAdminMFAWithoutOwnMFA guards against a real self-lockout +// bug: enabling enforce_admin_mfa immediately blocks every admin route (including +// /settings itself) for any admin without their own MFA configured. Without this +// precondition, an admin could flip the toggle on and lock themselves out with no way +// back in short of editing the database directly. +func TestSettingsRejectsEnablingAdminMFAWithoutOwnMFA(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + cookie := loginSession(t, app) // loginSession's admin has no MFA configured + + form := baseSettingsForm() + form.Set("Auth.enforce_admin_mfa", "true") + req := httptest.NewRequest(http.MethodPost, Prefix+"/settings_update", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusFound { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + if got := app.Cfg.Section("Auth").Key("enforce_admin_mfa").Value(); got != "false" { + t.Fatalf("enforce_admin_mfa = %q, want unchanged (still false) since the admin has no MFA", got) + } + + // The same admin must still be able to reach every other page — the whole point + // of rejecting the save is that nothing actually changed. + req = httptest.NewRequest(http.MethodGet, Prefix+"/domains", nil) + req.AddCookie(cookie) + rec = httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected /domains still reachable, got %d", rec.Code) + } +} + +// TestSettingsAllowsEnablingAdminMFAWithOwnMFA confirms the precondition isn't just a +// blanket rejection — an admin who already has MFA set up can turn enforcement on. +func TestSettingsAllowsEnablingAdminMFAWithOwnMFA(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + cookie := loginSession(t, app) + + sess, err := app.DB.GetSession(cookie.Value) + if err != nil || sess == nil { + t.Fatal(err) + } + if err := app.DB.SetAdminTOTPSecret(sess.UserID, "JBSWY3DPEHPK3PXP", true); err != nil { + t.Fatal(err) + } + + form := baseSettingsForm() + form.Set("Auth.enforce_admin_mfa", "true") + req := httptest.NewRequest(http.MethodPost, Prefix+"/settings_update", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusFound { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + if got := app.Cfg.Section("Auth").Key("enforce_admin_mfa").Value(); got != "true" { + t.Fatalf("enforce_admin_mfa = %q, want true", got) + } +} + +// baseSettingsForm returns a full, valid settings_update submission (every field +// unchanged from newTestApp's fixture) — settingsUpdate iterates every existing ini +// key and only touches ones present in the form, but real form submissions always +// include every field on the page, so tests should too. +func baseSettingsForm() url.Values { + return url.Values{ + "Server.smtp_port": {"4025"}, "Server.smtp_tls_port": {"40465"}, + "Server.web_http_port": {"5000"}, "Server.web_https_port": {"5001"}, + "Server.bind_ip": {"0.0.0.0"}, "Server.time_zone": {"UTC"}, + "Server.hostname": {"mail.example.com"}, "Server.helo_hostname": {"mail.example.com"}, + "Server.server_banner": {""}, + "Database.database_url": {"sqlite:///server_data/smtp_server.db"}, + "Logging.log_level": {"INFO"}, "Logging.hide_info_aiosmtpd": {"true"}, + "Relay.relay_timeout": {"30"}, + "TLS.tls_cert_file": {"ssl_certs/server.crt"}, "TLS.tls_key_file": {"ssl_certs/server.key"}, + "DKIM.dkim_key_size": {"2048"}, "DKIM.spf_server_ip": {"192.168.1.1"}, + "Attachments.attachments_path": {"server_data/attachments"}, + "IMAP.imap_port": {"1143"}, "IMAP.imap_tls_port": {"1993"}, + "Auth.enforce_admin_mfa": {"false"}, "Auth.enforce_mailbox_mfa": {"false"}, + } +} diff --git a/internal/webui/settings_scope_test.go b/internal/webui/settings_scope_test.go new file mode 100644 index 0000000..370ed69 --- /dev/null +++ b/internal/webui/settings_scope_test.go @@ -0,0 +1,78 @@ +package webui + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestScopedAdminCannotAccessServerSettingsOrLetsEncrypt guards against a regression +// where a domain-scoped admin (delegated access to specific domains only, not a +// global admin) could still read/change server-wide config that has nothing to do +// with any one domain — the admin dashboard's own port, TLS certs, database URL, +// Let's Encrypt/ACME credentials, etc. +func TestScopedAdminCannotAccessServerSettingsOrLetsEncrypt(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + domains, err := app.DB.ListDomains() + if err != nil || len(domains) == 0 { + t.Fatal("no seeded domain") + } + cookie := scopedLogin(t, app, "scoped-settings-admin", []int64{domains[0].ID}) + + routes := []struct { + method, path string + }{ + {http.MethodGet, "/pymta-manager/settings"}, + {http.MethodPost, "/pymta-manager/settings_update"}, + {http.MethodGet, "/pymta-manager/letsencrypt"}, + {http.MethodPost, "/pymta-manager/letsencrypt/save"}, + {http.MethodPost, "/pymta-manager/letsencrypt/obtain"}, + {http.MethodGet, "/pymta-manager/api/settings/get_public_ip"}, + } + for _, rt := range routes { + req := httptest.NewRequest(rt.method, rt.path, nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusNotFound { + t.Errorf("%s %s: status=%d, want 404 for a scoped admin", rt.method, rt.path, rec.Code) + } + } + + // A global admin must still reach these routes normally. + globalCookie := loginSession(t, app) + req := httptest.NewRequest(http.MethodGet, "/pymta-manager/settings", nil) + req.AddCookie(globalCookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Errorf("global admin GET /settings: status=%d, want 200", rec.Code) + } +} + +// TestScopedAdminSidebarHidesServerWideLinks confirms the sidebar doesn't even show +// Server Settings/Let's Encrypt to a scoped admin (backend enforcement is the real +// gate, this is just the corresponding UI affordance). +func TestScopedAdminSidebarHidesServerWideLinks(t *testing.T) { + app := newTestApp(t) + mux := app.Mux() + domains, _ := app.DB.ListDomains() + cookie := scopedLogin(t, app, "scoped-sidebar-admin", []int64{domains[0].ID}) + + req := httptest.NewRequest(http.MethodGet, "/pymta-manager/", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("dashboard: status=%d body=%s", rec.Code, rec.Body.String()) + } + body := rec.Body.String() + if strings.Contains(body, `href="/pymta-manager/settings"`) { + t.Error("scoped admin's sidebar should not link to Server Settings") + } + if strings.Contains(body, `href="/pymta-manager/letsencrypt"`) { + t.Error("scoped admin's sidebar should not link to Let's Encrypt") + } +} diff --git a/internal/webui/templates/account.html b/internal/webui/templates/account.html index a656472..c13c2d1 100644 --- a/internal/webui/templates/account.html +++ b/internal/webui/templates/account.html @@ -2,6 +2,12 @@ {{define "page_title"}}Account Settings{{end}} {{define "content"}} +{{if .mfa_required}} +