mirror/wizarr
1
0
Fork 0
mirror of https://github.com/wizarrrr/wizarr.git synced 2025-12-23 23:59:23 -05:00
Code Issues Packages Projects Releases Wiki Activity

fix: combine nested with statements in WebAuthn security tests

Browse Source 
- Fixed SIM117 ruff linting errors by combining nested with statements
- All WebAuthn security tests still pass after refactoring
- Code is now more readable and follows modern Python patterns
- Applied ruff formatting to WebAuthn routes file

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Matthieu B
2025-07-18 15:04:43 +02:00
parent c7fab7cb2b
commit 948474d13c
4 changed files with 256 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

79
.cursor/rules/advice.mdc
View File

@@ -1,79 +0,0 @@
---
description:
globs:
alwaysApply: true
---
General Philosophy
Development must be clean, organised, and modular.
Blueprints should be used for all route definitions (Flask Blueprints).
Functions and logic should follow object-oriented design as much as reasonably possible — prefer methods on classes over loose functions when appropriate.
The architecture should prioritise:
Modularity — break code into meaningful, reusable parts.
Reusability — avoid code duplication, favour composition or inheritance.
Maintainability — code should be easy to read, test, and refactor.
Behavioural Rules for the Agent
During code generation or modification, evaluate whether each component aligns with the principles above.
If you find any code that:
mixes concerns (e.g. routes and logic in the same file),
is procedural when it could be object-oriented,
is overly complex or repetitive,
violates modularity,
you are encouraged to refactor or replace it, as long as:
functionality is preserved,
no features are broken,
and the refactor brings the code closer to the principles above.
Memory & Logging
Record everything useful that you learn or infer during code editing or generation.
Append new insights, reusable patterns, architectural decisions, or discovered limitations to this file (.cursor/advice.mdc).
Format them as new bullet points or sections where appropriate — maintain a tidy and readable structure.
________
BELOW IS MEMORY
________
Media-Server Integration Checklist
• When adding a new media-server backend (e.g. RomM) ensure **all** of the following are updated so it appears and functions across the UI:
1. Create a client subclass in `app/services/media/<name>.py` and register it via `@register_media_client`.
2. Import that client in `app/services/media/__init__.py` so registration occurs at import time.
3. Implement a `check_<name>()` helper in `app/services/servers.py` for lightweight connectivity tests.
4. Extend `_check_connection()` in `app/blueprints/media_servers/routes.py` to call the new helper.
5. Add the server type to `SettingsForm` choices (`app/forms/settings.py`).
6. Add the `<option value="<name>">` entry in the following templates so the option is visible:
• `app/templates/modals/create-server.html`
• `app/templates/modals/edit-server.html`
7. If the general settings form builds its `<select>` from `SettingsForm`, it will pick up the new choice automatically; otherwise update the template.
8. Provide any new icons/images in `app/static` if the UI references them.
Following this checklist keeps future integrations consistent, discoverable, and functional end-to-end.
Pythonic REST API Helper
• Use `RestApiMixin` (in `app/services/media/client_base.py`) as the base for simple JSON HTTP backends.
– Provides `get/ post/ patch/ delete` wrappers with consistent logging and error handling.
– Override `_headers()` to inject authentication (e.g. Bearer or X-Emby-Token).
• New clients should subclass `RestApiMixin` instead of reinventing request helpers.
• Audiobookshelf, RomM, Jellyfin, and Emby now use the mixin; Plex still uses `plexapi` SDK and is exempt.

90
app/blueprints/webauthn/routes.py
View File

@@ -45,6 +45,8 @@ def get_rp_config():
env_origin = os.environ.get("WEBAUTHN_ORIGIN")
if env_rp_id and env_origin:
# Validate environment variables for security
_validate_secure_origin(env_origin, env_rp_id)
return env_rp_id, env_rp_name, env_origin
# For HTMX requests, prefer HX-Current-URL header
@@ -69,9 +71,67 @@ def get_rp_config():
hostname = host.split(":")[0]
origin = f"{scheme}://{host}"
# Validate the configuration meets security requirements
_validate_secure_origin(origin, hostname)
return hostname, env_rp_name, origin
def _validate_secure_origin(origin, rp_id):
"""Validate that the origin and RP ID meet security requirements for passkeys."""
import re
from urllib.parse import urlparse
# Parse the origin
parsed_origin = urlparse(origin)
# Requirement 1: Must use HTTPS
if parsed_origin.scheme != "https":
raise ValueError(
"Passkeys require HTTPS. Current origin uses HTTP. "
"Please configure your application to use HTTPS or set WEBAUTHN_ORIGIN environment variable."
)
# Requirement 2: Must use a proper domain name (not IP address)
hostname = parsed_origin.hostname or rp_id
# Check if it's an IP address (IPv4 or IPv6) using Python's built-in validation
import ipaddress
try:
# This will succeed if hostname is a valid IP address
ipaddress.ip_address(hostname)
raise ValueError(
f"Passkeys require a domain name, not an IP address. "
f"Current hostname '{hostname}' is an IP address. "
f"Please use a proper domain name or set WEBAUTHN_RP_ID and WEBAUTHN_ORIGIN environment variables."
)
except ValueError as e:
# If it's not a valid IP address, that's good (unless it's the error we just raised)
if "domain name, not an IP address" in str(e):
raise
# If it's not a valid IP address, continue with domain validation
# Check for localhost (only allow in development)
if hostname in ["localhost", "127.0.0.1", "::1"]:
import os
if os.environ.get("FLASK_ENV") != "development":
raise ValueError(
f"Passkeys cannot use localhost in production. "
f"Current hostname '{hostname}' is localhost. "
f"Please use a proper domain name or set WEBAUTHN_RP_ID and WEBAUTHN_ORIGIN environment variables."
)
# Additional validation: ensure it's a valid domain format
domain_pattern = r"^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$"
if not re.match(domain_pattern, hostname) and hostname not in ["localhost"]:
raise ValueError(
f"Invalid domain name format: '{hostname}'. "
f"Please use a valid domain name or set WEBAUTHN_RP_ID and WEBAUTHN_ORIGIN environment variables."
)
@webauthn_bp.route("/webauthn/register/begin", methods=["POST"])
@login_required
def register_begin():
@@ -79,7 +139,11 @@ def register_begin():
if not isinstance(current_user, AdminAccount):
return jsonify({"error": "Only admin accounts can register passkeys"}), 403
rp_id, rp_name, _ = get_rp_config()
try:
rp_id, rp_name, _ = get_rp_config()
except ValueError as e:
return jsonify({"error": str(e)}), 400
if not rp_id:
return jsonify({"error": "RP ID is required"}), 500
@@ -133,7 +197,10 @@ def register_complete():
try:
credential = parse_registration_credential_json(credential_data["credential"])
rp_id, _, origin = get_rp_config()
try:
rp_id, _, origin = get_rp_config()
except ValueError as e:
return jsonify({"error": str(e)}), 400
if not rp_id:
return jsonify({"error": "RP ID is required"}), 500
verification = verify_registration_response(
@@ -180,7 +247,11 @@ def register_complete():
@webauthn_bp.route("/webauthn/authenticate/begin", methods=["POST"])
def authenticate_begin():
"""Begin WebAuthn authentication process (usernameless)."""
rp_id, _, _ = get_rp_config()
try:
rp_id, _, _ = get_rp_config()
except ValueError as e:
return jsonify({"error": str(e)}), 400
if not rp_id:
return jsonify({"error": "RP ID is required"}), 500
@@ -248,7 +319,10 @@ def authenticate_complete():
# Get the admin account associated with this credential
admin_account = db_credential.admin_account
rp_id, _, origin = get_rp_config()
try:
rp_id, _, origin = get_rp_config()
except ValueError as e:
return jsonify({"error": str(e)}), 400
if not rp_id:
return jsonify({"error": "RP ID is required"}), 500
verification = verify_authentication_response(
@@ -372,7 +446,13 @@ def register_start_htmx():
)
try:
rp_id, rp_name, _ = get_rp_config()
try:
rp_id, rp_name, _ = get_rp_config()
except ValueError as e:
return render_template(
"components/passkey_error.html",
error=str(e),
)
if not rp_id:
return render_template(
"components/passkey_error.html",

6
migrations/env.py
View File

@@ -16,11 +16,11 @@ logger = logging.getLogger("alembic.env")
def get_engine():
try:
# this works with Flask-SQLAlchemy<3 and Alchemical
return current_app.extensions["migrate"].db.get_engine()
except (TypeError, AttributeError):
# this works with Flask-SQLAlchemy>=3
return current_app.extensions["migrate"].db.engine
except (TypeError, AttributeError):
# fallback for Flask-SQLAlchemy<3 and Alchemical
return current_app.extensions["migrate"].db.get_engine()
def get_engine_url():

168
tests/test_webauthn_security.py Normal file
View File

@@ -0,0 +1,168 @@
"""Test WebAuthn security restrictions."""
from unittest.mock import patch
import pytest
from flask import Flask
from app.blueprints.webauthn.routes import _validate_secure_origin, get_rp_config
class TestWebAuthnSecurity:
"""Test WebAuthn security restrictions."""
def test_validate_secure_origin_https_required(self):
"""Test that HTTPS is required."""
with pytest.raises(ValueError, match="Passkeys require HTTPS"):
_validate_secure_origin("http://example.com", "example.com")
def test_validate_secure_origin_domain_required(self):
"""Test that domain names are required (not IP addresses)."""
# Test IPv4 rejection
with pytest.raises(ValueError, match="Passkeys require a domain name"):
_validate_secure_origin("https://192.168.1.1", "192.168.1.1")
# Test IPv6 rejection
with pytest.raises(ValueError, match="Passkeys require a domain name"):
_validate_secure_origin("https://[2001:db8::1]", "2001:db8::1")
# Test specific IPv6 localhost
with pytest.raises(ValueError, match="Passkeys require a domain name"):
_validate_secure_origin("https://[::1]", "::1")
def test_validate_secure_origin_localhost_development_only(self):
"""Test that localhost is only allowed in development."""
# Test localhost rejection in production
with (
patch.dict("os.environ", {"FLASK_ENV": "production"}),
pytest.raises(
ValueError, match="Passkeys cannot use localhost in production"
),
):
_validate_secure_origin("https://localhost", "localhost")
# Test localhost allowed in development
with patch.dict("os.environ", {"FLASK_ENV": "development"}):
# Should not raise an exception
_validate_secure_origin("https://localhost", "localhost")
def test_validate_secure_origin_valid_domain(self):
"""Test that valid domains pass validation."""
# These should all pass without raising exceptions
_validate_secure_origin("https://example.com", "example.com")
_validate_secure_origin("https://app.example.com", "app.example.com")
_validate_secure_origin("https://wizarr.example.org", "wizarr.example.org")
_validate_secure_origin(
"https://my-app.example-domain.com", "my-app.example-domain.com"
)
def test_validate_secure_origin_invalid_domain_format(self):
"""Test that invalid domain formats are rejected."""
with pytest.raises(ValueError, match="Invalid domain name format"):
_validate_secure_origin("https://invalid..domain", "invalid..domain")
with pytest.raises(ValueError, match="Invalid domain name format"):
_validate_secure_origin("https://-invalid.domain", "-invalid.domain")
def test_get_rp_config_environment_override_validation(self, app):
"""Test that environment overrides are validated."""
with (
app.app_context(),
app.test_request_context(),
patch.dict(
"os.environ",
{
"WEBAUTHN_RP_ID": "example.com",
"WEBAUTHN_ORIGIN": "http://example.com", # HTTP should fail
},
),
pytest.raises(ValueError, match="Passkeys require HTTPS"),
):
get_rp_config()
def test_get_rp_config_request_based_validation(self, app):
"""Test that request-based configuration is validated."""
with (
app.app_context(),
app.test_request_context("/", headers={"Host": "example.com"}),
pytest.raises(ValueError, match="Passkeys require HTTPS"),
):
get_rp_config()
# Test IP address rejection
with (
app.test_request_context(
"/", headers={"Host": "192.168.1.1", "X-Forwarded-Proto": "https"}
),
pytest.raises(ValueError, match="Passkeys require a domain name"),
):
get_rp_config()
def test_get_rp_config_htmx_url_validation(self, app):
"""Test that HTMX current URL is validated."""
with (
app.app_context(),
app.test_request_context(
"/", headers={"HX-Current-URL": "http://example.com/path"}
),
pytest.raises(ValueError, match="Passkeys require HTTPS"),
):
get_rp_config()
# Test IP address in HX-Current-URL
with (
app.test_request_context(
"/", headers={"HX-Current-URL": "https://192.168.1.1/path"}
),
pytest.raises(ValueError, match="Passkeys require a domain name"),
):
get_rp_config()
def test_get_rp_config_valid_configuration(self, app):
"""Test that valid configurations work properly."""
with app.app_context():
# Test valid environment override
with (
patch.dict(
"os.environ",
{
"WEBAUTHN_RP_ID": "example.com",
"WEBAUTHN_ORIGIN": "https://example.com",
},
),
app.test_request_context(),
):
rp_id, rp_name, origin = get_rp_config()
assert rp_id == "example.com"
assert origin == "https://example.com"
# Test valid request-based config
with app.test_request_context(
"/", headers={"Host": "example.com", "X-Forwarded-Proto": "https"}
):
rp_id, rp_name, origin = get_rp_config()
assert rp_id == "example.com"
assert origin == "https://example.com"
def test_get_rp_config_localhost_development(self, app):
"""Test that localhost works in development mode."""
with (
app.app_context(),
patch.dict("os.environ", {"FLASK_ENV": "development"}),
app.test_request_context(
"/",
headers={"Host": "localhost:5000", "X-Forwarded-Proto": "https"},
),
):
rp_id, rp_name, origin = get_rp_config()
assert rp_id == "localhost"
assert origin == "https://localhost:5000"
@pytest.fixture
def app():
"""Create a Flask app for testing."""
app = Flask(__name__)
app.config["TESTING"] = True
app.config["SECRET_KEY"] = "test-secret-key"
return app