Skip to content

Commit 812491f

Browse files
committed
Refine device authorization flow
- device/init returns a relative verification_path; the client joins it with its own origin - Render the v2 approval screen via the named v2 router outlet (was blank) - DevicePair: RSpinner, keyboard-accessible scope chips, scrollable scopes; DevicePairShell uses always-light overlay tokens (no hex)
1 parent 519abc1 commit 812491f

23 files changed

Lines changed: 684 additions & 388 deletions

backend/alembic/versions/0084_devices_client_device_identifier.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Add client_device_identifier to devices
22
3-
Revision ID: 0080_devices_client_identifier
4-
Revises: 0079_add_rom_files_rom_id_index
3+
Revision ID: 0084_devices_client_identifier
4+
Revises: 0083_rom_category_soundtrack
55
Create Date: 2026-04-24 00:00:00.000000
66
77
"""

backend/alembic/versions/0085_client_tokens_device_id.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Add device_id FK to client_tokens
22
3-
Revision ID: 0081_client_tokens_device_id
4-
Revises: 0080_devices_client_identifier
3+
Revision ID: 0085_client_tokens_device_id
4+
Revises: 0084_devices_client_identifier
55
Create Date: 2026-04-24 00:00:00.000000
66
77
"""

backend/docker-compose.test.yml

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Local test infrastructure matching .github/workflows/pytest.yml.
2+
# Usage: docker compose -f docker-compose.test.yml up -d
3+
# uv run pytest
4+
services:
5+
mariadb:
6+
image: mariadb:10.11
7+
ports:
8+
- "3306:3306"
9+
environment:
10+
MYSQL_USER: romm_test
11+
MYSQL_PASSWORD: passwd
12+
MYSQL_DATABASE: romm_test
13+
MYSQL_ROOT_PASSWORD: passwd
14+
healthcheck:
15+
test:
16+
[
17+
"CMD",
18+
"mariadb-admin",
19+
"ping",
20+
"-h",
21+
"127.0.0.1",
22+
"-uroot",
23+
"-ppasswd",
24+
]
25+
interval: 5s
26+
timeout: 2s
27+
retries: 10
28+
29+
valkey:
30+
image: valkey/valkey:7.2
31+
ports:
32+
- "6379:6379"
33+
healthcheck:
34+
test: ["CMD", "redis-cli", "ping"]
35+
interval: 5s
36+
timeout: 2s
37+
retries: 10

backend/endpoints/device_auth.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
PENDING_TTL_SECONDS,
3939
POLL_DEFAULT_INTERVAL_SECONDS,
4040
FlowStatus,
41-
build_verification_urls,
41+
build_verification_paths,
4242
check_authorize_rate_limit,
4343
check_token_poll_rate_limit,
4444
consume_approved,
@@ -87,9 +87,7 @@ def device_auth_init(
8787
},
8888
)
8989

90-
verification_url, verification_url_complete = build_verification_urls(
91-
request, user_code
92-
)
90+
verification_path, verification_path_complete = build_verification_paths(user_code)
9391

9492
log.info(
9593
f"device_auth.init client={payload.client} "
@@ -100,8 +98,8 @@ def device_auth_init(
10098
return DeviceAuthInitResponse(
10199
device_code=device_code,
102100
user_code=user_code,
103-
verification_url=verification_url,
104-
verification_url_complete=verification_url_complete,
101+
verification_path=verification_path,
102+
verification_path_complete=verification_path_complete,
105103
expires_in=PENDING_TTL_SECONDS,
106104
interval=POLL_DEFAULT_INTERVAL_SECONDS,
107105
)
@@ -198,12 +196,14 @@ def approve(
198196
client_version = data.get("client_version")
199197
if client_version is not None:
200198
update_data["client_version"] = client_version
201-
db_device_handler.update_device(
202-
device_id=existing.id,
203-
user_id=request.user.id,
204-
data=update_data,
199+
device = (
200+
db_device_handler.update_device(
201+
device_id=existing.id,
202+
user_id=request.user.id,
203+
data=update_data,
204+
)
205+
or existing
205206
)
206-
device = existing
207207
else:
208208
device = db_device_handler.add_device(
209209
Device(

backend/endpoints/responses/device_auth.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,15 @@ class DeviceAuthInitPayload(BaseModel):
1515
class DeviceAuthInitResponse(BaseModel):
1616
device_code: str
1717
user_code: str
18-
verification_url: str
19-
verification_url_complete: str
18+
verification_path: str = Field(
19+
description=(
20+
"Relative web-UI path (/pair/device). The client joins it with the "
21+
"origin it was configured to reach; the server is origin-agnostic."
22+
)
23+
)
24+
verification_path_complete: str = Field(
25+
description="Same path with ?user_code= appended, for QR display."
26+
)
2027
expires_in: int
2128
interval: int
2229

backend/tests/endpoints/test_device_auth.py

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ def test_valid_request_returns_all_fields(self, client):
7070

7171
assert len(body["device_code"]) == df.DEVICE_CODE_BYTES * 2
7272
assert len(body["user_code"]) == df.USER_CODE_LENGTH
73-
assert body["verification_url"].endswith("/pair/device")
74-
assert body["user_code"] in body["verification_url_complete"]
73+
assert body["verification_path"].endswith("/pair/device")
74+
assert body["user_code"] in body["verification_path_complete"]
7575
assert body["expires_in"] == df.PENDING_TTL_SECONDS
7676
assert body["interval"] == df.POLL_DEFAULT_INTERVAL_SECONDS
7777

@@ -550,18 +550,17 @@ def test_per_ip_rate_limit(self, client):
550550
assert last_status == status.HTTP_429_TOO_MANY_REQUESTS
551551

552552

553-
class TestVerificationUrls:
554-
def test_verification_url_is_server_origin(self, client):
555-
# Never echoes client-supplied callback data — verification_url is
556-
# always the server's own origin + /pair/device. This is the XSS
557-
# surface removal the plan leans on.
553+
class TestVerificationPaths:
554+
def test_verification_path_is_relative(self, client):
555+
# The path is a fixed server constant (/pair/device); client metadata
556+
# like the device name is never interpolated into it.
558557
body = _authorize(
559558
client,
560559
payload={**AUTHORIZE_PAYLOAD, "name": "javascript:alert(1)"},
561560
)
562-
assert body["verification_url"].endswith("/pair/device")
563-
assert "javascript:" not in body["verification_url"]
564-
assert "javascript:" not in body["verification_url_complete"]
561+
assert body["verification_path"] == "/pair/device"
562+
assert "://" not in body["verification_path"]
563+
assert "javascript:" not in body["verification_path_complete"]
565564

566565

567566
class TestHelperFunctions:
@@ -711,9 +710,9 @@ def test_grout_pairs_ingests_play_session(
711710
)
712711
assert start_resp.status_code == status.HTTP_201_CREATED
713712
init = start_resp.json()
714-
assert init["verification_url"].endswith("/pair/device")
713+
assert init["verification_path"].endswith("/pair/device")
715714

716-
# Device displays QR from verification_url_complete; user scans; user
715+
# Device displays QR from verification_path_complete; user scans; user
717716
# is routed to /pair/device?user_code=... and authenticates.
718717

719718
# --- 2. Web UI fetches pending metadata ---

backend/utils/device_auth.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from typing import Any, Final
1212

1313
from fastapi import HTTPException, Request, status
14+
from yarl import URL
1415

1516
from handler.redis_handler import sync_cache
1617
from utils.client_tokens import PAIR_ALPHABET
@@ -54,16 +55,19 @@ def generate_user_code() -> str:
5455
return "".join(secrets.choice(PAIR_ALPHABET) for _ in range(USER_CODE_LENGTH))
5556

5657

57-
def build_verification_urls(request: Request, user_code: str) -> tuple[str, str]:
58-
"""Build server-origin URLs only — never from client input.
58+
def build_verification_paths(user_code: str) -> tuple[str, str]:
59+
"""Return the web-UI approval paths.
5960
60-
This is intentional: no user-supplied callback URLs, so no javascript:/data:
61-
XSS surface on the approval page.
61+
Only a fixed relative path is returned; the client joins it with the origin
62+
it was configured to reach, so the server stays origin-agnostic (in
63+
development the web UI and API run on different ports). The path is a server
64+
constant and never incorporates client input.
6265
"""
63-
base = str(request.base_url).rstrip("/")
64-
verification_url = f"{base}/pair/device"
65-
verification_url_complete = f"{verification_url}?user_code={user_code}"
66-
return verification_url, verification_url_complete
66+
verification_path = "/pair/device"
67+
verification_path_complete = str(
68+
URL(verification_path).with_query(user_code=user_code)
69+
)
70+
return verification_path, verification_path_complete
6771

6872

6973
def check_authorize_rate_limit(request: Request) -> None:

frontend/src/__generated__/models/ClientTokenAdminSchema.ts

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/src/__generated__/models/DeviceAuthApprovePayload.ts

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/src/__generated__/models/DeviceAuthApproveResponse.ts

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)