fix(appenroll): let the last phone go, at the box (#880)

The box's own page could remove phones down to the last one and then
stop: the row drew "add another before removing this one" where the
Remove button belongs. A household that lost its only phone, or wanted
to start clean, had no way back to an empty list from anywhere.

The refusal guards a real lockout, but only through one door. A phone
that empties the roster from the other side of the world leaves a box
nobody can administer, and nothing remote can mend it. Somebody at the
box is in a different position: the same page mints a fresh pairing
code, so the way back in is the button above the list.

Revoke now takes the presence of whoever is asking and refuses the last
owner over a session only. The decision stays in enrollment rather than
moving up to a screen; what is new is that the box is told which door
the request came through. That comes from the caller it named at
admission — LAN or kind "app" — never from a field a remote caller
could set. Demotion is unchanged at both doors: removing the last phone
leaves a list somebody at the box can fill again, and demoting it
leaves phones that can only look.

The device list follows. The last owner's row carries the same Remove,
and the warning lands at the press: a home down to one phone is told
nothing will see or change it until a phone is paired again with a code
from this page, and one with guests still paired is told those phones
will be left able to look and change nothing.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: frahlg

.changeset/remove-the-last-phone-at-the-box.md

@@ -0,0 +1,13 @@
+---
+"ftw": patch
+---
+
+A household can get back to no phones paired, from the box's own page. Until now the last owner could not be removed at all: the device list drew a sentence where the Remove button goes — add another before removing this one — so a home that lost its only phone, or wanted to start clean, had no way down to an empty list from anywhere.
+
+The refusal was right about one door and wrong about the other. What it guards against is a phone emptying the roster from anywhere in the world: remove the last owner over a session and nobody can administer the box, and nothing done remotely can mend that. Standing at the box is a different position. Whoever reads that page is in the building, and the same page mints a fresh pairing code — the way back in is the button above the list. Refusing there protected nothing and stranded the household the rule was written for.
+
+So `appenroll.Revoke` now takes the presence of whoever is asking, and refuses the last owner over a session only. The decision stays in enrollment rather than moving up to a screen; what is new is that the box is told which door the request came through. That fact comes from the caller the box named when it admitted the request — a LAN caller is minted as a local owner, an app session carries the kind `app` — and never from a field a remote caller could set. Removing the last owner through the app is still `E_LAST_OWNER_PROTECTED`, the same sentence and the same code.
+
+Stepping the last owner down is still refused at both doors, the box's own page included. Removing the last phone leaves a list somebody at the box can fill again; demoting it leaves a list of phones that can only look, which is the lockout rather than a way out of it.
+
+The box's device list follows. The last owner's row carries the Remove button every other row has, and the warning lands where it belongs — at the press, saying what is on the other side of it. A household down to one phone is told that nothing will see or change this home until a phone is paired again, with a code from this page. One with guests still paired is told those phones will be left able to look and change nothing. Both are what happens, which the old sentence was not: it described a rule the box no longer keeps.

go/cmd/ftw/app_link.go

@@ -689,8 +689,11 @@ func (a *appLinkAPI) SetDeviceRole(id, role string) error {
 	return appLinkError(a.enroll.SetRole(id, role))
 }
 
-func (a *appLinkAPI) RevokeDevice(id string) error {
-	key, err := a.enroll.Revoke(id)
+// RevokeDevice locks one phone out. atTheBox is the API's word for the door
+// the request came through, and it is the only thing that decides whether the
+// last owner may go; Presence is enrollment's word for the same fact.
+func (a *appLinkAPI) RevokeDevice(id string, atTheBox bool) error {
+	key, err := a.enroll.Revoke(id, appenroll.Presence(atTheBox))
 	if err != nil {
 		return appLinkError(err)
 	}

go/internal/api/api_app_link.go

@@ -47,8 +47,13 @@ type AppEnroller interface {
 	SetDeviceRole(id, role string) error
 	// RevokeDevice forgets one and tears down its live sessions. Returns
 	// ErrUnknownAppDevice when no row carries the id, and
-	// ErrLastAppOwnerProtected when it would leave the box with no owner.
-	RevokeDevice(id string) error
+	// ErrLastAppOwnerProtected when it would leave the box with no owner and
+	// the caller is not standing at it.
+	//
+	// atTheBox is that last fact, and it is the caller's to establish rather
+	// than the request's to claim: it comes from which door the request came
+	// through, never from anything a remote caller can put in one.
+	RevokeDevice(id string, atTheBox bool) error
 }
 
 // AppDevice is one paired phone. The id is a short prefix of its key —
@@ -61,8 +66,10 @@ type AppDevice struct {
 	// list rather than on a screen of its own, because a guest's phone is a
 	// paired phone and removing one is the same action as locking one out.
 	Role string `json:"role"`
-	// LastOwner marks the row that cannot be removed or demoted, so the page
-	// can say why before somebody presses the button.
+	// LastOwner marks the only phone left that can change anything here. It
+	// cannot be stepped down, and cannot be removed over a session — so a
+	// screen can say what it is before somebody presses a button. The box's
+	// own page may remove it, and warns instead of refusing.
 	LastOwner bool `json:"last_owner,omitempty"`
 }
 
@@ -232,6 +239,17 @@ func (s *Server) handleAppLinkDevices(w http.ResponseWriter, r *http.Request) {
 	writeJSON(w, http.StatusOK, map[string]any{"devices": s.deps.AppEnroll.Devices()})
 }
 
+// handleAppLinkDeviceRevoke locks one phone out, guest or owner.
+//
+// The last owner is the one row where the two doors part. Over a session the
+// removal is refused, because a phone that empties the roster from anywhere in
+// the world leaves a box nobody can administer and no remote way to mend it.
+// At the box it goes through: the person pressing the button is in the
+// building, and the same page mints a fresh pairing code, so refusing there
+// protects nothing and only strands a household that has lost the phone.
+//
+// Which door it was comes from the caller the box named at admission, never
+// from the request — there is no field a remote caller could set to claim it.
 func (s *Server) handleAppLinkDeviceRevoke(w http.ResponseWriter, r *http.Request) {
 	if !s.appLinkGate(w, r, appproto.ScopeMembersWrite) {
 		return
@@ -241,7 +259,7 @@ func (s *Server) handleAppLinkDeviceRevoke(w http.ResponseWriter, r *http.Reques
 		return
 	}
 	id := r.PathValue("id")
-	if err := s.deps.AppEnroll.RevokeDevice(id); err != nil {
+	if err := s.deps.AppEnroll.RevokeDevice(id, !appLinkOverSession(r)); err != nil {
 		if errors.Is(err, ErrUnknownAppDevice) {
 			writeAppLinkError(w, http.StatusNotFound, "that phone is no longer paired")
 			return

go/internal/api/api_app_link_session_test.go

@@ -73,8 +73,8 @@ func (e *realEnroller) SetDeviceRole(id, role string) error {
 	return enrollError(e.id.SetRole(id, role))
 }
 
-func (e *realEnroller) RevokeDevice(id string) error {
-	_, err := e.id.Revoke(id)
+func (e *realEnroller) RevokeDevice(id string, atTheBox bool) error {
+	_, err := e.id.Revoke(id, appenroll.Presence(atTheBox))
 	return enrollError(err)
 }
 

go/internal/api/api_app_link_sharing_test.go

@@ -15,6 +15,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/srcfl/ftw/go/internal/apiauth"
 	"github.com/srcfl/ftw/go/internal/appproto"
 )
 
@@ -30,6 +31,23 @@ func localRequest(method, target, body string) *http.Request {
 	return r
 }
 
+// sessionRequest is what the passthrough hands these handlers for an owner's
+// phone: a request built to look local on purpose — Host localhost, loopback
+// address, no forwarding header — carrying the caller the box named when the
+// session was admitted. An address alone cannot tell it from the page in the
+// kitchen, which is exactly why these handlers ask the caller instead.
+func sessionRequest(method, target, body string) *http.Request {
+	r := localRequest(method, target, body)
+	r.Host = "localhost"
+	r.RemoteAddr = "127.0.0.1:0"
+	return r.WithContext(apiauth.WithCaller(r.Context(), apiauth.Caller{
+		Subject: apiauth.KindApp + ":aaaa1111",
+		Kind:    apiauth.KindApp,
+		Role:    apiauth.RoleOwner,
+		Scopes:  apiauth.EveryScope(),
+	}))
+}
+
 // An invite is minted for the role that was asked for, and the answer names
 // that role back so the screen can say it in words above the code. A code
 // whose power is invisible is the one that gets read to the wrong person.
@@ -232,16 +250,22 @@ func TestARoleIsChangedFromTheDeviceList(t *testing.T) {
 
 // The last owner is protected, and the refusal has to be one a person can act
 // on: 409 with a sentence saying what to do first, not a 500.
+//
+// Removing it is refused over the session, where the household could be
+// anywhere. Stepping it down is refused at either door — see the box's own
+// removal test below for where the two part and why.
 func TestTheLastOwnerIsRefusedWithSomethingToDo(t *testing.T) {
 	for _, c := range []struct {
 		name  string
 		serve func(*Server, http.ResponseWriter, *http.Request)
 		req   *http.Request
 	}{
-		{"demote", (*Server).handleAppLinkDeviceRole,
+		{"demote at the box", (*Server).handleAppLinkDeviceRole,
 			localRequest(http.MethodPatch, "/api/app-link/devices/aaaa1111", `{"role":"viewer"}`)},
-		{"remove", (*Server).handleAppLinkDeviceRevoke,
-			localRequest(http.MethodDelete, "/api/app-link/devices/aaaa1111", "")},
+		{"demote from the app", (*Server).handleAppLinkDeviceRole,
+			sessionRequest(http.MethodPatch, "/api/app-link/devices/aaaa1111", `{"role":"viewer"}`)},
+		{"remove from the app", (*Server).handleAppLinkDeviceRevoke,
+			sessionRequest(http.MethodDelete, "/api/app-link/devices/aaaa1111", "")},
 	} {
 		t.Run(c.name, func(t *testing.T) {
 			enroll := &stubEnroller{lastOwner: true}
@@ -281,6 +305,66 @@ func TestTheLastOwnerIsRefusedWithSomethingToDo(t *testing.T) {
 	}
 }
 
+// At the box, the last phone comes off the list.
+//
+// The refusal above guards against a phone emptying the roster from anywhere
+// in the world, leaving a box nobody can administer and no remote way to mend
+// it. Whoever is standing at the box has the mend in front of them — the same
+// page shows a new pairing code — so refusing there protects nothing, and it
+// stranded every household that lost its only phone.
+//
+// The handler tells the two apart by the caller the box named, not by an
+// address and not by anything in the request. A remote caller has no field to
+// set that would get it here.
+func TestTheLastPhoneIsRemovedAtTheBox(t *testing.T) {
+	enroll := &stubEnroller{lastOwner: true}
+	s := New(&Deps{AppEnroll: enroll})
+
+	w := httptest.NewRecorder()
+	r := localRequest(http.MethodDelete, "/api/app-link/devices/aaaa1111", "")
+	r.SetPathValue("id", "aaaa1111")
+	s.handleAppLinkDeviceRevoke(w, r)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("got %d, want 200 — a household at its own box cannot start clean: %s",
+			w.Code, w.Body.String())
+	}
+	if enroll.revoked != 1 {
+		t.Fatalf("the phone was answered for but never removed: %+v", enroll)
+	}
+	if !enroll.revokedAtTheBox {
+		t.Fatal("the handler told enrolment the caller was remote; the last owner would survive a real box")
+	}
+}
+
+// Removing a phone that is not the last owner is the same at both doors.
+// Nothing about this change touches the ordinary case.
+func TestRemovingAPhoneThatIsNotTheLastOwnerIsUnchanged(t *testing.T) {
+	for _, c := range []struct {
+		name string
+		req  *http.Request
+	}{
+		{"at the box", localRequest(http.MethodDelete, "/api/app-link/devices/aaaa1111", "")},
+		{"from the app", sessionRequest(http.MethodDelete, "/api/app-link/devices/aaaa1111", "")},
+	} {
+		t.Run(c.name, func(t *testing.T) {
+			enroll := &stubEnroller{}
+			s := New(&Deps{AppEnroll: enroll})
+
+			w := httptest.NewRecorder()
+			c.req.SetPathValue("id", "aaaa1111")
+			s.handleAppLinkDeviceRevoke(w, c.req)
+
+			if w.Code != http.StatusOK {
+				t.Fatalf("got %d, want 200: %s", w.Code, w.Body.String())
+			}
+			if enroll.revoked != 1 {
+				t.Fatalf("the phone was answered for but never removed: %+v", enroll)
+			}
+		})
+	}
+}
+
 // A role the registry does not define is a 400, not a 500 and not a silent
 // success. The list of roles lives in contract/registry.yaml and the answer
 // has to come from there rather than from a literal on this floor.

go/internal/api/api_app_link_test.go

@@ -17,9 +17,13 @@ type stubEnroller struct {
 	mintedRole string
 	roleSet    string
 	err        error
-	// lastOwner makes the stub refuse to remove or demote its one row, the
-	// way appenroll does when it is the only owner left.
+	// lastOwner makes the stub refuse to demote its one row, and to remove it
+	// for anyone who is not standing at the box — the way appenroll does when
+	// it is the only owner left.
 	lastOwner bool
+	// revokedAtTheBox records the door the last removal came through, so a
+	// test can prove the handler passed the fact rather than a constant.
+	revokedAtTheBox bool
 }
 
 func (s *stubEnroller) MintPairingCode(role string) ([]byte, time.Time, error) {
@@ -68,14 +72,15 @@ func (s *stubEnroller) SetDeviceRole(id, role string) error {
 	return nil
 }
 
-func (s *stubEnroller) RevokeDevice(id string) error {
+func (s *stubEnroller) RevokeDevice(id string, atTheBox bool) error {
 	if id != "aaaa1111" {
 		return ErrUnknownAppDevice
 	}
-	if s.lastOwner {
+	if s.lastOwner && !atTheBox {
 		return ErrLastAppOwnerProtected
 	}
 	s.revoked++
+	s.revokedAtTheBox = atTheBox
 	return nil
 }
 

go/internal/appenroll/enroll.go

@@ -92,7 +92,8 @@ var (
 	ErrBadPairing = errors.New("appenroll: pairing code is not valid")
 	// ErrUnknownRole is a role that is not in contract/registry.yaml.
 	ErrUnknownRole = errors.New("appenroll: no such role")
-	// ErrLastOwnerProtected is an attempt to remove or demote the only owner.
+	// ErrLastOwnerProtected is an attempt to demote the only owner, or to
+	// remove it from somewhere other than the box itself.
 	ErrLastOwnerProtected = errors.New("appenroll: that is the only owner")
 )
 
@@ -156,8 +157,10 @@ type DeviceInfo struct {
 	// and locking out are the same screen: a household that cannot see which
 	// phone is a guest cannot decide which one to remove.
 	Role string
-	// LastOwner marks the row that cannot be removed or demoted, so the
-	// screen can say why before somebody presses the button rather than after.
+	// LastOwner marks the only phone left that can change anything here. It
+	// cannot be stepped down at either door, and cannot be removed over a
+	// session — so a screen can say what it is before somebody presses a
+	// button rather than after.
 	LastOwner bool
 }
 
@@ -659,6 +662,12 @@ func (i *Identity) GrantFor(id string) (Grant, bool) {
 // Refused for the only owner, here rather than in the API layer — otherwise
 // the box's own web UI could do what the app cannot, and the protection would
 // be a property of the screen instead of a property of the box.
+//
+// Refused at both doors, the box's own page included. Removing the last owner
+// there is allowed — see Revoke — because a household whose phone is gone has
+// to be able to empty the list and start again. Stepping the last owner down
+// leaves that same box carrying a list of phones that can only look, which is
+// the shape of the problem rather than a way out of it.
 func (i *Identity) SetRole(id, role string) error {
 	if err := knownRole(role); err != nil {
 		return err
@@ -714,6 +723,21 @@ func (i *Identity) Devices() []DeviceInfo {
 // ErrUnknownDevice is a revoke aimed at an id no paired phone carries.
 var ErrUnknownDevice = errors.New("appenroll: no such device")
 
+// Presence says whether whoever is asking is standing at the box.
+//
+// It is not a role and it is not a scope: an owner's phone holds every scope
+// this box grants and still cannot prove where it is. Presence is the one
+// thing only the LAN door establishes, and here it settles exactly one
+// question — whether the last owner may go.
+type Presence bool
+
+const (
+	// AtTheBox is a request off the box's own page, on the home network.
+	AtTheBox Presence = true
+	// OverASession is an enrolled phone, which could be anywhere on earth.
+	OverASession Presence = false
+)
+
 // Revoke forgets a phone by its device id and returns the full key, so the
 // caller can also tear down any session that key is running right now. The
 // next handshake from it meets ErrNoPairing like any stranger's.
@@ -722,17 +746,27 @@ var ErrUnknownDevice = errors.New("appenroll: no such device")
 // Sharing does not get its own gesture with its own bugs: a shared phone is a
 // row in the same list, removed by the same button.
 //
-// Refused for the only owner. A household can always pair a new owner at the
-// box and then remove the old one; what it cannot do is reduce itself to a set
-// of phones that can only look.
-func (i *Identity) Revoke(id string) ([]byte, error) {
+// Refused for the only owner over a session, and allowed at the box.
+//
+// What the refusal guards against is a phone locking a household out of its
+// own box from anywhere in the world: remove the last owner remotely and
+// nobody can administer the box, and nothing done remotely can undo that.
+// Presence is the way back — whoever is standing at the box can mint a fresh
+// owner's code on the same page the Remove button is on. So at the box the
+// refusal protects nothing, and all it does is stop a household emptying the
+// list after a phone is lost or starting clean.
+//
+// The decision stays here rather than in the API layer. What is new is that
+// the box is told which door the request came through; which screen drew the
+// button still decides nothing.
+func (i *Identity) Revoke(id string, from Presence) ([]byte, error) {
 	i.mu.Lock()
 	fullKey, meta := i.findLocked(id)
 	if meta == nil {
 		i.mu.Unlock()
 		return nil, ErrUnknownDevice
 	}
-	if meta.role == apiauth.RoleOwner && i.ownerCountLocked() == 1 {
+	if from == OverASession && meta.role == apiauth.RoleOwner && i.ownerCountLocked() == 1 {
 		i.mu.Unlock()
 		return nil, ErrLastOwnerProtected
 	}

go/internal/appenroll/enroll_test.go

@@ -374,7 +374,7 @@ func TestDeviceListAndRevoke(t *testing.T) {
 
 	// Revoke by row id: the key comes back so live sessions can be dropped,
 	// and the next handshake meets ErrNoPairing like any stranger's.
-	key, err := id.Revoke(devices[1].ID)
+	key, err := id.Revoke(devices[1].ID, OverASession)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -384,7 +384,7 @@ func TestDeviceListAndRevoke(t *testing.T) {
 	if _, err := id.Authorise(second, nil); !errors.Is(err, ErrNoPairing) {
 		t.Fatalf("a revoked key reconnected: %v", err)
 	}
-	if _, err := id.Revoke("nosuchid"); !errors.Is(err, ErrUnknownDevice) {
+	if _, err := id.Revoke("nosuchid", OverASession); !errors.Is(err, ErrUnknownDevice) {
 		t.Fatalf("revoking a ghost: %v", err)
 	}
 
@@ -513,7 +513,7 @@ func TestARevokedDeviceHasNoLiveGrant(t *testing.T) {
 	if live, ok := id.GrantFor(guest.DeviceID); !ok || live.Epoch != guest.Epoch {
 		t.Fatalf("grant before the revoke = %+v/%v, want %+v/live", live, ok, guest)
 	}
-	if _, err := id.Revoke(guest.DeviceID); err != nil {
+	if _, err := id.Revoke(guest.DeviceID, OverASession); err != nil {
 		t.Fatalf("Revoke: %v", err)
 	}
 	if live, ok := id.GrantFor(guest.DeviceID); ok {