Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions config/apps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,18 @@ apps:
spoo-raycast:
name: Raycast Extension
icon: raycast.svg
description: Shorten links from Raycast
description: Shorten and manage your spoo.me links from Raycast
verified: true
status: coming_soon
status: live
type: device_auth
redirect_uris:
- https://raycast.com/redirect?*
links:
raycast: https://www.raycast.com/spoo-me/spoo
permissions:
- Access your spoo.me account
- Create and manage short URLs
- View and export your analytics

spoo-vscode:
name: VS Code Extension
Expand Down
16 changes: 12 additions & 4 deletions routes/auth/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,15 @@ def _device_error(request: Request, error: str, status_code: int = 400) -> Respo


def _build_callback_redirect(
code: str, state: str, redirect_uri: str, app: AppEntry
code: str,
state: str,
redirect_uri: str,
app: AppEntry,
svc: DeviceAuthSvc,
) -> RedirectResponse:
"""Build the redirect to the callback page or a registered redirect_uri."""
params = urlencode({"code": code, "state": state})
if redirect_uri and redirect_uri in app.redirect_uris:
if redirect_uri and svc.validate_redirect_uri(redirect_uri, app):
separator = "&" if "?" in redirect_uri else "?"
return RedirectResponse(f"{redirect_uri}{separator}{params}", status_code=302)
return RedirectResponse(f"/auth/device/callback?{params}", status_code=302)
Expand Down Expand Up @@ -122,7 +126,9 @@ async def device_login(
code = await device_auth_service.create_device_auth_code(
profile.id, profile.email, app_id=app_id
)
return _build_callback_redirect(code, state, redirect_uri, app)
return _build_callback_redirect(
code, state, redirect_uri, app, device_auth_service
)

# No grant: show consent screen
csrf_token = generate_secure_token(_CSRF_TOKEN_BYTES)
Expand Down Expand Up @@ -193,7 +199,9 @@ async def device_consent_approve(
)

# Clear CSRF cookie and redirect
response = _build_callback_redirect(code, state, redirect_uri, app)
response = _build_callback_redirect(
code, state, redirect_uri, app, device_auth_service
)
response.delete_cookie(_CSRF_COOKIE_NAME)
return response

Expand Down
18 changes: 16 additions & 2 deletions services/auth/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,22 @@ def resolve_app(self, app_id: str) -> AppEntry | None:
return entry if entry and entry.is_live_device_app() else None

def validate_redirect_uri(self, redirect_uri: str, app: AppEntry) -> bool:
"""Return True if redirect_uri is empty or in the app's allowlist."""
return not redirect_uri or redirect_uri in app.redirect_uris
"""Return True if redirect_uri is empty, exact-matches an allowlist
entry, or prefix-matches an entry ending with ``*``.

The ``*`` suffix is used for OAuth clients (e.g. Raycast) that append
a varying query string to a fixed redirect URL. Only exact or
explicit-wildcard entries are accepted; implicit wildcards are not.
"""
if not redirect_uri:
return True
for allowed in app.redirect_uris:
if allowed.endswith("*"):
if redirect_uri.startswith(allowed[:-1]):
Comment thread
Zingzy marked this conversation as resolved.
return True
elif redirect_uri == allowed:
return True
return False
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
Comment thread
Zingzy marked this conversation as resolved.
Comment thread
Zingzy marked this conversation as resolved.

async def create_device_auth_code(
self, user_id: ObjectId, email: str, app_id: str | None = None
Expand Down
8 changes: 7 additions & 1 deletion tests/integration/test_device_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,13 @@ def device_auth_svc():
# resolve_app and validate_redirect_uri are sync — must not be coroutines
svc.resolve_app = MagicMock(side_effect=_resolve_app)
svc.validate_redirect_uri = MagicMock(
side_effect=lambda uri, app: not uri or uri in app.redirect_uris
side_effect=lambda uri, app: (
not uri
or any(
uri.startswith(a[:-1]) if a.endswith("*") else uri == a
for a in app.redirect_uris
)
)
)
return svc

Expand Down
77 changes: 77 additions & 0 deletions tests/unit/services/test_redirect_uri.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
"""Unit tests for wildcard redirect URI matching."""

from __future__ import annotations

from schemas.models.app import AppEntry
from services.auth.device import DeviceAuthService


def _app(redirect_uris: list[str]) -> AppEntry:
return AppEntry(name="test", description="test app", redirect_uris=redirect_uris)


class TestValidateRedirectUri:
"""Tests for DeviceAuthService.validate_redirect_uri."""

def setup_method(self):
self.svc = DeviceAuthService.__new__(DeviceAuthService)

def test_empty_uri_allowed(self):
app = _app(["https://example.com/callback"])
assert self.svc.validate_redirect_uri("", app) is True

def test_exact_match(self):
app = _app(["https://example.com/callback"])
assert (
self.svc.validate_redirect_uri("https://example.com/callback", app) is True
)

def test_exact_no_match(self):
app = _app(["https://example.com/callback"])
assert self.svc.validate_redirect_uri("https://evil.com/callback", app) is False

def test_wildcard_query_match(self):
app = _app(["https://raycast.com/redirect?*"])
assert (
self.svc.validate_redirect_uri(
"https://raycast.com/redirect?packageName=spoo&state=abc", app
)
is True
)

def test_wildcard_prefix_only(self):
app = _app(["https://raycast.com/redirect?*"])
assert (
self.svc.validate_redirect_uri("https://raycast.com/redirect?", app) is True
)

def test_wildcard_rejects_different_path(self):
app = _app(["https://raycast.com/redirect?*"])
assert (
self.svc.validate_redirect_uri("https://raycast.com/redirected", app)
is False
)

def test_wildcard_rejects_different_host(self):
app = _app(["https://raycast.com/redirect?*"])
assert (
self.svc.validate_redirect_uri("https://evil.com/redirect?foo=bar", app)
is False
)

def test_no_uris_rejects(self):
app = _app([])
assert self.svc.validate_redirect_uri("https://example.com", app) is False

def test_multiple_uris_mixed(self):
app = _app(["https://a.com/cb", "https://b.com/redirect?*"])
assert self.svc.validate_redirect_uri("https://a.com/cb", app) is True
assert self.svc.validate_redirect_uri("https://b.com/redirect?x=1", app) is True
assert self.svc.validate_redirect_uri("https://c.com/cb", app) is False

def test_bare_star_matches_everything(self):
"""A bare '*' entry matches any URI — intentional if configured."""
app = _app(["*"])
assert (
self.svc.validate_redirect_uri("https://anything.com/whatever", app) is True
)
Comment on lines +72 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Bare * allowlist is a footgun — consider rejecting it explicitly.

Documenting this behavior in a test locks in a configuration mode where a single typo (- "*") in apps.yaml silently disables redirect URI validation for an app, allowing an attacker to receive a freshly minted device auth code on any host. There's no legitimate need for a bare * entry today (every real app has a fixed callback or a host-anchored prefix), so it's safer to treat it as a misconfiguration.

Suggest either:

  • Rejecting bare * (and any allowlist entry whose prefix collapses to "" after stripping the trailing *) inside DeviceAuthService.validate_redirect_uri and/or at app-registry load time, and replacing this test with one that asserts the rejection; or
  • At minimum, also requiring a scheme+host before the * (e.g. enforce that a[:-1] contains :// and a /).

Tracking this at the service layer is preferable to relying on YAML hygiene.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/services/test_redirect_uri.py` around lines 72 - 77, The test in
test_bare_star_matches_everything locks in a dangerous behavior: a bare "*" in
an app allowlist lets any redirect pass; instead modify
DeviceAuthService.validate_redirect_uri to explicitly reject bare "*" or any
allowlist entry that collapses to an empty prefix after stripping a trailing '*'
(i.e., treat entries equal to "*" or where a[:-1].strip() == "" as invalid),
return False (or raise a validation error) for those entries, and update the
test to assert that such entries are rejected; alternatively, enforce that any
wildcard entry must include a scheme+host (ensure the substring before the
trailing '*' contains "://" and a "/") when checking in validate_redirect_uri so
misconfigured bare wildcards fail at service layer.

Loading