Nobody could change their own password

POST /api/auth/password. The cost of its absence was measured today
rather than argued: rotating three production accounts took a shell on
the host, three round trips, and briefly left a PLATFORM ADMIN - the
account that reads every company on the estate - with the password
PASTE_IT_HERE, because a placeholder in a pasted command was taken
literally and there was no way to correct it from the product.

A manager could always reset somebody ELSE's password. A platform admin
could be reset by nobody: they have no client, so the team routes are
not theirs, and `provision user` on the host was the only route. For
software that puts accounts on shop-floor PCs and staff phones, this is
not a feature - it is what makes every other credential decision
recoverable.

Three decisions:

- **authed, not tenantOnly.** A session is not a company's data, and the
  account with no company is precisely the one that had no route. Scoping
  this by client would have reproduced the hole it exists to close, which
  is also why SetUserPassword is not scoped by client the way
  ResetMemberPassword beside it is. The user id comes from the verified
  session, never the request, so there is nothing to point at anyone else.

- **The current password is required.** An access token lives twelve
  hours and travels on devices that get lost and shared; without this a
  stolen one owns the account permanently instead of until it expires.

- **Every OTHER session is revoked, and the caller's is kept.** Somebody
  changing their password because they believe it is known must not have
  to wonder whether the device that already had it is still signed in -
  and must not be signed out of the one in their hand while dealing with
  it. A failure there is logged, not returned: the password IS changed by
  then, and reporting an error would send them to retry with a current
  password that no longer exists.

The suite's login() helper fatals on anything but 200, which is right
everywhere else and useless here - half of what these tests assert is
that a password has STOPPED working. loginCode() returns the status.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGcjxF1cNLcuwc3DAPcnfj

server/internal/api/api.go

@@ -57,6 +57,7 @@ type Store interface {
 	UserSessions(ctx context.Context, userID string) ([]DeviceSession, error)
 	RevokeUserSession(ctx context.Context, userID, sessionID string) error
 	RevokeOtherSessions(ctx context.Context, userID, keepSessionID string) (int, error)
+	SetUserPassword(ctx context.Context, userID, hash string) error
 
 	// --- team and invitations ---
 	// Registration is by invitation: the code carries the address and the role
@@ -294,6 +295,10 @@ func (s *Server) Routes() *http.ServeMux {
 	// Devices. A person may list and revoke their own sessions; removing a
 	// colleague's access is a different question, answered by deactivating them
 	// on the team endpoint below.
+	// Changing your own password. `authed`, not `tenantOnly`: a session is not
+	// a company's data, and a platform admin has no company but must still be
+	// able to do this - they were the account with no route at all.
+	mux.HandleFunc("POST /api/auth/password", s.authed(s.handleChangePassword))
 	mux.HandleFunc("GET /api/auth/sessions", s.authed(s.handleSessions))
 	mux.HandleFunc("DELETE /api/auth/sessions/{id}", s.authed(s.handleRevokeSession))
 	mux.HandleFunc("POST /api/auth/sessions/revoke-others",

server/internal/api/fake_test.go

@@ -317,6 +317,19 @@ func (f *fakeStore) SiteHealth(_ context.Context, clientID string) ([]SiteHealth
 	return out, nil
 }
 
+func (f *fakeStore) SetUserPassword(_ context.Context, userID, hash string) error {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	for email, u := range f.users {
+		if u.ID == userID {
+			u.PasswordHash = hash
+			f.users[email] = u
+			return nil
+		}
+	}
+	return errors.New("no such user")
+}
+
 func (f *fakeStore) Sales(_ context.Context, q SaleQuery) ([]Sale, error) {
 	f.mu.Lock()
 	defer f.mu.Unlock()

server/internal/api/handlers_password.go

@@ -0,0 +1,92 @@
+// Changing your own password.
+//
+// This did not exist, and the cost of that was measured rather than guessed:
+// rotating three production accounts took a shell on the server, three round
+// trips, and briefly left a PLATFORM ADMIN - the account that reads every
+// company on the estate - with a password anyone watching could guess, because
+// a placeholder in a pasted command was taken literally.
+//
+// A manager could always reset somebody ELSE's password, and a platform admin
+// could be reset by nobody at all: they have no client, so the team routes are
+// not theirs, and `provision user` on the host was the only way. For a product
+// that puts accounts on shop-floor PCs and staff phones, "change my password"
+// is not a feature, it is the thing that makes every other credential decision
+// recoverable.
+package api
+
+import (
+	"net/http"
+
+	"github.com/loyaly/behavision-server/internal/auth"
+)
+
+// handleChangePassword is on `authed`, NOT `tenantOnly`.
+//
+// A session is not a company's data. A platform admin has no client and must
+// still be able to change their own password - they are precisely the account
+// for which there was no other route.
+func (s *Server) handleChangePassword(w http.ResponseWriter, r *http.Request) {
+	p := PrincipalFrom(r.Context())
+
+	var in ChangePassword
+	if err := decode(w, r, &in); err != nil {
+		badRequest(w, err.Error())
+		return
+	}
+
+	// The CURRENT password is required, and that is the whole security
+	// argument. An access token lives twelve hours and travels on shop-floor
+	// devices; without this, anyone holding a stolen one could set a new
+	// password and own the account permanently rather than for the rest of
+	// the day.
+	rec, err := s.Store.UserByEmail(r.Context(), p.Email)
+	if err != nil {
+		s.serverError(w, "change password", err)
+		return
+	}
+	if !rec.Found || !auth.VerifyPassword(rec.PasswordHash, in.CurrentPassword) {
+		// Deliberately not throttled separately: this needs a live session, so
+		// it is not reachable by anyone guessing from outside, and the login
+		// throttle already governs getting one.
+		writeErr(w, http.StatusForbidden, "wrong_password",
+			"That is not your current password.")
+		return
+	}
+	if in.CurrentPassword == in.NewPassword {
+		badRequest(w, "the new password is the same as the old one")
+		return
+	}
+
+	hash, err := auth.HashPassword(in.NewPassword)
+	if err != nil {
+		// HashPassword enforces the length floor, and its message names it.
+		badRequest(w, err.Error())
+		return
+	}
+	if err := s.Store.SetUserPassword(r.Context(), p.UserID, hash); err != nil {
+		s.serverError(w, "change password", err)
+		return
+	}
+
+	// Every OTHER session goes, and the caller's stays. Somebody changing
+	// their password because they think it is known must not have to guess
+	// whether the change took effect on the device that already had it - and
+	// must not be signed out of the one in their hand while they deal with it.
+	revoked, err := s.Store.RevokeOtherSessions(r.Context(), p.UserID, p.SessionID)
+	if err != nil {
+		// The password IS changed. Reporting a failure here would tell the
+		// user to try again, and the retry would fail on the current password
+		// they just replaced.
+		s.Log.Printf("change password: revoke other sessions: %v", err)
+	}
+
+	s.Store.Audit(r.Context(), AuditEntry{
+		ClientID: p.ClientID, ActorID: p.UserID, ActorKind: "user",
+		Action: "auth.change_password", Entity: "user", EntityID: p.UserID,
+		Detail: map[string]any{"sessions_revoked": revoked},
+	})
+	writeJSON(w, http.StatusOK, map[string]any{
+		"changed":          true,
+		"sessions_revoked": revoked,
+	})
+}

server/internal/api/password_test.go

@@ -0,0 +1,115 @@
+package api
+
+import (
+	"net/http"
+	"testing"
+)
+
+const pwPath = "/api/auth/password"
+
+// loginCode signs in and returns only the status. The suite's login() fatals
+// on anything but 200, which is right everywhere else and useless here: half
+// of what these tests assert is that a password has STOPPED working.
+func loginCode(t *testing.T, s *Server, email, password string) int {
+	t.Helper()
+	return do(t, s, "POST", "/api/auth/login", "",
+		map[string]string{"email": email, "password": password}).Code
+}
+
+// The account this endpoint exists for. A platform admin has no company, so
+// the team routes are not theirs and tenantOnly refuses them - before this,
+// changing their password needed a shell on the production host.
+func TestAPlatformAdminCanChangeTheirOwnPassword(t *testing.T) {
+	s, fs := newServer(t)
+	seedPlatformAdmin(fs)
+	sess := login(t, s, "root@loyaly.ai", "admin123")
+
+	rec := do(t, s, "POST", pwPath, sess.Token, map[string]string{
+		"current_password": "admin123", "new_password": "a-much-longer-one",
+	})
+	if rec.Code != http.StatusOK {
+		t.Fatalf("got %d: %s", rec.Code, rec.Body.String())
+	}
+	// The new one works and the old one does not - asserted by signing in,
+	// because that is the only thing a user actually cares about here.
+	if c := loginCode(t, s, "root@loyaly.ai", "a-much-longer-one"); c != http.StatusOK {
+		t.Errorf("new password signs in: got %d, want 200", c)
+	}
+	if c := loginCode(t, s, "root@loyaly.ai", "admin123"); c == http.StatusOK {
+		t.Error("the old password still signs in")
+	}
+}
+
+func TestAnOrdinaryUserCanChangeTheirOwnPassword(t *testing.T) {
+	s, fs := newServer(t)
+	seedUser(fs)
+	sess := login(t, s, "manager@acme.com", "correct horse battery")
+
+	rec := do(t, s, "POST", pwPath, sess.Token, map[string]string{
+		"current_password": "correct horse battery", "new_password": "staple-battery-horse",
+	})
+	if rec.Code != http.StatusOK {
+		t.Fatalf("got %d: %s", rec.Code, rec.Body.String())
+	}
+}
+
+// The whole security argument. An access token lives twelve hours and travels
+// on shop-floor PCs and staff phones; without this, a stolen one owns the
+// account permanently instead of until it expires.
+func TestChangingAPasswordRequiresTheCurrentOne(t *testing.T) {
+	s, fs := newServer(t)
+	seedUser(fs)
+	sess := login(t, s, "manager@acme.com", "correct horse battery")
+
+	rec := do(t, s, "POST", pwPath, sess.Token, map[string]string{
+		"current_password": "not the password", "new_password": "a-much-longer-one",
+	})
+	if rec.Code != http.StatusForbidden {
+		t.Fatalf("got %d, want 403 - a token alone must not be enough: %s",
+			rec.Code, rec.Body.String())
+	}
+	// And it must not have changed anything.
+	if c := loginCode(t, s, "manager@acme.com", "correct horse battery"); c != http.StatusOK {
+		t.Errorf("a refused change must leave the old password working: got %d", c)
+	}
+}
+
+// The floor lives in HashPassword, so this asserts the endpoint routes through
+// it rather than re-implementing a check that could drift from the constant.
+func TestAShortNewPasswordIsRefused(t *testing.T) {
+	s, fs := newServer(t)
+	seedUser(fs)
+	sess := login(t, s, "manager@acme.com", "correct horse battery")
+
+	rec := do(t, s, "POST", pwPath, sess.Token, map[string]string{
+		"current_password": "correct horse battery", "new_password": "short",
+	})
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("got %d, want 400 for a password under the floor", rec.Code)
+	}
+}
+
+func TestReusingTheSamePasswordIsRefused(t *testing.T) {
+	s, fs := newServer(t)
+	seedUser(fs)
+	sess := login(t, s, "manager@acme.com", "correct horse battery")
+
+	rec := do(t, s, "POST", pwPath, sess.Token, map[string]string{
+		"current_password": "correct horse battery",
+		"new_password":     "correct horse battery",
+	})
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("got %d, want 400 - a no-op change reads as success and is not", rec.Code)
+	}
+}
+
+func TestChangingAPasswordNeedsASession(t *testing.T) {
+	s, fs := newServer(t)
+	seedUser(fs)
+	rec := do(t, s, "POST", pwPath, "", map[string]string{
+		"current_password": "correct horse battery", "new_password": "a-much-longer-one",
+	})
+	if rec.Code != http.StatusUnauthorized {
+		t.Errorf("got %d, want 401", rec.Code)
+	}
+}

server/internal/api/types.go

@@ -172,6 +172,14 @@ type DashboardSummary struct {
 	Timezone          string  `json:"timezone"`
 }
 
+// ChangePassword is the body of POST /api/auth/password. The current password
+// is required: an access token alone must not be enough to take an account
+// over permanently.
+type ChangePassword struct {
+	CurrentPassword string `json:"current_password"`
+	NewPassword     string `json:"new_password"`
+}
+
 type Customer struct {
 	ID string `json:"id"`
 	// Ref is the customer number - "V-42" - and is accepted anywhere this

server/internal/store/api_team.go

@@ -414,3 +414,30 @@ func (s *Store) ResetMemberPassword(ctx context.Context, clientID, userID,
 	}
 	return m, nil
 }
+
+// SetUserPassword changes one account's password, by user id.
+//
+// Deliberately NOT scoped by client, unlike ResetMemberPassword beside it.
+// That one is a manager acting on somebody else in their company, so the
+// tenant is the boundary. This is an account acting on ITSELF, and the caller
+// is the session - a platform admin has no client at all and was, before this,
+// the one account nobody could change the password of without a shell on the
+// host. Scoping by client here would have reproduced exactly that hole.
+//
+// The id comes from the verified session and never from the request, so there
+// is nothing here for a caller to point at somebody else.
+func (s *Store) SetUserPassword(ctx context.Context, userID, hash string) error {
+	tag, err := s.pool.Exec(ctx, `
+		UPDATE app_users SET password_hash = $2
+		 WHERE id = $1::uuid AND active`, userID, hash)
+	if err != nil {
+		return err
+	}
+	if tag.RowsAffected() == 0 {
+		// Deactivated mid-session: their sessions are already revoked, so this
+		// is unreachable in practice, and silently succeeding would report a
+		// password change that did not happen.
+		return fmt.Errorf("no active account %s", userID)
+	}
+	return nil
+}