Stop an owner deleting the account they are signed in as

Deleting your own user succeeded, and logged you out doing it:
local_sessions.user_id is ON DELETE CASCADE, so the delete took the
caller's own live session with it. Nothing refused this. The
last-owner guard is the only thing in the path, and it passes cleanly
as soon as a second owner exists -- which is exactly the state you are
in just after creating one.

The way back in was then whatever other account happened to exist, and
-seed-admin could not help: it skipped whenever *any* local user was
present, so the command documented as the way to create an
administrator refused precisely when there was no usable one, because
some other account still existed. It now asks whether the admin
account itself is missing, which is what its own help text always
claimed, and what makes it useful as recovery rather than only as
first-run bootstrap.

TestCanDeleteAnOwnerWhenAnotherRemains signed in as admin1 and deleted
admin1, asserting 204 -- it encoded the lockout as intended behaviour.
It now deletes the other owner, which is what it meant to cover, and a
new test holds the refusal in place.

runSeedAdmin takes a small interface so the bootstrap path is tested
without a Postgres pool; it had no tests before.

Signed-off-by: John Coffey <[email protected]>

api/localauth/handler_test.go

@@ -543,16 +543,49 @@ func TestCannotDeleteTheLastOwner(t *testing.T) {
 
 func TestCanDeleteAnOwnerWhenAnotherRemains(t *testing.T) {
 	fs := newFakeStore()
-	owner1 := mustCreateUser(t, fs, "admin1", "adminpass1", authz.RoleOwner)
+	mustCreateUser(t, fs, "admin1", "adminpass1", authz.RoleOwner)
+	// The caller deletes the *other* owner, not itself. This test used
+	// to sign in as admin1 and delete admin1, which passed only because
+	// self-deletion was unguarded -- it asserted the lockout as if it
+	// were the intended behaviour. What it means to test is that the
+	// last-owner guard doesn't fire while a second owner remains, and
+	// that holds without deleting the caller.
+	owner2 := mustCreateUser(t, fs, "admin2", "adminpass2", authz.RoleOwner)
+	_, mux := newTestHandler(t, fs)
+
+	login := doRequest(t, mux, http.MethodPost, "/auth/login", `{"username":"admin1","password":"adminpass1"}`, nil)
+	cookie := sessionCookieFrom(login)
+
+	rec := doRequest(t, mux, http.MethodDelete, "/auth/users/"+owner2.ID, "", cookie)
+	if rec.Code != http.StatusNoContent {
+		t.Fatalf("deleting one of two owners: status = %d, want 204, body=%s", rec.Code, rec.Body.String())
+	}
+}
+
+// The lockout this guards against: an owner creates a second owner,
+// deletes their own account, and is signed out by the resulting
+// local_sessions cascade with no supported way back in unless they
+// already know the other account's password.
+func TestCannotDeleteYourOwnAccount(t *testing.T) {
+	fs := newFakeStore()
+	owner := mustCreateUser(t, fs, "admin1", "adminpass1", authz.RoleOwner)
+	// A second owner exists, so the last-owner guard is satisfied and
+	// cannot be what refuses this.
 	mustCreateUser(t, fs, "admin2", "adminpass2", authz.RoleOwner)
 	_, mux := newTestHandler(t, fs)
 
 	login := doRequest(t, mux, http.MethodPost, "/auth/login", `{"username":"admin1","password":"adminpass1"}`, nil)
 	cookie := sessionCookieFrom(login)
 
-	rec := doRequest(t, mux, http.MethodDelete, "/auth/users/"+owner1.ID, "", cookie)
-	if rec.Code != http.StatusNoContent {
-		t.Fatalf("deleting one of two owners: status = %d, want 204, body=%s", rec.Code, rec.Body.String())
+	rec := doRequest(t, mux, http.MethodDelete, "/auth/users/"+owner.ID, "", cookie)
+	if rec.Code != http.StatusConflict {
+		t.Fatalf("deleting your own account: status = %d, want 409, body=%s", rec.Code, rec.Body.String())
+	}
+
+	// Still signed in, and the account still exists.
+	sess := doRequest(t, mux, http.MethodGet, "/auth/session", "", cookie)
+	if sess.Code != http.StatusOK {
+		t.Fatalf("session after a refused self-delete: status = %d, want 200, body=%s", sess.Code, sess.Body.String())
 	}
 }