diff --git a/PROGRESS.md b/PROGRESS.md index 90629a0..7e5d3d4 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -4,6 +4,59 @@ Source of truth for current development state. --- All phases complete. **v1.0.0 tagged.** All packages pass `go test ./...`; `golangci-lint run ./...` clean. +### 2026-03-12 — Checkpoint: password change UI enforcement + migration recovery + +**internal/ui/handlers_accounts.go** +- `handleAdminResetPassword`: added server-side admin role check at the top of + the handler; any authenticated non-admin calling this route now receives 403. + Previously only cookie validity + CSRF were checked. + +**internal/ui/handlers_auth.go** +- Added `handleProfilePage`: renders the new `/profile` page for any + authenticated user. +- Added `handleSelfChangePassword`: self-service password change for non-admin + users; validates current password (Argon2id, lockout-checked), enforces + server-side confirmation equality check, hashes new password, revokes all + other sessions, audits as `{"via":"ui_self_service"}`. + +**internal/ui/ui.go** +- Added `ProfileData` view model. +- Registered `GET /profile` and `PUT /profile/password` routes (cookie auth + + CSRF; no admin role required). +- Added `password_change_form.html` to shared template list; added `profile` + page template. +- Nav bar actor-name span changed to a link pointing to `/profile`. + +**web/templates/fragments/password_change_form.html** (new) +- HTMX form with `current_password`, `new_password`, `confirm_password` fields. +- Client-side JS confirmation guard; server-side equality check in handler. + +**web/templates/profile.html** (new) +- Profile page hosting the self-service password change form. + +**internal/db/migrate.go** +- Compatibility shim now only calls `m.Force(legacyVersion)` when + `schema_migrations` is completely empty (`ErrNilVersion`); leaves existing + version entries (including dirty ones) alone to prevent re-running already- + attempted migrations. +- Added duplicate-column-name recovery: when `m.Up()` fails with "duplicate + column name" and the dirty version equals `LatestSchemaVersion`, the migrator + is force-cleaned and returns nil (handles databases where columns were added + outside the runner before migration 006 existed). +- Added `ForceSchemaVersion(database *DB, version int) error`: break-glass + exported function; forces golang-migrate version without running SQL. + +**cmd/mciasdb/schema.go** +- Added `schema force --version N` subcommand backed by `db.ForceSchemaVersion`. + +**cmd/mciasdb/main.go** +- `schema` commands now open the database via `openDBRaw` (no auto-migration) + so the tool stays usable when the database is in a dirty migration state. +- `openDB` refactored to call `openDBRaw` then `db.Migrate`. +- Updated usage text. + +All tests pass; `golangci-lint run ./...` clean. + ### 2026-03-12 — Password change: self-service and admin reset Added the ability for users to change their own password and for admins to @@ -394,9 +447,10 @@ All tests pass (`go test ./...`); `golangci-lint run ./...` reports 0 issues. - `engine.go` — `Evaluate(input, operatorRules) (Effect, *Rule)`: pure function; merges operator rules with default rules, sorts by priority, deny-wins, then first allow, then default-deny -- `defaults.go` — 6 compiled-in rules (IDs -1 to -6, Priority 0): admin - wildcard, self-service logout/renew, self-service TOTP, system account own - pgcreds, system account own service token, public login/validate endpoints +- `defaults.go` — 7 compiled-in rules (IDs -1 to -7, Priority 0): admin + wildcard, self-service logout/renew, self-service TOTP, self-service password + change (human only), system account own pgcreds, system account own service + token, public login/validate endpoints - `engine_wrapper.go` — `Engine` struct with `sync.RWMutex`; `SetRules()` decodes DB records; `PolicyRecord` type avoids import cycle - `engine_test.go` — 11 tests: DefaultDeny, AdminWildcard, SelfService*, diff --git a/cmd/mciasdb/main.go b/cmd/mciasdb/main.go index 49050d7..498ab8e 100644 --- a/cmd/mciasdb/main.go +++ b/cmd/mciasdb/main.go @@ -15,6 +15,7 @@ // // schema verify // schema migrate +// schema force --version N // // account list // account get --id UUID @@ -62,7 +63,22 @@ func main() { os.Exit(1) } - database, masterKey, err := openDB(*configPath) + command := args[0] + subArgs := args[1:] + + // schema subcommands manage migrations themselves and must not trigger + // auto-migration on open (a dirty database would prevent the tool from + // opening at all, blocking recovery operations like "schema force"). + var ( + database *db.DB + masterKey []byte + err error + ) + if command == "schema" { + database, masterKey, err = openDBRaw(*configPath) + } else { + database, masterKey, err = openDB(*configPath) + } if err != nil { fatalf("%v", err) } @@ -76,9 +92,6 @@ func main() { tool := &tool{db: database, masterKey: masterKey} - command := args[0] - subArgs := args[1:] - switch command { case "schema": tool.runSchema(subArgs) @@ -111,6 +124,21 @@ type tool struct { // the same passphrase always yields the same key and encrypted secrets remain // readable. The passphrase env var is unset immediately after reading. func openDB(configPath string) (*db.DB, []byte, error) { + database, masterKey, err := openDBRaw(configPath) + if err != nil { + return nil, nil, err + } + if err := db.Migrate(database); err != nil { + _ = database.Close() + return nil, nil, fmt.Errorf("migrate database: %w", err) + } + return database, masterKey, nil +} + +// openDBRaw opens the database without running migrations. Used by schema +// subcommands so they remain operational even when the database is in a dirty +// migration state (e.g. to allow "schema force" to clear a dirty flag). +func openDBRaw(configPath string) (*db.DB, []byte, error) { cfg, err := config.Load(configPath) if err != nil { return nil, nil, fmt.Errorf("load config: %w", err) @@ -121,11 +149,6 @@ func openDB(configPath string) (*db.DB, []byte, error) { return nil, nil, fmt.Errorf("open database %q: %w", cfg.Database.Path, err) } - if err := db.Migrate(database); err != nil { - _ = database.Close() - return nil, nil, fmt.Errorf("migrate database: %w", err) - } - masterKey, err := deriveMasterKey(cfg, database) if err != nil { _ = database.Close() @@ -210,6 +233,7 @@ Global flags: Commands: schema verify Check schema version; exit 1 if migrations pending schema migrate Apply any pending schema migrations + schema force --version N Force schema version (clears dirty state) account list List all accounts account get --id UUID diff --git a/cmd/mciasdb/schema.go b/cmd/mciasdb/schema.go index f5ace9a..fcd4c9a 100644 --- a/cmd/mciasdb/schema.go +++ b/cmd/mciasdb/schema.go @@ -1,6 +1,7 @@ package main import ( + "flag" "fmt" "git.wntrmute.dev/kyle/mcias/internal/db" @@ -8,13 +9,15 @@ import ( func (t *tool) runSchema(args []string) { if len(args) == 0 { - fatalf("schema requires a subcommand: verify, migrate") + fatalf("schema requires a subcommand: verify, migrate, force") } switch args[0] { case "verify": t.schemaVerify() case "migrate": t.schemaMigrate() + case "force": + t.schemaForce(args[1:]) default: fatalf("unknown schema subcommand %q", args[0]) } @@ -39,6 +42,26 @@ func (t *tool) schemaVerify() { fmt.Println("schema is up-to-date") } +// schemaForce marks the database as being at a specific migration version +// without running any SQL. Use this to clear a dirty migration state after +// you have verified that the schema already reflects the target version. +// +// Example: mciasdb schema force --version 6 +func (t *tool) schemaForce(args []string) { + fs := flag.NewFlagSet("schema force", flag.ExitOnError) + version := fs.Int("version", 0, "schema version to force (required)") + _ = fs.Parse(args) + + if *version <= 0 { + fatalf("--version must be a positive integer") + } + + if err := db.ForceSchemaVersion(t.db, *version); err != nil { + fatalf("force schema version: %v", err) + } + fmt.Printf("schema version forced to %d; run 'schema migrate' to apply any remaining migrations\n", *version) +} + // schemaMigrate applies any pending migrations and reports each one. func (t *tool) schemaMigrate() { before, err := db.SchemaVersion(t.db) diff --git a/internal/db/migrate.go b/internal/db/migrate.go index ecdb665..0b2d8c4 100644 --- a/internal/db/migrate.go +++ b/internal/db/migrate.go @@ -5,6 +5,7 @@ import ( "embed" "errors" "fmt" + "strings" "github.com/golang-migrate/migrate/v4" sqlitedriver "github.com/golang-migrate/migrate/v4/database/sqlite" @@ -93,19 +94,65 @@ func Migrate(database *DB) error { defer func() { src, drv := m.Close(); _ = src; _ = drv }() if legacyVersion > 0 { - // Force the migrator to treat the database as already at - // legacyVersion so Up only applies newer migrations. - if err := m.Force(legacyVersion); err != nil { - return fmt.Errorf("db: force legacy schema version %d: %w", legacyVersion, err) + // Only fast-forward from the legacy version when golang-migrate has no + // version record of its own yet (ErrNilVersion). If schema_migrations + // already has an entry — including a dirty entry from a previously + // failed migration — leave it alone and let golang-migrate handle it. + // Overriding a non-nil version would discard progress (or a dirty + // state that needs idempotent re-application) and cause migrations to + // be retried unnecessarily. + _, _, versionErr := m.Version() + if errors.Is(versionErr, migrate.ErrNilVersion) { + if err := m.Force(legacyVersion); err != nil { + return fmt.Errorf("db: force legacy schema version %d: %w", legacyVersion, err) + } } } if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) { + // A "duplicate column name" error means the failing migration is an + // ADD COLUMN that was already applied outside the migration runner + // (common during development before a migration file existed). + // If this is the last migration and its version matches LatestSchemaVersion, + // force it clean so subsequent starts succeed. + // + // This is intentionally narrow: we only suppress the error when the + // dirty version equals the latest known version, preventing accidental + // masking of errors in intermediate migrations. + if strings.Contains(err.Error(), "duplicate column name") { + v, dirty, verErr := m.Version() + if verErr == nil && dirty && int(v) == LatestSchemaVersion { //nolint:gosec // G115: safe conversion + if forceErr := m.Force(LatestSchemaVersion); forceErr != nil { + return fmt.Errorf("db: force after duplicate column: %w", forceErr) + } + return nil + } + } return fmt.Errorf("db: apply migrations: %w", err) } return nil } +// ForceSchemaVersion marks the database as being at the given version without +// running any SQL. This is a break-glass operation: use it to clear a dirty +// migration state after verifying (or manually applying) the migration SQL. +// +// Passing a version that has never been recorded by golang-migrate is safe; +// it simply sets the version and clears the dirty flag. The next call to +// Migrate will apply any versions higher than the forced one. +func ForceSchemaVersion(database *DB, version int) error { + m, err := newMigrate(database) + if err != nil { + return err + } + defer func() { src, drv := m.Close(); _ = src; _ = drv }() + + if err := m.Force(version); err != nil { + return fmt.Errorf("db: force schema version %d: %w", version, err) + } + return nil +} + // SchemaVersion returns the current applied schema version of the database. // Returns 0 if no migrations have been applied yet. func SchemaVersion(database *DB) (int, error) { diff --git a/internal/policy/defaults.go b/internal/policy/defaults.go index 36c6fee..fe05478 100644 --- a/internal/policy/defaults.go +++ b/internal/policy/defaults.go @@ -42,6 +42,18 @@ var defaultRules = []Rule{ Actions: []Action{ActionEnrollTOTP}, Effect: Allow, }, + { + // Self-service password change: any authenticated human account may + // change their own password. The handler derives the target exclusively + // from the JWT subject (claims.Subject) and requires the current + // password, so a non-admin caller can only affect their own account. + ID: -7, + Description: "Self-service: any human account may change their own password", + Priority: 0, + AccountTypes: []string{"human"}, + Actions: []Action{ActionChangePassword}, + Effect: Allow, + }, { // System accounts reading their own pgcreds: a service that has already // authenticated (e.g. via its bearer service token) may retrieve its own diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 1336fd9..65dff8b 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -42,8 +42,9 @@ const ( ActionEnrollTOTP Action = "totp:enroll" // self-service ActionRemoveTOTP Action = "totp:remove" // admin - ActionLogin Action = "auth:login" // public - ActionLogout Action = "auth:logout" // self-service + ActionLogin Action = "auth:login" // public + ActionLogout Action = "auth:logout" // self-service + ActionChangePassword Action = "auth:change_password" // self-service ActionListRules Action = "policy:list" ActionManageRules Action = "policy:manage" diff --git a/internal/ui/handlers_accounts.go b/internal/ui/handlers_accounts.go index e56509a..e4b1f4e 100644 --- a/internal/ui/handlers_accounts.go +++ b/internal/ui/handlers_accounts.go @@ -901,10 +901,32 @@ func (u *UIServer) handleCreatePGCreds(w http.ResponseWriter, r *http.Request) { // for the target account are revoked so a compromised account is fully // invalidated. // -// Security: new password is validated (minimum 12 chars) and hashed with -// Argon2id before storage. The plaintext is never logged or included in any -// response. Audit event EventPasswordChanged is recorded on success. +// Security: caller must hold the admin role; the check is performed server-side +// against the JWT claims so it cannot be bypassed by client-side tricks. +// New password is validated (minimum 12 chars) and hashed with Argon2id before +// storage. The plaintext is never logged or included in any response. +// Audit event EventPasswordChanged is recorded on success. func (u *UIServer) handleAdminResetPassword(w http.ResponseWriter, r *http.Request) { + // Security: enforce admin role; requireCookieAuth only validates the token, + // it does not check roles. A non-admin with a valid session must not be + // able to reset arbitrary accounts' passwords. + callerClaims := claimsFromContext(r.Context()) + if callerClaims == nil { + u.renderError(w, r, http.StatusUnauthorized, "unauthorized") + return + } + isAdmin := false + for _, role := range callerClaims.Roles { + if role == "admin" { + isAdmin = true + break + } + } + if !isAdmin { + u.renderError(w, r, http.StatusForbidden, "admin role required") + return + } + r.Body = http.MaxBytesReader(w, r.Body, maxFormBytes) if err := r.ParseForm(); err != nil { u.renderError(w, r, http.StatusBadRequest, "invalid form") diff --git a/internal/ui/handlers_auth.go b/internal/ui/handlers_auth.go index 30a2f27..0fec7f6 100644 --- a/internal/ui/handlers_auth.go +++ b/internal/ui/handlers_auth.go @@ -8,6 +8,7 @@ import ( "git.wntrmute.dev/kyle/mcias/internal/crypto" "git.wntrmute.dev/kyle/mcias/internal/model" "git.wntrmute.dev/kyle/mcias/internal/token" + "git.wntrmute.dev/kyle/mcias/internal/validate" ) // handleLoginPage renders the login form. @@ -255,3 +256,127 @@ func (u *UIServer) writeAudit(r *http.Request, eventType string, actorID, target u.logger.Warn("write audit event", "type", eventType, "error", err) } } + +// handleProfilePage renders the profile page for the currently logged-in user. +func (u *UIServer) handleProfilePage(w http.ResponseWriter, r *http.Request) { + csrfToken, _ := u.setCSRFCookies(w) + u.render(w, "profile", ProfileData{ + PageData: PageData{ + CSRFToken: csrfToken, + ActorName: u.actorName(r), + }, + }) +} + +// handleSelfChangePassword allows an authenticated human user to change their +// own password. The current password must be supplied to prevent a stolen +// session token from being used to take over an account. +// +// Security: current password is verified with Argon2id (constant-time) before +// the new hash is written. Lockout is checked first so the endpoint cannot +// be used to brute-force the existing password. On success all other active +// sessions are revoked; the caller's own session is preserved so they remain +// logged in. The plaintext passwords are never logged or returned. +func (u *UIServer) handleSelfChangePassword(w http.ResponseWriter, r *http.Request) { + r.Body = http.MaxBytesReader(w, r.Body, maxFormBytes) + if err := r.ParseForm(); err != nil { + u.renderError(w, r, http.StatusBadRequest, "invalid form") + return + } + + claims := claimsFromContext(r.Context()) + if claims == nil { + u.renderError(w, r, http.StatusUnauthorized, "unauthorized") + return + } + + acct, err := u.db.GetAccountByUUID(claims.Subject) + if err != nil { + u.renderError(w, r, http.StatusUnauthorized, "account not found") + return + } + if acct.AccountType != model.AccountTypeHuman { + u.renderError(w, r, http.StatusBadRequest, "password change is only available for human accounts") + return + } + + currentPassword := r.FormValue("current_password") + newPassword := r.FormValue("new_password") + confirmPassword := r.FormValue("confirm_password") + + if currentPassword == "" || newPassword == "" { + u.renderError(w, r, http.StatusBadRequest, "current and new password are required") + return + } + // Server-side confirmation check mirrors the client-side guard; defends + // against direct POST requests that bypass the JavaScript validation. + if newPassword != confirmPassword { + u.renderError(w, r, http.StatusBadRequest, "passwords do not match") + return + } + + // Security: check lockout before running Argon2 to prevent brute-force. + locked, lockErr := u.db.IsLockedOut(acct.ID) + if lockErr != nil { + u.logger.Error("lockout check (UI self-service password change)", "error", lockErr) + } + if locked { + u.writeAudit(r, model.EventPasswordChanged, &acct.ID, &acct.ID, `{"result":"locked"}`) + u.renderError(w, r, http.StatusTooManyRequests, "account temporarily locked, please try again later") + return + } + + // Security: verify current password with constant-time Argon2id path used + // at login so this endpoint cannot serve as a timing oracle. + ok, verifyErr := auth.VerifyPassword(currentPassword, acct.PasswordHash) + if verifyErr != nil || !ok { + _ = u.db.RecordLoginFailure(acct.ID) + u.writeAudit(r, model.EventPasswordChanged, &acct.ID, &acct.ID, `{"result":"wrong_current_password"}`) + u.renderError(w, r, http.StatusUnauthorized, "current password is incorrect") + return + } + + // Security (F-13): enforce minimum length before hashing. + if err := validate.Password(newPassword); err != nil { + u.renderError(w, r, http.StatusBadRequest, err.Error()) + return + } + + hash, err := auth.HashPassword(newPassword, auth.ArgonParams{ + Time: u.cfg.Argon2.Time, + Memory: u.cfg.Argon2.Memory, + Threads: u.cfg.Argon2.Threads, + }) + if err != nil { + u.logger.Error("hash password (UI self-service)", "error", err) + u.renderError(w, r, http.StatusInternalServerError, "internal error") + return + } + + if err := u.db.UpdatePasswordHash(acct.ID, hash); err != nil { + u.logger.Error("update password hash", "error", err) + u.renderError(w, r, http.StatusInternalServerError, "failed to update password") + return + } + + // Security: clear failure counter (user proved knowledge of current + // password), then revoke all sessions except the current one so stale + // tokens are invalidated while the caller stays logged in. + _ = u.db.ClearLoginFailures(acct.ID) + if err := u.db.RevokeAllUserTokensExcept(acct.ID, claims.JTI, "password_changed"); err != nil { + u.logger.Error("revoke other tokens on UI password change", "account_id", acct.ID, "error", err) + u.renderError(w, r, http.StatusInternalServerError, "password updated but session revocation failed; revoke tokens manually") + return + } + + u.writeAudit(r, model.EventPasswordChanged, &acct.ID, &acct.ID, `{"via":"ui_self_service"}`) + + csrfToken, _ := u.setCSRFCookies(w) + u.render(w, "password_change_result", ProfileData{ + PageData: PageData{ + CSRFToken: csrfToken, + ActorName: u.actorName(r), + Flash: "Password updated successfully. Other active sessions have been revoked.", + }, + }) +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go index 1674a87..ccdb179 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -191,6 +191,7 @@ func New(database *db.DB, cfg *config.Config, priv ed25519.PrivateKey, pub ed255 "templates/fragments/policy_row.html", "templates/fragments/policy_form.html", "templates/fragments/password_reset_form.html", + "templates/fragments/password_change_form.html", } base, err := template.New("").Funcs(funcMap).ParseFS(web.TemplateFS, sharedFiles...) if err != nil { @@ -208,6 +209,7 @@ func New(database *db.DB, cfg *config.Config, priv ed25519.PrivateKey, pub ed255 "audit_detail": "templates/audit_detail.html", "policies": "templates/policies.html", "pgcreds": "templates/pgcreds.html", + "profile": "templates/profile.html", } tmpls := make(map[string]*template.Template, len(pageFiles)) for name, file := range pageFiles { @@ -296,6 +298,10 @@ func (u *UIServer) Register(mux *http.ServeMux) { uiMux.Handle("PUT /accounts/{id}/tags", admin(u.handleSetAccountTags)) uiMux.Handle("PUT /accounts/{id}/password", admin(u.handleAdminResetPassword)) + // Profile routes — accessible to any authenticated user (not admin-only). + uiMux.Handle("GET /profile", adminGet(u.handleProfilePage)) + uiMux.Handle("PUT /profile/password", auth(u.requireCSRF(http.HandlerFunc(u.handleSelfChangePassword)))) + // Mount the wrapped UI mux on the parent mux. The "/" pattern acts as a // catch-all for all UI paths; the more-specific /v1/ API patterns registered // on the parent mux continue to take precedence per Go's routing rules. @@ -611,6 +617,11 @@ type PoliciesData struct { AllActions []string } +// ProfileData is the view model for the profile/settings page. +type ProfileData struct { + PageData +} + // PGCredsData is the view model for the "My PG Credentials" list page. // It shows all pg_credentials sets accessible to the currently logged-in user: // those they own and those they have been granted access to. diff --git a/openapi.yaml b/openapi.yaml index d908ac7..10a01af 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1257,7 +1257,7 @@ paths: summary: List policy rules (admin) description: | Return all operator-defined policy rules ordered by priority (ascending). - Built-in default rules (IDs -1 to -6) are not included. + Built-in default rules (IDs -1 to -7) are not included. operationId: listPolicyRules tags: [Admin — Policy] security: diff --git a/web/templates/base.html b/web/templates/base.html index 83fdd08..d4c3af5 100644 --- a/web/templates/base.html +++ b/web/templates/base.html @@ -16,7 +16,7 @@
  • Audit
  • Policies
  • PG Creds
  • - {{if .ActorName}}
  • {{.ActorName}}
  • {{end}} + {{if .ActorName}}
  • {{.ActorName}}
  • {{end}}
  • diff --git a/web/templates/fragments/password_change_form.html b/web/templates/fragments/password_change_form.html new file mode 100644 index 0000000..87f9bb7 --- /dev/null +++ b/web/templates/fragments/password_change_form.html @@ -0,0 +1,53 @@ +{{define "password_change_form"}} +
    +
    + + +
    +
    + + +
    +
    + + +
    + + +
    + +{{end}} + +{{define "password_change_result"}} +{{if .Flash}} + +{{end}} +{{template "password_change_form" .}} +{{end}} diff --git a/web/templates/profile.html b/web/templates/profile.html new file mode 100644 index 0000000..29868c1 --- /dev/null +++ b/web/templates/profile.html @@ -0,0 +1,16 @@ +{{define "profile"}}{{template "base" .}}{{end}} +{{define "title"}}Profile — MCIAS{{end}} +{{define "content"}} + +
    +

    Change Password

    +

    + Enter your current password and choose a new one. Other active sessions will be revoked. +

    +
    + {{template "password_change_form" .}} +
    +
    +{{end}}