Skip to content

Commit a511fff

Browse files
terafinterafin
authored
fix(helper-auth): rebind app-password on stale device ID instead of returning 409
[squashed; see PR body for full description] 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: terafin <terafin@users.noreply.github.com>
1 parent dd97766 commit a511fff

3 files changed

Lines changed: 266 additions & 3 deletions

File tree

‎backend/cmd/server/helper_auth.go‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,22 @@ func (a *app) authenticateHelperKey(compactKey string, identity helperIdentity,
358358
hasBoundIdentity := strings.TrimSpace(record.BoundDeviceType) != "" && strings.TrimSpace(record.BoundFingerprint) != ""
359359
if hasBoundIdentity {
360360
if identity.isComplete() && !deviceIdentityMatches(identity.DeviceType, identity.Fingerprint, record.BoundDeviceType, record.BoundFingerprint) {
361-
return helperAuthContext{}, http.StatusConflict, "app password is already bound to another device"
361+
// A helper can present a different device_type per source while keeping a
362+
// stable fingerprint — e.g. the SGM Steam Deck helper enrolls as
363+
// device_type "steamdeck" but uploads saves tagged with the source's
364+
// emulator type ("retroarch"). The fingerprint is the stable per-device
365+
// identity; device_type is variable metadata. When the fingerprint still
366+
// matches the bound one, treat it as the same physical device and keep the
367+
// bound identity (so the existing device resolves and we rebind cleanly).
368+
// Only a genuinely different fingerprint is another device — reject that.
369+
if strings.EqualFold(strings.TrimSpace(identity.Fingerprint), strings.TrimSpace(record.BoundFingerprint)) {
370+
identity = helperIdentity{
371+
DeviceType: record.BoundDeviceType,
372+
Fingerprint: record.BoundFingerprint,
373+
}
374+
} else {
375+
return helperAuthContext{}, http.StatusConflict, "app password is already bound to another device"
376+
}
362377
}
363378
if !identity.isComplete() {
364379
identity = helperIdentity{
@@ -379,10 +394,21 @@ func (a *app) authenticateHelperKey(compactKey string, identity helperIdentity,
379394
return helperAuthContext{}, http.StatusConflict, "device already has a different app password bound"
380395
}
381396

397+
// CHECK 3 — the app-password record stores a device ID that differs from the
398+
// device we resolved by identity. This happens routinely after an RSM restart
399+
// reloads security_device_state.json: a device can be re-created (or re-keyed)
400+
// with a new ID even though it is the same physical machine. We must only reject
401+
// the request when the stale ID belongs to a genuinely different device — i.e.
402+
// one whose fingerprint does not match. Otherwise we fall through and let the
403+
// rebind below repoint the app-password at the current device ID.
382404
if record.BoundDeviceID != nil && *record.BoundDeviceID != boundDevice.ID {
383-
if _, exists := a.devices[*record.BoundDeviceID]; exists {
384-
return helperAuthContext{}, http.StatusConflict, "app password is already bound to another device"
405+
if oldDevice, exists := a.devices[*record.BoundDeviceID]; exists {
406+
if !deviceIdentityMatches(oldDevice.DeviceType, oldDevice.Fingerprint, boundDevice.DeviceType, boundDevice.Fingerprint) {
407+
return helperAuthContext{}, http.StatusConflict, "app password is already bound to another device"
408+
}
409+
// Same physical device under a different ID — rebind to the current device.
385410
}
411+
// Old bound device no longer exists (state reload) — rebind to the current device.
386412
}
387413

388414
now := time.Now().UTC()

‎backend/cmd/server/helper_auth_device_binding_test.go‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
package main
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
"time"
7+
)
8+
9+
// newHelperAuthTestApp returns an app with empty device / app-password state so
10+
// each test can construct the exact binding scenario it needs without the seed
11+
// data created by newApp(). securityStateFile is cleared so no state is persisted
12+
// to disk during the test.
13+
func newHelperAuthTestApp() *app {
14+
a := newApp()
15+
a.devices = map[int]device{}
16+
a.appPasswords = map[string]appPassword{}
17+
a.nextDeviceID = 1
18+
a.nextAppPasswordID = 1
19+
a.securityStateFile = ""
20+
return a
21+
}
22+
23+
// mintHelperAppPassword creates an app-password and returns its record ID along
24+
// with the compact form a helper would present for authentication.
25+
func mintHelperAppPassword(t *testing.T, a *app, now time.Time) (string, string) {
26+
t.Helper()
27+
record, plain := a.createAppPasswordLocked("test-helper", now)
28+
_, compact, ok := normalizeAppPasswordInput(plain)
29+
if !ok {
30+
t.Fatalf("failed to normalize generated app password %q", plain)
31+
}
32+
return record.ID, compact
33+
}
34+
35+
// An app-password that is correctly bound to the requesting device must
36+
// authenticate cleanly (this is the regular happy-path upload).
37+
func TestAuthenticateHelperKeyCorrectlyBoundSucceeds(t *testing.T) {
38+
a := newHelperAuthTestApp()
39+
now := time.Now().UTC()
40+
a.devices[1] = device{ID: 1, DeviceType: "linux-x86", Fingerprint: "deck-1", LastSeenAt: now, SyncAll: true, CreatedAt: now}
41+
a.nextDeviceID = 2
42+
43+
keyID, compact := mintHelperAppPassword(t, a, now)
44+
rec := a.appPasswords[keyID]
45+
devID := 1
46+
rec.BoundDeviceID = &devID
47+
rec.BoundDeviceType = "linux-x86"
48+
rec.BoundFingerprint = "deck-1"
49+
a.appPasswords[keyID] = rec
50+
d := a.devices[1]
51+
d.BoundAppPasswordID = &keyID
52+
a.devices[1] = d
53+
54+
ctx, status, msg := a.authenticateHelperKey(compact, helperIdentity{DeviceType: "linux-x86", Fingerprint: "deck-1"}, helperMetadata{})
55+
if status != 0 {
56+
t.Fatalf("expected success, got status %d (%s)", status, msg)
57+
}
58+
if ctx.AppPassword.BoundDeviceID == nil || *ctx.AppPassword.BoundDeviceID != 1 {
59+
t.Fatalf("expected app password bound to device 1, got %v", ctx.AppPassword.BoundDeviceID)
60+
}
61+
}
62+
63+
// The regression case: the record stores a stale device ID (the old device no
64+
// longer exists after a state reload) but the same physical fingerprint resolves
65+
// to a current device. The request must rebind rather than return 409.
66+
func TestAuthenticateHelperKeyRebindsStaleDeviceID(t *testing.T) {
67+
a := newHelperAuthTestApp()
68+
now := time.Now().UTC()
69+
// The live device is ID 7 (e.g. re-created with a new ID after a restart).
70+
a.devices[7] = device{ID: 7, DeviceType: "linux-x86", Fingerprint: "deck-1", LastSeenAt: now, SyncAll: true, CreatedAt: now}
71+
a.nextDeviceID = 8
72+
73+
keyID, compact := mintHelperAppPassword(t, a, now)
74+
rec := a.appPasswords[keyID]
75+
staleID := 99 // device 99 no longer exists
76+
rec.BoundDeviceID = &staleID
77+
rec.BoundDeviceType = "linux-x86"
78+
rec.BoundFingerprint = "deck-1"
79+
a.appPasswords[keyID] = rec
80+
81+
ctx, status, msg := a.authenticateHelperKey(compact, helperIdentity{DeviceType: "linux-x86", Fingerprint: "deck-1"}, helperMetadata{})
82+
if status != 0 {
83+
t.Fatalf("expected rebind success, got status %d (%s)", status, msg)
84+
}
85+
if got := a.appPasswords[keyID].BoundDeviceID; got == nil || *got != 7 {
86+
t.Fatalf("expected app password rebound to device 7, got %v", got)
87+
}
88+
if ctx.Device.ID != 7 {
89+
t.Fatalf("expected context device 7, got %d", ctx.Device.ID)
90+
}
91+
}
92+
93+
// A stale device ID that points to a device with a *different* fingerprint is a
94+
// genuinely different machine: the 409 guard must still fire.
95+
func TestAuthenticateHelperKeyRejectsDifferentDevice(t *testing.T) {
96+
a := newHelperAuthTestApp()
97+
now := time.Now().UTC()
98+
a.devices[1] = device{ID: 1, DeviceType: "linux-x86", Fingerprint: "deck-1", LastSeenAt: now, SyncAll: true, CreatedAt: now}
99+
a.devices[2] = device{ID: 2, DeviceType: "linux-x86", Fingerprint: "deck-2", LastSeenAt: now, SyncAll: true, CreatedAt: now}
100+
a.nextDeviceID = 3
101+
102+
keyID, compact := mintHelperAppPassword(t, a, now)
103+
rec := a.appPasswords[keyID]
104+
otherID := 2 // points at a device with a different fingerprint
105+
rec.BoundDeviceID = &otherID
106+
rec.BoundDeviceType = "linux-x86"
107+
rec.BoundFingerprint = "deck-1"
108+
a.appPasswords[keyID] = rec
109+
110+
_, status, _ := a.authenticateHelperKey(compact, helperIdentity{DeviceType: "linux-x86", Fingerprint: "deck-1"}, helperMetadata{})
111+
if status != http.StatusConflict {
112+
t.Fatalf("expected 409 conflict for a genuinely different device, got %d", status)
113+
}
114+
if got := a.appPasswords[keyID].BoundDeviceID; got == nil || *got != 2 {
115+
t.Fatalf("expected binding to remain on device 2 after rejection, got %v", got)
116+
}
117+
}
118+
119+
// Rebinding a device to a new app-password must delete the password it displaces
120+
// (now unbound and unusable) rather than leaving it to accumulate as an orphan.
121+
func TestBindAppPasswordDeletesDisplacedOrphan(t *testing.T) {
122+
a := newHelperAuthTestApp()
123+
now := time.Now().UTC()
124+
a.devices[1] = device{ID: 1, DeviceType: "linux-x86", Fingerprint: "deck-1", LastSeenAt: now, SyncAll: true, CreatedAt: now}
125+
a.nextDeviceID = 2
126+
127+
old, _ := a.createAppPasswordLocked("old", now)
128+
a.bindAppPasswordToDeviceLocked(old.ID, a.devices[1])
129+
fresh, _ := a.createAppPasswordLocked("fresh", now)
130+
131+
a.bindAppPasswordToDeviceLocked(fresh.ID, a.devices[1])
132+
133+
if _, ok := a.appPasswords[old.ID]; ok {
134+
t.Fatalf("expected displaced orphan %s to be deleted", old.ID)
135+
}
136+
if got := a.appPasswords[fresh.ID].BoundDeviceID; got == nil || *got != 1 {
137+
t.Fatalf("expected fresh password bound to device 1, got %v", got)
138+
}
139+
if got := a.devices[1].BoundAppPasswordID; got == nil || *got != fresh.ID {
140+
t.Fatalf("expected device 1 to reference fresh password, got %v", got)
141+
}
142+
}
143+
144+
// A displaced password that is still referenced by another device must NOT be
145+
// deleted by the orphan sweep.
146+
func TestBindAppPasswordKeepsPasswordReferencedElsewhere(t *testing.T) {
147+
a := newHelperAuthTestApp()
148+
now := time.Now().UTC()
149+
shared, _ := a.createAppPasswordLocked("shared", now)
150+
sharedID := shared.ID
151+
a.devices[1] = device{ID: 1, DeviceType: "linux-x86", Fingerprint: "deck-1", BoundAppPasswordID: &sharedID, LastSeenAt: now, SyncAll: true, CreatedAt: now}
152+
a.devices[2] = device{ID: 2, DeviceType: "linux-x86", Fingerprint: "deck-2", BoundAppPasswordID: &sharedID, LastSeenAt: now, SyncAll: true, CreatedAt: now}
153+
a.nextDeviceID = 3
154+
155+
fresh, _ := a.createAppPasswordLocked("fresh", now)
156+
a.bindAppPasswordToDeviceLocked(fresh.ID, a.devices[1])
157+
158+
if _, ok := a.appPasswords[sharedID]; !ok {
159+
t.Fatalf("password still referenced by device 2 must not be deleted")
160+
}
161+
}
162+
163+
// A helper may present a different device_type while keeping a stable fingerprint
164+
// (e.g. the SGM Steam Deck helper enrolls as "steamdeck" but uploads saves tagged
165+
// with the source emulator type "retroarch"). The fingerprint is the device
166+
// identity, so this must authenticate and stay bound to the same device.
167+
func TestAuthenticateHelperKeyDifferentDeviceTypeSameFingerprintSucceeds(t *testing.T) {
168+
a := newHelperAuthTestApp()
169+
now := time.Now().UTC()
170+
a.devices[1] = device{ID: 1, DeviceType: "steamdeck", Fingerprint: "deck-1", LastSeenAt: now, SyncAll: true, CreatedAt: now}
171+
a.nextDeviceID = 2
172+
173+
keyID, compact := mintHelperAppPassword(t, a, now)
174+
rec := a.appPasswords[keyID]
175+
devID := 1
176+
rec.BoundDeviceID = &devID
177+
rec.BoundDeviceType = "steamdeck"
178+
rec.BoundFingerprint = "deck-1"
179+
a.appPasswords[keyID] = rec
180+
d := a.devices[1]
181+
d.BoundAppPasswordID = &keyID
182+
a.devices[1] = d
183+
184+
ctx, status, msg := a.authenticateHelperKey(compact, helperIdentity{DeviceType: "retroarch", Fingerprint: "deck-1"}, helperMetadata{})
185+
if status != 0 {
186+
t.Fatalf("expected success on same-fingerprint different-device_type, got %d (%s)", status, msg)
187+
}
188+
if ctx.AppPassword.BoundDeviceID == nil || *ctx.AppPassword.BoundDeviceID != 1 {
189+
t.Fatalf("expected app password to stay bound to device 1, got %v", ctx.AppPassword.BoundDeviceID)
190+
}
191+
}
192+
193+
// A genuinely different fingerprint is a different physical device and must still
194+
// be rejected — an app-password is bound to one device.
195+
func TestAuthenticateHelperKeyDifferentFingerprintRejected(t *testing.T) {
196+
a := newHelperAuthTestApp()
197+
now := time.Now().UTC()
198+
a.devices[1] = device{ID: 1, DeviceType: "steamdeck", Fingerprint: "deck-1", LastSeenAt: now, SyncAll: true, CreatedAt: now}
199+
a.nextDeviceID = 2
200+
201+
keyID, compact := mintHelperAppPassword(t, a, now)
202+
rec := a.appPasswords[keyID]
203+
devID := 1
204+
rec.BoundDeviceID = &devID
205+
rec.BoundDeviceType = "steamdeck"
206+
rec.BoundFingerprint = "deck-1"
207+
a.appPasswords[keyID] = rec
208+
209+
if _, status, _ := a.authenticateHelperKey(compact, helperIdentity{DeviceType: "steamdeck", Fingerprint: "other-deck"}, helperMetadata{}); status != http.StatusConflict {
210+
t.Fatalf("expected 409 on different fingerprint, got %d", status)
211+
}
212+
}

‎backend/cmd/server/security_state.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,10 @@ func (a *app) bindAppPasswordToDeviceLocked(passwordID string, d device) {
597597
previous.BoundDeviceType = ""
598598
previous.BoundFingerprint = ""
599599
a.appPasswords[*current] = previous
600+
// The displaced password is no longer bound to this device. If no other
601+
// device references it either, delete it instead of leaving an orphaned,
602+
// unusable record behind to accumulate over repeated (re)bindings.
603+
a.deleteAppPasswordIfOrphanedLocked(*current, d.ID)
600604
}
601605
}
602606

@@ -626,6 +630,27 @@ func (a *app) bindAppPasswordToDeviceLocked(passwordID string, d device) {
626630
a.saveDeviceLocked(d)
627631
}
628632

633+
// deleteAppPasswordIfOrphanedLocked removes an app-password that is no longer
634+
// bound to any device. excludeDeviceID is the device currently being rebound to a
635+
// different password (its own stale link to passwordID is about to be replaced),
636+
// so it is not treated as a remaining reference. This keeps displaced, unusable
637+
// app-passwords from accumulating without disturbing passwords still in use.
638+
func (a *app) deleteAppPasswordIfOrphanedLocked(passwordID string, excludeDeviceID int) {
639+
record, ok := a.appPasswords[passwordID]
640+
if !ok || record.BoundDeviceID != nil {
641+
return
642+
}
643+
for deviceID, candidate := range a.devices {
644+
if deviceID == excludeDeviceID {
645+
continue
646+
}
647+
if candidate.BoundAppPasswordID != nil && *candidate.BoundAppPasswordID == passwordID {
648+
return
649+
}
650+
}
651+
delete(a.appPasswords, passwordID)
652+
}
653+
629654
func applyHelperMetadataToDevice(input device, metadata helperMetadata, seenAt time.Time) device {
630655
out := input
631656
if seenAt.IsZero() {

0 commit comments

Comments
 (0)