diff --git a/docs/superpowers/plans/2026-07-31-user-facing-device-settings.md b/docs/superpowers/plans/2026-07-31-user-facing-device-settings.md new file mode 100644 index 00000000..cb1db366 --- /dev/null +++ b/docs/superpowers/plans/2026-07-31-user-facing-device-settings.md @@ -0,0 +1,510 @@ +# User-Facing Device Settings Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. +> +> Commands assume the repository root is the cwd. + +**Goal:** Let a person see and change the device settings for every device they watch on, from whichever device they are holding — and let the household parent do the same for everyone on the account. + +**Architecture:** No new storage and no schema change. `user_devices` and `user_setting_values` are already keyed `(user_id, profile_id, device_id, …)` and both list queries are already account-wide, so the work is authorization plus routes plus UI. Two identity widenings on the existing canonical settings API, each behind a guard: a caller may name a `device_id` other than the request's own (checked against `user_devices`), and a household parent may name a `profile_id` other than their own (checked by the existing `canManageHouseholdProfiles`). One new self-service device registry endpoint, deliberately profile-filtered by default. One new settings page reusing `SettingsGroup`/`SettingRow`. + +**Tech Stack:** Go, `net/http` handler tests, `internal/userstore/storetest` conformance suite, React 19 + react-router v7 + TanStack Query, Vitest + Testing Library, shadcn/ui primitives. + +**Design source:** `docs/superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md`. Mockups were reviewed out-of-band; the shipping shape is the "B2 + B3" pair — a searchable device list with an editable detail pane, plus a household scope switch for the primary profile. + +## Global Constraints + +- **Additive only.** No existing response field is renamed, retyped, or removed, and no status code is repurposed. New behavior arrives as new endpoints or new optional query parameters. See `CLAUDE.md` "v1 API rules". +- **The header stays the default.** When `device_id` / `profile_id` are absent from the query, every existing route must behave exactly as it does today. Existing clients must not need a change. +- **No new hand-written setting metadata.** Labels, descriptions, controls, options and bounds come from `contracts/settings/v1/manifest.json` via the generated `web/src/lib/settingsContract.ts`. A per-key table beside the generated one is exactly the drift the contract exists to remove. +- **Never render raw setting keys** in user-facing UI. +- **Scope wording is mandated** by the design spec: "this device, for your profile only". Do not invent alternatives such as "global", "default", or a bare "this device". +- **Restrictions are not preferences.** Policy caps are explained with the permitted value and the reason; they are never rendered as a disabled control with no explanation, and this screen never authors a restriction. +- Prove each authorization regression RED before writing production code. +- Do not edit this plan file while implementing it. + +--- + +## Phase 1 — Server: identity widening and the device registry + +### Task 1: Reject a device the caller does not own + +`completeIdentity` validates an identity's *shape* but never that a `profile_device` identity names a device belonging to the caller. That is safe today only because `DeviceID` is taken from the request's own header. Task 2 removes that guarantee, so the check lands first. + +**Files:** +- Modify: `internal/api/handlers/settings_values.go` +- Modify: `internal/userstore/store.go` +- Modify: `internal/userstore/pgstore/settings.go` +- Modify: `internal/userdb/settings.go` (the per-user SQLite backend) +- Test: `internal/api/handlers/settings_values_test.go` +- Test: `internal/userstore/storetest/settingvalues.go` + +**Interfaces:** +- Produces: `DeviceRegistry.DeviceExists(ctx, profileID, deviceID string) (bool, error)` — a targeted existence check rather than a full `ListDevices` scan on every write. +- Produces: a device-ownership guard invoked from `completeIdentity` for `ScopeProfileDevice`. + +- [ ] **Step 1: Write the failing ownership test** + +Add `TestSetValue_RejectsDeviceNotOwnedByCaller` to `settings_values_test.go`: register device `dev-a` for the caller, then `PUT /settings/values/player.hdr_enabled?scope=profile_device&device_id=dev-someone-else`. Expect `404` with error code `not_found` — not `403`, which would confirm the device id exists. + +Because Task 2 has not landed, the query parameter is ignored and the write silently succeeds against the header device. Assert on the *stored* row, so the test fails for the right reason: + +```go +if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "" { + t.Fatalf("wrote a row for device %q; want no write", got) +} +``` + +- [ ] **Step 2: Run the test and verify RED** + +```bash +go test ./internal/api/handlers/ -run TestSetValue_RejectsDeviceNotOwnedByCaller -v +``` + +- [ ] **Step 3: Add `DeviceExists` to the store interface and both backends** + +Postgres: `SELECT EXISTS(SELECT 1 FROM user_devices WHERE user_id = $1 AND profile_id = $2 AND device_id = $3)`. SQLite: the same without `user_id`, matching the existing `ListDevices` asymmetry (one DB per user). Add the case to the shared conformance suite in `internal/userstore/storetest/settingvalues.go` so both backends are held to it. + +- [ ] **Step 4: Enforce it in `completeIdentity`** + +For `ScopeProfileDevice`, when the device id did **not** come from the request header, verify it exists for `(profileID, deviceID)`. A device that is unknown returns 404 `not_found`. Registering-on-write stays the behavior for the caller's own header device, so a brand-new device can still store its first value. + +- [ ] **Step 5: Run and verify GREEN** + +```bash +go test ./internal/api/handlers/ ./internal/userstore/... -run 'Device|SettingValue' -v +``` + +--- + +### Task 2: Accept an explicit `device_id` on the self-service settings routes + +**Files:** +- Modify: `internal/api/handlers/settings_values.go` +- Test: `internal/api/handlers/settings_values_test.go` + +**Interfaces:** +- Consumes: optional `device_id` query parameter on `GET|PUT|DELETE /settings/values/{key}` and `GET /settings/values` when `scope=profile_device`. +- Produces: `identityForSessionKey` resolving `DeviceID` from the query when present, else from `X-Silo-Device-Id` exactly as today. + +- [ ] **Step 1: Write the failing tests** + +Three cases in `settings_values_test.go`: +- `TestSetValue_WritesNamedDevice` — register `dev-b`, `PUT …?scope=profile_device&device_id=dev-b` from a request whose header is `dev-a`; assert the stored row is on `dev-b` and `dev-a` has none. +- `TestGetValues_ReadsNamedDevice` — same shape for the read path. +- `TestSetValue_FallsBackToHeaderDevice` — no `device_id` in the query writes the header device. This is the regression guard for every existing client. + +- [ ] **Step 2: Run and verify RED** + +```bash +go test ./internal/api/handlers/ -run 'NamedDevice|FallsBackToHeaderDevice' -v +``` + +- [ ] **Step 3: Implement** + +In `identityForSessionKey`, prefer a non-empty `device_id` from the query and fall through to `deviceMetadataFromRequest(r).DeviceID`. Update the comment at the profile assignment so it still describes reality: the profile remains session-derived here; only the device may be named. Keep the existing 400 when neither source yields a device id. + +Do **not** call `registerWritingDevice` for a named device — registration is a statement that *this* device is in use, and a remote write is not that. + +- [ ] **Step 4: Run and verify GREEN, then run the whole handler package** + +```bash +go test ./internal/api/handlers/ -v +``` + +--- + +### Task 3: `GET /api/v1/devices` — list your own devices + +**Files:** +- Create: `internal/api/handlers/devices.go` +- Create: `internal/api/handlers/devices_test.go` +- Modify: `internal/api/router.go` + +**Interfaces:** +- Produces: `GET /api/v1/devices` → `{"devices":[{device_id, device_name, device_platform, last_seen_at, profile_id, profile_name, is_current_device, changed_count}]}`. +- Consumes: `DeviceRegistry.ListDevices`, `ListAllSettingValues`, `X-Silo-Device-Id`. + +**The trap this task exists to avoid:** `ListDevices` is account-wide in *both* backends — `WHERE user_id = $1` in `internal/userstore/pgstore/settings.go:115`, and no `WHERE` clause at all in `internal/userdb/settings.go:112` because there is one DB per user. `profile_id` is a selected column, never a predicate. A naive passthrough would show every household member's devices to everyone. The handler must filter. + +- [ ] **Step 1: Write the failing tests** + +- `TestListDevices_FiltersToCallingProfile` — seed devices for profile A and profile B, call as A, assert only A's are returned. **This is the security test; write it first.** +- `TestListDevices_CountsChangedSettings` — `changed_count` equals the number of `profile_device` rows for that `(profile, device)`. +- `TestListDevices_MarksCurrentDevice` — the device matching `X-Silo-Device-Id` has `is_current_device: true`. + +- [ ] **Step 2: Run and verify RED** + +```bash +go test ./internal/api/handlers/ -run TestListDevices -v +``` + +- [ ] **Step 3: Implement the handler** + +Filter `ListDevices` output to `apimw.GetProfileID(ctx)`. Derive `changed_count` from `ListAllSettingValues` filtered to `scope == profile_device` and the same profile — one store round trip, not one per device. `profile_name` comes from the existing `listProfileNamesByID` helper pattern in `internal/api/handlers/admin.go`. + +- [ ] **Step 4: Register the route** + +In `internal/api/router.go`, inside the authenticated group, `r.With(apimw.RequireProfile).Get("/devices", devicesHandler.HandleListDevices)`. Place it away from the `/devices/push/apple` line so the two device namespaces stay visibly distinct. + +- [ ] **Step 5: Run and verify GREEN** + +```bash +go test ./internal/api/handlers/ -run TestListDevices -v +``` + +--- + +### Task 4: Forget a device, and bulk-clear one device + +**Files:** +- Modify: `internal/api/handlers/devices.go` +- Modify: `internal/api/router.go` +- Modify: `internal/userstore/store.go` (if a targeted delete is missing) +- Test: `internal/api/handlers/devices_test.go` + +**Interfaces:** +- Produces: `DELETE /api/v1/devices/{device_id}` — forget: clears settings **and** the registry row. +- Produces: `DELETE /api/v1/devices/{device_id}/settings` — clear overrides, keep the device. + +The design spec requires Forget device (`…-design.md:416`) and lists it as outstanding (`:1202`). `DeleteAllDeviceSettings` exists and clears both storage generations but is reachable only through profile deletion today. Bulk clear also fixes the admin screen's 30-sequential-DELETE loop in `web/src/hooks/queries/admin/users.ts:451`. + +- [ ] **Step 1: Write failing tests** + +- `TestForgetDevice_RemovesSettingsAndRegistryRow` +- `TestForgetDevice_RejectsOtherProfilesDevice` → 404 +- `TestClearDeviceSettings_KeepsRegistryRow` +- `TestForgetDevice_IsIdempotent` — a second call returns 204, not 500 + +- [ ] **Step 2: Run and verify RED** + +```bash +go test ./internal/api/handlers/ -run 'ForgetDevice|ClearDeviceSettings' -v +``` + +- [ ] **Step 3: Implement both routes** + +Reuse `DeleteSettingValuesForDevice` (`internal/userstore/store.go:216`) and `DeleteAllDeviceSettings`. Publish `user_settings.changed` once per cleared key so other devices invalidate — or once for the device if a batch event shape is added; do not skip the event. + +- [ ] **Step 4: Run and verify GREEN** + +```bash +go test ./internal/api/handlers/ ./internal/userstore/... -v +``` + +--- + +## Phase 2 — Server: the household tier + +### Task 5: Extract the household-parent guard + +`canManageHouseholdProfiles` (`internal/api/handlers/profiles.go:146`) already encodes exactly the right rule — server admin, or an `is_primary` active profile, and when that profile has a PIN a verified `X-Profile-Token` so sending only `X-Profile-Id` cannot bypass the profile lock. It is a method on `ProfileHandler`, so `SettingValuesHandler` cannot call it. + +**Files:** +- Create: `internal/api/handlers/household.go` +- Modify: `internal/api/handlers/profiles.go` +- Modify: `internal/api/handlers/settings_values.go` +- Modify: `internal/api/router.go` +- Test: `internal/api/handlers/household_test.go` + +**Interfaces:** +- Produces: `canManageHousehold(r *http.Request, store userstore.UserStore, tokens ProfileTokenValidator) (bool, error)` — a package-level function. +- `ProfileHandler.canManageHouseholdProfiles` becomes a thin wrapper so its four existing call sites and their behavior are untouched. +- `SettingValuesHandler` gains a `ProfileTokens` field, wired in `internal/api/router.go` next to `profileHandler.ProfileTokens = profileTokenService` (`router.go:812`). + +- [ ] **Step 1: Characterization tests before moving anything** + +In `household_test.go`, cover: admin → true; primary without PIN → true; primary with PIN and no token → `access.ErrProfileUnverified`; primary with PIN and valid token → true; non-primary → false; no active profile → false. Run them against the *existing* method first so the extraction is provably behavior-preserving. + +- [ ] **Step 2: Extract, then re-run** + +Move the body to the package-level function; leave the method delegating. Run the full profiles suite — those four call sites are the regression surface: + +```bash +go test ./internal/api/handlers/ -run 'Profile|Household' -v +``` + +- [ ] **Step 3: Wire `ProfileTokens` into `SettingValuesHandler`** + +Nil `ProfileTokens` must mean "no household widening", never "allow" — assert that in a test. + +--- + +### Task 6: Accept an explicit `profile_id` for the household parent + +**Files:** +- Modify: `internal/api/handlers/settings_values.go` +- Modify: `internal/api/handlers/devices.go` +- Test: `internal/api/handlers/settings_values_test.go` +- Test: `internal/api/handlers/devices_test.go` + +**Interfaces:** +- Consumes: optional `profile_id` query parameter on the settings-value routes and on `GET /api/v1/devices`. +- Produces: identity resolution that permits a non-own `profile_id` only when `canManageHousehold` passes. + +- [ ] **Step 1: Write the three refusal tests first** + +These are the security surface. All three must be RED before any production code: +- `TestSetValue_NonPrimaryCannotNameSiblingProfile` → 403 +- `TestSetValue_PrimaryWithUnverifiedPINCannotNameSibling` → 403, code `forbidden`, message naming PIN verification +- `TestSetValue_ProfileFromAnotherAccountIsNotFound` → 404 + +Then the positive cases: `TestSetValue_PrimaryWritesSiblingProfileDeviceSetting`, `TestListDevices_PrimarySeesHouseholdWhenRequested`. + +- [ ] **Step 2: Run and verify RED** + +```bash +go test ./internal/api/handlers/ -run 'NameSibling|AnotherAccount|PrimaryWrites|PrimarySees' -v +``` + +- [ ] **Step 3: Implement** + +In `identityForSessionKey`: when `profile_id` is present and differs from the session profile, require `canManageHousehold`; on failure return 403 without disclosing whether the profile exists. Then resolve the profile through the caller's **own** store, exactly as `internal/access/resolver.go:73-86` does — a profile from another account is simply absent, which yields the 404 and preserves the cross-account boundary for free. + +For `GET /api/v1/devices`, add `?scope=household` (default: own profile only). Do not make household the default; the plain screen must stay private by construction. + +- [ ] **Step 4: Run and verify GREEN, then the full package** + +```bash +go test ./internal/api/handlers/ -v +``` + +**Record in the PR:** this widening does not create a new capability. The primary profile can already rewrite a sibling's canonical settings rows through `PUT /profiles/{id}` (`internal/api/handlers/profiles_settings_sync.go:218-231`), and `GET /profiles` (`profiles.go:263`) already returns every sibling's resolved preferences to any profile with no gate. All profiles share one login session, so `X-Profile-Id` is self-asserted for PIN-less profiles — stated in-repo at `internal/api/middleware/auth.go:180-184`. This task replaces an unlabelled path with a guarded, audited one. + +--- + +### Task 7: Audit cross-profile and device settings mutations + +`internal/activitylog` is only an HTTP request-log middleware mounted globally before auth (`internal/api/router.go:236-239`); no handler writes to it. Its entries carry method, path pattern, status, user and session but **no profile id and no body**, so a settings write is indistinguishable from any other `PUT`. Tolerable while every write is your own; not once one profile can change another's. The settings spec already requires admin clear/reset to be audited. + +**Files:** +- Modify: `internal/activitylog/` (new entry type or a settings-audit sink) +- Modify: `internal/api/handlers/settings_values.go` +- Modify: `internal/api/handlers/settings_values_admin.go` +- Modify: `internal/api/handlers/devices.go` +- Test: `internal/api/handlers/settings_values_test.go` + +- [ ] **Step 1: Write failing tests** + +`TestSetValue_AuditsCrossProfileWrite` asserts an entry recording actor profile, target profile, device, key, and action. `TestSetValue_DoesNotAuditOwnWrite` keeps ordinary self-service writes out of the audit trail — otherwise volume makes it useless. + +- [ ] **Step 2: Run RED, implement, verify GREEN** + +Record the *identity* of what changed, never the value: `user_settings.changed` deliberately carries no value because admins receive other accounts' events (`internal/api/handlers/user_settings_events.go:8-13`). The same reasoning applies to a stored audit row. Also audit Forget device and bulk clear. + +--- + +## Phase 3 — Web: the device settings screen + +### Task 8: Query hooks for devices and cross-identity settings + +**Files:** +- Create: `web/src/hooks/queries/devices.ts` +- Modify: `web/src/hooks/queries/settingValues.ts` +- Modify: `web/src/hooks/queries/keys.ts` +- Modify: `web/src/api/types.ts` +- Test: `web/src/hooks/queries/devices.test.ts` + +**Interfaces:** +- Produces: `useMyDevices({ household? })`, `useForgetDevice()`, `useClearDeviceSettings()`. +- Modifies: `SettingIdentity` gains optional `deviceId` and `profileId`; `identityQuery` (`settingValues.ts:60`) serializes them. + +- [ ] **Step 1: Extend `SettingIdentity` and `identityQuery`** + +Both fields optional, so every existing caller compiles and behaves identically. + +- [ ] **Step 2: Fix the cache key — this is a real bug if skipped** + +`effectiveSettingsQueryKey` (`settingValues.ts:76`) namespaces by `activeProfileId()`. Reading another profile's or another device's values through the same key would collide with the current device's cache and serve one device's settings as another's. Add `deviceId` and `profileId` to the key, and add a test that two devices' reads occupy distinct entries. + +- [ ] **Step 3: Write the device hooks and tests** + +Follow the existing `api()` + TanStack Query conventions. Invalidate `[...settingsKeys.all, "values"]` on every mutation, as `useSetSettingValue` does. + +```bash +cd web && pnpm vitest run src/hooks/queries/devices.test.ts +``` + +--- + +### Task 9: The device list pane + +**Files:** +- Create: `web/src/pages/settings/DeviceSettings.tsx` +- Create: `web/src/components/settings/DeviceList.tsx` +- Create: `web/src/components/settings/deviceDisplay.ts` +- Modify: `web/src/pages/SettingsLayout.tsx` +- Modify: `web/src/App.tsx` +- Modify: `web/src/lib/documentTitle.ts` +- Test: `web/src/components/settings/DeviceList.test.tsx` + +**Interfaces:** +- Produces: a master-detail layout — searchable list on the left, selected device on the right, stacking to a single column below the `md` breakpoint. +- Produces: `deviceDisplay.ts` — platform icon/label classification and relative-time formatting. Adapt the existing helpers in `web/src/components/admin/deviceOverrides.tsx:40-146` rather than duplicating them; move them here and have the admin page import from the shared module. + +- [ ] **Step 1: Add the route and nav entry** + +One `NavSection` item under "Account" in `SettingsLayout.tsx` (`NAV_SECTIONS`, from line 52) with `settings: settingIndex(...)` so the entries reach the settings search index, and one `` in `App.tsx` beside the other settings routes. + +- [ ] **Step 2: Build the list with tests** + +Rows are fixed height and carry name, last-used, and a changed-count pill; a device with nothing changed shows a dash rather than "0". Group by recency — Using now / This week / Earlier. Search filters by name and platform. Tests: grouping boundaries, the current device is marked, count pill renders a dash at zero, and search matches on platform as well as name. + +```bash +cd web && pnpm vitest run src/components/settings/DeviceList.test.tsx +``` + +--- + +### Task 10: The device detail pane + +**Files:** +- Create: `web/src/components/settings/DeviceSettingGroups.tsx` +- Create: `web/src/lib/deviceSettingGroups.ts` +- Test: `web/src/lib/deviceSettingGroups.test.ts` +- Test: `web/src/components/settings/DeviceSettingGroups.test.tsx` + +**Interfaces:** +- Produces: `groupDeviceSettings(keys)` → Picture / Sound / Subtitles / Episodes, derived from each definition's `category` plus a small key→group map for the cases `category` does not separate (`player.*` splits across Picture and Sound). +- Consumes: `ALL_DEVICE_SETTING_KEYS` (`web/src/lib/settingsDisplay.ts:126`) and `RegistrySettingControl` (`web/src/components/settings/RegistrySettingControl.tsx`). + +- [ ] **Step 1: Group mapping, with a completeness test** + +`TestEveryDeviceKeyIsGrouped` — every key in `ALL_DEVICE_SETTING_KEYS` lands in exactly one group. This is what stops a newly added manifest key from silently vanishing from the UI. + +- [ ] **Step 2: Render rows through the shared primitives** + +Use `SettingRow` and `SettingsGroup`. No raw keys. "Changed here" badge when a `profile_device` row exists; "Use my setting" clears at `profile_device` — a DELETE, never a copy of the profile value into the device row. Sliders and steppers must round-trip as numbers; do not reuse the admin screen's string round-trip (`web/src/hooks/queries/admin/users.ts:106-145`). + +- [ ] **Step 3: Policy-capped rows** + +When the effective response carries `constrained_by`, render `permitted_values` only and state the limit and who set it. Never a disabled control with no reason. Test both a capped select and a `locked` constraint. + +- [ ] **Step 4: Run** + +```bash +cd web && pnpm vitest run src/components/settings/ src/lib/deviceSettingGroups.test.ts +``` + +--- + +### Task 11: Remote-device editing and its copy + +**Files:** +- Modify: `web/src/pages/settings/DeviceSettings.tsx` +- Modify: `web/src/components/settings/DeviceSettingGroups.tsx` +- Test: `web/src/pages/settings/DeviceSettings.test.tsx` + +- [ ] **Step 1: Write to the selected device** + +Every mutation passes `deviceId` explicitly rather than relying on the header, so selecting a device and editing it writes that device. Test that editing a non-current device sends its id. + +- [ ] **Step 2: Scope and sync copy** + +Show the mandated scope sentence once per device — "this device, for your profile only" — not per row. For a non-current device, state that it picks the change up next time it is on. `useSettingValuesRealtime` (`settingValues.ts:245`) already invalidates on `user_settings.changed`, so no polling. + +- [ ] **Step 3: Forget and bulk clear, with confirmation** + +Both are destructive and both name their target: "Clear all N changes on this device", "Forget this device". + +--- + +## Phase 4 — Web: the household view + +### Task 12: Household scope switch and person grouping + +**Files:** +- Modify: `web/src/pages/settings/DeviceSettings.tsx` +- Modify: `web/src/components/settings/DeviceList.tsx` +- Test: `web/src/pages/settings/DeviceSettings.household.test.tsx` + +**Interfaces:** +- Consumes: `useIsActingAdmin`, `useCurrentProfile`, and `profile.is_primary` — the same rule as `RequirePrimaryOrAdmin` (`web/src/App.tsx:209`) and `isActingAdmin` (`web/src/lib/permissions.ts:19`). Do not write a fourth definition of this predicate. +- Consumes: `GET /profiles` for names, avatars, `is_child`. + +- [ ] **Step 1: The switch appears only for the household parent** + +Test that a non-primary, non-admin profile never sees it, and that the page still works fully for them in "just mine" mode. The switch is additive; nothing is taken away from anyone. + +- [ ] **Step 2: Group devices by person** + +Person header with avatar, name, a "You" or "Kid" tag, and a device count; devices nested beneath. When two profiles have registered the same physical TV, say so — "Same TV as yours — separate settings per person" — because that is the single most confusable thing on this screen. + +- [ ] **Step 3: Acting-on-behalf copy** + +A persistent banner while a sibling's device is selected: "You're changing Robin's settings, not your own." Reset actions name the person: "Use Robin's setting". Test that the banner is absent for one's own devices. + +--- + +### Task 13: Household boundaries in the UI + +**Files:** +- Modify: `web/src/components/settings/DeviceSettingGroups.tsx` +- Modify: `web/src/pages/settings/DeviceSettings.tsx` +- Test: `web/src/pages/settings/DeviceSettings.household.test.tsx` + +- [ ] **Step 1: Household limits read as limits, and link out** + +A value capped by parental controls shows a lock pill naming who set it and links to the profiles screen. This screen never authors a restriction — settings answer "what does this user want", policy answers "what are they allowed to have" (design spec, "Preferences versus restrictions"). + +- [ ] **Step 2: State the privacy boundary** + +A short block: this page shows how Silo is set up per device, not what anyone watched. Viewing history stays private per profile. Test that it renders in household mode. + +- [ ] **Step 3: Full check** + +```bash +cd web && pnpm run lint && pnpm run format:check && pnpm vitest run +``` + +--- + +## Phase 5 — Verification + +### Task 14: Cross-cutting checks + +- [ ] **Step 1: Full suite** + +```bash +make lint +make test +cd web && pnpm run lint && pnpm run format:check +make verify-local-paths +make verify-settings-bindings-all +``` + +Four Go failures (auth, catalog, jellycompat, notifications) pre-exist on some local Postgres provisioning and are not caused by this work — verify against the branch base before chasing one. `make lint` runs `golangci-lint` over the whole tree while CI runs `--new-from-merge-base`, so expect pre-existing findings that CI will not fail on; do not add to them. + +- [ ] **Step 2: Browser verification** + +Use the `web-ui-testing` skill against a real backend. Capture, for the PR: the device list at ten or more devices, a device detail pane, a remote-device edit, the household switch, and a policy-capped row. UI changes need screenshots (`CLAUDE.md`, "Pull requests"). + +- [ ] **Step 3: Manual authorization pass** + +With `curl` against a dev server, confirm each refusal returns the intended status and leaks nothing: +- non-primary naming a sibling profile → 403 +- primary with an unverified PIN → 403 +- a profile id from another account → 404 +- a device id belonging to another profile → 404 + +- [ ] **Step 4: Cross-repo follow-up** + +The two identity widenings are additive server capabilities that Apple and Android may adopt later; nothing in those clients breaks without a change. Note in the PR whether follow-up issues are wanted, per `CLAUDE.md` "Multi-repo". + +--- + +## Out of scope, and why + +- **Renaming a device.** `device_name` is client-reported and re-registration overwrites it, so a user-set name needs a separate column and a precedence rule. Worth doing; not part of this plan. +- **"Copy my settings from another device".** Appealing, but it writes up to 30 keys in one request, which is the argument for the batch mutations endpoint the design spec names (`POST /api/v1/settings/mutations`) and which does not exist. Build that first. +- **Transferring the primary designation.** `is_primary` is assigned implicitly to the first profile created (`internal/userstore/pgstore/profiles.go:60-73`) and cannot be moved. The household view makes that visible, so it likely needs to exist — as its own issue. +- **Watch history in the household view.** A different privacy question that a settings screen should not quietly answer. +- **Admin device screen rework.** Once users self-serve, `/admin/devices` can go on being a fleet console. Grouping its rows by what they affect and hiding raw keys behind a disclosure is worth doing separately. + +## Adjacent gaps found while planning + +Same shape as this work, but not blockers — each deserves its own issue: + +- `PUT`/`DELETE /profiles/{id}/avatar` (`internal/api/handlers/profile_avatars.go:202`, `:290`) apply no household guard and no self-check: any profile can change or delete any sibling's avatar, including the primary's. +- `GET /profiles` (`internal/api/handlers/profiles.go:263`) returns every sibling's resolved preferences, `has_pin`, `is_child`, content rating and library restrictions to any profile on the account. +- `POST /profiles/{id}/verify-pin` (`internal/api/handlers/profiles.go:701`) is open to any authenticated user of the account for any profile id, with no rate limiting in the handler. diff --git a/internal/api/handlers/devices.go b/internal/api/handlers/devices.go new file mode 100644 index 00000000..534ec01e --- /dev/null +++ b/internal/api/handlers/devices.go @@ -0,0 +1,326 @@ +package handlers + +import ( + "log/slog" + "net/http" + "sort" + "strings" + + "github.com/go-chi/chi/v5" + + "github.com/Silo-Server/silo-server/internal/access" + apimw "github.com/Silo-Server/silo-server/internal/api/middleware" + evt "github.com/Silo-Server/silo-server/internal/events" + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// DeviceHandler serves a viewer's own device registry: the devices they watch +// on, and the settings those devices carry. +// +// This is the self-service twin of the admin device routes. The important +// difference is scoping. The store's ListDevices is account-wide by +// construction — "WHERE user_id" in Postgres, and no WHERE at all in the +// per-user SQLite backend, which keeps one database per account — so every +// read here filters to a profile in the handler. Returning the store's rows +// unfiltered would show every household member's devices to everyone. +type DeviceHandler struct { + storeProvider userstore.UserStoreProvider + + // EventsHub, when set, receives a user_settings.changed event for every + // key a forget or clear removed. Nil (as in tests) skips publishing. + EventsHub *evt.Hub + + // UserRepo and ProfileTokens enable ?scope=household for the household + // parent. Both nil means the whole-household read is simply unavailable — + // never that it is unguarded. + UserRepo userLookup + ProfileTokens *access.ProfileTokenService +} + +func NewDeviceHandler(provider userstore.UserStoreProvider) *DeviceHandler { + return &DeviceHandler{storeProvider: provider} +} + +type deviceResponse struct { + DeviceID string `json:"device_id"` + DeviceName string `json:"device_name"` + DevicePlatform string `json:"device_platform"` + LastSeenAt string `json:"last_seen_at"` + ProfileID string `json:"profile_id"` + ProfileName string `json:"profile_name"` + // IsCurrentDevice marks the device this request came from, so a client can + // say "you're here" without repeating the header-matching rule. + IsCurrentDevice bool `json:"is_current_device"` + // ChangedCount is how many settings this (profile, device) pair overrides. + // It is the one number a device list needs: everything else about a device + // is either its identity or its last-seen time. + ChangedCount int `json:"changed_count"` +} + +type deviceListResponse struct { + Devices []deviceResponse `json:"devices"` +} + +// HandleListDevices handles GET /devices. +func (h *DeviceHandler) HandleListDevices(w http.ResponseWriter, r *http.Request) { + store, ok := h.storeFor(w, r) + if !ok { + return + } + registry, ok := store.(userstore.DeviceRegistry) + if !ok { + writeJSON(w, http.StatusOK, deviceListResponse{Devices: []deviceResponse{}}) + return + } + + profileID := strings.TrimSpace(apimw.GetProfileID(r.Context())) + if profileID == "" { + writeError(w, http.StatusBadRequest, "bad_request", "X-Profile-Id header is required") + return + } + + // Household scope is opt-in and guarded. The default is this profile alone, + // so the ordinary screen stays private by construction rather than by a + // caller remembering to ask for less. + household := strings.EqualFold(strings.TrimSpace(r.URL.Query().Get("scope")), "household") + if household { + allowed, err := canManageHousehold(r, store, h.UserRepo, h.ProfileTokens) + if err != nil { + writeProfileManagementPermissionError(w, err) + return + } + if !allowed { + writeError(w, http.StatusForbidden, "forbidden", + "Viewing the household's devices requires the primary profile or admin access") + return + } + } + + devices, err := registry.ListDevices(r.Context()) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list devices") + return + } + + // One pass over the stored values rather than a count query per device. + counts, err := deviceOverrideCounts(r, store) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to read device settings") + return + } + + profileNames, err := listProfileNamesByID(r.Context(), store) + if err != nil { + slog.WarnContext(r.Context(), "device list profile lookup failed", + "component", "api", "error", err) + profileNames = map[string]string{} + } + + currentDeviceID := deviceMetadataFromRequest(r).DeviceID + resp := deviceListResponse{Devices: make([]deviceResponse, 0, len(devices))} + for _, device := range devices { + if !household && device.ProfileID != profileID { + continue + } + resp.Devices = append(resp.Devices, deviceResponse{ + DeviceID: device.DeviceID, + DeviceName: device.DeviceName, + DevicePlatform: device.DevicePlatform, + LastSeenAt: device.LastSeenAt, + ProfileID: device.ProfileID, + ProfileName: profileNames[device.ProfileID], + IsCurrentDevice: device.DeviceID == currentDeviceID, + ChangedCount: counts[deviceKey{profileID: device.ProfileID, deviceID: device.DeviceID}], + }) + } + + // ListDevices already orders by last_seen_at; keep that and make the tie + // break deterministic so a client's list does not reshuffle between reads. + sort.SliceStable(resp.Devices, func(i, j int) bool { + if resp.Devices[i].LastSeenAt != resp.Devices[j].LastSeenAt { + return resp.Devices[i].LastSeenAt > resp.Devices[j].LastSeenAt + } + return resp.Devices[i].DeviceID < resp.Devices[j].DeviceID + }) + + writeJSON(w, http.StatusOK, resp) +} + +// HandleForgetDevice handles DELETE /devices/{device_id}: clear the device's +// settings and drop it from the registry. +func (h *DeviceHandler) HandleForgetDevice(w http.ResponseWriter, r *http.Request) { + h.removeDevice(w, r, true) +} + +// HandleClearDeviceSettings handles DELETE /devices/{device_id}/settings: +// return the device to the profile's own values without forgetting it. +func (h *DeviceHandler) HandleClearDeviceSettings(w http.ResponseWriter, r *http.Request) { + h.removeDevice(w, r, false) +} + +func (h *DeviceHandler) removeDevice(w http.ResponseWriter, r *http.Request, forget bool) { + store, ok := h.storeFor(w, r) + if !ok { + return + } + profileID := strings.TrimSpace(apimw.GetProfileID(r.Context())) + if profileID == "" { + writeError(w, http.StatusBadRequest, "bad_request", "X-Profile-Id header is required") + return + } + deviceID := strings.TrimSpace(chi.URLParam(r, "device_id")) + if deviceID == "" { + writeError(w, http.StatusBadRequest, "bad_request", "A device id is required") + return + } + // The household parent may forget or clear a family member's device, on the + // same guard the settings routes use. + if named := strings.TrimSpace(r.URL.Query().Get("profile_id")); named != "" && named != profileID { + allowed, err := canManageHousehold(r, store, h.UserRepo, h.ProfileTokens) + if err != nil { + writeProfileManagementPermissionError(w, err) + return + } + if !allowed { + writeError(w, http.StatusForbidden, "forbidden", + "Managing another profile's devices requires the primary profile or admin access") + return + } + profile, err := store.GetProfile(r.Context(), named) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to load profile") + return + } + if profile == nil { + writeError(w, http.StatusNotFound, "not_found", "Profile not found") + return + } + profileID = named + } + + // A device this profile never registered is not this caller's to remove. + // 404 rather than 403: a 403 would confirm the id exists somewhere. + // + // "This profile has no trace of it" covers both a device that never existed + // and one already forgotten, which is what makes a repeated forget answer + // 204 rather than inventing a failure for work it already did. A device + // belonging to *another* profile is equally traceless here, so it takes the + // same path and learns nothing about that device's existence. + registry, isRegistry := store.(userstore.DeviceRegistry) + owned := false + if isRegistry { + exists, err := registry.DeviceExists(r.Context(), profileID, deviceID) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to look up device") + return + } + owned = exists + } + keys, err := h.deviceSettingKeys(r, store, profileID, deviceID) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to read device settings") + return + } + if !owned && len(keys) == 0 { + writeError(w, http.StatusNotFound, "not_found", "Device not found") + return + } + + if _, err := store.DeleteSettingValuesForDevice(r.Context(), profileID, deviceID); err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to clear device settings") + return + } + // The legacy string-keyed device settings are a second generation of the + // same data; clearing one without the other would leave the device + // half-reset. + if err := store.DeleteAllDeviceSettings(r.Context(), profileID, deviceID); err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to clear device settings") + return + } + if forget && isRegistry { + if err := registry.ForgetDevice(r.Context(), profileID, deviceID); err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to forget device") + return + } + } + + userID := apimw.GetUserID(r.Context()) + for _, key := range keys { + publishUserSettingsEvent(r.Context(), h.EventsHub, userID, profileID, + key, string(settingscontract.ScopeProfileDevice)) + } + action := "clear_device" + if forget { + action = "forget_device" + } + auditSettingsForOther(r.Context(), settingsAuditRecord{ + Action: action, + ActorProfileID: actingProfileID(r.Context()), + TargetProfileID: profileID, + TargetUserID: userID, + DeviceID: deviceID, + Scope: string(settingscontract.ScopeProfileDevice), + }) + + w.WriteHeader(http.StatusNoContent) +} + +type deviceKey struct { + profileID string + deviceID string +} + +// deviceOverrideCounts counts stored profile_device rows per (profile, device). +// ListAllSettingValues spans the whole account, so the caller filters the +// result to the profile it is answering for. +func deviceOverrideCounts(r *http.Request, store userstore.UserStore) (map[deviceKey]int, error) { + values, err := store.ListAllSettingValues(r.Context()) + if err != nil { + return nil, err + } + counts := make(map[deviceKey]int) + for _, value := range values { + if value.Scope != settingscontract.ScopeProfileDevice { + continue + } + counts[deviceKey{profileID: value.ProfileID, deviceID: value.DeviceID}]++ + } + return counts, nil +} + +// deviceSettingKeys returns the canonical keys stored for one device, so a +// clear can publish an invalidation per key that actually moved. +func (h *DeviceHandler) deviceSettingKeys( + r *http.Request, store userstore.UserStore, profileID, deviceID string, +) ([]string, error) { + values, err := store.ListAllSettingValues(r.Context()) + if err != nil { + return nil, err + } + var keys []string + for _, value := range values { + if value.Scope != settingscontract.ScopeProfileDevice { + continue + } + if value.ProfileID != profileID || value.DeviceID != deviceID { + continue + } + keys = append(keys, value.Key) + } + return keys, nil +} + +func (h *DeviceHandler) storeFor(w http.ResponseWriter, r *http.Request) (userstore.UserStore, bool) { + userID := apimw.GetUserID(r.Context()) + if userID == 0 { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required") + return nil, false + } + store, err := h.storeProvider.ForUser(r.Context(), userID) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to access user store") + return nil, false + } + return store, true +} diff --git a/internal/api/handlers/devices_test.go b/internal/api/handlers/devices_test.go new file mode 100644 index 00000000..a0ec65ba --- /dev/null +++ b/internal/api/handlers/devices_test.go @@ -0,0 +1,405 @@ +package handlers + +import ( + "context" + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "github.com/Silo-Server/silo-server/internal/access" + apimw "github.com/Silo-Server/silo-server/internal/api/middleware" + "github.com/Silo-Server/silo-server/internal/auth" + "github.com/Silo-Server/silo-server/internal/models" + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userdb" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +func newDevicesTestHandler(t *testing.T) (*DeviceHandler, userstore.UserStore) { + t.Helper() + + dsn := "file:" + strings.NewReplacer("/", "_", " ", "_").Replace(t.Name()) + + "?mode=memory&cache=shared" + db, err := sql.Open("sqlite3", dsn) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := userdb.InitSchema(db); err != nil { + t.Fatalf("init schema: %v", err) + } + + store := userdb.NewSQLiteUserStore(db) + ctx := context.Background() + for _, p := range []userstore.Profile{ + {ID: "profile-1", Name: "Sam", IsPrimary: true}, + {ID: "profile-2", Name: "Robin"}, + } { + if err := store.CreateProfile(ctx, p); err != nil { + t.Fatalf("create profile %s: %v", p.ID, err) + } + } + + return NewDeviceHandler(testUserStoreProvider{store: store}), store +} + +func seedDevice(t *testing.T, store userstore.UserStore, profileID, deviceID, name string) { + t.Helper() + registry, ok := store.(userstore.DeviceRegistry) + if !ok { + t.Fatal("store does not implement DeviceRegistry") + } + if err := registry.RegisterDevice(context.Background(), userstore.DeviceEntry{ + ProfileID: profileID, DeviceID: deviceID, DeviceName: name, DevicePlatform: "web", + }); err != nil { + t.Fatalf("registering %s: %v", deviceID, err) + } +} + +func seedDeviceValue(t *testing.T, store userstore.UserStore, profileID, deviceID, key, value string) { + t.Helper() + if _, err := store.UpsertSettingValue(context.Background(), userstore.SettingIdentity{ + Key: key, + Scope: settingscontract.ScopeProfileDevice, + ProfileID: profileID, + DeviceID: deviceID, + }, json.RawMessage(value)); err != nil { + t.Fatalf("seeding %s on %s: %v", key, deviceID, err) + } +} + +func devicesRequest(method, target, profileID string) *http.Request { + req := httptest.NewRequest(method, target, nil) + req.Header.Set(deviceIDHeader, "device-1") + ctx := apimw.SetClaims(req.Context(), &auth.Claims{UserID: 1}) + return req.WithContext(apimw.SetProfileID(ctx, profileID)) +} + +func listDevices(t *testing.T, h *DeviceHandler, query, profileID string) deviceListResponse { + t.Helper() + target := "/devices" + if query != "" { + target += "?" + query + } + rec := httptest.NewRecorder() + h.HandleListDevices(rec, devicesRequest(http.MethodGet, target, profileID)) + if rec.Code != http.StatusOK { + t.Fatalf("GET %s = %d: %s", target, rec.Code, rec.Body.String()) + } + var body deviceListResponse + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decoding: %v", err) + } + return body +} + +// TestListDevices_FiltersToCallingProfile is the security test for this +// endpoint. ListDevices is account-wide by construction in both backends — +// "WHERE user_id" in Postgres and no WHERE at all in the per-user SQLite — so a +// passthrough would show every household member's devices to everyone. +func TestListDevices_FiltersToCallingProfile(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "device-1", "Sam's laptop") + seedDevice(t, store, "profile-2", "device-9", "Robin's iPad") + + body := listDevices(t, handler, "", "profile-1") + + if len(body.Devices) != 1 { + t.Fatalf("returned %d devices, want 1: %+v", len(body.Devices), body.Devices) + } + if body.Devices[0].DeviceID != "device-1" { + t.Errorf("returned device %q, want device-1", body.Devices[0].DeviceID) + } + for _, device := range body.Devices { + if device.ProfileID != "profile-1" { + t.Errorf("leaked device %q from profile %q", device.DeviceID, device.ProfileID) + } + } +} + +func TestListDevices_CountsChangedSettings(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "device-1", "Laptop") + seedDevice(t, store, "profile-1", "device-2", "Apple TV") + seedDeviceValue(t, store, "profile-1", "device-2", "player.hdr_enabled", `false`) + seedDeviceValue(t, store, "profile-1", "device-2", "playback.subtitle_mode", `"always"`) + // Another profile's row on the same device must not be counted. + seedDeviceValue(t, store, "profile-2", "device-2", "player.seek_cache_enabled", `false`) + + body := listDevices(t, handler, "", "profile-1") + + counts := map[string]int{} + for _, device := range body.Devices { + counts[device.DeviceID] = device.ChangedCount + } + if counts["device-2"] != 2 { + t.Errorf("device-2 changed_count = %d, want 2", counts["device-2"]) + } + if counts["device-1"] != 0 { + t.Errorf("device-1 changed_count = %d, want 0", counts["device-1"]) + } +} + +func TestListDevices_MarksCurrentDevice(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "device-1", "This browser") + seedDevice(t, store, "profile-1", "device-2", "Apple TV") + + body := listDevices(t, handler, "", "profile-1") + + for _, device := range body.Devices { + want := device.DeviceID == "device-1" + if device.IsCurrentDevice != want { + t.Errorf("device %q is_current_device = %v, want %v", + device.DeviceID, device.IsCurrentDevice, want) + } + } +} + +func TestListDevices_IncludesProfileName(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "device-1", "Laptop") + + body := listDevices(t, handler, "", "profile-1") + + if len(body.Devices) != 1 || body.Devices[0].ProfileName != "Sam" { + t.Errorf("profile_name = %q, want Sam", body.Devices[0].ProfileName) + } +} + +func routeDevice( + t *testing.T, h *DeviceHandler, method, target, deviceID, profileID string, + handle func(http.ResponseWriter, *http.Request), +) *httptest.ResponseRecorder { + t.Helper() + req := devicesRequest(method, target, profileID) + routeCtx := chi.NewRouteContext() + routeCtx.URLParams.Add("device_id", deviceID) + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, routeCtx)) + + rec := httptest.NewRecorder() + handle(rec, req) + return rec +} + +func TestForgetDevice_RemovesSettingsAndRegistryRow(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "device-2", "Apple TV") + seedDeviceValue(t, store, "profile-1", "device-2", "player.hdr_enabled", `false`) + + rec := routeDevice(t, handler, http.MethodDelete, "/devices/device-2", "device-2", + "profile-1", handler.HandleForgetDevice) + if rec.Code != http.StatusNoContent { + t.Fatalf("DELETE = %d: %s", rec.Code, rec.Body.String()) + } + + registry := store.(userstore.DeviceRegistry) + exists, err := registry.DeviceExists(context.Background(), "profile-1", "device-2") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if exists { + t.Error("registry row survived forget") + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "" { + t.Errorf("setting row survived forget on device %q", got) + } +} + +func TestForgetDevice_RejectsOtherProfilesDevice(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-2", "device-9", "Robin's iPad") + seedDeviceValue(t, store, "profile-2", "device-9", "player.hdr_enabled", `false`) + + rec := routeDevice(t, handler, http.MethodDelete, "/devices/device-9", "device-9", + "profile-1", handler.HandleForgetDevice) + if rec.Code != http.StatusNotFound { + t.Fatalf("DELETE another profile's device = %d, want 404", rec.Code) + } + + registry := store.(userstore.DeviceRegistry) + exists, err := registry.DeviceExists(context.Background(), "profile-2", "device-9") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if !exists { + t.Error("another profile's device was removed") + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "device-9" { + t.Errorf("another profile's setting row was removed (device %q)", got) + } +} + +// A repeated forget reports 404, not 500 or a partial delete: once the device +// is gone this profile has no trace of it, which is indistinguishable from a +// device that was never here — and deliberately so, since the same answer is +// what keeps another profile's device ids from being probeable. +func TestForgetDevice_SecondCallIsNotFound(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "device-2", "Apple TV") + + if rec := routeDevice(t, handler, http.MethodDelete, "/devices/device-2", "device-2", + "profile-1", handler.HandleForgetDevice); rec.Code != http.StatusNoContent { + t.Fatalf("first DELETE = %d, want 204: %s", rec.Code, rec.Body.String()) + } + if rec := routeDevice(t, handler, http.MethodDelete, "/devices/device-2", "device-2", + "profile-1", handler.HandleForgetDevice); rec.Code != http.StatusNotFound { + t.Fatalf("second DELETE = %d, want 404", rec.Code) + } +} + +// Forgetting a device two profiles share removes only the caller's half. +func TestForgetDevice_LeavesOtherProfilesRowOnSharedDevice(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "shared-tv", "Living Room TV") + seedDevice(t, store, "profile-2", "shared-tv", "Living Room TV") + seedDeviceValue(t, store, "profile-2", "shared-tv", "player.hdr_enabled", `false`) + + rec := routeDevice(t, handler, http.MethodDelete, "/devices/shared-tv", "shared-tv", + "profile-1", handler.HandleForgetDevice) + if rec.Code != http.StatusNoContent { + t.Fatalf("DELETE = %d: %s", rec.Code, rec.Body.String()) + } + + registry := store.(userstore.DeviceRegistry) + stillThere, err := registry.DeviceExists(context.Background(), "profile-2", "shared-tv") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if !stillThere { + t.Error("forgetting one profile's half removed the other profile's row") + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "shared-tv" { + t.Errorf("the other profile's setting row was removed (device %q)", got) + } +} + +func TestClearDeviceSettings_KeepsRegistryRow(t *testing.T) { + handler, store := newDevicesTestHandler(t) + seedDevice(t, store, "profile-1", "device-2", "Apple TV") + seedDeviceValue(t, store, "profile-1", "device-2", "player.hdr_enabled", `false`) + + rec := routeDevice(t, handler, http.MethodDelete, "/devices/device-2/settings", "device-2", + "profile-1", handler.HandleClearDeviceSettings) + if rec.Code != http.StatusNoContent { + t.Fatalf("DELETE = %d: %s", rec.Code, rec.Body.String()) + } + + registry := store.(userstore.DeviceRegistry) + exists, err := registry.DeviceExists(context.Background(), "profile-1", "device-2") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if !exists { + t.Error("registry row was removed; clear must keep the device") + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "" { + t.Errorf("setting row survived clear on device %q", got) + } +} + +// --- Household scope --- + +func householdDevicesHandler(t *testing.T) (*DeviceHandler, userstore.UserStore) { + t.Helper() + handler, store := newDevicesTestHandler(t) + handler.UserRepo = stubUserRepo{user: &models.User{ID: 1}} + handler.ProfileTokens = access.NewProfileTokenService("test-secret-value-at-least-32-chars", 0) + return handler, store +} + +func TestListDevices_PrimarySeesHouseholdWhenRequested(t *testing.T) { + handler, store := householdDevicesHandler(t) + seedDevice(t, store, "profile-1", "device-1", "Sam's laptop") + seedDevice(t, store, "profile-2", "device-9", "Robin's iPad") + + body := listDevices(t, handler, "scope=household", "profile-1") + + if len(body.Devices) != 2 { + t.Fatalf("household scope returned %d devices, want 2: %+v", len(body.Devices), body.Devices) + } + names := map[string]string{} + for _, device := range body.Devices { + names[device.DeviceID] = device.ProfileName + } + if names["device-9"] != "Robin" { + t.Errorf("device-9 profile_name = %q, want Robin", names["device-9"]) + } +} + +func TestListDevices_NonPrimaryCannotRequestHousehold(t *testing.T) { + handler, store := householdDevicesHandler(t) + seedDevice(t, store, "profile-1", "device-1", "Sam's laptop") + seedDevice(t, store, "profile-2", "device-9", "Robin's iPad") + + rec := httptest.NewRecorder() + handler.HandleListDevices(rec, devicesRequest(http.MethodGet, "/devices?scope=household", "profile-2")) + + if rec.Code != http.StatusForbidden { + t.Fatalf("non-primary household read = %d, want 403: %s", rec.Code, rec.Body.String()) + } + if strings.Contains(rec.Body.String(), "device-1") { + t.Error("refusal leaked another profile's device") + } +} + +// Default scope stays private even for the household parent, so the ordinary +// screen cannot show the family's devices by forgetting to ask for less. +func TestListDevices_PrimaryDefaultsToOwnProfile(t *testing.T) { + handler, store := householdDevicesHandler(t) + seedDevice(t, store, "profile-1", "device-1", "Sam's laptop") + seedDevice(t, store, "profile-2", "device-9", "Robin's iPad") + + body := listDevices(t, handler, "", "profile-1") + + if len(body.Devices) != 1 || body.Devices[0].DeviceID != "device-1" { + t.Fatalf("default scope returned %+v, want only device-1", body.Devices) + } +} + +func TestForgetDevice_PrimaryMayForgetHouseholdDevice(t *testing.T) { + handler, store := householdDevicesHandler(t) + seedDevice(t, store, "profile-2", "device-9", "Robin's iPad") + seedDeviceValue(t, store, "profile-2", "device-9", "player.hdr_enabled", `false`) + + rec := routeDevice(t, handler, http.MethodDelete, "/devices/device-9?profile_id=profile-2", + "device-9", "profile-1", handler.HandleForgetDevice) + if rec.Code != http.StatusNoContent { + t.Fatalf("primary forgetting a household device = %d: %s", rec.Code, rec.Body.String()) + } + + registry := store.(userstore.DeviceRegistry) + exists, err := registry.DeviceExists(context.Background(), "profile-2", "device-9") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if exists { + t.Error("device survived the household forget") + } +} + +func TestForgetDevice_NonPrimaryCannotForgetSiblingsDevice(t *testing.T) { + handler, store := householdDevicesHandler(t) + seedDevice(t, store, "profile-1", "device-1", "Sam's laptop") + + rec := routeDevice(t, handler, http.MethodDelete, "/devices/device-1?profile_id=profile-1", + "device-1", "profile-2", handler.HandleForgetDevice) + if rec.Code != http.StatusForbidden { + t.Fatalf("non-primary forgetting a sibling's device = %d, want 403: %s", + rec.Code, rec.Body.String()) + } + + registry := store.(userstore.DeviceRegistry) + exists, err := registry.DeviceExists(context.Background(), "profile-1", "device-1") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if !exists { + t.Error("a non-primary profile removed a sibling's device") + } +} diff --git a/internal/api/handlers/household.go b/internal/api/handlers/household.go new file mode 100644 index 00000000..5f255c60 --- /dev/null +++ b/internal/api/handlers/household.go @@ -0,0 +1,118 @@ +package handlers + +import ( + "context" + "fmt" + "net/http" + + "github.com/Silo-Server/silo-server/internal/access" + apimw "github.com/Silo-Server/silo-server/internal/api/middleware" + "github.com/Silo-Server/silo-server/internal/models" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// userLookup is the slice of the user repository the household check needs: a +// profile token is only valid against the access-policy revision it was minted +// for, so verifying one means reading the user's current revision. +type userLookup interface { + GetByID(ctx context.Context, id int) (*models.User, error) +} + +// canManageHousehold reports whether the caller may act for the whole household +// — every profile on their own account — rather than only for themselves. +// +// Server admins always may. Otherwise the caller's active profile must be the +// one flagged is_primary, which is the household parent and is deliberately +// *not* the server-wide admin role: a household parent manages their family, +// an admin manages the server. +// +// When that primary profile has a PIN, management additionally requires a valid +// X-Profile-Token from /profiles/{id}/verify-pin. Without that, a client could +// walk past a profile lock by sending only X-Profile-Id. +// +// This is a policy boundary for well-behaved clients rather than a defense +// against the account holder: every profile on an account shares one login +// session, so X-Profile-Id is self-asserted (see the note in +// internal/api/middleware/auth.go). It is the same boundary profile management +// has always used, and it is applied here so household settings management is +// guarded and auditable rather than implicit. +func canManageHousehold( + r *http.Request, + store userstore.UserStore, + users userLookup, + tokens *access.ProfileTokenService, +) (bool, error) { + ctx := r.Context() + if apimw.IsAdmin(ctx) { + return true, nil + } + activeProfileID := apimw.GetProfileID(ctx) + if activeProfileID == "" { + activeProfileID = r.Header.Get("X-Profile-Id") + } + if activeProfileID == "" { + return false, nil + } + active, err := store.GetProfile(ctx, activeProfileID) + if err != nil { + return false, err + } + if active == nil { + return false, nil + } + if !active.IsPrimary { + return false, nil + } + if active.PINHash == "" { + return true, nil + } + if err := verifyProfileToken(r, users, tokens, active.ID); err != nil { + return false, err + } + return true, nil +} + +// verifyProfileToken checks the X-Profile-Token a PIN-locked profile must +// present. Missing dependencies fail closed: a handler wired without a token +// service cannot verify a PIN, and "cannot verify" is not "verified". +func verifyProfileToken( + r *http.Request, + users userLookup, + tokens *access.ProfileTokenService, + profileID string, +) error { + if users == nil || tokens == nil { + return access.ErrProfileUnverified + } + + claims := apimw.GetClaims(r.Context()) + if claims == nil || claims.SessionID == "" { + return access.ErrProfileUnverified + } + + userID := apimw.GetUserID(r.Context()) + if userID == 0 { + return access.ErrProfileUnverified + } + + user, err := users.GetByID(r.Context(), userID) + if err != nil { + return fmt.Errorf("loading user policy: %w", err) + } + if user == nil { + return access.ErrProfileUnverified + } + + profileClaims, err := tokens.Validate(r.Header.Get("X-Profile-Token")) + if err != nil { + return err + } + if profileClaims.UserID != userID || + profileClaims.SessionID != claims.SessionID || + profileClaims.ProfileID != profileID || + profileClaims.PolicyRevision != user.AccessPolicyRevision { + return access.ErrProfileUnverified + } + + return nil +} diff --git a/internal/api/handlers/household_test.go b/internal/api/handlers/household_test.go new file mode 100644 index 00000000..6a82464d --- /dev/null +++ b/internal/api/handlers/household_test.go @@ -0,0 +1,165 @@ +package handlers + +import ( + "context" + "database/sql" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Silo-Server/silo-server/internal/access" + apimw "github.com/Silo-Server/silo-server/internal/api/middleware" + "github.com/Silo-Server/silo-server/internal/auth" + "github.com/Silo-Server/silo-server/internal/models" + "github.com/Silo-Server/silo-server/internal/userdb" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +type stubUserRepo struct { + user *models.User +} + +func (s stubUserRepo) GetByID(context.Context, int) (*models.User, error) { + return s.user, nil +} + +func newHouseholdTestStore(t *testing.T) userstore.UserStore { + t.Helper() + dsn := "file:" + strings.NewReplacer("/", "_", " ", "_").Replace(t.Name()) + + "?mode=memory&cache=shared" + db, err := sql.Open("sqlite3", dsn) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := userdb.InitSchema(db); err != nil { + t.Fatalf("init schema: %v", err) + } + return userdb.NewSQLiteUserStore(db) +} + +func householdRequest(profileID string, admin bool, profileToken string) *http.Request { + req := httptest.NewRequest(http.MethodGet, "/", nil) + claims := &auth.Claims{UserID: 1, SessionID: "session-1"} + if admin { + claims.Role = "admin" + } + ctx := apimw.SetClaims(req.Context(), claims) + if profileID != "" { + ctx = apimw.SetProfileID(ctx, profileID) + } + if profileToken != "" { + req.Header.Set("X-Profile-Token", profileToken) + } + return req.WithContext(ctx) +} + +// TestCanManageHousehold pins the household-parent boundary before it is shared +// with the settings routes. The rule is deliberately narrow: a server admin, or +// the one profile flagged is_primary — and when that profile carries a PIN, a +// verified profile token, so sending only X-Profile-Id cannot walk past a +// profile lock. +func TestCanManageHousehold(t *testing.T) { + ctx := context.Background() + + setup := func(t *testing.T, pin string) (userstore.UserStore, *access.ProfileTokenService) { + t.Helper() + store := newHouseholdTestStore(t) + if err := store.CreateProfile(ctx, userstore.Profile{ + ID: "primary", Name: "Sam", IsPrimary: true, + }); err != nil { + t.Fatalf("create primary: %v", err) + } + if err := store.CreateProfile(ctx, userstore.Profile{ + ID: "child", Name: "Robin", + }); err != nil { + t.Fatalf("create child: %v", err) + } + if pin != "" { + if err := store.UpdateProfile(ctx, "primary", userstore.UpdateProfileInput{ + PIN: &pin, + }); err != nil { + t.Fatalf("set pin: %v", err) + } + } + return store, access.NewProfileTokenService("test-secret-value-at-least-32-chars", 0) + } + + t.Run("admin always may", func(t *testing.T) { + store, tokens := setup(t, "") + // An admin with no active profile at all still manages. + ok, err := canManageHousehold(householdRequest("", true, ""), store, nil, tokens) + if err != nil || !ok { + t.Fatalf("admin = (%v, %v), want (true, nil)", ok, err) + } + }) + + t.Run("primary without pin may", func(t *testing.T) { + store, tokens := setup(t, "") + ok, err := canManageHousehold(householdRequest("primary", false, ""), store, nil, tokens) + if err != nil || !ok { + t.Fatalf("primary = (%v, %v), want (true, nil)", ok, err) + } + }) + + t.Run("non-primary may not", func(t *testing.T) { + store, tokens := setup(t, "") + ok, err := canManageHousehold(householdRequest("child", false, ""), store, nil, tokens) + if err != nil || ok { + t.Fatalf("non-primary = (%v, %v), want (false, nil)", ok, err) + } + }) + + t.Run("no active profile may not", func(t *testing.T) { + store, tokens := setup(t, "") + ok, err := canManageHousehold(householdRequest("", false, ""), store, nil, tokens) + if err != nil || ok { + t.Fatalf("no profile = (%v, %v), want (false, nil)", ok, err) + } + }) + + t.Run("unknown profile may not", func(t *testing.T) { + store, tokens := setup(t, "") + ok, err := canManageHousehold(householdRequest("ghost", false, ""), store, nil, tokens) + if err != nil || ok { + t.Fatalf("unknown profile = (%v, %v), want (false, nil)", ok, err) + } + }) + + t.Run("primary with pin and no token may not", func(t *testing.T) { + store, tokens := setup(t, "1234") + repo := stubUserRepo{user: &models.User{ID: 1}} + _, err := canManageHousehold(householdRequest("primary", false, ""), store, repo, tokens) + if !errors.Is(err, access.ErrProfileUnverified) { + t.Fatalf("pin without token err = %v, want ErrProfileUnverified", err) + } + }) + + t.Run("primary with pin and valid token may", func(t *testing.T) { + store, tokens := setup(t, "1234") + repo := stubUserRepo{user: &models.User{ID: 1}} + token, _, err := tokens.Mint(access.ProfileTokenClaims{ + UserID: 1, SessionID: "session-1", ProfileID: "primary", PolicyRevision: 0, + }) + if err != nil { + t.Fatalf("issuing token: %v", err) + } + ok, err := canManageHousehold(householdRequest("primary", false, token), store, repo, tokens) + if err != nil || !ok { + t.Fatalf("pin with token = (%v, %v), want (true, nil)", ok, err) + } + }) + + // Nil dependencies must fail closed. A settings handler built without a + // token service is not a reason to skip the PIN check. + t.Run("pin with no token service may not", func(t *testing.T) { + store, _ := setup(t, "1234") + repo := stubUserRepo{user: &models.User{ID: 1}} + _, err := canManageHousehold(householdRequest("primary", false, "tok"), store, repo, nil) + if !errors.Is(err, access.ErrProfileUnverified) { + t.Fatalf("nil token service err = %v, want ErrProfileUnverified", err) + } + }) +} diff --git a/internal/api/handlers/profiles.go b/internal/api/handlers/profiles.go index 7af9e2b1..45a06c57 100644 --- a/internal/api/handlers/profiles.go +++ b/internal/api/handlers/profiles.go @@ -137,78 +137,24 @@ type verifyPINResponse struct { } // canManageHouseholdProfiles reports whether the caller may create/update/delete -// profiles belonging to their user. Server admins always can. Otherwise the -// caller's active profile must be the primary profile for the household. +// profiles belonging to their user. // -// When that primary profile has a PIN, management also requires a valid -// X-Profile-Token from `/profiles/{id}/verify-pin`; otherwise a client could -// bypass the profile lock by sending only X-Profile-Id. +// The rule lives in household.go so the settings routes can apply the same one: +// a household parent who may edit a child's profile may also edit that child's +// device settings, and two definitions of "is this the household parent" would +// eventually disagree. func (h *ProfileHandler) canManageHouseholdProfiles(r *http.Request, store userstore.UserStore) (bool, error) { - ctx := r.Context() - if apimw.IsAdmin(ctx) { - return true, nil - } - activeProfileID := apimw.GetProfileID(ctx) - if activeProfileID == "" { - activeProfileID = r.Header.Get("X-Profile-Id") - } - if activeProfileID == "" { - return false, nil - } - active, err := store.GetProfile(ctx, activeProfileID) - if err != nil { - return false, err - } - if active == nil { - return false, nil - } - if !active.IsPrimary { - return false, nil - } - if active.PINHash == "" { - return true, nil - } - if err := h.requireVerifiedProfileToken(r, active.ID); err != nil { - return false, err - } - return true, nil + return canManageHousehold(r, store, h.userLookupOrNil(), h.ProfileTokens) } -func (h *ProfileHandler) requireVerifiedProfileToken(r *http.Request, profileID string) error { - if h.UserRepo == nil || h.ProfileTokens == nil { - return access.ErrProfileUnverified +// userLookupOrNil returns UserRepo as the narrow interface the household check +// wants, preserving nil-ness: a typed nil in a non-nil interface would defeat +// the fail-closed check there. +func (h *ProfileHandler) userLookupOrNil() userLookup { + if h.UserRepo == nil { + return nil } - - claims := apimw.GetClaims(r.Context()) - if claims == nil || claims.SessionID == "" { - return access.ErrProfileUnverified - } - - userID := apimw.GetUserID(r.Context()) - if userID == 0 { - return access.ErrProfileUnverified - } - - user, err := h.UserRepo.GetByID(r.Context(), userID) - if err != nil { - return fmt.Errorf("loading user policy: %w", err) - } - if user == nil { - return access.ErrProfileUnverified - } - - profileClaims, err := h.ProfileTokens.Validate(r.Header.Get("X-Profile-Token")) - if err != nil { - return err - } - if profileClaims.UserID != userID || - profileClaims.SessionID != claims.SessionID || - profileClaims.ProfileID != profileID || - profileClaims.PolicyRevision != user.AccessPolicyRevision { - return access.ErrProfileUnverified - } - - return nil + return h.UserRepo } func writeProfileManagementPermissionError(w http.ResponseWriter, err error) { diff --git a/internal/api/handlers/settings_audit.go b/internal/api/handlers/settings_audit.go new file mode 100644 index 00000000..fa30cdfd --- /dev/null +++ b/internal/api/handlers/settings_audit.go @@ -0,0 +1,80 @@ +package handlers + +import ( + "context" + "log/slog" + + apimw "github.com/Silo-Server/silo-server/internal/api/middleware" +) + +// Audit records for settings changes made *for someone else*. +// +// Ordinary self-service writes are deliberately not audited: they are the +// common case by orders of magnitude, and a trail that records everything +// answers nothing. What needs an answer is "who turned subtitles on for +// Robin?" — a household parent acting for another profile, or an admin acting +// on an account. +// +// The record carries identity only, never the value. The realtime event makes +// the same choice for the same reason (see user_settings_events.go): a value +// here would put one profile's private settings into a log an operator reads. +// +// This is a structured log record rather than a row in activity_log: +// activitylog is an HTTP request-log middleware whose schema has no profile or +// body, so it cannot express "actor P changed key K for profile Q". Persisting +// these needs its own table and migration. +const settingsAuditMsg = "settings changed for another profile" + +// logComponentKey is the structured-log attribute every handler in this package +// tags itself with. +const logComponentKey = "component" + +type settingsAuditRecord struct { + Action string + ActorProfileID string + TargetProfileID string + // TargetUserID is the account the change lands on. It differs from the + // actor's own account only on the admin routes, where profile ids alone + // would not say whose settings moved. + TargetUserID int + DeviceID string + Key string + Scope string +} + +// auditSettingsForOther emits the record when, and only when, the actor is +// acting for someone else — another profile, or (on the admin routes) another +// account entirely. +func auditSettingsForOther(ctx context.Context, record settingsAuditRecord) { + actorUserID := apimw.GetUserID(ctx) + sameProfile := record.TargetProfileID == "" || record.TargetProfileID == record.ActorProfileID + sameUser := record.TargetUserID == 0 || record.TargetUserID == actorUserID + if sameProfile && sameUser { + return + } + attrs := []any{ + logComponentKey, "api", + "action", record.Action, + "actor_user_id", actorUserID, + "actor_profile_id", record.ActorProfileID, + "target_user_id", record.TargetUserID, + "target_profile_id", record.TargetProfileID, + "acting_as_admin", apimw.IsAdmin(ctx), + } + if record.Key != "" { + attrs = append(attrs, "setting_key", record.Key) + } + if record.Scope != "" { + attrs = append(attrs, "scope", record.Scope) + } + if record.DeviceID != "" { + attrs = append(attrs, "device_id", record.DeviceID) + } + slog.InfoContext(ctx, settingsAuditMsg, attrs...) +} + +// actingProfileID is the profile the caller is signed in as, which is not +// necessarily the profile a request addresses. +func actingProfileID(ctx context.Context) string { + return apimw.GetProfileID(ctx) +} diff --git a/internal/api/handlers/settings_values.go b/internal/api/handlers/settings_values.go index 5a68a689..0a12d695 100644 --- a/internal/api/handlers/settings_values.go +++ b/internal/api/handlers/settings_values.go @@ -11,6 +11,7 @@ import ( "log/slog" "net/http" "net/url" + "slices" "strconv" "strings" "time" @@ -52,12 +53,24 @@ type SettingValuesHandler struct { // EventsHub, when set, receives a user_settings.changed event after every // successful write or delete. Nil (as in tests) simply skips publishing. EventsHub *evt.Hub + + // UserRepo and ProfileTokens enable household management: a primary profile + // naming another profile on its own account. Both nil means the widening is + // simply unavailable — never that it is unguarded. + UserRepo userLookup + ProfileTokens *access.ProfileTokenService } +// languageSuggestionSource supplies the distinct original_language values the +// accessible catalog actually contains. It decorates catalog.metadata_language +// suggestions only: original_language is a plain indexed scalar column, so the +// listing is a cheap DISTINCT scan. The audio and subtitle pickers deliberately +// do NOT get observed values — their track-derived listings walk every media +// file (tens of seconds on large catalogs), and since those settings are open +// language_tag values, clients offer free entry beyond the contract floor +// instead. type languageSuggestionSource interface { ListOriginalLanguages(context.Context, catalog.BrowseFilters) ([]string, error) - ListAudioLanguages(context.Context, catalog.BrowseFilters) ([]string, error) - ListSubtitleLanguages(context.Context, catalog.BrowseFilters) ([]string, error) } // SetLibraryLookup wires the catalog lookup used to reject profile_library @@ -67,9 +80,10 @@ func (h *SettingValuesHandler) SetLibraryLookup(lookup libraryLookup) { h.libraryLookup = lookup } -// SetLanguageSuggestionSource wires deployment-observed media languages into -// effective setting responses. The contract option set remains the stable -// floor; a missing source or failed catalog lookup simply returns that floor. +// SetLanguageSuggestionSource wires deployment-observed original languages +// into effective metadata-language responses. The contract option set remains +// the stable floor; a missing source or failed catalog lookup simply returns +// that floor. func (h *SettingValuesHandler) SetLanguageSuggestionSource(source languageSuggestionSource) { h.languageSource = source } @@ -439,15 +453,33 @@ func (h *SettingValuesHandler) setValueAt( // updated_at included — rather than a reconstruction of the input. h.recordMutation(r, store, mutationID, requestHash, response) } + acting := actingProfileID(r.Context()) if identity.Scope == settingscontract.ScopeProfileDevice { // A device that only ever writes canonically must still appear in // ListDevices and the device-management surfaces, or it can never be // discovered and forgotten. The legacy device route registers on every // touch; the canonical route matches it on device writes. - h.registerWritingDevice(r, store, identity.ProfileID) + // + // Only when the caller is writing its own device for its own profile, + // though. Registration asserts "this device is in use by this profile", + // which a write aimed at another device — or made on another profile's + // behalf — is not. Registering here would invent a device nobody holds: + // the parent's browser filed under the child's profile. + if identity.DeviceID == deviceMetadataFromRequest(r).DeviceID && identity.ProfileID == acting { + h.registerWritingDevice(r, store, identity.ProfileID) + } } publishUserSettingsEvent(r.Context(), h.EventsHub, eventUserID, identity.ProfileID, identity.Key, string(identity.Scope)) + auditSettingsForOther(r.Context(), settingsAuditRecord{ + Action: "set", + ActorProfileID: acting, + TargetProfileID: identity.ProfileID, + TargetUserID: eventUserID, + DeviceID: identity.DeviceID, + Key: identity.Key, + Scope: string(identity.Scope), + }) writeJSON(w, http.StatusOK, response) } @@ -520,6 +552,15 @@ func (h *SettingValuesHandler) deleteValueAt( writeError(w, http.StatusNotFound, "not_found", "No value is set at this scope") return } + auditSettingsForOther(r.Context(), settingsAuditRecord{ + Action: "clear", + ActorProfileID: actingProfileID(r.Context()), + TargetProfileID: identity.ProfileID, + TargetUserID: eventUserID, + DeviceID: identity.DeviceID, + Key: identity.Key, + Scope: string(identity.Scope), + }) publishUserSettingsEvent(r.Context(), h.EventsHub, eventUserID, identity.ProfileID, identity.Key, string(identity.Scope)) w.WriteHeader(http.StatusNoContent) @@ -568,6 +609,23 @@ func (h *SettingValuesHandler) HandleGetEffective(w http.ResponseWriter, r *http SeriesIDs: splitCSV(r.URL.Query().Get("series_ids")), } + // A device-settings screen resolves what some *other* device sees, so this + // read accepts the same explicit identity the write path does, under the + // same guards: the device must belong to the profile, and naming another + // profile requires the household parent. + if named := strings.TrimSpace(r.URL.Query().Get("profile_id")); named != "" && named != rc.ProfileID { + if !h.mayActForProfile(w, r, named) { + return + } + rc.ProfileID = named + } + if named := strings.TrimSpace(r.URL.Query().Get("device_id")); named != "" { + if !h.deviceBelongsToProfile(w, r, rc.ProfileID, named) { + return + } + rc.DeviceID = named + } + // The SQLite backend expands these into IN lists, whose host-parameter // budget is finite; an unbounded request could fail the whole resolution. // The bound is far above any real batch — a season view resolves a @@ -871,8 +929,9 @@ func (h *SettingValuesHandler) identityForSessionKey( identity := userstore.SettingIdentity{Key: key, Scope: scope} - // Profile and device come from the session headers rather than the query, - // so one profile cannot write another's settings by naming it. + // The profile defaults to the session header, so an ordinary caller cannot + // write another's settings by naming it. A household parent may name a + // different profile on their own account — authorized below. if scope != settingscontract.ScopeAccount { identity.ProfileID = strings.TrimSpace(apimw.GetProfileID(r.Context())) if identity.ProfileID == "" { @@ -880,19 +939,108 @@ func (h *SettingValuesHandler) identityForSessionKey( "X-Profile-Id header is required for this scope") return userstore.SettingIdentity{}, false } + if named := strings.TrimSpace(r.URL.Query().Get("profile_id")); named != "" && + named != identity.ProfileID { + if !h.mayActForProfile(w, r, named) { + return userstore.SettingIdentity{}, false + } + identity.ProfileID = named + } } if scope == settingscontract.ScopeProfileDevice { - identity.DeviceID = deviceMetadataFromRequest(r).DeviceID + // A device may be named explicitly so one device can manage another's + // settings — the screen that lists your devices edits them in place. + // Unlike the profile above, that is safe to accept from the query only + // because the device is then checked against this profile's registry. + named := strings.TrimSpace(r.URL.Query().Get("device_id")) + identity.DeviceID = named + if identity.DeviceID == "" { + identity.DeviceID = deviceMetadataFromRequest(r).DeviceID + } if identity.DeviceID == "" { writeError(w, http.StatusBadRequest, "bad_request", "X-Silo-Device-Id header is required for a device override") return userstore.SettingIdentity{}, false } + if named != "" && !h.deviceBelongsToProfile(w, r, identity.ProfileID, named) { + return userstore.SettingIdentity{}, false + } } return h.completeIdentity(w, r.Context(), r.URL.Query(), identity) } +// mayActForProfile authorizes acting for a profile other than the caller's own. +// +// Two checks, in this order and for different reasons. First the household +// guard: only the primary profile (or a server admin) manages the household, so +// an ordinary member naming a sibling is 403 — the profile plainly exists, and +// pretending otherwise would be a lie the caller can already disprove through +// GET /profiles. Then existence, resolved through the caller's *own* user +// store, which is what confines this to one account: a profile id from another +// account is simply absent there, so it is 404 and the caller learns nothing. +func (h *SettingValuesHandler) mayActForProfile( + w http.ResponseWriter, r *http.Request, profileID string, +) bool { + store, ok := h.storeFor(w, r) + if !ok { + return false + } + + allowed, err := canManageHousehold(r, store, h.UserRepo, h.ProfileTokens) + if err != nil { + writeProfileManagementPermissionError(w, err) + return false + } + if !allowed { + writeError(w, http.StatusForbidden, "forbidden", + "Managing another profile's settings requires the primary profile or admin access") + return false + } + + profile, err := store.GetProfile(r.Context(), profileID) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to load profile") + return false + } + if profile == nil { + writeError(w, http.StatusNotFound, "not_found", "Profile not found") + return false + } + return true +} + +// deviceBelongsToProfile authorizes a device id that came from the query rather +// than from this request's own header. It answers 404 rather than 403 for an +// unknown device: a 403 would confirm the id exists somewhere. +// +// The caller's own header device is deliberately not checked. Registration is +// lazy — a device's first write is what registers it — so requiring a row there +// would reject every new device's first setting. +func (h *SettingValuesHandler) deviceBelongsToProfile( + w http.ResponseWriter, r *http.Request, profileID, deviceID string, +) bool { + store, ok := h.storeFor(w, r) + if !ok { + return false + } + registry, ok := store.(userstore.DeviceRegistry) + if !ok { + writeError(w, http.StatusNotFound, "not_found", "Device not found") + return false + } + exists, err := registry.DeviceExists(r.Context(), profileID, deviceID) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to look up device") + return false + } + if !exists { + writeError(w, http.StatusNotFound, "not_found", "Device not found") + return false + } + return true +} + // keyedScopeFromRequest parses the parts every scoped request names: a key // that exists in the contract and is remote, plus an explicit scope. func (h *SettingValuesHandler) keyedScopeFromRequest( @@ -1073,19 +1221,9 @@ func (h *SettingValuesHandler) observedLanguageSuggestions( if h.languageSource == nil { return result } - - wantsMetadata, wantsAudio, wantsSubtitles := false, false, false - for _, eff := range resolved { - switch eff.Key { - case settingskeys.CatalogMetadataLanguage: - wantsMetadata = true - case settingskeys.PlaybackAudioLanguage: - wantsAudio = true - case settingskeys.PlaybackSubtitleLanguage: - wantsSubtitles = true - } - } - if !wantsMetadata && !wantsAudio && !wantsSubtitles { + if !slices.ContainsFunc(resolved, func(eff settingsresolve.Effective) bool { + return eff.Key == settingskeys.CatalogMetadataLanguage + }) { return result } @@ -1095,33 +1233,13 @@ func (h *SettingValuesHandler) observedLanguageSuggestions( filters.DisabledLibraryIDs = scope.DisabledLibraryIDs filters.MaxContentRating = scope.MaxContentRating } - if wantsMetadata { - values, err := h.languageSource.ListOriginalLanguages(r.Context(), filters) - if err != nil { - slog.WarnContext(r.Context(), "settings: listing metadata language suggestions", - "component", "settings", "error", err) - } else { - result[settingskeys.CatalogMetadataLanguage] = values - } - } - if wantsAudio { - values, err := h.languageSource.ListAudioLanguages(r.Context(), filters) - if err != nil { - slog.WarnContext(r.Context(), "settings: listing audio language suggestions", - "component", "settings", "error", err) - } else { - result[settingskeys.PlaybackAudioLanguage] = values - } - } - if wantsSubtitles { - values, err := h.languageSource.ListSubtitleLanguages(r.Context(), filters) - if err != nil { - slog.WarnContext(r.Context(), "settings: listing subtitle language suggestions", - "component", "settings", "error", err) - } else { - result[settingskeys.PlaybackSubtitleLanguage] = values - } + values, err := h.languageSource.ListOriginalLanguages(r.Context(), filters) + if err != nil { + slog.WarnContext(r.Context(), "settings: listing metadata language suggestions", + "component", "settings", "error", err) + return result } + result[settingskeys.CatalogMetadataLanguage] = values return result } diff --git a/internal/api/handlers/settings_values_test.go b/internal/api/handlers/settings_values_test.go index 0ef30367..56b04eee 100644 --- a/internal/api/handlers/settings_values_test.go +++ b/internal/api/handlers/settings_values_test.go @@ -6,6 +6,7 @@ import ( "database/sql" "encoding/json" "errors" + "log/slog" "net/http" "net/http/httptest" "slices" @@ -831,6 +832,537 @@ func TestCapabilitiesReportTheContractRevision(t *testing.T) { } } +// storedDeviceIDFor reports the device a profile_device row was written for, or +// "" when the key has no device-scoped row at all. The device-widening tests +// assert on stored rows rather than status codes: before the query parameter is +// honored a named device is silently ignored and the write lands on the header +// device, which is a 200 either way. +func storedDeviceIDFor(t *testing.T, store userstore.UserStore, key string) string { + t.Helper() + values, err := store.ListAllSettingValues(context.Background()) + if err != nil { + t.Fatalf("listing stored values: %v", err) + } + for _, value := range values { + if value.Key == key && value.Scope == settingscontract.ScopeProfileDevice { + return value.DeviceID + } + } + return "" +} + +func TestSetValue_RejectsDeviceNotOwnedByCaller(t *testing.T) { + handler, store := newValuesTestHandler(t) + + registry, ok := store.(userstore.DeviceRegistry) + if !ok { + t.Fatal("store does not implement DeviceRegistry") + } + if err := registry.RegisterDevice(context.Background(), userstore.DeviceEntry{ + ProfileID: "profile-1", DeviceID: "device-1", DeviceName: "Laptop", + }); err != nil { + t.Fatalf("registering caller device: %v", err) + } + + rec := routeValues(t, handler, http.MethodPut, "player.hdr_enabled", + "scope=profile_device&device_id=device-someone-else", []byte(`{"value":false}`)) + + // 404 rather than 403: a 403 would confirm the device id exists. + if rec.Code != http.StatusNotFound { + t.Errorf("PUT naming an unknown device = %d, want 404: %s", rec.Code, rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "" { + t.Fatalf("wrote a row for device %q; want no write", got) + } +} + +func TestSetValue_WritesNamedDevice(t *testing.T) { + handler, store := newValuesTestHandler(t) + + registry := store.(userstore.DeviceRegistry) + for _, id := range []string{"device-1", "device-b"} { + if err := registry.RegisterDevice(context.Background(), userstore.DeviceEntry{ + ProfileID: "profile-1", DeviceID: id, + }); err != nil { + t.Fatalf("registering %s: %v", id, err) + } + } + + // The request's own header is device-1; the query names device-b. + rec := routeValues(t, handler, http.MethodPut, "player.hdr_enabled", + "scope=profile_device&device_id=device-b", []byte(`{"value":false}`)) + if rec.Code != http.StatusOK { + t.Fatalf("PUT = %d: %s", rec.Code, rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "device-b" { + t.Errorf("stored on device %q, want device-b", got) + } +} + +func TestGetValue_ReadsNamedDevice(t *testing.T) { + handler, store := newValuesTestHandler(t) + + registry := store.(userstore.DeviceRegistry) + if err := registry.RegisterDevice(context.Background(), userstore.DeviceEntry{ + ProfileID: "profile-1", DeviceID: "device-b", + }); err != nil { + t.Fatalf("registering device-b: %v", err) + } + if _, err := store.UpsertSettingValue(context.Background(), userstore.SettingIdentity{ + Key: "player.hdr_enabled", + Scope: settingscontract.ScopeProfileDevice, + ProfileID: "profile-1", DeviceID: "device-b", + }, json.RawMessage(`false`)); err != nil { + t.Fatalf("seeding device-b value: %v", err) + } + + rec := routeValues(t, handler, http.MethodGet, "player.hdr_enabled", + "scope=profile_device&device_id=device-b", nil) + if rec.Code != http.StatusOK { + t.Fatalf("GET named device = %d: %s", rec.Code, rec.Body.String()) + } + var got settingValueResponse + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decoding: %v", err) + } + if got.DeviceID != "device-b" || string(got.Value) != "false" { + t.Errorf("read %s on %q, want false on device-b", got.Value, got.DeviceID) + } + + // The caller's own device has no row, so it must still 404. + if rec := routeValues(t, handler, http.MethodGet, "player.hdr_enabled", + "scope=profile_device", nil); rec.Code != http.StatusNotFound { + t.Errorf("GET own device = %d, want 404", rec.Code) + } +} + +// The regression guard for every existing client: with no device_id in the +// query the header device is used, exactly as before this parameter existed. +func TestSetValue_FallsBackToHeaderDevice(t *testing.T) { + handler, store := newValuesTestHandler(t) + + rec := routeValues(t, handler, http.MethodPut, "player.hdr_enabled", + "scope=profile_device", []byte(`{"value":false}`)) + if rec.Code != http.StatusOK { + t.Fatalf("PUT = %d: %s", rec.Code, rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "device-1" { + t.Errorf("stored on device %q, want the header device device-1", got) + } +} + +func TestDeleteValue_RejectsDeviceNotOwnedByCaller(t *testing.T) { + handler, _ := newValuesTestHandler(t) + + rec := routeValues(t, handler, http.MethodDelete, "player.hdr_enabled", + "scope=profile_device&device_id=device-someone-else", nil) + if rec.Code != http.StatusNotFound { + t.Errorf("DELETE naming an unknown device = %d, want 404", rec.Code) + } +} + +// --- Household widening: a primary profile addressing a sibling profile --- + +// newHouseholdValuesHandler builds a handler with two profiles on one account: +// "profile-1" is the household parent, "profile-2" is another member. +func newHouseholdValuesHandler(t *testing.T, pin string) (*SettingValuesHandler, userstore.UserStore) { + t.Helper() + handler, store := newValuesTestHandler(t) + + // profile-1 is already the household parent: is_primary is assigned to the + // first profile an account creates and is not settable afterwards. + ctx := context.Background() + if err := store.CreateProfile(ctx, userstore.Profile{ID: "profile-2", Name: "Robin"}); err != nil { + t.Fatalf("create sibling: %v", err) + } + primary, err := store.GetProfile(ctx, "profile-1") + if err != nil || primary == nil || !primary.IsPrimary { + t.Fatalf("profile-1 is not the primary profile (%+v, %v)", primary, err) + } + if pin != "" { + if err := store.UpdateProfile(ctx, "profile-1", userstore.UpdateProfileInput{ + PIN: &pin, + }); err != nil { + t.Fatalf("set pin: %v", err) + } + } + + registry := store.(userstore.DeviceRegistry) + if err := registry.RegisterDevice(ctx, userstore.DeviceEntry{ + ProfileID: "profile-2", DeviceID: "robin-ipad", DeviceName: "Robin's iPad", + }); err != nil { + t.Fatalf("registering sibling device: %v", err) + } + + handler.UserRepo = stubUserRepo{user: &models.User{ID: 1}} + handler.ProfileTokens = access.NewProfileTokenService("test-secret-value-at-least-32-chars", 0) + return handler, store +} + +// routeValuesAs is routeValues with an explicit acting profile, so a test can +// call as a non-primary member of the same household. +func routeValuesAs( + t *testing.T, h *SettingValuesHandler, actingProfileID, method, key, query string, body []byte, +) *httptest.ResponseRecorder { + t.Helper() + target := "/settings/values/" + key + if query != "" { + target += "?" + query + } + req := valuesRequest(method, target, body) + req = req.WithContext(apimw.SetProfileID(req.Context(), actingProfileID)) + + routeCtx := chi.NewRouteContext() + routeCtx.URLParams.Add("key", key) + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, routeCtx)) + + rec := httptest.NewRecorder() + switch method { + case http.MethodGet: + h.HandleGetValue(rec, req) + case http.MethodPut: + h.HandleSetValue(rec, req) + case http.MethodDelete: + h.HandleDeleteValue(rec, req) + } + return rec +} + +func TestSetValue_NonPrimaryCannotNameSiblingProfile(t *testing.T) { + handler, store := newHouseholdValuesHandler(t, "") + + rec := routeValuesAs(t, handler, "profile-2", http.MethodPut, "player.hdr_enabled", + "scope=profile_device&profile_id=profile-1&device_id=device-1", []byte(`{"value":false}`)) + + if rec.Code != http.StatusForbidden { + t.Fatalf("non-primary naming a sibling = %d, want 403: %s", rec.Code, rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "" { + t.Errorf("wrote a row on device %q; want no write", got) + } +} + +func TestSetValue_PrimaryWithUnverifiedPINCannotNameSibling(t *testing.T) { + handler, store := newHouseholdValuesHandler(t, "1234") + + rec := routeValuesAs(t, handler, "profile-1", http.MethodPut, "player.hdr_enabled", + "scope=profile_device&profile_id=profile-2&device_id=robin-ipad", []byte(`{"value":false}`)) + + if rec.Code != http.StatusForbidden { + t.Fatalf("primary with unverified PIN = %d, want 403: %s", rec.Code, rec.Body.String()) + } + if !strings.Contains(strings.ToLower(rec.Body.String()), "pin") { + t.Errorf("error does not mention the PIN: %s", rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "" { + t.Errorf("wrote a row on device %q; want no write", got) + } +} + +// A profile id that is not on this account resolves out of the caller's own +// store, so it is simply absent — 404, and the caller learns nothing about +// whether it exists elsewhere. +func TestSetValue_ProfileFromAnotherAccountIsNotFound(t *testing.T) { + handler, store := newHouseholdValuesHandler(t, "") + + rec := routeValuesAs(t, handler, "profile-1", http.MethodPut, "player.hdr_enabled", + "scope=profile_device&profile_id=someone-elses-profile&device_id=device-1", + []byte(`{"value":false}`)) + + if rec.Code != http.StatusNotFound { + t.Fatalf("foreign profile = %d, want 404: %s", rec.Code, rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "" { + t.Errorf("wrote a row on device %q; want no write", got) + } +} + +func TestSetValue_PrimaryWritesSiblingProfileDeviceSetting(t *testing.T) { + handler, store := newHouseholdValuesHandler(t, "") + + rec := routeValuesAs(t, handler, "profile-1", http.MethodPut, "playback.subtitle_mode", + "scope=profile_device&profile_id=profile-2&device_id=robin-ipad", []byte(`{"value":"always"}`)) + if rec.Code != http.StatusOK { + t.Fatalf("primary writing a sibling = %d: %s", rec.Code, rec.Body.String()) + } + + values, err := store.ListAllSettingValues(context.Background()) + if err != nil { + t.Fatalf("listing: %v", err) + } + var found bool + for _, value := range values { + if value.Key != "playback.subtitle_mode" { + continue + } + found = true + if value.ProfileID != "profile-2" || value.DeviceID != "robin-ipad" { + t.Errorf("stored on (%s, %s), want (profile-2, robin-ipad)", + value.ProfileID, value.DeviceID) + } + } + if !found { + t.Error("no row stored for the sibling profile") + } +} + +// A device belonging to a different profile than the one being addressed is +// still rejected: the household widening changes who you may act for, not +// which devices belong to whom. +func TestSetValue_PrimaryCannotMixSiblingProfileWithForeignDevice(t *testing.T) { + handler, _ := newHouseholdValuesHandler(t, "") + + rec := routeValuesAs(t, handler, "profile-1", http.MethodPut, "player.hdr_enabled", + "scope=profile_device&profile_id=profile-2&device_id=not-robins-device", + []byte(`{"value":false}`)) + if rec.Code != http.StatusNotFound { + t.Fatalf("sibling profile with a foreign device = %d, want 404: %s", + rec.Code, rec.Body.String()) + } +} + +// Naming your own profile explicitly is not a household action and must work +// for anyone — it is what a client does when it sends the identity it read back. +func TestSetValue_NamingOwnProfileIsAllowedForAnyone(t *testing.T) { + handler, store := newHouseholdValuesHandler(t, "") + + rec := routeValuesAs(t, handler, "profile-2", http.MethodPut, "player.hdr_enabled", + "scope=profile_device&profile_id=profile-2&device_id=robin-ipad", []byte(`{"value":false}`)) + if rec.Code != http.StatusOK { + t.Fatalf("naming own profile = %d: %s", rec.Code, rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "robin-ipad" { + t.Errorf("stored on device %q, want robin-ipad", got) + } +} + +// Registration means "this device is in use by this profile". A write aimed at +// some *other* device, or made on another profile's behalf, is not that — and +// registering the actor's browser under the target profile would invent a +// device nobody is holding. +func TestSetValue_DoesNotRegisterWhenActingOnAnotherDevice(t *testing.T) { + handler, store := newValuesTestHandler(t) + + registry := store.(userstore.DeviceRegistry) + if err := registry.RegisterDevice(context.Background(), userstore.DeviceEntry{ + ProfileID: "profile-1", DeviceID: "apple-tv", DeviceName: "Apple TV", + }); err != nil { + t.Fatalf("registering apple-tv: %v", err) + } + + // Header device is device-1; the write targets apple-tv. + if rec := routeValues(t, handler, http.MethodPut, "player.hdr_enabled", + "scope=profile_device&device_id=apple-tv", []byte(`{"value":false}`)); rec.Code != http.StatusOK { + t.Fatalf("PUT = %d: %s", rec.Code, rec.Body.String()) + } + + exists, err := registry.DeviceExists(context.Background(), "profile-1", "device-1") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if exists { + t.Error("registered the acting device while writing to a different device") + } +} + +func TestSetValue_DoesNotRegisterActorsDeviceUnderAnotherProfile(t *testing.T) { + handler, store := newHouseholdValuesHandler(t, "") + + if rec := routeValuesAs(t, handler, "profile-1", http.MethodPut, "player.hdr_enabled", + "scope=profile_device&profile_id=profile-2&device_id=robin-ipad", + []byte(`{"value":false}`)); rec.Code != http.StatusOK { + t.Fatalf("PUT = %d: %s", rec.Code, rec.Body.String()) + } + + registry := store.(userstore.DeviceRegistry) + exists, err := registry.DeviceExists(context.Background(), "profile-2", "device-1") + if err != nil { + t.Fatalf("DeviceExists: %v", err) + } + if exists { + t.Error("registered the parent's browser under the child's profile") + } +} + +// captureAuditLogs swaps the default slog handler for the duration of a test. +// The returned function parses whatever has been emitted so far and keeps only +// the settings-audit records. +func captureAuditLogs(t *testing.T) func() []map[string]any { + t.Helper() + var buf bytes.Buffer + previous := slog.Default() + slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo}))) + t.Cleanup(func() { slog.SetDefault(previous) }) + + return func() []map[string]any { + var records []map[string]any + for _, line := range strings.Split(strings.TrimSpace(buf.String()), "\n") { + if line == "" { + continue + } + var record map[string]any + if err := json.Unmarshal([]byte(line), &record); err != nil { + continue + } + if record["msg"] == settingsAuditMsg { + records = append(records, record) + } + } + return records + } +} + +func TestSetValue_AuditsCrossProfileWrite(t *testing.T) { + handler, _ := newHouseholdValuesHandler(t, "") + audited := captureAuditLogs(t) + + if rec := routeValuesAs(t, handler, "profile-1", http.MethodPut, "playback.subtitle_mode", + "scope=profile_device&profile_id=profile-2&device_id=robin-ipad", + []byte(`{"value":"always"}`)); rec.Code != http.StatusOK { + t.Fatalf("PUT = %d: %s", rec.Code, rec.Body.String()) + } + + records := audited() + if len(records) != 1 { + t.Fatalf("emitted %d audit records, want 1: %+v", len(records), records) + } + record := records[0] + if record["actor_profile_id"] != "profile-1" || record["target_profile_id"] != "profile-2" { + t.Errorf("actor/target = %v/%v, want profile-1/profile-2", + record["actor_profile_id"], record["target_profile_id"]) + } + if record["setting_key"] != "playback.subtitle_mode" || record["device_id"] != "robin-ipad" { + t.Errorf("key/device = %v/%v", record["setting_key"], record["device_id"]) + } + // Identity only: the value must never reach an operator's log. + for key, value := range record { + if text, ok := value.(string); ok && text == "always" { + t.Errorf("audit record leaked the value under %q", key) + } + } +} + +// Ordinary self-service writes stay out of the trail. A record of everything +// answers nothing, and the question this exists for is "who changed it for me". +func TestSetValue_DoesNotAuditOwnWrite(t *testing.T) { + handler, _ := newHouseholdValuesHandler(t, "") + audited := captureAuditLogs(t) + + if rec := routeValuesAs(t, handler, "profile-1", http.MethodPut, "playback.subtitle_mode", + "scope=profile", []byte(`{"value":"always"}`)); rec.Code != http.StatusOK { + t.Fatalf("PUT = %d: %s", rec.Code, rec.Body.String()) + } + + if records := audited(); len(records) != 0 { + t.Errorf("audited an ordinary self-service write: %+v", records) + } +} + +func TestGetEffective_ResolvesNamedDevice(t *testing.T) { + handler, store := newValuesTestHandler(t) + + registry := store.(userstore.DeviceRegistry) + if err := registry.RegisterDevice(context.Background(), userstore.DeviceEntry{ + ProfileID: "profile-1", DeviceID: "apple-tv", + }); err != nil { + t.Fatalf("registering apple-tv: %v", err) + } + // The Apple TV overrides subtitle mode; this browser does not. + if _, err := store.UpsertSettingValue(context.Background(), userstore.SettingIdentity{ + Key: "playback.subtitle_mode", + Scope: settingscontract.ScopeProfileDevice, + ProfileID: "profile-1", DeviceID: "apple-tv", + }, json.RawMessage(`"always"`)); err != nil { + t.Fatalf("seeding: %v", err) + } + + read := func(query string) map[string]any { + t.Helper() + req := valuesRequest(http.MethodGet, "/settings/values/effective?"+query, nil) + rec := httptest.NewRecorder() + handler.HandleGetEffective(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("GET effective?%s = %d: %s", query, rec.Code, rec.Body.String()) + } + var body struct { + Settings []map[string]any `json:"settings"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decoding: %v", err) + } + if len(body.Settings) != 1 { + t.Fatalf("resolved %d settings, want 1", len(body.Settings)) + } + return body.Settings[0] + } + + named := read("keys=playback.subtitle_mode&device_id=apple-tv") + if named["value"] != "always" || named["source"] != "profile_device" { + t.Errorf("named device resolved %v from %v, want always from profile_device", + named["value"], named["source"]) + } + + // Without the parameter this browser still resolves its own answer. + own := read("keys=playback.subtitle_mode") + if own["source"] == "profile_device" { + t.Errorf("this browser resolved a device override it does not have: %v", own) + } +} + +func TestGetEffective_RejectsDeviceNotOwnedByCaller(t *testing.T) { + handler, _ := newValuesTestHandler(t) + + req := valuesRequest(http.MethodGet, + "/settings/values/effective?keys=playback.subtitle_mode&device_id=not-mine", nil) + rec := httptest.NewRecorder() + handler.HandleGetEffective(rec, req) + if rec.Code != http.StatusNotFound { + t.Fatalf("effective read of a foreign device = %d, want 404: %s", rec.Code, rec.Body.String()) + } +} + +func TestGetEffective_NonPrimaryCannotResolveSiblingProfile(t *testing.T) { + handler, _ := newHouseholdValuesHandler(t, "") + + req := valuesRequest(http.MethodGet, + "/settings/values/effective?keys=playback.subtitle_mode&profile_id=profile-1", nil) + req = req.WithContext(apimw.SetProfileID(req.Context(), "profile-2")) + rec := httptest.NewRecorder() + handler.HandleGetEffective(rec, req) + if rec.Code != http.StatusForbidden { + t.Fatalf("non-primary effective read of a sibling = %d, want 403: %s", + rec.Code, rec.Body.String()) + } +} + +// A server admin may act for any profile, including from a non-primary active +// profile: apimw.IsAdmin short-circuits the household check by design. Pinned +// because it is easy to mistake for the non-primary refusal above — the +// difference is the account's role, not the profile's. +func TestSetValue_ServerAdminMayNameAnyProfile(t *testing.T) { + handler, store := newHouseholdValuesHandler(t, "") + + target := "/settings/values/player.hdr_enabled" + + "?scope=profile_device&profile_id=profile-2&device_id=robin-ipad" + req := valuesRequest(http.MethodPut, target, []byte(`{"value":false}`)) + // Acting as the *non-primary* profile, but on an admin account. + ctx := apimw.SetClaims(req.Context(), &auth.Claims{UserID: 1, Role: "admin"}) + req = req.WithContext(apimw.SetProfileID(ctx, "profile-2")) + + routeCtx := chi.NewRouteContext() + routeCtx.URLParams.Add("key", "player.hdr_enabled") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, routeCtx)) + + rec := httptest.NewRecorder() + handler.HandleSetValue(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("admin naming a profile = %d, want 200: %s", rec.Code, rec.Body.String()) + } + if got := storedDeviceIDFor(t, store, "player.hdr_enabled"); got != "robin-ipad" { + t.Errorf("stored on device %q, want robin-ipad", got) + } +} + func TestMergeLanguageSuggestionsKeepsFloorObservedAndCurrent(t *testing.T) { got := mergeLanguageSuggestions( []string{"en", "fr", "pt"}, @@ -858,27 +1390,17 @@ func TestMergeLanguageSuggestionsUsesExactCurrentAlias(t *testing.T) { type recordingLanguageSuggestionSource struct { filters catalog.BrowseFilters original []string + calls int } func (s *recordingLanguageSuggestionSource) ListOriginalLanguages( _ context.Context, filters catalog.BrowseFilters, ) ([]string, error) { s.filters = filters + s.calls++ return s.original, nil } -func (*recordingLanguageSuggestionSource) ListAudioLanguages( - context.Context, catalog.BrowseFilters, -) ([]string, error) { - return nil, nil -} - -func (*recordingLanguageSuggestionSource) ListSubtitleLanguages( - context.Context, catalog.BrowseFilters, -) ([]string, error) { - return nil, nil -} - func TestObservedLanguageSuggestionsIncludesAccessibleOriginalLanguages(t *testing.T) { source := &recordingLanguageSuggestionSource{original: []string{"is", "no"}} handler, _ := newValuesTestHandler(t) @@ -903,3 +1425,27 @@ func TestObservedLanguageSuggestionsIncludesAccessibleOriginalLanguages(t *testi t.Fatalf("catalog filters = %#v", source.filters) } } + +// TestObservedLanguageSuggestionsSkipsPlaybackKeys pins the design decision +// that only catalog.metadata_language gets deployment-observed suggestions. +// The audio/subtitle track listings walk every media file — tens of seconds +// on large catalogs — so those pickers ship the contract floor and clients +// offer free entry for anything beyond it. +func TestObservedLanguageSuggestionsSkipsPlaybackKeys(t *testing.T) { + source := &recordingLanguageSuggestionSource{original: []string{"is"}} + handler, _ := newValuesTestHandler(t) + handler.SetLanguageSuggestionSource(source) + + req := valuesRequest(http.MethodGet, "/settings/values/effective", nil) + observed := handler.observedLanguageSuggestions(req, []settingsresolve.Effective{ + {Key: settingskeys.PlaybackAudioLanguage}, + {Key: settingskeys.PlaybackSubtitleLanguage}, + }) + + if len(observed) != 0 { + t.Fatalf("observed suggestions for playback keys = %v, want none", observed) + } + if source.calls != 0 { + t.Fatalf("catalog scans = %d, want 0 for playback-only requests", source.calls) + } +} diff --git a/internal/api/router.go b/internal/api/router.go index e530e32b..9bba5311 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -795,6 +795,7 @@ func NewRouter(deps Dependencies) chi.Router { var collectionHandler *handlers.CollectionHandler var settingsHandler *handlers.SettingsHandler var settingValuesHandler *handlers.SettingValuesHandler + var deviceHandler *handlers.DeviceHandler var homeDismissalHandler *handlers.HomeDismissalHandler var subtitlePrefHandler *handlers.SubtitlePrefHandler var audioPrefHandler *handlers.AudioPrefHandler @@ -853,6 +854,13 @@ func NewRouter(deps Dependencies) chi.Router { if contract, err := settingscontract.Load(); err == nil { settingValuesHandler = handlers.NewSettingValuesHandler(deps.UserStoreProvider, contract) settingValuesHandler.EventsHub = deps.EventsHub + // Household management: a primary profile acting for another + // profile on its own account. Without both of these the widening + // is unavailable rather than unguarded. + if userRepo != nil { + settingValuesHandler.UserRepo = userRepo + } + settingValuesHandler.ProfileTokens = profileTokenService if deps.FolderRepo != nil { settingValuesHandler.SetLibraryLookup(deps.FolderRepo) } else if deps.DB != nil { @@ -862,6 +870,12 @@ func NewRouter(deps Dependencies) chi.Router { settingValuesHandler.SetLanguageSuggestionSource(catalog.NewBrowseRepository(deps.DB)) } } + deviceHandler = handlers.NewDeviceHandler(deps.UserStoreProvider) + deviceHandler.EventsHub = deps.EventsHub + if userRepo != nil { + deviceHandler.UserRepo = userRepo + } + deviceHandler.ProfileTokens = profileTokenService homeDismissalHandler = handlers.NewHomeDismissalHandler(deps.UserStoreProvider) homeDismissalHandler.EventsHub = deps.EventsHub subtitlePrefHandler = handlers.NewSubtitlePrefHandler(deps.UserStoreProvider) @@ -2166,6 +2180,19 @@ func NewRouter(deps Dependencies) chi.Router { }) } + // The viewer's own device registry. Distinct from the push + // device routes under /notifications and from the TV login + // pairing flow under /auth/device: this is the installation + // identity that carries device-scoped settings. + if deviceHandler != nil { + r.Route("/devices", func(r chi.Router) { + r.Use(apimw.RequireProfile) + r.Get("/", deviceHandler.HandleListDevices) + r.Delete("/{device_id}", deviceHandler.HandleForgetDevice) + r.Delete("/{device_id}/settings", deviceHandler.HandleClearDeviceSettings) + }) + } + // Favorites, watchlist, and history routes (profile-scoped). if personalDataHandler != nil && itemsHandler != nil { r.Route("/watched", func(r chi.Router) { diff --git a/internal/catalog/browse.go b/internal/catalog/browse.go index 2cbe77c7..41b06d3e 100644 --- a/internal/catalog/browse.go +++ b/internal/catalog/browse.go @@ -865,19 +865,24 @@ func listSubtitleLanguagesWithSource( // `subtitle_language_codes`; external subs still need a JSONB unnest // because the generated column only covers subtitle_tracks // (audit 2026-05-01 §2.5b). + // + // Each arm applies its own DISTINCT before the UNION ALL so the merge + // only sees the handful of distinct language codes per arm. A plain + // UNION here forces one deduplication pass across every unnested track + // row — millions of rows on a large catalog. query := fmt.Sprintf(` SELECT DISTINCT value FROM ( - SELECT lang AS value + SELECT DISTINCT lang AS value FROM %s JOIN media_files mf ON %s CROSS JOIN LATERAL UNNEST(mf.subtitle_language_codes) AS lang %s AND mf.missing_since IS NULL - UNION + UNION ALL - SELECT LOWER(COALESCE(track->>'language', '')) AS value + SELECT DISTINCT LOWER(COALESCE(track->>'language', '')) AS value FROM %s JOIN media_files mf ON %s CROSS JOIN LATERAL jsonb_array_elements(COALESCE(mf.external_subtitles, '[]'::jsonb)) AS track @@ -886,7 +891,8 @@ func listSubtitleLanguagesWithSource( ) languages WHERE value IS NOT NULL AND value <> '' ORDER BY value ASC - `, fromClause, mediaFileJoin, browseFilterPrefix(whereClause), fromClause, mediaFileJoin, browseFilterPrefix(whereClause)) + LIMIT %d + `, fromClause, mediaFileJoin, browseFilterPrefix(whereClause), fromClause, mediaFileJoin, browseFilterPrefix(whereClause), catalogFacetMaxValues) return queryDistinctStrings(ctx, pool, query, args) } diff --git a/internal/userdb/settings.go b/internal/userdb/settings.go index e241671a..c05f678f 100644 --- a/internal/userdb/settings.go +++ b/internal/userdb/settings.go @@ -131,6 +131,39 @@ func ListDevices(db *sql.DB) ([]userstore.DeviceEntry, error) { return entries, rows.Err() } +// DeviceExists carries no user_id predicate: this backend keeps one database +// per user, so the table is already scoped to the account. +func DeviceExists(db *sql.DB, profileID, deviceID string) (bool, error) { + if strings.TrimSpace(profileID) == "" || strings.TrimSpace(deviceID) == "" { + return false, nil + } + var exists bool + err := db.QueryRow( + `SELECT EXISTS( + SELECT 1 FROM user_devices + WHERE profile_id = ? AND device_id = ?)`, + profileID, deviceID, + ).Scan(&exists) + if err != nil { + return false, fmt.Errorf("checking device %q: %w", deviceID, err) + } + return exists, nil +} + +func ForgetDevice(db *sql.DB, profileID, deviceID string) error { + if strings.TrimSpace(profileID) == "" || strings.TrimSpace(deviceID) == "" { + return nil + } + _, err := db.Exec( + `DELETE FROM user_devices WHERE profile_id = ? AND device_id = ?`, + profileID, deviceID, + ) + if err != nil { + return fmt.Errorf("forgetting device %q: %w", deviceID, err) + } + return nil +} + func SetDeviceSetting(db *sql.DB, entry userstore.DeviceSettingEntry) error { if err := RegisterDevice(db, userstore.DeviceEntry{ ProfileID: entry.ProfileID, diff --git a/internal/userdb/sqlitestore.go b/internal/userdb/sqlitestore.go index 2aaba05b..29f6d5d6 100644 --- a/internal/userdb/sqlitestore.go +++ b/internal/userdb/sqlitestore.go @@ -346,6 +346,14 @@ func (s *SQLiteUserStore) ListDevices(_ context.Context) ([]userstore.DeviceEntr return ListDevices(s.db) } +func (s *SQLiteUserStore) DeviceExists(_ context.Context, profileID, deviceID string) (bool, error) { + return DeviceExists(s.db, profileID, deviceID) +} + +func (s *SQLiteUserStore) ForgetDevice(_ context.Context, profileID, deviceID string) error { + return ForgetDevice(s.db, profileID, deviceID) +} + func (s *SQLiteUserStore) SetDeviceSetting(_ context.Context, entry userstore.DeviceSettingEntry) error { return SetDeviceSetting(s.db, entry) } diff --git a/internal/userstore/pgstore/settings.go b/internal/userstore/pgstore/settings.go index 38d7cfac..f3a5113c 100644 --- a/internal/userstore/pgstore/settings.go +++ b/internal/userstore/pgstore/settings.go @@ -136,6 +136,37 @@ func (s *PostgresUserStore) ListDevices(ctx context.Context) ([]userstore.Device return entries, rows.Err() } +func (s *PostgresUserStore) DeviceExists(ctx context.Context, profileID, deviceID string) (bool, error) { + if strings.TrimSpace(profileID) == "" || strings.TrimSpace(deviceID) == "" { + return false, nil + } + var exists bool + if err := s.pool.QueryRow(ctx, + `SELECT EXISTS( + SELECT 1 FROM user_devices + WHERE user_id = $1 AND profile_id = $2 AND device_id = $3)`, + s.userID, profileID, deviceID, + ).Scan(&exists); err != nil { + return false, fmt.Errorf("checking device %q: %w", deviceID, err) + } + return exists, nil +} + +func (s *PostgresUserStore) ForgetDevice(ctx context.Context, profileID, deviceID string) error { + if strings.TrimSpace(profileID) == "" || strings.TrimSpace(deviceID) == "" { + return nil + } + _, err := s.pool.Exec(ctx, + `DELETE FROM user_devices + WHERE user_id = $1 AND profile_id = $2 AND device_id = $3`, + s.userID, profileID, deviceID, + ) + if err != nil { + return fmt.Errorf("forgetting device %q: %w", deviceID, err) + } + return nil +} + func (s *PostgresUserStore) SetDeviceSetting(ctx context.Context, entry userstore.DeviceSettingEntry) error { if err := s.RegisterDevice(ctx, userstore.DeviceEntry{ ProfileID: entry.ProfileID, diff --git a/internal/userstore/store.go b/internal/userstore/store.go index e159655b..1750d147 100644 --- a/internal/userstore/store.go +++ b/internal/userstore/store.go @@ -234,4 +234,13 @@ type UserStore interface { type DeviceRegistry interface { RegisterDevice(ctx context.Context, entry DeviceEntry) error ListDevices(ctx context.Context) ([]DeviceEntry, error) + // DeviceExists reports whether one device is registered to one profile. It + // exists so a write naming a device can be authorized without scanning the + // whole account's registry: ListDevices is account-wide by construction, so + // filtering its result per write would read every household member's rows. + DeviceExists(ctx context.Context, profileID, deviceID string) (bool, error) + // ForgetDevice removes one profile's registry row for a device. Settings + // are deleted separately through the scoped setting deletes, so forgetting + // a device shared by two profiles leaves the other profile's row intact. + ForgetDevice(ctx context.Context, profileID, deviceID string) error } diff --git a/internal/userstore/storetest/settingvalues.go b/internal/userstore/storetest/settingvalues.go index 9dc4502e..0d30e84a 100644 --- a/internal/userstore/storetest/settingvalues.go +++ b/internal/userstore/storetest/settingvalues.go @@ -49,6 +49,54 @@ func RunSettingValues(t *testing.T, newStore func(t *testing.T) userstore.UserSt t.Run("MutationIdempotency", func(t *testing.T) { testSettingMutationIdempotency(t, newStore) }) + t.Run("DeviceExists", func(t *testing.T) { + testDeviceExists(t, newStore) + }) +} + +// testDeviceExists pins the check that authorizes a device named in a request +// rather than taken from its headers. It must be scoped to the profile, not +// merely to the account: two profiles in one household register the same TV +// separately, and a device is only "yours" for the profile that registered it. +func testDeviceExists(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + registry, ok := store.(userstore.DeviceRegistry) + if !ok { + t.Skip("store does not implement DeviceRegistry") + } + + seedSettingProfiles(t, ctx, store, "p1", "p2") + if err := registry.RegisterDevice(ctx, userstore.DeviceEntry{ + ProfileID: "p1", DeviceID: deviceApple, DeviceName: "Apple TV", + }); err != nil { + t.Fatalf("RegisterDevice: %v", err) + } + + cases := []struct { + name string + profileID string + deviceID string + want bool + }{ + {"registered", "p1", deviceApple, true}, + {"other profile same device", "p2", deviceApple, false}, + {"unknown device", "p1", "no-such-device", false}, + {"empty device", "p1", "", false}, + {"empty profile", "", deviceApple, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := registry.DeviceExists(ctx, tc.profileID, tc.deviceID) + if err != nil { + t.Fatalf("DeviceExists(%q, %q): %v", tc.profileID, tc.deviceID, err) + } + if got != tc.want { + t.Errorf("DeviceExists(%q, %q) = %v, want %v", + tc.profileID, tc.deviceID, got, tc.want) + } + }) + } } // testPreferenceSettingsTransactionRollback proves that a failure after both diff --git a/web/src/App.tsx b/web/src/App.tsx index 08254c0d..9673c6d1 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -41,6 +41,7 @@ import PersonDetail from "@/pages/PersonDetail"; import Collections from "@/pages/Collections"; import CollectionEditor from "@/pages/CollectionEditor"; import Notifications from "@/pages/Notifications"; +import DeviceSettings from "@/pages/settings/DeviceSettings"; import NotificationsSettings from "@/pages/settings/NotificationsSettings"; import Requests from "@/pages/Requests"; import RequestBrowse from "@/pages/RequestBrowse"; @@ -489,6 +490,7 @@ function AppRoutes() { } /> } /> } /> + } /> } /> } /> } /> diff --git a/web/src/api/types.ts b/web/src/api/types.ts index d87489f6..374a794a 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -2993,6 +2993,24 @@ export interface AdminDeviceProfileSummary { last_updated: string; } +/** One device the signed-in viewer watches on. */ +export interface UserDevice { + device_id: string; + device_name: string; + device_platform: string; + last_seen_at: string; + profile_id: string; + profile_name: string; + /** True for the device this browser is. */ + is_current_device: boolean; + /** How many settings this (profile, device) pair overrides. */ + changed_count: number; +} + +export interface UserDeviceListResponse { + devices: UserDevice[]; +} + export interface AdminDeviceSummary { user_id: number; username: string; diff --git a/web/src/components/settings/DeviceList.test.tsx b/web/src/components/settings/DeviceList.test.tsx new file mode 100644 index 00000000..2fedb4a6 --- /dev/null +++ b/web/src/components/settings/DeviceList.test.tsx @@ -0,0 +1,359 @@ +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import type { UserDevice } from "@/api/types"; +import { DeviceList, lastSeenLabel } from "@/components/settings/DeviceList"; + +const NOW = Date.parse("2026-07-31T12:00:00Z"); + +function device(overrides: Partial = {}): UserDevice { + return { + device_id: "device-1", + device_name: "Chrome on macOS", + device_platform: "macOS Web", + last_seen_at: new Date(NOW - 60 * 60 * 1000).toISOString(), + profile_id: "profile-1", + profile_name: "Sam", + is_current_device: false, + changed_count: 0, + ...overrides, + }; +} + +function renderList( + devices: UserDevice[], + props: Partial<{ + groupByProfile: boolean; + profileFilter: string | null; + onProfileFilterChange: (id: string | null) => void; + ownProfileId: string; + }> = {}, +) { + const onSelect = vi.fn(); + const onProfileFilterChange = props.onProfileFilterChange ?? vi.fn(); + render( + , + ); + return { onSelect, onProfileFilterChange }; +} + +const HOUSEHOLD: UserDevice[] = [ + device({ device_id: "a", device_name: "Sam's laptop", profile_name: "Sam" }), + device({ device_id: "b", device_name: "Sam's TV", profile_name: "Sam" }), + device({ + device_id: "c", + device_name: "Robin's iPad", + profile_id: "profile-2", + profile_name: "Robin", + }), +]; + +describe("DeviceList", () => { + it("groups by recency and marks the current device", () => { + renderList([ + device({ device_id: "here", device_name: "This browser", is_current_device: true }), + device({ + device_id: "recent", + device_name: "Apple TV", + last_seen_at: new Date(NOW - 2 * 24 * 60 * 60 * 1000).toISOString(), + }), + device({ + device_id: "old", + device_name: "Old iPad", + last_seen_at: new Date(NOW - 60 * 24 * 60 * 60 * 1000).toISOString(), + }), + ]); + + expect(screen.getByText("Using now")).toBeInTheDocument(); + expect(screen.getByText("This week")).toBeInTheDocument(); + expect(screen.getByText("Earlier")).toBeInTheDocument(); + expect(screen.getByLabelText("You're on this device")).toBeInTheDocument(); + }); + + // A device with nothing changed shows a dash, not "0": the list exists to + // answer "which one did I change?" at a glance. + it("shows a dash rather than a zero when nothing is changed", () => { + renderList([ + device({ device_id: "clean", device_name: "Clean", changed_count: 0 }), + device({ device_id: "dirty", device_name: "Dirty", changed_count: 3 }), + ]); + + expect(screen.getByLabelText("Nothing changed")).toHaveTextContent("—"); + expect(screen.getByLabelText("3 settings changed here")).toHaveTextContent("3"); + expect(screen.queryByText("0")).not.toBeInTheDocument(); + }); + + it("selects a device when its row is clicked", async () => { + const { onSelect } = renderList([device({ device_id: "tv", device_name: "Apple TV" })]); + + await userEvent.click(screen.getByRole("button", { name: /Apple TV/ })); + + expect(onSelect).toHaveBeenCalledWith(expect.objectContaining({ device_id: "tv" })); + }); + + it("groups by person in the household view", () => { + renderList( + [ + device({ device_id: "a", device_name: "Sam's laptop", profile_name: "Sam" }), + device({ + device_id: "b", + device_name: "Robin's iPad", + profile_id: "profile-2", + profile_name: "Robin", + }), + ], + { groupByProfile: true }, + ); + + // Scoped to headings: the profile-filter chips carry these names too. + expect(screen.getByRole("heading", { name: "Sam" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Robin" })).toBeInTheDocument(); + expect(screen.queryByText("Using now")).not.toBeInTheDocument(); + }); + + it("stays readable with many devices", () => { + const many = Array.from({ length: 11 }, (_, index) => + device({ + device_id: `device-${index}`, + device_name: `Device ${index}`, + last_seen_at: new Date(NOW - index * 5 * 24 * 60 * 60 * 1000).toISOString(), + }), + ); + renderList(many); + + const list = screen.getAllByRole("listitem"); + expect(list).toHaveLength(11); + expect(screen.getByLabelText("Search devices")).toHaveAttribute( + "placeholder", + "Search 11 devices", + ); + }); + + it("filters by platform as well as name", async () => { + const onSearchChange = vi.fn(); + const { rerender } = render( + , + ); + + rerender( + , + ); + + const items = screen.getAllByRole("listitem"); + expect(items).toHaveLength(1); + const [onlyItem] = items; + expect(within(onlyItem!).getByText("Living Room")).toBeInTheDocument(); + }); +}); + +describe("DeviceList at scale", () => { + const OLD = new Date(NOW - 200 * 24 * 60 * 60 * 1000).toISOString(); + + // A real account carried 260 devices, over half of them one-off sessions + // that had never changed a setting. Listing them all produced a + // thirteen-thousand-pixel page, so the settings never came into view. + function bigFleet(): UserDevice[] { + return [ + device({ device_id: "here", device_name: "This browser", is_current_device: true }), + device({ device_id: "tv", device_name: "Apple TV", changed_count: 5 }), + // Old but configured: still worth showing, since someone set it up. + device({ + device_id: "kept", + device_name: "Old but configured", + last_seen_at: OLD, + changed_count: 2, + }), + ...Array.from({ length: 40 }, (_, i) => + device({ device_id: `junk-${i}`, device_name: `Silo-PR111-build-${i}`, last_seen_at: OLD }), + ), + ]; + } + + it("hides devices nobody has used and that carry no settings", () => { + renderList(bigFleet()); + + expect(screen.getByText("This browser")).toBeInTheDocument(); + expect(screen.getByText("Apple TV")).toBeInTheDocument(); + expect(screen.getByText("Old but configured")).toBeInTheDocument(); + expect(screen.queryByText("Silo-PR111-build-0")).not.toBeInTheDocument(); + expect(screen.getAllByRole("listitem")).toHaveLength(3); + }); + + it("says how many it is holding back, and reveals them on request", async () => { + renderList(bigFleet()); + + const toggle = screen.getByRole("button", { name: "Show 40 unused devices" }); + await userEvent.click(toggle); + + expect(screen.getByText("Silo-PR111-build-0")).toBeInTheDocument(); + expect(screen.getAllByRole("listitem")).toHaveLength(43); + expect(screen.getByRole("button", { name: "Hide unused devices" })).toBeInTheDocument(); + }); + + // Searching means looking for something specific; hiding a device from its + // own name would read as the device having disappeared. + it("searches across hidden devices without expanding them", () => { + render( + , + ); + + expect(screen.getByText("Silo-PR111-build-7")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /unused devices/ })).not.toBeInTheDocument(); + }); + + it("keeps the current device visible even when it is old and unconfigured", () => { + renderList([ + device({ + device_id: "here", + device_name: "This browser", + is_current_device: true, + last_seen_at: OLD, + }), + device({ device_id: "junk", device_name: "Forgotten", last_seen_at: OLD }), + ]); + + expect(screen.getByText("This browser")).toBeInTheDocument(); + expect(screen.queryByText("Forgotten")).not.toBeInTheDocument(); + }); + + it("offers no toggle when nothing is dormant", () => { + renderList([device({ device_id: "a", device_name: "Recent", changed_count: 1 })]); + expect(screen.queryByRole("button", { name: /unused devices/ })).not.toBeInTheDocument(); + }); +}); + +describe("DeviceList profile filter", () => { + it("offers one chip per profile, plus everyone, with counts", () => { + renderList(HOUSEHOLD, { groupByProfile: true }); + + expect(screen.getByRole("button", { name: "Everyone, 3 devices" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Sam, 2 devices" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Robin, 1 device" })).toBeInTheDocument(); + }); + + // At eight profiles, arrival order buried the person actually using the + // screen; the rest sort by name so a chip does not move as devices are used. + it("leads with the viewer's own profile, then sorts by name", () => { + renderList( + [ + device({ device_id: "z", profile_id: "profile-9", profile_name: "Zoe" }), + device({ device_id: "c", profile_id: "profile-3", profile_name: "Casey" }), + device({ device_id: "a", profile_id: "profile-1", profile_name: "Sam" }), + ], + { groupByProfile: true, ownProfileId: "profile-1" }, + ); + + const chips = screen + .getByRole("group", { name: "Filter by profile" }) + .querySelectorAll("button"); + const names = [...chips].map((chip) => chip.getAttribute("aria-label")?.split(",")[0]); + expect(names).toEqual(["Everyone", "Sam", "Casey", "Zoe"]); + }); + + // Someone looking at their own devices has exactly one profile, so a filter + // with a single option would be chrome that explains nothing. + it("stays hidden outside the household view", () => { + renderList(HOUSEHOLD, { groupByProfile: false }); + expect(screen.queryByRole("group", { name: "Filter by profile" })).not.toBeInTheDocument(); + }); + + it("stays hidden when the household has only one profile", () => { + renderList([HOUSEHOLD[0]!, HOUSEHOLD[1]!], { groupByProfile: true }); + expect(screen.queryByRole("group", { name: "Filter by profile" })).not.toBeInTheDocument(); + }); + + it("reports the chosen profile", async () => { + const { onProfileFilterChange } = renderList(HOUSEHOLD, { groupByProfile: true }); + + await userEvent.click(screen.getByRole("button", { name: "Robin, 1 device" })); + + expect(onProfileFilterChange).toHaveBeenCalledWith("profile-2"); + }); + + it("clears the filter when the active chip is clicked again", async () => { + const { onProfileFilterChange } = renderList(HOUSEHOLD, { + groupByProfile: true, + profileFilter: "profile-2", + }); + + await userEvent.click(screen.getByRole("button", { name: "Robin, 1 device" })); + + expect(onProfileFilterChange).toHaveBeenCalledWith(null); + }); + + it("shows only the filtered profile's devices", () => { + renderList(HOUSEHOLD, { groupByProfile: true, profileFilter: "profile-2" }); + + expect(screen.getByText("Robin's iPad")).toBeInTheDocument(); + expect(screen.queryByText("Sam's laptop")).not.toBeInTheDocument(); + expect(screen.getAllByRole("listitem")).toHaveLength(1); + }); + + // With one person selected, a person heading would just repeat the chip. + it("falls back to recency headings once a profile is chosen", () => { + renderList(HOUSEHOLD, { groupByProfile: true, profileFilter: "profile-2" }); + + expect(screen.getByText("This week")).toBeInTheDocument(); + expect(screen.queryByRole("heading", { name: "Robin" })).not.toBeInTheDocument(); + }); + + it("keeps chip counts stable while a filter is active", () => { + renderList(HOUSEHOLD, { groupByProfile: true, profileFilter: "profile-2" }); + + // Sam's chip still says 2 even though none of Sam's devices are listed. + expect(screen.getByRole("button", { name: "Sam, 2 devices" })).toBeInTheDocument(); + }); +}); + +describe("lastSeenLabel", () => { + it("reads as plain language", () => { + expect(lastSeenLabel(new Date(NOW - 30 * 60 * 1000).toISOString(), NOW)).toBe( + "Less than an hour ago", + ); + expect(lastSeenLabel(new Date(NOW - 3 * 60 * 60 * 1000).toISOString(), NOW)).toBe( + "3 hours ago", + ); + expect(lastSeenLabel(new Date(NOW - 24 * 60 * 60 * 1000).toISOString(), NOW)).toBe("Yesterday"); + expect(lastSeenLabel(new Date(NOW - 10 * 24 * 60 * 60 * 1000).toISOString(), NOW)).toBe( + "10 days ago", + ); + expect(lastSeenLabel("not-a-date", NOW)).toBe("Never used"); + }); +}); diff --git a/web/src/components/settings/DeviceList.tsx b/web/src/components/settings/DeviceList.tsx new file mode 100644 index 00000000..8774e00f --- /dev/null +++ b/web/src/components/settings/DeviceList.tsx @@ -0,0 +1,398 @@ +import { useMemo, useState } from "react"; +import { ChevronRight, Search } from "lucide-react"; + +import type { UserDevice } from "@/api/types"; +import { Input } from "@/components/ui/input"; +import { + classifyPlatform, + PlatformIcon, + platformKindLabel, +} from "@/components/admin/deviceOverrides"; +import { deviceRecencyGroup, type DeviceRecencyGroup } from "@/hooks/queries/devices"; +import { cn } from "@/lib/utils"; + +const GROUP_TITLES: Record = { + current: "Using now", + week: "This week", + earlier: "Earlier", +}; + +const GROUP_ORDER: DeviceRecencyGroup[] = ["current", "week", "earlier"]; + +/** + * A device is "dormant" when nobody has used it for three months and it carries + * no settings of its own. + * + * Accounts accumulate these relentlessly — every browser profile, private + * window, reinstall and test build registers an identity, and nothing prunes + * them. A real account had 260 devices, over half of them one-off sessions that + * had never changed a setting. Listing them alongside the living-room TV is + * what turned this screen into sixteen screens of scrolling, so they collapse + * behind a count until asked for. + */ +const DORMANT_AFTER_MS = 90 * 24 * 60 * 60 * 1000; + +export function isDormantDevice(device: UserDevice, now: number): boolean { + if (device.is_current_device || device.changed_count > 0) return false; + const seen = Date.parse(device.last_seen_at); + if (Number.isNaN(seen)) return true; + return now - seen > DORMANT_AFTER_MS; +} + +export interface DeviceListProps { + devices: UserDevice[]; + selectedDeviceId: string | null; + onSelect: (device: UserDevice) => void; + search: string; + onSearchChange: (value: string) => void; + /** Group by person first. Only the household view sets this. */ + groupByProfile?: boolean; + /** + * Show only this profile's devices. Null means everyone. Only meaningful + * alongside groupByProfile — a viewer looking at their own devices has just + * the one profile, so a filter with a single option would be noise. + */ + profileFilter?: string | null; + onProfileFilterChange?: (profileId: string | null) => void; + /** The viewer's own profile, so their chip leads the row. */ + ownProfileId?: string; + /** + * "Now" for relative-time labels. Passed in rather than read during render so + * the component stays pure — and so a test can pin the clock. + */ + now: number; +} + +/** + * The device list. + * + * Fixed-height rows carrying a name, when it was last used, and the one number + * that matters — how many settings differ there. A device with nothing changed + * shows a dash rather than a zero, so "which one did I change?" is answerable + * by scanning rather than by opening each device in turn. + */ +export function DeviceList({ + devices, + selectedDeviceId, + onSelect, + search, + onSearchChange, + groupByProfile = false, + profileFilter = null, + onProfileFilterChange, + ownProfileId, + now, +}: DeviceListProps) { + const [showDormant, setShowDormant] = useState(false); + // Counts come from the unfiltered list so a chip keeps showing how many + // devices it would reveal, rather than collapsing to zero once another chip + // is active. + const profiles = useMemo(() => profileOptions(devices, ownProfileId), [devices, ownProfileId]); + + const matching = useMemo(() => { + const query = search.trim().toLowerCase(); + return devices.filter((device) => { + if (profileFilter && device.profile_id !== profileFilter) return false; + if (!query) return true; + const platform = platformKindLabel(classifyPlatform(device.device_platform)); + return ( + device.device_name.toLowerCase().includes(query) || + device.device_platform.toLowerCase().includes(query) || + platform.toLowerCase().includes(query) || + device.profile_name.toLowerCase().includes(query) + ); + }); + }, [devices, search, profileFilter]); + + // Searching means you are looking for something specific, so a search spans + // everything — hiding a dormant device from its own name would be a bug. + const searching = search.trim().length > 0; + const dormantCount = useMemo( + () => (searching ? 0 : matching.filter((device) => isDormantDevice(device, now)).length), + [matching, searching, now], + ); + const filtered = useMemo( + () => + searching || showDormant + ? matching + : matching.filter((device) => !isDormantDevice(device, now)), + [matching, searching, showDormant, now], + ); + + const sections = useMemo(() => { + if (groupByProfile && !profileFilter) { + const byProfile = new Map(); + for (const device of filtered) { + const existing = byProfile.get(device.profile_id); + if (existing) { + existing.devices.push(device); + } else { + byProfile.set(device.profile_id, { + title: device.profile_name || "Other profile", + devices: [device], + }); + } + } + return [...byProfile.values()]; + } + + return GROUP_ORDER.map((group) => ({ + title: GROUP_TITLES[group], + devices: filtered.filter((device) => deviceRecencyGroup(device, now) === group), + })).filter((section) => section.devices.length > 0); + }, [filtered, groupByProfile, profileFilter, now]); + + return ( +
+
+ + onSearchChange(event.target.value)} + placeholder={`Search ${devices.length} ${devices.length === 1 ? "device" : "devices"}`} + aria-label="Search devices" + // text-base on mobile: iOS Safari zooms the viewport when a focused + // input's font is under 16px, and the zoom does not undo itself. + className="h-11 pl-8 text-base xl:h-9 xl:text-[13px]" + /> +
+ + {groupByProfile && profiles.length > 1 && onProfileFilterChange ? ( + // A wrapping row cost three lines and pushed the list off a phone + // screen at eight profiles. Scrolling horizontally keeps it to one + // line at any household size — the same trade the settings shell's own + // mobile tab bar makes — and unwraps into a normal row from xl up. +
+ onProfileFilterChange(null)} + /> + {profiles.map((profile) => ( + + onProfileFilterChange(profileFilter === profile.id ? null : profile.id) + } + /> + ))} +
+ ) : null} + + {sections.length === 0 ? ( +

+ {devices.length === 0 + ? "No devices yet." + : searching + ? "No devices match that search." + : "Nothing here. Try showing unused devices."} +

+ ) : null} + + {/* Capped and scrollable rather than growing without limit: an account + with a few hundred devices produced a thirteen-thousand-pixel page, + so the settings never came into view. The list keeps its own scroll + and the page stays roughly one screen. */} +
+ {sections.map((section) => ( +
+

+ {section.title} +

+
    + {section.devices.map((device) => ( +
  • + +
    • + ))} +
+
+ ))} +
+ + {/* The long tail is opt-in, and says how big it is so nobody wonders + whether a device is missing. */} + {dormantCount > 0 ? ( + + ) : null} +
+ ); +} + +interface ProfileOption { + id: string; + name: string; + count: number; +} + +/** + * One entry per profile that owns at least one device. + * + * The viewer's own profile comes first — at eight profiles the arrival order + * put the person actually using the screen last — and the rest are alphabetical + * so a chip stays where someone last saw it, rather than moving whenever a + * device is used. + */ +function profileOptions(devices: UserDevice[], ownProfileId?: string): ProfileOption[] { + const byId = new Map(); + for (const device of devices) { + const existing = byId.get(device.profile_id); + if (existing) { + existing.count += 1; + } else { + byId.set(device.profile_id, { + id: device.profile_id, + name: device.profile_name || "Other profile", + count: 1, + }); + } + } + return [...byId.values()].sort((a, b) => { + if (a.id === ownProfileId) return -1; + if (b.id === ownProfileId) return 1; + return a.name.localeCompare(b.name); + }); +} + +function ProfileChip({ + label, + count, + active, + onClick, +}: { + label: string; + count: number; + active: boolean; + onClick: () => void; +}) { + return ( + + ); +} + +function DeviceRow({ + device, + selected, + onSelect, + now, +}: { + device: UserDevice; + selected: boolean; + onSelect: (device: UserDevice) => void; + now: number; +}) { + const kind = classifyPlatform(device.device_platform); + return ( + + ); +} + +function ChangedCount({ count }: { count: number }) { + if (count <= 0) { + return ( + + — + + ); + } + return ( + + {count} + + ); +} + +/** Plain relative time; the exact timestamp is not what anyone is asking. */ +export function lastSeenLabel(iso: string, now: number): string { + const seen = Date.parse(iso); + if (Number.isNaN(seen)) return "Never used"; + const minutes = Math.max(0, Math.round((now - seen) / 60000)); + if (minutes < 60) return "Less than an hour ago"; + const hours = Math.round(minutes / 60); + if (hours < 24) return `${hours} ${hours === 1 ? "hour" : "hours"} ago`; + const days = Math.round(hours / 24); + if (days === 1) return "Yesterday"; + if (days < 30) return `${days} days ago`; + const months = Math.round(days / 30); + if (months < 12) return `${months} ${months === 1 ? "month" : "months"} ago`; + const years = Math.round(months / 12); + return `${years} ${years === 1 ? "year" : "years"} ago`; +} diff --git a/web/src/components/settings/DeviceSettingGroups.test.tsx b/web/src/components/settings/DeviceSettingGroups.test.tsx new file mode 100644 index 00000000..0e019b60 --- /dev/null +++ b/web/src/components/settings/DeviceSettingGroups.test.tsx @@ -0,0 +1,205 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import { DeviceSettingGroups } from "@/components/settings/DeviceSettingGroups"; +import type { EffectiveSetting } from "@/hooks/queries/settingValues"; +import type { SettingKey } from "@/lib/settingsContract"; + +// Radix Slider and Select read element sizes via ResizeObserver, which jsdom +// does not provide. A no-op polyfill is enough to render them. +class ResizeObserverStub { + observe() {} + unobserve() {} + disconnect() {} +} +if (typeof globalThis.ResizeObserver === "undefined") { + (globalThis as unknown as { ResizeObserver: typeof ResizeObserverStub }).ResizeObserver = + ResizeObserverStub; +} +// Radix Select opens through pointer capture, which jsdom also lacks. +if (typeof window !== "undefined" && !window.HTMLElement.prototype.hasPointerCapture) { + window.HTMLElement.prototype.hasPointerCapture = () => false; + window.HTMLElement.prototype.scrollIntoView = () => {}; +} + +function renderGroups( + settings: Partial>, + props: Partial<{ ownerLabel: string }> = {}, +) { + const onChange = vi.fn(); + const onReset = vi.fn(); + render( + , + ); + return { onChange, onReset }; +} + +function effective(overrides: Partial = {}): EffectiveSetting { + return { key: "player.hdr_enabled", value: true, source: "default", ...overrides }; +} + +describe("DeviceSettingGroups", () => { + it("groups settings under headings a viewer would look under", () => { + renderGroups({}); + + // Scoped to headings: "Subtitles" is also a setting label inside the group. + const headings = screen.getAllByRole("heading").map((node) => node.textContent); + expect(headings).toEqual(["Picture", "Sound", "Subtitles", "Episodes"]); + }); + + // The screen is for people who do not know what a manifest key is. Matching + // on the dotted key shape rather than the prefix alone, because a manifest + // description may legitimately end a sentence with the word "playback". + it("never shows a raw setting key", () => { + const { container } = render( + , + ); + expect(container.textContent).not.toMatch(/\b(?:player|playback|ui)\.[a-z0-9_]+/); + }); + + it("marks a value stored on this device and offers to clear it", async () => { + const { onReset } = renderGroups({ + "player.hdr_enabled": effective({ + value: false, + source: "profile_device", + scope: "profile_device", + }), + }); + + expect(screen.getByText("Changed here")).toBeInTheDocument(); + await userEvent.click(screen.getByRole("button", { name: /Use your setting/ })); + expect(onReset).toHaveBeenCalledWith("player.hdr_enabled"); + }); + + it("names the person when the household parent is acting for someone else", () => { + renderGroups( + { + "player.hdr_enabled": effective({ source: "profile_device", scope: "profile_device" }), + }, + { ownerLabel: "Robin's" }, + ); + + expect(screen.getByRole("button", { name: /Use Robin's setting/ })).toBeInTheDocument(); + }); + + it("does not offer a reset when nothing is stored on this device", () => { + renderGroups({ "player.hdr_enabled": effective({ source: "profile" }) }); + + expect(screen.queryByText("Changed here")).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /Use your setting/ })).not.toBeInTheDocument(); + }); + + it("sends typed values rather than strings", async () => { + const { onChange } = renderGroups({ + "player.hdr_enabled": effective({ value: true, source: "default" }), + }); + + const [firstToggle] = screen.getAllByRole("switch"); + await userEvent.click(firstToggle!); + + expect(onChange).toHaveBeenCalled(); + const [firstCall] = onChange.mock.calls; + expect(typeof firstCall![1]).toBe("boolean"); + }); + + // A capped setting explains the cap. A disabled control with no reason is + // exactly what the settings contract's UX rules forbid. + it("explains a household limit instead of silently narrowing", () => { + renderGroups({ + "playback.preferred_quality": effective({ + key: "playback.preferred_quality", + value: "1080p", + stored_value: "2160p", + source: "profile_device", + scope: "profile_device", + constrained: true, + constraint_kind: "ceiling", + }), + }); + + expect(screen.getByText("Household limit")).toBeInTheDocument(); + expect(screen.getByText(/limit this to 1080p/)).toBeInTheDocument(); + expect(screen.getByText(/your choice of 2160p isn't available/)).toBeInTheDocument(); + }); + + // The bandwidth cap is declared as an integer range with a select control + // and no members, so the generic path rendered a dropdown with one blank + // entry — unusable, and silent about the value it was already storing. + it("offers real bandwidth choices for a numeric range with no members", async () => { + const { onChange } = renderGroups({ + "playback.max_bitrate_kbps": effective({ + key: "playback.max_bitrate_kbps", + value: 2000, + source: "profile_device", + scope: "profile_device", + }), + }); + + expect(screen.getByText("2 Mbps")).toBeInTheDocument(); + + await userEvent.click(screen.getByRole("combobox", { name: /Maximum bitrate/i })); + const option = await screen.findByRole("option", { name: "10 Mbps" }); + await userEvent.click(option); + + // Indexed rather than .at(-1): the app tsconfig targets ES2020. + const calls = onChange.mock.calls; + const call = calls[calls.length - 1]; + expect(call?.[0]).toBe("playback.max_bitrate_kbps"); + expect(call?.[1]).toBe(10000); + }); + + // The ladder is filtered by the definition's own range, which tops out at + // 200 Mbps. An earlier hardcoded list stopped at 40 and silently capped + // people below what their server could already send. + it("offers the full range the contract allows", async () => { + renderGroups({ + "playback.max_bitrate_kbps": effective({ + key: "playback.max_bitrate_kbps", + value: null, + source: "default", + }), + }); + + await userEvent.click(screen.getByRole("combobox", { name: /Maximum bitrate/i })); + + expect(await screen.findByRole("option", { name: "200 Mbps" })).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "100 Mbps" })).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "1.5 Mbps" })).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "No limit" })).toBeInTheDocument(); + }); + + it("keeps a stored bandwidth value selectable even when it is not a preset", () => { + renderGroups({ + "playback.max_bitrate_kbps": effective({ + key: "playback.max_bitrate_kbps", + value: 3500, + source: "profile_device", + scope: "profile_device", + }), + }); + + expect(screen.getByText("3.5 Mbps")).toBeInTheDocument(); + }); + + it("states who set a locked value rather than disabling it silently", () => { + renderGroups({ + "playback.preferred_quality": effective({ + key: "playback.preferred_quality", + value: "720p", + source: "profile", + constrained: true, + constraint_kind: "locked", + }), + }); + + expect( + screen.getByText(/set for your household and can't be changed here/), + ).toBeInTheDocument(); + }); +}); diff --git a/web/src/components/settings/DeviceSettingGroups.tsx b/web/src/components/settings/DeviceSettingGroups.tsx new file mode 100644 index 00000000..03496ea8 --- /dev/null +++ b/web/src/components/settings/DeviceSettingGroups.tsx @@ -0,0 +1,441 @@ +import { Lock, RotateCcw } from "lucide-react"; + +import { Button } from "@/components/ui/button"; +import { Slider } from "@/components/ui/slider"; +import { Switch } from "@/components/ui/switch"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { LanguageSelect } from "@/components/settings/LanguageSelect"; +import { SettingsGroup } from "@/components/settings/SettingsGroup"; +import { groupDeviceSettings } from "@/lib/deviceSettingGroups"; +import type { EffectiveSetting } from "@/hooks/queries/settingValues"; +import { SETTING_DEFINITIONS, type SettingKey } from "@/lib/settingsContract"; +import { formatQualityBitrate } from "@/player/hooks/useTranscodeQuality"; +import { namedLanguageOptionsFor } from "@/lib/languageOptions"; +import { controlKindFor, optionsFor } from "@/lib/settingsDisplay"; +import { cn } from "@/lib/utils"; + +const EMPTY_SELECT_VALUE = "__empty__"; + +export interface DeviceSettingGroupsProps { + /** Effective values resolved for the device being edited. */ + settings: Partial>; + /** + * Whose settings these are, for the reset label. "your" on your own devices, + * a name when the household parent is acting for someone else. + */ + ownerLabel: string; + disabled?: boolean; + onChange: (key: SettingKey, value: unknown) => void; + onReset: (key: SettingKey) => void; + /** Opens the subtitle appearance panel, which is not an inline control. */ + onOpenPanel?: (key: SettingKey) => void; +} + +export function DeviceSettingGroups({ + settings, + ownerLabel, + disabled = false, + onChange, + onReset, + onOpenPanel, +}: DeviceSettingGroupsProps) { + return ( +
+ {groupDeviceSettings().map((group) => ( + + {group.keys.map((key) => ( + + ))} + + ))} +
+ ); +} + +interface DeviceSettingRowProps { + settingKey: SettingKey; + effective: EffectiveSetting | undefined; + ownerLabel: string; + disabled: boolean; + onChange: (key: SettingKey, value: unknown) => void; + onReset: (key: SettingKey) => void; + onOpenPanel?: (key: SettingKey) => void; +} + +function DeviceSettingRow({ + settingKey, + effective, + ownerLabel, + disabled, + onChange, + onReset, + onOpenPanel, +}: DeviceSettingRowProps) { + const definition = SETTING_DEFINITIONS[settingKey]; + if (!definition) return null; + + // "Changed here" means a row exists at this exact device, which is also what + // makes the reset meaningful — reset clears that row rather than copying the + // profile value into it. + const changedHere = effective?.scope === "profile_device"; + const locked = effective?.constraint_kind === "locked"; + const constrained = Boolean(effective?.constrained); + const value = effective?.value ?? definition.defaultValue; + const inlineControl = controlKindFor(definition) === "switch"; + + return ( +
+
+
+ {definition.label} + {changedHere ? ( + + Changed here + + ) : null} + {constrained ? ( + + + Household limit + + ) : null} +
+

+ {definition.description} +

+ {constrained ? ( +

+ {constraintExplanation(effective)} +

+ ) : null} +
+ +
+ {changedHere && !locked ? ( + + ) : null} + +
+
+ ); +} + +function constraintExplanation(effective: EffectiveSetting | undefined): string { + if (!effective) return ""; + const permitted = effective.value; + if (effective.constraint_kind === "locked") { + return "This is set for your household and can't be changed here."; + } + // The stored preference is still theirs; it is just capped today. Saying so + // beats silently showing a value they did not choose. + if (effective.stored_value !== undefined && effective.stored_value !== permitted) { + return `Your household settings limit this to ${String(permitted)}, so your choice of ${String(effective.stored_value)} isn't available right now.`; + } + return "Your household settings limit this option."; +} + +interface DeviceSettingControlProps { + settingKey: SettingKey; + effective: EffectiveSetting | undefined; + value: unknown; + disabled: boolean; + onChange: (key: SettingKey, value: unknown) => void; + onOpenPanel?: (key: SettingKey) => void; +} + +/** + * The control for one device setting. + * + * Values are typed JSON here, not strings: a slider round-tripping through + * text is a hazard on a screen a viewer uses, and the contract already knows + * every value's type. When policy narrows the choices, the select renders the + * permitted list rather than the manifest's. + */ +function DeviceSettingControl({ + settingKey, + effective, + value, + disabled, + onChange, + onOpenPanel, +}: DeviceSettingControlProps) { + const definition = SETTING_DEFINITIONS[settingKey]; + const control = controlKindFor(definition); + + if (control === "panel" || definition.type === "object") { + return ( + + ); + } + + if (control === "switch") { + return ( + + onChange(settingKey, checked)} + /> + + ); + } + + if (control === "slider" || control === "stepper") { + const numeric = typeof value === "number" ? value : Number(definition.defaultValue ?? 0); + return ( +
+ onChange(settingKey, values[0] ?? numeric)} + /> + + {numeric} + {definition.unit ? ` ${definition.unit}` : ""} + +
+ ); + } + + const options = permittedOptions(settingKey, effective); + const asString = value === null || value === undefined ? "" : String(value); + + // Open language values get the shared picker with "Other…" free entry — + // the contract floor is a short authored list, and any tag beyond it is + // typed rather than fetched from the catalog. A permitted_values constraint + // pins the list closed, so the free entry disappears with it. + if (definition.type === "language_tag") { + const permitted = (effective as { permitted_values?: unknown[] } | undefined)?.permitted_values; + const languageOptions = namedLanguageOptionsFor(settingKey, asString || undefined).filter( + (option) => !permitted?.length || permitted.some((entry) => String(entry) === option.value), + ); + return ( +
+ onChange(settingKey, next === EMPTY_SELECT_VALUE ? null : next)} + > + {definition.nullable && ( + {definition.unsetLabel ?? "Unset"} + )} + +
+ ); + } + + // A numeric "select" the manifest gives no members — the bandwidth cap is + // declared as a range, not a list — would render as a one-entry dropdown + // showing nothing at all. Present the range as bandwidth choices people + // recognise instead, and keep the stored value visible if it is not one of + // them. + const numericChoices = numericSelectChoices(settingKey, definition, options, asString); + if (numericChoices) { + return ( + + ); + } + + return ( + + ); +} + +/** + * The options this viewer may actually pick. `permitted_values` narrows the + * manifest's list for a viewer under a policy cap, so a child's quality picker + * shows what they can have rather than offering 4K and delivering 1080p. + */ +function permittedOptions(settingKey: SettingKey, effective: EffectiveSetting | undefined) { + const definition = SETTING_DEFINITIONS[settingKey]; + const all = optionsFor(definition); + const permitted = (effective as { permitted_values?: unknown[] } | undefined)?.permitted_values; + if (!permitted?.length) return all; + const allowed = new Set(permitted.map((entry) => String(entry))); + const narrowed = all.filter((option) => option.value === "" || allowed.has(option.value)); + return narrowed.length > 0 ? narrowed : all; +} + +/** + * Bandwidth caps people recognise, bounded by the definition's own range. + * + * Returns null for any select the manifest actually gives members, which is + * every other one — this exists only for a numeric range declared with a + * select control. + */ +/** + * The bandwidth ladder, in kbps. + * + * The low end matches the in-player quality switcher + * (web/src/player/hooks/useTranscodeQuality.ts) so a cap chosen here lines up + * with what the player offers mid-playback. Above that it keeps climbing to + * the definition's own ceiling of 200 Mbps, which is what remuxed 4K HDR and + * untouched Blu-ray rips actually need — a ladder that stopped short would cap + * people below what their server can already send them. + * + * Entries outside a definition's declared range are filtered out, so this list + * can cover more ground than any single setting allows. + */ +const BITRATE_CHOICES_KBPS = [ + 1500, 2000, 4000, 6000, 10000, 15000, 20000, 30000, 40000, 60000, 80000, 100000, 150000, 200000, +]; + +function numericSelectChoices( + settingKey: SettingKey, + definition: (typeof SETTING_DEFINITIONS)[SettingKey], + options: { value: string; label: string }[], + currentValue: string, +): { value: string; label: string }[] | null { + const isNumeric = definition.type === "integer" || definition.type === "number"; + const hasMembers = options.some((option) => option.value !== ""); + if (!isNumeric || hasMembers) return null; + + const min = definition.minimum ?? 0; + const max = definition.maximum ?? Number.MAX_SAFE_INTEGER; + const choices = BITRATE_CHOICES_KBPS.filter((kbps) => kbps >= min && kbps <= max).map((kbps) => ({ + value: String(kbps), + label: formatBitrate(kbps), + })); + + // A value set elsewhere (an API call, another client) must stay selectable + // rather than silently reading as "No limit". + if (currentValue !== "" && !choices.some((choice) => choice.value === currentValue)) { + const parsed = Number(currentValue); + if (Number.isFinite(parsed)) { + choices.push({ value: currentValue, label: formatBitrate(parsed) }); + choices.sort((a, b) => Number(a.value) - Number(b.value)); + } + } + + const unsetLabel = settingKey === "playback.max_bitrate_kbps" ? "No limit" : "Unset"; + return [{ value: "", label: unsetLabel }, ...choices]; +} + +function formatBitrate(kbps: number): string { + return formatQualityBitrate(kbps); +} + +/** Selects edit strings; integers travel back as numbers. */ +function typedSelectValue(settingKey: SettingKey, raw: string): unknown { + const definition = SETTING_DEFINITIONS[settingKey]; + if (definition.type === "integer" || definition.type === "number") { + const parsed = Number(raw); + return Number.isFinite(parsed) ? parsed : raw; + } + return raw; +} diff --git a/web/src/components/settings/LanguageSelect.test.tsx b/web/src/components/settings/LanguageSelect.test.tsx new file mode 100644 index 00000000..1bb96521 --- /dev/null +++ b/web/src/components/settings/LanguageSelect.test.tsx @@ -0,0 +1,98 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import { LanguageSelect } from "@/components/settings/LanguageSelect"; +import { SelectItem } from "@/components/ui/select"; + +// Radix Select reads element sizes via ResizeObserver, which jsdom does not +// provide, and opens through pointer capture, which jsdom also lacks. +class ResizeObserverStub { + observe() {} + unobserve() {} + disconnect() {} +} +if (typeof globalThis.ResizeObserver === "undefined") { + (globalThis as unknown as { ResizeObserver: typeof ResizeObserverStub }).ResizeObserver = + ResizeObserverStub; +} +if (typeof window !== "undefined" && !window.HTMLElement.prototype.hasPointerCapture) { + window.HTMLElement.prototype.hasPointerCapture = () => false; + window.HTMLElement.prototype.scrollIntoView = () => {}; +} + +const OPTIONS = [ + { value: "en", label: "English" }, + { value: "fr", label: "French" }, +]; + +function renderSelect(props: Partial[0]> = {}) { + const onValueChange = vi.fn(); + render( + + No preference + , + ); + return { onValueChange }; +} + +describe("LanguageSelect", () => { + it("commits a typed tag through the Other entry", async () => { + const user = userEvent.setup(); + const { onValueChange } = renderSelect(); + + await user.click(screen.getByRole("combobox", { name: "Spoken language" })); + await user.click(screen.getByRole("option", { name: "Other…" })); + + const input = screen.getByRole("textbox", { name: "Language code" }); + await user.type(input, "is"); + // The preview names the language before anything is saved. + expect(screen.getByText("Icelandic")).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Use" })); + expect(onValueChange).toHaveBeenCalledWith("is"); + // Committing closes the free-entry row. + expect(screen.queryByRole("textbox", { name: "Language code" })).not.toBeInTheDocument(); + }); + + it("refuses to commit an invalid tag and explains why", async () => { + const user = userEvent.setup(); + const { onValueChange } = renderSelect(); + + await user.click(screen.getByRole("combobox", { name: "Spoken language" })); + await user.click(screen.getByRole("option", { name: "Other…" })); + + const input = screen.getByRole("textbox", { name: "Language code" }); + await user.type(input, "not a language{Enter}"); + + expect(onValueChange).not.toHaveBeenCalled(); + expect(screen.getByRole("alert")).toHaveTextContent("Not a valid language tag"); + expect(screen.getByRole("button", { name: "Use" })).toBeDisabled(); + }); + + it("keeps the current selection when Other is dismissed", async () => { + const user = userEvent.setup(); + const { onValueChange } = renderSelect({ value: "en" }); + + await user.click(screen.getByRole("combobox", { name: "Spoken language" })); + await user.click(screen.getByRole("option", { name: "Other…" })); + await user.click(screen.getByRole("button", { name: "Cancel" })); + + expect(onValueChange).not.toHaveBeenCalled(); + expect(screen.getByRole("combobox", { name: "Spoken language" })).toHaveTextContent("English"); + }); + + it("hides the Other entry when the value is constrained", async () => { + const user = userEvent.setup(); + renderSelect({ allowOther: false }); + + await user.click(screen.getByRole("combobox", { name: "Spoken language" })); + expect(screen.queryByRole("option", { name: "Other…" })).not.toBeInTheDocument(); + }); +}); diff --git a/web/src/components/settings/LanguageSelect.tsx b/web/src/components/settings/LanguageSelect.tsx new file mode 100644 index 00000000..27b5191b --- /dev/null +++ b/web/src/components/settings/LanguageSelect.tsx @@ -0,0 +1,178 @@ +import { useState, type ReactNode } from "react"; + +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { + Select, + SelectContent, + SelectItem, + SelectSeparator, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { canonicalLanguageTag, getLanguageName } from "@/lib/languageNames"; +import type { SettingOption } from "@/lib/languageOptions"; + +const OTHER_VALUE = "__other__"; + +interface LanguageSelectProps { + /** Raw select value, including any caller sentinel ("none", inherit, …). */ + value: string; + /** + * Receives either a caller sentinel from {@link children}, an option value, + * or a free-typed BCP 47 tag committed through "Other…". + */ + onValueChange: (value: string) => void; + options: readonly SettingOption[]; + id?: string; + disabled?: boolean; + placeholder?: string; + /** Class for the select trigger. */ + className?: string; + /** + * Whether the "Other…" free entry is offered. Callers pass false when the + * value is constrained to an explicit permitted list, where a typed tag + * would only be rejected on save. + */ + allowOther?: boolean; + "aria-label"?: string; + "aria-describedby"?: string; + /** Leading sentinel items ("No preference", "Inherit", …) as SelectItems. */ + children?: ReactNode; +} + +/** + * A language picker over the contract's advisory option floor. + * + * The floor deliberately stays a short authored list — the server no longer + * scans the catalog for every audio and subtitle track language (that walk + * took tens of seconds on large deployments). Because these settings are open + * `language_tag` values, any language beyond the floor is reachable through + * "Other…", which accepts a BCP 47 tag and previews the resolved language + * name before committing. A stored off-floor value stays selectable because + * the option builders always synthesize the current value into the list. + */ +export function LanguageSelect({ + value, + onValueChange, + options, + id, + disabled = false, + placeholder, + className, + allowOther = true, + "aria-label": ariaLabel, + "aria-describedby": ariaDescribedBy, + children, +}: LanguageSelectProps) { + // null = closed; otherwise the tag being typed. + const [draft, setDraft] = useState(null); + + const trimmed = (draft ?? "").trim(); + const draftTag = trimmed ? canonicalLanguageTag(trimmed) : null; + const draftInvalid = trimmed.length > 0 && draftTag === null; + + const commitDraft = () => { + if (!draftTag) return; + onValueChange(trimmed); + setDraft(null); + }; + + return ( +
+ + + {draft !== null && ( +
+
+ setDraft(event.target.value)} + onKeyDown={(event) => { + if (event.key === "Enter") { + event.preventDefault(); + commitDraft(); + } else if (event.key === "Escape") { + event.preventDefault(); + setDraft(null); + } + }} + /> + + +
+

+ {draftInvalid + ? "Not a valid language tag. Use an ISO code such as is, yue, or pt-BR." + : draftTag + ? getLanguageName(trimmed) + : "Type an ISO language code to use a language not in the list."} +

+
+ )} +
+ ); +} diff --git a/web/src/hooks/queries/devices.test.ts b/web/src/hooks/queries/devices.test.ts new file mode 100644 index 00000000..77dffcae --- /dev/null +++ b/web/src/hooks/queries/devices.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from "vitest"; + +import type { UserDevice } from "@/api/types"; +import { deviceRecencyGroup } from "@/hooks/queries/devices"; +import { effectiveSettingsQueryKey } from "@/hooks/queries/settingValues"; + +const NOW = Date.parse("2026-07-31T12:00:00Z"); + +function device(overrides: Partial = {}): UserDevice { + return { + device_id: "device-1", + device_name: "Chrome", + device_platform: "macOS Web", + last_seen_at: new Date(NOW).toISOString(), + profile_id: "profile-1", + profile_name: "Sam", + is_current_device: false, + changed_count: 0, + ...overrides, + }; +} + +describe("deviceRecencyGroup", () => { + it("puts the current device first regardless of its timestamp", () => { + const stale = device({ + is_current_device: true, + last_seen_at: new Date(NOW - 400 * 24 * 60 * 60 * 1000).toISOString(), + }); + expect(deviceRecencyGroup(stale, NOW)).toBe("current"); + }); + + it("splits the rest at a week", () => { + const recent = device({ last_seen_at: new Date(NOW - 6 * 24 * 60 * 60 * 1000).toISOString() }); + const older = device({ last_seen_at: new Date(NOW - 8 * 24 * 60 * 60 * 1000).toISOString() }); + expect(deviceRecencyGroup(recent, NOW)).toBe("week"); + expect(deviceRecencyGroup(older, NOW)).toBe("earlier"); + }); + + it("treats an unparseable timestamp as old rather than throwing", () => { + expect(deviceRecencyGroup(device({ last_seen_at: "nonsense" }), NOW)).toBe("earlier"); + }); +}); + +describe("effectiveSettingsQueryKey", () => { + // Without device and profile in the key, reading the Apple TV's values would + // land on the same cache entry as this browser's and serve one device's + // settings as another's. + it("gives each device its own cache entry", () => { + const keys = ["player.hdr_enabled"] as const; + const own = effectiveSettingsQueryKey({ keys }); + const appleTv = effectiveSettingsQueryKey({ keys, deviceId: "apple-tv" }); + const iPad = effectiveSettingsQueryKey({ keys, deviceId: "ipad" }); + + expect(appleTv).not.toEqual(own); + expect(appleTv).not.toEqual(iPad); + }); + + it("gives each profile its own cache entry", () => { + const keys = ["player.hdr_enabled"] as const; + const mine = effectiveSettingsQueryKey({ keys, deviceId: "shared-tv" }); + const theirs = effectiveSettingsQueryKey({ + keys, + deviceId: "shared-tv", + profileId: "profile-2", + }); + + expect(theirs).not.toEqual(mine); + }); + + it("is stable for the same request", () => { + const keys = ["player.hdr_enabled"] as const; + expect(effectiveSettingsQueryKey({ keys, deviceId: "tv" })).toEqual( + effectiveSettingsQueryKey({ keys, deviceId: "tv" }), + ); + }); +}); diff --git a/web/src/hooks/queries/devices.ts b/web/src/hooks/queries/devices.ts new file mode 100644 index 00000000..df6af957 --- /dev/null +++ b/web/src/hooks/queries/devices.ts @@ -0,0 +1,79 @@ +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; + +import { api } from "@/api/client"; +import type { UserDevice, UserDeviceListResponse } from "@/api/types"; + +import { deviceKeys, settingsKeys } from "./keys"; + +/** + * The signed-in viewer's own device registry. + * + * `household` is opt-in and only succeeds for the household parent — the server + * answers 403 otherwise. It defaults off so the ordinary screen cannot show the + * family's devices by forgetting to ask for less. + */ +export function useMyDevices(options?: { household?: boolean; enabled?: boolean }) { + const household = options?.household ?? false; + + return useQuery({ + queryKey: deviceKeys.list(household ? "household" : "own"), + queryFn: async () => { + const result = await api( + `/devices${household ? "?scope=household" : ""}`, + ); + return result.devices ?? []; + }, + enabled: options?.enabled ?? true, + staleTime: 30 * 1000, + }); +} + +/** Both mutations move settings, so both invalidate the value caches too. */ +function useDeviceMutation(path: (device: DeviceTarget) => string) { + const qc = useQueryClient(); + + return useMutation({ + mutationFn: (device: DeviceTarget) => api(path(device), { method: "DELETE" }), + onSettled: () => { + void qc.invalidateQueries({ queryKey: deviceKeys.all }); + void qc.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); + }, + }); +} + +export interface DeviceTarget { + deviceId: string; + /** Set only when acting for another profile, as the household parent. */ + profileId?: string; +} + +function targetQuery(device: DeviceTarget): string { + return device.profileId ? `?profile_id=${encodeURIComponent(device.profileId)}` : ""; +} + +/** Remove the device's settings and drop it from the registry. */ +export function useForgetDevice() { + return useDeviceMutation( + (device) => `/devices/${encodeURIComponent(device.deviceId)}${targetQuery(device)}`, + ); +} + +/** Return a device to the profile's own values without forgetting it. */ +export function useClearDeviceSettings() { + return useDeviceMutation( + (device) => `/devices/${encodeURIComponent(device.deviceId)}/settings${targetQuery(device)}`, + ); +} + +/** + * Devices grouped the way the list reads them: by how recently they were used, + * because that is how someone identifies a device they own. + */ +export type DeviceRecencyGroup = "current" | "week" | "earlier"; + +export function deviceRecencyGroup(device: UserDevice, now: number): DeviceRecencyGroup { + if (device.is_current_device) return "current"; + const seen = Date.parse(device.last_seen_at); + if (Number.isNaN(seen)) return "earlier"; + return now - seen < 7 * 24 * 60 * 60 * 1000 ? "week" : "earlier"; +} diff --git a/web/src/hooks/queries/keys.ts b/web/src/hooks/queries/keys.ts index 3d917653..52c7149c 100644 --- a/web/src/hooks/queries/keys.ts +++ b/web/src/hooks/queries/keys.ts @@ -208,6 +208,11 @@ export const progressKeys = { list: (status?: string, libraryId?: number) => ["progress", "list", status, libraryId] as const, }; +export const deviceKeys = { + all: ["devices"] as const, + list: (scope: "own" | "household") => ["devices", "list", scope] as const, +}; + export const settingsKeys = { // The canonical value queries live under ["settings", "values", …] and build // their own key (effectiveSettingsQueryKey), so one invalidation of that diff --git a/web/src/hooks/queries/settingValues.ts b/web/src/hooks/queries/settingValues.ts index 69eef38c..65353cdd 100644 --- a/web/src/hooks/queries/settingValues.ts +++ b/web/src/hooks/queries/settingValues.ts @@ -4,7 +4,7 @@ import { api } from "@/api/client"; import { storage } from "@/utils/storage"; import { SETTING_DEFINITIONS, SETTINGS_REVISION, type SettingKey } from "@/lib/settingsContract"; import { useEventChannel } from "@/components/realtimeEventsContext"; -import { settingsKeys } from "./keys"; +import { deviceKeys, settingsKeys } from "./keys"; /** * Typed access to the canonical settings API. @@ -33,6 +33,16 @@ export interface SettingIdentity { libraryId?: number; /** Required for profile_series. */ seriesId?: string; + /** + * A device other than the one this browser is. Omit to address the current + * device, which is what every screen except "your devices" wants. + */ + deviceId?: string; + /** + * A profile other than the signed-in one. Only the household parent may set + * this; the server answers 403 for anyone else. + */ + profileId?: string; } export interface EffectiveSetting { @@ -63,6 +73,8 @@ function identityQuery(identity: SettingIdentity): string { const params = new URLSearchParams({ scope: identity.scope }); if (identity.libraryId !== undefined) params.set("library_id", String(identity.libraryId)); if (identity.seriesId !== undefined) params.set("series_id", identity.seriesId); + if (identity.deviceId !== undefined) params.set("device_id", identity.deviceId); + if (identity.profileId !== undefined) params.set("profile_id", identity.profileId); return params.toString(); } @@ -79,13 +91,20 @@ export function effectiveSettingsQueryKey(options?: { keys?: readonly SettingKey[]; libraryIds?: readonly number[]; seriesIds?: readonly string[]; + deviceId?: string; + profileId?: string; }) { - const { keys, libraryIds, seriesIds } = options ?? {}; + const { keys, libraryIds, seriesIds, deviceId, profileId } = options ?? {}; return [ ...settingsKeys.all, "values", "effective", - activeProfileId(), + // The device and profile a read resolved for are part of the identity of + // the answer, not just of the request. Without them a read of the Apple + // TV's values would land on the same cache entry as this browser's and + // serve one device's settings as another's. + profileId ?? activeProfileId(), + deviceId ?? "", keys ? [...keys].sort().join(",") : "*", libraryIds ? [...libraryIds].sort().join(",") : "", seriesIds ? [...seriesIds].sort().join(",") : "", @@ -104,19 +123,27 @@ export function useEffectiveSettings(options?: { keys?: readonly SettingKey[]; libraryIds?: readonly number[]; seriesIds?: readonly string[]; + /** Resolve for a device other than this browser. */ + deviceId?: string; + /** Resolve for another profile on the account (household parent only). */ + profileId?: string; enabled?: boolean; }) { const keys = options?.keys; const libraryIds = options?.libraryIds; const seriesIds = options?.seriesIds; + const deviceId = options?.deviceId; + const profileId = options?.profileId; return useQuery({ - queryKey: effectiveSettingsQueryKey({ keys, libraryIds, seriesIds }), + queryKey: effectiveSettingsQueryKey({ keys, libraryIds, seriesIds, deviceId, profileId }), queryFn: async () => { const params = new URLSearchParams(); if (keys?.length) params.set("keys", keys.join(",")); if (libraryIds?.length) params.set("library_ids", libraryIds.join(",")); if (seriesIds?.length) params.set("series_ids", seriesIds.join(",")); + if (deviceId) params.set("device_id", deviceId); + if (profileId) params.set("profile_id", profileId); const query = params.toString(); const result = await api( `/settings/values/effective${query ? `?${query}` : ""}`, @@ -181,8 +208,14 @@ export function useSetSettingValue() { }, body: JSON.stringify({ value }), }), - onSettled: () => { - qc.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); + onSettled: (_data, _error, variables) => { + void qc.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); + // A device-scoped write changes that device's "how many things differ" + // count, which the device list shows. Without this the badge stays stale + // until the list's own staleTime expires. + if (variables.identity.scope === "profile_device") { + void qc.invalidateQueries({ queryKey: deviceKeys.all }); + } }, }); } @@ -194,8 +227,11 @@ export function useClearSettingValue() { return useMutation({ mutationFn: ({ key, identity }: { key: SettingKey; identity: SettingIdentity }) => api(`/settings/values/${key}?${identityQuery(identity)}`, { method: "DELETE" }), - onSettled: () => { - qc.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); + onSettled: (_data, _error, variables) => { + void qc.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); + if (variables.identity.scope === "profile_device") { + void qc.invalidateQueries({ queryKey: deviceKeys.all }); + } }, }); } diff --git a/web/src/lib/deviceSettingGroups.test.ts b/web/src/lib/deviceSettingGroups.test.ts new file mode 100644 index 00000000..e5184c14 --- /dev/null +++ b/web/src/lib/deviceSettingGroups.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; + +import { + groupDeviceSettings, + groupForDeviceSetting, + hiddenDeviceSettingKeys, +} from "@/lib/deviceSettingGroups"; +import { ALL_DEVICE_SETTING_KEYS } from "@/lib/settingsDisplay"; + +describe("deviceSettingGroups", () => { + // The guard that matters: a key added to the manifest must either land in a + // group or be deliberately hidden. Without this, a new device setting simply + // never appears on the screen and nobody notices. + it("places every device-scoped key in exactly one group or hides it deliberately", () => { + const grouped = new Map(); + for (const group of groupDeviceSettings()) { + for (const key of group.keys) { + grouped.set(key, [...(grouped.get(key) ?? []), group.id]); + } + } + const hidden = new Set(hiddenDeviceSettingKeys()); + + const unplaced = ALL_DEVICE_SETTING_KEYS.filter((key) => !grouped.has(key) && !hidden.has(key)); + expect(unplaced).toEqual([]); + + const duplicated = [...grouped.entries()].filter(([, groups]) => groups.length > 1); + expect(duplicated).toEqual([]); + }); + + it("puts each setting where someone would look for it", () => { + expect(groupForDeviceSetting("player.hdr_enabled")).toBe("picture"); + expect(groupForDeviceSetting("player.audio_sync_ms")).toBe("sound"); + expect(groupForDeviceSetting("playback.audio_language")).toBe("sound"); + expect(groupForDeviceSetting("playback.subtitle_mode")).toBe("subtitles"); + expect(groupForDeviceSetting("player.subtitle_sync_ms")).toBe("subtitles"); + expect(groupForDeviceSetting("playback.auto_play_next")).toBe("episodes"); + }); + + it("keeps appearance settings off the device screen", () => { + expect(groupForDeviceSetting("ui.theme")).toBeNull(); + expect(groupForDeviceSetting("ui.library_page_state")).toBeNull(); + }); + + it("returns groups in reading order and omits empty ones", () => { + const ids = groupDeviceSettings().map((group) => group.id); + expect(ids).toEqual(["picture", "sound", "subtitles", "episodes"]); + + const single = groupDeviceSettings(["player.hdr_enabled"]); + expect(single.map((group) => group.id)).toEqual(["picture"]); + }); +}); diff --git a/web/src/lib/deviceSettingGroups.ts b/web/src/lib/deviceSettingGroups.ts new file mode 100644 index 00000000..44b5dd20 --- /dev/null +++ b/web/src/lib/deviceSettingGroups.ts @@ -0,0 +1,125 @@ +import { SETTING_DEFINITIONS, type SettingKey } from "@/lib/settingsContract"; +import { ALL_DEVICE_SETTING_KEYS } from "@/lib/settingsDisplay"; + +/** + * Device settings, grouped by what they affect rather than by manifest order. + * + * The manifest's own `category` is close but not sufficient: `player.*` holds + * both picture and sound keys, and `playback.*` spans subtitles and episode + * behaviour. Someone looking for "why does the sound lag" reads down a Sound + * heading, not down an alphabetical list of 30 keys. + * + * The small override table below is the only per-key knowledge in the settings + * UI, and it is checked by a test that every device-scoped key lands in exactly + * one group — so a key added to the manifest cannot silently disappear from + * this screen. + */ +export type DeviceSettingGroupId = "picture" | "sound" | "subtitles" | "episodes"; + +export interface DeviceSettingGroup { + id: DeviceSettingGroupId; + title: string; + /** Shown beside the title; names the device so the scope stays concrete. */ + description: string; + keys: SettingKey[]; +} + +const GROUP_META: Record = { + picture: { title: "Picture", description: "How video looks on this device" }, + sound: { title: "Sound", description: "Audio on this device" }, + subtitles: { title: "Subtitles", description: "On this device" }, + episodes: { title: "Episodes", description: "What happens between episodes" }, +}; + +const GROUP_ORDER: DeviceSettingGroupId[] = ["picture", "sound", "subtitles", "episodes"]; + +/** Keys whose group is not implied by their manifest category. */ +const EXPLICIT_GROUPS: Partial> = { + "playback.audio_language": "sound", + "playback.subtitle_language": "subtitles", + "playback.subtitle_mode": "subtitles", + "playback.show_forced_subtitles": "subtitles", + "playback.subtitle_appearance": "subtitles", + "playback.preferred_quality": "picture", + "playback.max_bitrate_kbps": "picture", + "playback.auto_skip_intro": "episodes", + "playback.auto_skip_credits": "episodes", + "playback.auto_skip_recap": "episodes", + "playback.auto_play_next": "episodes", + "playback.auto_play_next_preview": "episodes", + "playback.next_up_prompt_seconds": "episodes", + "player.hdr_enabled": "picture", + "player.dolby_vision_enabled": "picture", + "player.dv_profile7_hdr10_fallback": "picture", + "player.match_frame_rate": "picture", + "player.video_gravity": "picture", + "player.orientation_mode": "picture", + "player.seek_cache_enabled": "picture", + "player.audio_sync_ms": "sound", + "player.playback_speed": "sound", + "player.subtitle_sync_ms": "subtitles", + "player.sleep_timer_default_minutes": "episodes", +}; + +/** + * Keys deliberately kept off this screen. + * + * `ui.*` device overrides exist in the contract but belong to the Appearance + * screen, which already edits them at profile scope; showing them here would + * give one setting two homes. `ui.library_page_state` is remembered browse + * state rather than a preference — it has no control in the manifest at all. + */ +const HIDDEN_KEYS = new Set([ + "ui.theme", + "ui.text_scale", + "ui.text_weight", + "ui.high_contrast", + "ui.library_page_state", + "ui.remember_library_page_state", +]); + +export function groupForDeviceSetting(key: SettingKey): DeviceSettingGroupId | null { + if (HIDDEN_KEYS.has(key)) return null; + const explicit = EXPLICIT_GROUPS[key]; + if (explicit) return explicit; + + // Anything new falls back to its manifest category, so an added key shows up + // somewhere sensible rather than vanishing. + const definition = SETTING_DEFINITIONS[key]; + switch (definition?.category) { + case "player": + case "playback": + return "picture"; + default: + return null; + } +} + +/** The groups to render, in reading order, with empty groups omitted. */ +export function groupDeviceSettings( + keys: readonly SettingKey[] = ALL_DEVICE_SETTING_KEYS, +): DeviceSettingGroup[] { + const byGroup = new Map(); + for (const key of keys) { + const group = groupForDeviceSetting(key); + if (!group) continue; + const existing = byGroup.get(group); + if (existing) { + existing.push(key); + } else { + byGroup.set(group, [key]); + } + } + + return GROUP_ORDER.filter((id) => (byGroup.get(id)?.length ?? 0) > 0).map((id) => ({ + id, + title: GROUP_META[id].title, + description: GROUP_META[id].description, + keys: byGroup.get(id) ?? [], + })); +} + +/** Every device-scoped key this screen deliberately does not show. */ +export function hiddenDeviceSettingKeys(): SettingKey[] { + return ALL_DEVICE_SETTING_KEYS.filter((key) => HIDDEN_KEYS.has(key)); +} diff --git a/web/src/lib/documentTitle.ts b/web/src/lib/documentTitle.ts index 1485c0c9..04a0e72c 100644 --- a/web/src/lib/documentTitle.ts +++ b/web/src/lib/documentTitle.ts @@ -45,6 +45,7 @@ const ADMIN_TITLES: Record = { "api-keys": "Admin API Keys", collections: "Admin Collections", devices: "Admin Devices", + "settings/devices": "Your Devices", diagnostics: "Admin Client Diagnostics", history: "Admin Playback History", "history-import": "Admin History Import", diff --git a/web/src/lib/languageOptions.ts b/web/src/lib/languageOptions.ts index 5c921179..9a723254 100644 --- a/web/src/lib/languageOptions.ts +++ b/web/src/lib/languageOptions.ts @@ -32,11 +32,13 @@ export function withCurrentLanguageOption( /** * Suggested values for one open language setting. * - * The generated contract list is the stable floor. A newer server may add - * values observed in this deployment's catalog, and the current stored tag is - * always synthesized into the list so an open value such as `pt-BR` never - * leaves a select with no matching row. True language aliases are de-duplicated, - * with the exact current wire value winning so the control remains selected. + * The generated contract list is the stable floor. The server adds observed + * original languages for catalog.metadata_language only (the audio/subtitle + * track scans were too expensive; those pickers offer free entry via + * LanguageSelect's "Other…" instead), and the current stored tag is always + * synthesized into the list so an open value such as `pt-BR` never leaves a + * select with no matching row. True language aliases are de-duplicated, with + * the exact current wire value winning so the control remains selected. */ export function namedLanguageOptionsFor( key: SettingKey, diff --git a/web/src/pages/SettingsLayout.tsx b/web/src/pages/SettingsLayout.tsx index 1a286777..561f430a 100644 --- a/web/src/pages/SettingsLayout.tsx +++ b/web/src/pages/SettingsLayout.tsx @@ -16,6 +16,7 @@ import { Server, Sparkles, Bell, + MonitorSmartphone, } from "lucide-react"; // Sparkles is used by the Personalization nav entry below. import type { LucideIcon } from "lucide-react"; @@ -49,6 +50,12 @@ interface NavSection { const settingIndex = (...labels: string[]) => labels.map((label) => ({ label })); +/** + * Settings pages that manage their own multi-column layout, so the shell's + * reading-width cap would squeeze them instead of helping. + */ +const WIDE_SETTINGS_PAGES = new Set(["devices"]); + const NAV_SECTIONS: NavSection[] = [ { label: "Playback", @@ -284,6 +291,37 @@ const NAV_SECTIONS: NavSection[] = [ { label: "Account", items: [ + { + path: "devices", + label: "Your Devices", + icon: MonitorSmartphone, + description: "Settings for each device you watch on", + keywords: [ + "devices", + "tv", + "phone", + "tablet", + "browser", + "this device", + "forget device", + "hdr", + "dolby vision", + "sound delay", + "lip sync", + ], + settings: settingIndex( + "Video quality", + "Data use limit", + "HDR", + "Dolby Vision", + "Play Dolby Vision films as HDR10", + "Match content frame rate", + "How video fills the screen", + "Audio sync offset", + "Subtitle sync offset", + "Forget this device", + ), + }, { path: "notifications", label: "Notifications", @@ -358,6 +396,9 @@ export default function SettingsLayout() { const segments = location.pathname.split("/"); const activeSegment = segments[2] || "playback"; const canManageProfiles = actingAdmin || profile?.is_primary === true; + // Most settings pages are a single column of rows and read best measured. + // A page that is itself two panes needs the room, so it opts out. + const wideSetting = WIDE_SETTINGS_PAGES.has(activeSegment); const visibleSections = useMemo( () => @@ -469,7 +510,7 @@ export default function SettingsLayout() {
-
+
diff --git a/web/src/pages/settings/DeviceSettings.tsx b/web/src/pages/settings/DeviceSettings.tsx new file mode 100644 index 00000000..e8c50067 --- /dev/null +++ b/web/src/pages/settings/DeviceSettings.tsx @@ -0,0 +1,447 @@ +import { useEffect, useMemo, useState } from "react"; +import { ChevronLeft, Info, ShieldCheck, Trash2, Users } from "lucide-react"; +import { toast } from "sonner"; + +import type { UserDevice } from "@/api/types"; +import { Button } from "@/components/ui/button"; +import { Skeleton } from "@/components/ui/skeleton"; +import { + classifyPlatform, + PlatformIcon, + platformKindLabel, +} from "@/components/admin/deviceOverrides"; +import { DeviceList, lastSeenLabel } from "@/components/settings/DeviceList"; +import { DeviceSettingGroups } from "@/components/settings/DeviceSettingGroups"; +import { useClearDeviceSettings, useForgetDevice, useMyDevices } from "@/hooks/queries/devices"; +import { + useClearSettingValue, + useEffectiveSettings, + useSetSettingValue, +} from "@/hooks/queries/settingValues"; +import { useCurrentProfile } from "@/hooks/useCurrentProfile"; +import { useIsActingAdmin } from "@/hooks/useIsActingAdmin"; +import { ALL_DEVICE_SETTING_KEYS } from "@/lib/settingsDisplay"; +import type { SettingKey } from "@/lib/settingsContract"; +import { cn } from "@/lib/utils"; + +/** + * "Your devices" — see and change how Silo behaves on each device you watch on. + * + * Two things make this screen different from the admin device console. Every + * device is editable from wherever you are, because the settings API accepts an + * explicit device rather than only the one in the request headers. And the + * household parent can switch to the whole household, so a parent can fix a + * kid's iPad without borrowing it. + */ +export default function DeviceSettings() { + const actingAdmin = useIsActingAdmin(); + const { profile } = useCurrentProfile(); + const canSeeHousehold = actingAdmin || profile?.is_primary === true; + + const [household, setHousehold] = useState(false); + const [search, setSearch] = useState(""); + const [profileFilter, setProfileFilter] = useState(null); + const [selectedId, setSelectedId] = useState(null); + /** + * On a phone the list and the settings are two screens, not two panes. + * + * Stacking them vertically put a device list over a thousand pixels tall + * above the first setting, so reaching "turn HDR off" meant scrolling past + * every other device first. Below xl the list hands off to the detail view + * and a back control returns; from xl up both are visible and this is + * ignored. + */ + const [showDetailOnMobile, setShowDetailOnMobile] = useState(false); + // One clock for the whole screen, taken once per mount: relative labels only + // need to be right to the minute, and reading the clock during render is not + // something a pure component may do. + const [now] = useState(() => Date.now()); + + const { data: devices = [], isLoading } = useMyDevices({ + household: household && canSeeHousehold, + }); + + // Filtering to one person must not leave someone else's device open in the + // detail pane — the list and the pane would then disagree about who is being + // edited, which is the one thing this screen cannot afford to be vague about. + const selectable = useMemo( + () => + profileFilter ? devices.filter((device) => device.profile_id === profileFilter) : devices, + [devices, profileFilter], + ); + + // Default to the device you are on: it is the one you can check the effect of + // immediately, and the one most people came here for. + const selected = useMemo(() => { + if (selectable.length === 0) return null; + return selectable.find((device) => device.device_id === selectedId) ?? selectable[0]; + }, [selectable, selectedId]); + + useEffect(() => { + if (selected && selected.device_id !== selectedId) { + setSelectedId(selected.device_id); + } + }, [selected, selectedId]); + + return ( +
+ {/* Hidden below xl while the detail view is open: on a phone that screen + is about one device, and its own header says which. */} +
+

Your devices

+

+ Every phone, tablet, TV and browser you watch on. Pick one to see what's set + differently there and change it — from here, whichever device you're holding. +

+
+ + {canSeeHousehold ? ( +
+ { + setHousehold(next); + if (!next) setProfileFilter(null); + }} + count={devices.length} + /> +
+ ) : null} + +
+ {isLoading ? ( + + ) : ( +
+ { + setSelectedId(device.device_id); + setShowDetailOnMobile(true); + // Swapping the panes leaves the scroll position where the list + // was, which on a phone lands mid-settings with no context. + if (window.matchMedia("(max-width: 1279px)").matches) { + window.scrollTo({ top: 0, behavior: "instant" }); + } + }} + search={search} + onSearchChange={setSearch} + groupByProfile={household && canSeeHousehold} + profileFilter={profileFilter} + onProfileFilterChange={setProfileFilter} + ownProfileId={profile?.id} + now={now} + /> +
+ )} + + {isLoading ? ( + + ) : selected ? ( +
+ { + setShowDetailOnMobile(false); + window.scrollTo({ top: 0, behavior: "instant" }); + }} + /> +
+ ) : ( +

+ No devices yet. They appear here as you sign in on them. +

+ )} +
+
+ ); +} + +function HouseholdSwitch({ + household, + onChange, + count, +}: { + household: boolean; + onChange: (value: boolean) => void; + count: number; +}) { + return ( +
+ onChange(false)}> + Just mine + + onChange(true)}> + + Everyone{household && count > 0 ? ` (${count})` : ""} + +
+ ); +} + +function SwitchButton({ + active, + onClick, + children, +}: { + active: boolean; + onClick: () => void; + children: React.ReactNode; +}) { + return ( + + ); +} + +function DeviceDetail({ + device, + actingProfileId, + now, + onBack, +}: { + device: UserDevice; + actingProfileId: string; + now: number; + /** Returns to the list below xl, where the two are separate screens. */ + onBack: () => void; +}) { + // Acting for someone else changes the copy throughout: the banner, the reset + // labels, and every mutation's identity. + const forSomeoneElse = Boolean(device.profile_id) && device.profile_id !== actingProfileId; + const ownerLabel = forSomeoneElse ? `${device.profile_name}'s` : "your"; + const targetProfileId = forSomeoneElse ? device.profile_id : undefined; + + const { data: settings = {}, isLoading } = useEffectiveSettings({ + keys: ALL_DEVICE_SETTING_KEYS, + deviceId: device.device_id, + profileId: targetProfileId, + }); + + const setValue = useSetSettingValue(); + const clearValue = useClearSettingValue(); + const clearDevice = useClearDeviceSettings(); + const forgetDevice = useForgetDevice(); + + const identity = { + scope: "profile_device" as const, + deviceId: device.device_id, + profileId: targetProfileId, + }; + + const changedCount = device.changed_count; + const kind = classifyPlatform(device.device_platform); + + return ( +
+ + +
+
+ + + +
+ {/* Wraps rather than truncating: on a phone the name is the whole + subject of the screen, and "Living Room Apple…" is not it. */} +

+ {device.device_name || "Unknown device"} +

+

+ {[ + platformKindLabel(kind), + device.is_current_device ? "using now" : lastSeenLabel(device.last_seen_at, now), + changedCount > 0 + ? `${changedCount} ${changedCount === 1 ? "thing" : "things"} set differently` + : "nothing changed here", + ].join(" · ")} +

+
+
+
+ {changedCount > 0 ? ( + + ) : null} + {!device.is_current_device ? ( + + ) : null} +
+
+ + {forSomeoneElse ? ( + }> + + You're changing {device.profile_name}'s settings, not your own. + {" "} + {device.profile_name} will see these change on this device. + + ) : null} + + {/* The scope sentence is required copy and must not be paraphrased away, + but five lines of prose is a wall on a phone. The mandated clause + leads; the elaboration is there for anyone who wants it. */} + }> + + These apply to{" "} + + this device, for {forSomeoneElse ? `${device.profile_name}'s` : "your"} profile only + + . + + + {forSomeoneElse ? `${device.profile_name}'s` : "Your"} other devices, and anyone else who + uses this one, are unaffected. + {!device.is_current_device + ? " This device picks up your changes the next time it's used." + : ""} + + + + {isLoading ? ( + + ) : ( + + setValue.mutate( + { key, value, identity }, + { + onError: (error) => + toast.error(error instanceof Error ? error.message : "Couldn't save"), + }, + ) + } + onReset={(key: SettingKey) => + clearValue.mutate( + { key, identity }, + { + onError: (error) => + toast.error(error instanceof Error ? error.message : "Couldn't reset"), + }, + ) + } + /> + )} + + {forSomeoneElse ? ( + }> + What you can't see here.{" "} + This page shows how Silo is set up on each device — not what anyone watched. Viewing + history stays private to each profile. + + ) : null} +
+ ); +} + +function Callout({ + tone, + icon, + children, +}: { + tone: "info" | "warning" | "muted"; + icon: React.ReactNode; + children: React.ReactNode; +}) { + return ( +
+ + {icon} + +

{children}

+
+ ); +} diff --git a/web/src/pages/settings/LibrarySettings.tsx b/web/src/pages/settings/LibrarySettings.tsx index 1c4f4172..4924ad68 100644 --- a/web/src/pages/settings/LibrarySettings.tsx +++ b/web/src/pages/settings/LibrarySettings.tsx @@ -1,6 +1,7 @@ import { useEffect, useId, useState, type ReactNode } from "react"; import type { UserLibrary } from "@/api/types"; import { Badge } from "@/components/ui/badge"; +import { LanguageSelect } from "@/components/settings/LanguageSelect"; import { SettingRow } from "@/components/settings/SettingRow"; import { SettingsGroup } from "@/components/settings/SettingsGroup"; import { Button } from "@/components/ui/button"; @@ -48,7 +49,7 @@ import { NONE_VALUE, SUBTITLE_MODE_OPTIONS, } from "./libraryPlaybackPreferences"; -import { namedLanguageOptionsFor } from "@/lib/languageOptions"; +import { namedLanguageOptionsFor, type SettingOption } from "@/lib/languageOptions"; import { toast } from "sonner"; import { ChevronDown, ChevronRight, Eye, EyeOff, GripVertical, RotateCcw } from "lucide-react"; import { @@ -174,6 +175,46 @@ function PlaybackField({ ); } +/** PlaybackField for open language values, with the shared "Other…" entry. */ +function LanguageField({ + label, + value, + options, + disabled, + hint, + onChange, + children, +}: { + label: string; + value: string; + options: readonly SettingOption[]; + disabled?: boolean; + hint?: string; + onChange: (value: string) => void; + children: ReactNode; +}) { + const controlId = useId(); + + return ( +
+ + + {children} + + {hint &&

{hint}

} +
+ ); +} + function SortableLibraryCard({ id, children }: { id: number; children: React.ReactNode }) { const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({ id, @@ -371,9 +412,10 @@ function LibraryCard({ automatically.

- handlePlaybackChange("audioLanguage", value)} @@ -381,16 +423,12 @@ function LibraryCard({ {buildInheritedLanguageLabel(profileDefaults.audioLanguage ?? "")} - {audioLanguageOptions.map((language) => ( - - {language.label} - - ))} - + - handlePlaybackChange("subtitleLanguage", value)} @@ -399,12 +437,7 @@ function LibraryCard({ {buildInheritedSubtitleLanguageLabel(profileDefaults.subtitleLanguage ?? "")} None - {subtitleLanguageOptions.map((language) => ( - - {language.label} - - ))} - + ( -
- + No preference +
)} /> diff --git a/web/src/pages/settings/SubtitleAppearanceSettings.tsx b/web/src/pages/settings/SubtitleAppearanceSettings.tsx index 66a93969..def5c636 100644 --- a/web/src/pages/settings/SubtitleAppearanceSettings.tsx +++ b/web/src/pages/settings/SubtitleAppearanceSettings.tsx @@ -1,6 +1,7 @@ import { useId, useState, type ReactNode } from "react"; import { RotateCcw } from "lucide-react"; import { SettingsGroup } from "@/components/settings/SettingsGroup"; +import { LanguageSelect } from "@/components/settings/LanguageSelect"; import { Button } from "@/components/ui/button"; import { Label } from "@/components/ui/label"; import { Slider } from "@/components/ui/slider"; @@ -234,34 +235,27 @@ export default function SubtitleAppearanceSettings() { description="Pick a subtitle language or leave subtitles off by default." > {({ id, descriptionId }) => ( - + +
)} diff --git a/web/src/player/hooks/useTranscodeQuality.ts b/web/src/player/hooks/useTranscodeQuality.ts index 26935652..6dcc062c 100644 --- a/web/src/player/hooks/useTranscodeQuality.ts +++ b/web/src/player/hooks/useTranscodeQuality.ts @@ -97,7 +97,11 @@ export const COMPATIBILITY_QUALITY_ID = "compatibility"; // Quality-menu bitrate label: Mbps with collapsed integers ("8 Mbps", not // "8.0 Mbps") — a deliberately different display policy than the canonical // formatBitrate/formatMbpsFromKbps in @/lib/mediaFormat. -function formatQualityBitrate(kbps: number): string { +// +// Exported so the device-settings bandwidth cap reads the same as the in-player +// switcher; the two pick from the same ladder and should not disagree about how +// to spell a number. +export function formatQualityBitrate(kbps: number): string { if (kbps >= 1000) { const mbps = kbps / 1000; return mbps % 1 === 0 ? `${mbps} Mbps` : `${mbps.toFixed(1)} Mbps`;