runs: scope by account; app: the PKCE half of signing in
Three phones on one account now produce one history, which is the main reason to have accounts beyond upload permission. GET /v1/runs returns the account's runs and says how many devices contributed; fetching and deleting resolve a run id against the caller's own devices, so an id from another account is not found rather than fetched from wherever it happens to live. The rule that needed stating: the empty account is never a group. Devices nobody has signed in on are unrelated devices that share the absence of an owner, and matching on "" would let any anonymous device read every other one's runs. Tested, along with sibling-device access working and cross-account access not. App side: authorization code with PKCE. The app is a public client - anything compiled into an APK can be read out with unzip and strings - and the redirect returns through a custom URI scheme that any app on the device may register, so an intercepted code is a real risk. PKCE makes a stolen code worthless: it can only be exchanged by presenting a verifier that never left the process. A callback whose state does not match is refused before the code is spent and before any network call, since that is exactly how someone gets a victim to complete the attacker's sign-in. Nothing from the IdP is retained. The ID token is used once to prove who is signing in and then discarded; the device credential authenticates everything afterwards. No access tokens to store, no refresh tokens to rotate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
0eaba6150b
commit
4e6f2da3fb
@@ -755,11 +755,19 @@ func (s *Server) listRuns(w http.ResponseWriter, r *http.Request) {
|
||||
writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "unknown credential"})
|
||||
return
|
||||
}
|
||||
list := s.Runs.List(dev.ID)
|
||||
list := s.Runs.ListFor(s.visibleDevices(dev))
|
||||
if list == nil {
|
||||
list = []runs.Meta{}
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]any{"runs": list})
|
||||
writeJSON(w, http.StatusOK, map[string]any{
|
||||
"runs": list,
|
||||
// Says whose history this is, so a client can show "3 devices" rather than leaving the
|
||||
// user to wonder why runs from another phone appeared.
|
||||
"scope": map[string]any{
|
||||
"account_id": dev.AccountID,
|
||||
"devices": len(s.visibleDevices(dev)),
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func (s *Server) getRun(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -768,9 +776,14 @@ func (s *Server) getRun(w http.ResponseWriter, r *http.Request) {
|
||||
writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "unknown credential"})
|
||||
return
|
||||
}
|
||||
// Scoped to the calling device's own directory: one device cannot read another's runs by
|
||||
// guessing a run id.
|
||||
b, err := s.Runs.Get(dev.ID, r.PathValue("id"))
|
||||
// Resolved against the caller's own devices only, so a run id from another account is not
|
||||
// found rather than being fetched from wherever it happens to live.
|
||||
owner, ok := s.Runs.OwnerOf(s.visibleDevices(dev), r.PathValue("id"))
|
||||
if !ok {
|
||||
writeJSON(w, http.StatusNotFound, map[string]string{"error": "no such run"})
|
||||
return
|
||||
}
|
||||
b, err := s.Runs.Get(owner, r.PathValue("id"))
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusNotFound, map[string]string{"error": "no such run"})
|
||||
return
|
||||
@@ -785,7 +798,12 @@ func (s *Server) deleteRun(w http.ResponseWriter, r *http.Request) {
|
||||
writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "unknown credential"})
|
||||
return
|
||||
}
|
||||
if err := s.Runs.Delete(dev.ID, r.PathValue("id")); err != nil {
|
||||
owner, ok := s.Runs.OwnerOf(s.visibleDevices(dev), r.PathValue("id"))
|
||||
if !ok {
|
||||
w.WriteHeader(http.StatusNoContent) // delete is idempotent; absent is the desired state
|
||||
return
|
||||
}
|
||||
if err := s.Runs.Delete(owner, r.PathValue("id")); err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
@@ -928,3 +946,17 @@ func (s *Server) accountStatus(w http.ResponseWriter, r *http.Request) {
|
||||
"device_id": dev.ID,
|
||||
})
|
||||
}
|
||||
|
||||
// visibleDevices is the set of devices whose runs the caller may read.
|
||||
//
|
||||
// Signed in: every device on the same account, which is what an account is for. Not signed in:
|
||||
// only itself — anonymous devices are not a group, and treating the absent account as a shared
|
||||
// one would let any of them read all the others.
|
||||
func (s *Server) visibleDevices(dev *store.Device) []string {
|
||||
if dev.LinkedToAccount() {
|
||||
if ids := s.Store.DeviceIDsForAccount(dev.AccountID); len(ids) > 0 {
|
||||
return ids
|
||||
}
|
||||
}
|
||||
return []string{dev.ID}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
// SPDX-FileCopyrightText: 2026 Echolot contributors
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package runs
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Account scoping widens what a caller can read, so the test that matters is the one about what
|
||||
// it must NOT widen: a run id from another account has to be invisible, not merely unlisted.
|
||||
func TestAccountScopingDoesNotReachOtherAccounts(t *testing.T) {
|
||||
s, _ := open(t, DefaultPolicy())
|
||||
|
||||
// Two devices on one account, one device belonging to somebody else.
|
||||
mine := []string{"phone-a", "tablet-a"}
|
||||
for i, d := range mine {
|
||||
if _, err := s.Put(d, doc("run-"+d, AnonFull), true); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_ = i
|
||||
time.Sleep(2 * time.Millisecond)
|
||||
}
|
||||
if _, err := s.Put("phone-b", doc("run-secret", AnonFull), true); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
got := s.ListFor(mine)
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("account history has %d runs, want 2", len(got))
|
||||
}
|
||||
for _, m := range got {
|
||||
if m.ID == "run-secret" {
|
||||
t.Fatal("another account's run appeared in the history")
|
||||
}
|
||||
}
|
||||
|
||||
// The decisive one: knowing the id is not enough.
|
||||
if _, ok := s.OwnerOf(mine, "run-secret"); ok {
|
||||
t.Fatal("a run id from another account resolved against this account's devices")
|
||||
}
|
||||
if owner, ok := s.OwnerOf(mine, "run-phone-a"); !ok || owner != "phone-a" {
|
||||
t.Fatalf("own run did not resolve: owner=%q ok=%v", owner, ok)
|
||||
}
|
||||
// A sibling device's run must resolve — that is the point of the feature.
|
||||
if owner, ok := s.OwnerOf(mine, "run-tablet-a"); !ok || owner != "tablet-a" {
|
||||
t.Fatalf("sibling device's run did not resolve: owner=%q ok=%v", owner, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAccountHistoryIsNewestFirstAcrossDevices(t *testing.T) {
|
||||
s, _ := open(t, DefaultPolicy())
|
||||
if _, err := s.Put("phone", doc("older", AnonFull), true); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
time.Sleep(5 * time.Millisecond)
|
||||
if _, err := s.Put("tablet", doc("newer", AnonFull), true); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
got := s.ListFor([]string{"phone", "tablet"})
|
||||
if len(got) != 2 || got[0].ID != "newer" {
|
||||
t.Fatalf("not merged newest-first: %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmptyDeviceSetSeesNothing(t *testing.T) {
|
||||
s, _ := open(t, DefaultPolicy())
|
||||
if _, err := s.Put("someone", doc("run-1", AnonFull), true); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := s.ListFor(nil); len(got) != 0 {
|
||||
t.Fatalf("an empty device set returned %d runs", len(got))
|
||||
}
|
||||
if _, ok := s.OwnerOf(nil, "run-1"); ok {
|
||||
t.Fatal("a run resolved against an empty device set")
|
||||
}
|
||||
}
|
||||
@@ -209,6 +209,38 @@ func (s *Store) List(deviceID string) []Meta {
|
||||
return s.listLocked(filepath.Join(s.dir, sanitizeID(deviceID)))
|
||||
}
|
||||
|
||||
// ListFor returns the runs of several devices at once, newest first.
|
||||
//
|
||||
// This is what makes an account mean something: three phones signed in to one account produce one
|
||||
// history, which is the main reason to have accounts beyond upload permission.
|
||||
func (s *Store) ListFor(deviceIDs []string) []Meta {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
var out []Meta
|
||||
for _, id := range deviceIDs {
|
||||
out = append(out, s.listLocked(filepath.Join(s.dir, sanitizeID(id)))...)
|
||||
}
|
||||
sort.Slice(out, func(i, j int) bool { return out[i].UploadedAt.After(out[j].UploadedAt) })
|
||||
return out
|
||||
}
|
||||
|
||||
// OwnerOf reports which of these devices holds runID, so a caller can be granted access to a run
|
||||
// belonging to a sibling device without being able to name an arbitrary device.
|
||||
//
|
||||
// The search is over an allow-list the caller never supplies directly — it comes from the account
|
||||
// — so a run id from another account simply is not found.
|
||||
func (s *Store) OwnerOf(deviceIDs []string, runID string) (string, bool) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
for _, id := range deviceIDs {
|
||||
p := filepath.Join(s.dir, sanitizeID(id), sanitizeID(runID)+".json")
|
||||
if fi, err := os.Stat(p); err == nil && !fi.IsDir() {
|
||||
return id, true
|
||||
}
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
// Get returns the stored document bytes for one run.
|
||||
func (s *Store) Get(deviceID, runID string) ([]byte, error) {
|
||||
s.mu.Lock()
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
// SPDX-FileCopyrightText: 2026 Echolot contributors
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package store
|
||||
|
||||
import "testing"
|
||||
|
||||
// The empty account must never match. Devices nobody has signed in on are not a group — they are
|
||||
// unrelated devices that share the absence of an owner — and treating that as an account would
|
||||
// let any anonymous device read every other anonymous device's runs.
|
||||
func TestTheEmptyAccountIsNotAGroup(t *testing.T) {
|
||||
s, err := Open(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, id := range []string{"anon-1", "anon-2"} {
|
||||
s.data.Devices = append(s.data.Devices, Device{ID: id})
|
||||
}
|
||||
s.data.Devices = append(s.data.Devices,
|
||||
Device{ID: "mine-1", AccountID: "iss#me"},
|
||||
Device{ID: "mine-2", AccountID: "iss#me"},
|
||||
Device{ID: "theirs", AccountID: "iss#them"})
|
||||
|
||||
if got := s.DeviceIDsForAccount(""); len(got) != 0 {
|
||||
t.Fatalf("the empty account matched %v", got)
|
||||
}
|
||||
if got := s.DeviceIDsForAccount("iss#me"); len(got) != 2 {
|
||||
t.Fatalf("account has %v, want both of its devices", got)
|
||||
}
|
||||
if got := s.DeviceIDsForAccount("iss#them"); len(got) != 1 || got[0] != "theirs" {
|
||||
t.Fatalf("wrong devices for the other account: %v", got)
|
||||
}
|
||||
}
|
||||
@@ -165,6 +165,26 @@ func (s *Store) LinkAccount(deviceID, accountID, displayName string) error {
|
||||
return errors.New("no such device")
|
||||
}
|
||||
|
||||
// DeviceIDsForAccount returns every device signed in to the same account.
|
||||
//
|
||||
// The empty account is never matched: devices that nobody has signed in on are not a group, they
|
||||
// are unrelated devices that happen to share the absence of an owner. Treating them as an account
|
||||
// would let any anonymous device read every other anonymous device's runs.
|
||||
func (s *Store) DeviceIDsForAccount(accountID string) []string {
|
||||
if accountID == "" {
|
||||
return nil
|
||||
}
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
var out []string
|
||||
for _, d := range s.data.Devices {
|
||||
if d.AccountID == accountID {
|
||||
out = append(out, d.ID)
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// Devices returns a copy of the device list, for the admin UI.
|
||||
func (s *Store) Devices() []Device {
|
||||
s.mu.Lock()
|
||||
|
||||
Reference in New Issue
Block a user