mirror of
https://github.com/jxxghp/MoviePilot.git
synced 2026-08-21 16:23:34 +08:00
fix(auth): secure passkey challenge transactions (#6178)
This commit is contained in:
@@ -15,7 +15,7 @@ from app.core.cache import (
|
||||
cached,
|
||||
)
|
||||
from app.core.config import settings
|
||||
from app.helper.redis import AsyncRedisHelper, RedisHelper
|
||||
from app.helper.redis import AsyncRedisHelper, RedisHelper, serialize
|
||||
|
||||
def test_file_backend_items_keep_relative_keys_and_bytes(tmp_path):
|
||||
"""
|
||||
@@ -548,6 +548,27 @@ def test_redis_helper_uses_blocking_pool_settings(monkeypatch):
|
||||
|
||||
helper.close()
|
||||
|
||||
|
||||
def test_redis_helper_pop_uses_atomic_getdel():
|
||||
"""Redis 缓存领取必须通过单条 GETDEL 命令完成。"""
|
||||
calls = []
|
||||
|
||||
class FakeClient:
|
||||
def getdel(self, key):
|
||||
calls.append(key)
|
||||
return serialize({"challenge": "value"})
|
||||
|
||||
helper = RedisHelper()
|
||||
helper.client = FakeClient()
|
||||
try:
|
||||
value = helper.pop("token", region="passkey_challenge")
|
||||
finally:
|
||||
helper.client = None
|
||||
|
||||
assert value == {"challenge": "value"}
|
||||
assert calls == ["region:passkey_challenge:key:token"]
|
||||
|
||||
|
||||
def test_async_redis_helper_uses_blocking_pool_settings(monkeypatch):
|
||||
"""
|
||||
Redis 异步客户端应使用阻塞连接池,避免高并发缓存读取立刻抛出连接耗尽错误。
|
||||
|
||||
97
tests/test_login_mfa_methods.py
Normal file
97
tests/test_login_mfa_methods.py
Normal file
@@ -0,0 +1,97 @@
|
||||
import json
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
from starlette.requests import Request
|
||||
from starlette.responses import Response
|
||||
|
||||
from app.api.endpoints import login as login_endpoint
|
||||
from app.chain.user import MfaRequired, UserChain
|
||||
|
||||
|
||||
def _request() -> Request:
|
||||
"""构造登录接口所需的最小请求。"""
|
||||
return Request(
|
||||
{
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/api/v1/login/access-token",
|
||||
"headers": [(b"host", b"testserver")],
|
||||
"scheme": "http",
|
||||
"server": ("testserver", 80),
|
||||
"client": ("testclient", 123),
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _form() -> SimpleNamespace:
|
||||
"""构造密码登录表单契约。"""
|
||||
return SimpleNamespace(username="user", password="password")
|
||||
|
||||
|
||||
def test_verify_mfa_requires_otp_when_enabled():
|
||||
"""密码通过后应返回账号已启用的 OTP 二次验证方式。"""
|
||||
user = SimpleNamespace(id=1, name="user", is_otp=True, otp_secret="")
|
||||
|
||||
result = UserChain._verify_mfa(user=user, mfa_code=None)
|
||||
|
||||
assert isinstance(result, MfaRequired)
|
||||
assert result.methods == ("otp",)
|
||||
|
||||
|
||||
def test_verify_mfa_ignores_passkeys_when_otp_is_disabled():
|
||||
"""Passkey 独立登录能力不应改变密码登录结果。"""
|
||||
user = SimpleNamespace(id=1, name="user", is_otp=False, otp_secret="")
|
||||
|
||||
assert UserChain._verify_mfa(user=user, mfa_code=None) is True
|
||||
|
||||
|
||||
def test_login_mfa_response_contains_methods_after_password_verification(monkeypatch):
|
||||
"""MFA 响应应保持旧标记并补充结构化方法列表。"""
|
||||
|
||||
class FakeUserChain:
|
||||
"""返回已通过密码校验的 MFA 要求。"""
|
||||
|
||||
def user_authenticate(self, username, password, mfa_code=None):
|
||||
"""模拟账号启用了 OTP。"""
|
||||
return False, MfaRequired(methods=("otp",))
|
||||
|
||||
monkeypatch.setattr(login_endpoint, "UserChain", FakeUserChain)
|
||||
|
||||
response = login_endpoint.login_access_token(
|
||||
request=_request(),
|
||||
response=Response(),
|
||||
form_data=_form(),
|
||||
)
|
||||
|
||||
assert response.status_code == 401
|
||||
assert response.headers["x-mfa-required"] == "true"
|
||||
assert json.loads(response.body) == {
|
||||
"detail": "需要二次验证",
|
||||
"mfa_methods": ["otp"],
|
||||
}
|
||||
|
||||
|
||||
def test_login_invalid_password_does_not_expose_mfa_methods(monkeypatch):
|
||||
"""密码未通过时不得返回账号的 MFA 能力。"""
|
||||
|
||||
class FakeUserChain:
|
||||
"""返回普通认证失败。"""
|
||||
|
||||
def user_authenticate(self, username, password, mfa_code=None):
|
||||
"""模拟错误密码。"""
|
||||
return False, "用户名、密码或验证码错误"
|
||||
|
||||
monkeypatch.setattr(login_endpoint, "UserChain", FakeUserChain)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
login_endpoint.login_access_token(
|
||||
request=_request(),
|
||||
response=Response(),
|
||||
form_data=_form(),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 401
|
||||
assert exc_info.value.detail == "用户名或密码错误"
|
||||
assert "X-MFA-Required" not in (exc_info.value.headers or {})
|
||||
134
tests/test_mfa_passkey_registration_errors.py
Normal file
134
tests/test_mfa_passkey_registration_errors.py
Normal file
@@ -0,0 +1,134 @@
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from webauthn.helpers.exceptions import InvalidRegistrationResponse
|
||||
|
||||
from app.api.endpoints import mfa as mfa_endpoint
|
||||
from app.helper import passkey as passkey_helper
|
||||
from app.helper.passkey import (
|
||||
PassKeyHelper,
|
||||
PassKeyRegistrationOriginMismatchError,
|
||||
PassKeyRegistrationVerificationError,
|
||||
)
|
||||
from app.helper.passkey_challenge import PasskeyChallengeStore
|
||||
|
||||
|
||||
def _registration_request(user_id: int = 1) -> mfa_endpoint.PassKeyRegistrationFinish:
|
||||
"""构造只用于错误路径的注册完成请求。"""
|
||||
transaction_token = PasskeyChallengeStore.issue(
|
||||
challenge="challenge",
|
||||
purpose="registration",
|
||||
user_id=user_id,
|
||||
)
|
||||
return mfa_endpoint.PassKeyRegistrationFinish(
|
||||
credential={"id": "credential-id"},
|
||||
transaction_token=transaction_token,
|
||||
name="测试通行密钥",
|
||||
)
|
||||
|
||||
|
||||
def _current_user() -> SimpleNamespace:
|
||||
"""构造注册错误路径所需的当前用户契约。"""
|
||||
return SimpleNamespace(id=1, name="admin")
|
||||
|
||||
|
||||
def test_passkey_helper_classifies_origin_mismatch():
|
||||
"""来源不一致应在 WebAuthn 边界转换为稳定的业务异常。"""
|
||||
library_error = InvalidRegistrationResponse(
|
||||
'Unexpected client data origin "http://localhost:5173", '
|
||||
'expected "http://localhost:3000"'
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
passkey_helper,
|
||||
"parse_registration_credential_json",
|
||||
return_value=object(),
|
||||
), patch.object(
|
||||
passkey_helper,
|
||||
"verify_registration_response",
|
||||
side_effect=library_error,
|
||||
), pytest.raises(PassKeyRegistrationOriginMismatchError) as exc_info:
|
||||
PassKeyHelper.verify_registration_response(
|
||||
credential={"id": "credential-id"},
|
||||
expected_challenge="Y2hhbGxlbmdl",
|
||||
)
|
||||
|
||||
assert exc_info.value.__cause__ is library_error
|
||||
|
||||
|
||||
def test_passkey_helper_classifies_other_verification_failure():
|
||||
"""其他注册验证错误不应被误判为来源配置问题。"""
|
||||
library_error = InvalidRegistrationResponse(
|
||||
"Client data challenge was not expected challenge"
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
passkey_helper,
|
||||
"parse_registration_credential_json",
|
||||
return_value=object(),
|
||||
), patch.object(
|
||||
passkey_helper,
|
||||
"verify_registration_response",
|
||||
side_effect=library_error,
|
||||
), pytest.raises(PassKeyRegistrationVerificationError) as exc_info:
|
||||
PassKeyHelper.verify_registration_response(
|
||||
credential={"id": "credential-id"},
|
||||
expected_challenge="Y2hhbGxlbmdl",
|
||||
)
|
||||
|
||||
assert exc_info.value.__cause__ is library_error
|
||||
|
||||
|
||||
def test_passkey_register_finish_returns_actionable_origin_message():
|
||||
"""来源配置不一致时应告诉管理员如何修正访问地址。"""
|
||||
with patch.object(
|
||||
mfa_endpoint.PassKeyHelper,
|
||||
"verify_registration_response",
|
||||
side_effect=PassKeyRegistrationOriginMismatchError(),
|
||||
):
|
||||
response = mfa_endpoint.passkey_register_finish(
|
||||
passkey_req=_registration_request(),
|
||||
current_user=_current_user(),
|
||||
)
|
||||
|
||||
assert not response.success
|
||||
assert response.message == "访问域名与系统配置不一致,请使用配置的域名重试"
|
||||
assert "APP_DOMAIN" not in response.message
|
||||
assert "Unexpected client data origin" not in response.message
|
||||
|
||||
|
||||
def test_passkey_register_finish_hides_other_verification_details():
|
||||
"""其他 WebAuthn 验证细节只记录在服务端,不返回给客户端。"""
|
||||
with patch.object(
|
||||
mfa_endpoint.PassKeyHelper,
|
||||
"verify_registration_response",
|
||||
side_effect=PassKeyRegistrationVerificationError(
|
||||
"Client data challenge was not expected challenge"
|
||||
),
|
||||
):
|
||||
response = mfa_endpoint.passkey_register_finish(
|
||||
passkey_req=_registration_request(),
|
||||
current_user=_current_user(),
|
||||
)
|
||||
|
||||
assert not response.success
|
||||
assert response.message == "通行密钥注册验证失败,请重新发起注册后重试"
|
||||
assert "challenge" not in response.message
|
||||
|
||||
|
||||
def test_passkey_register_finish_hides_unexpected_error_details():
|
||||
"""未知内部异常应返回通用提示,避免泄露实现信息。"""
|
||||
with patch.object(
|
||||
mfa_endpoint.PassKeyHelper,
|
||||
"verify_registration_response",
|
||||
side_effect=RuntimeError("database connection details"),
|
||||
):
|
||||
response = mfa_endpoint.passkey_register_finish(
|
||||
passkey_req=_registration_request(),
|
||||
current_user=_current_user(),
|
||||
)
|
||||
|
||||
assert not response.success
|
||||
assert response.message == "通行密钥注册失败,请稍后重试"
|
||||
assert "database" not in response.message
|
||||
194
tests/test_mfa_passkey_transactions.py
Normal file
194
tests/test_mfa_passkey_transactions.py
Normal file
@@ -0,0 +1,194 @@
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
from starlette.requests import Request
|
||||
from starlette.responses import Response
|
||||
|
||||
from app.api.endpoints import mfa as mfa_endpoint
|
||||
from app.helper.passkey_challenge import PasskeyChallengeStore
|
||||
|
||||
|
||||
def _request() -> Request:
|
||||
return Request(
|
||||
{
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/api/v1/mfa/passkey/authenticate/finish",
|
||||
"headers": [(b"host", b"testserver")],
|
||||
"scheme": "http",
|
||||
"server": ("testserver", 80),
|
||||
"client": ("testclient", 123),
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def setup_function():
|
||||
PasskeyChallengeStore._cache.clear()
|
||||
|
||||
|
||||
def test_registration_transaction_is_bound_to_current_user():
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="registration",
|
||||
user_id=1,
|
||||
)
|
||||
request = mfa_endpoint.PassKeyRegistrationFinish(
|
||||
credential={"id": "credential-id"},
|
||||
transaction_token=token,
|
||||
name="test",
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
mfa_endpoint.PassKeyHelper,
|
||||
"verify_registration_response",
|
||||
) as verify:
|
||||
result = mfa_endpoint.passkey_register_finish(
|
||||
passkey_req=request,
|
||||
current_user=SimpleNamespace(id=2, name="other"),
|
||||
)
|
||||
|
||||
assert not result.success
|
||||
assert result.message == "注册请求已失效,请重新发起注册"
|
||||
verify.assert_not_called()
|
||||
|
||||
|
||||
def test_registration_uses_server_challenge():
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="registration",
|
||||
user_id=1,
|
||||
)
|
||||
request = mfa_endpoint.PassKeyRegistrationFinish(
|
||||
credential={"id": "credential-id", "challenge": "client-challenge"},
|
||||
transaction_token=token,
|
||||
name="test",
|
||||
)
|
||||
passkey = Mock()
|
||||
|
||||
with patch.object(
|
||||
mfa_endpoint.PassKeyHelper,
|
||||
"verify_registration_response",
|
||||
return_value=("credential-id", b"public-key", 0, "aaguid"),
|
||||
) as verify, patch.object(mfa_endpoint, "PassKey", return_value=passkey):
|
||||
result = mfa_endpoint.passkey_register_finish(
|
||||
passkey_req=request,
|
||||
current_user=SimpleNamespace(id=1, name="user"),
|
||||
)
|
||||
|
||||
assert result.success
|
||||
verify.assert_called_once_with(
|
||||
credential=request.credential,
|
||||
expected_challenge="server-challenge",
|
||||
)
|
||||
passkey.create.assert_called_once_with()
|
||||
|
||||
|
||||
def test_authentication_transaction_rejects_other_user_credential():
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="authentication",
|
||||
user_id=1,
|
||||
)
|
||||
request = mfa_endpoint.PassKeyAuthenticationFinish(
|
||||
credential={"id": "credential-id"},
|
||||
transaction_token=token,
|
||||
)
|
||||
passkey = SimpleNamespace(user_id=2)
|
||||
user = SimpleNamespace(id=2, is_active=True)
|
||||
|
||||
with patch.object(
|
||||
mfa_endpoint,
|
||||
"_extract_and_standardize_credential_id",
|
||||
return_value="credential-id",
|
||||
), patch.object(
|
||||
mfa_endpoint.PassKey,
|
||||
"get_by_credential_id",
|
||||
return_value=passkey,
|
||||
), patch.object(
|
||||
mfa_endpoint.User,
|
||||
"get_by_id",
|
||||
return_value=user,
|
||||
), patch.object(
|
||||
mfa_endpoint,
|
||||
"_verify_passkey_and_update",
|
||||
) as verify:
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
mfa_endpoint.passkey_authenticate_finish(
|
||||
request=_request(),
|
||||
response=Response(),
|
||||
passkey_req=request,
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 401
|
||||
verify.assert_not_called()
|
||||
|
||||
|
||||
def test_authentication_finish_token_cannot_be_replayed():
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="authentication",
|
||||
user_id=1,
|
||||
)
|
||||
request = mfa_endpoint.PassKeyAuthenticationFinish(
|
||||
credential={"id": "credential-id"},
|
||||
transaction_token=token,
|
||||
)
|
||||
passkey = SimpleNamespace(user_id=1)
|
||||
user = SimpleNamespace(
|
||||
id=1,
|
||||
name="user",
|
||||
is_active=True,
|
||||
is_superuser=False,
|
||||
avatar="",
|
||||
permissions={},
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
mfa_endpoint,
|
||||
"_extract_and_standardize_credential_id",
|
||||
return_value="credential-id",
|
||||
), patch.object(
|
||||
mfa_endpoint.PassKey,
|
||||
"get_by_credential_id",
|
||||
return_value=passkey,
|
||||
), patch.object(
|
||||
mfa_endpoint.User,
|
||||
"get_by_id",
|
||||
return_value=user,
|
||||
), patch.object(
|
||||
mfa_endpoint,
|
||||
"_verify_passkey_and_update",
|
||||
return_value=(True, 0),
|
||||
), patch.object(
|
||||
mfa_endpoint,
|
||||
"SitesHelper",
|
||||
return_value=SimpleNamespace(auth_level=1),
|
||||
), patch.object(
|
||||
mfa_endpoint,
|
||||
"SystemConfigOper",
|
||||
return_value=SimpleNamespace(get=lambda _: True),
|
||||
), patch.object(
|
||||
mfa_endpoint.security,
|
||||
"create_access_token",
|
||||
return_value="access-token",
|
||||
), patch.object(
|
||||
mfa_endpoint.security,
|
||||
"set_or_refresh_resource_token_cookie",
|
||||
):
|
||||
result = mfa_endpoint.passkey_authenticate_finish(
|
||||
request=_request(),
|
||||
response=Response(),
|
||||
passkey_req=request,
|
||||
)
|
||||
with pytest.raises(HTTPException) as replay_error:
|
||||
mfa_endpoint.passkey_authenticate_finish(
|
||||
request=_request(),
|
||||
response=Response(),
|
||||
passkey_req=request,
|
||||
)
|
||||
|
||||
assert result.access_token == "access-token"
|
||||
assert replay_error.value.status_code == 401
|
||||
assert replay_error.value.detail == "认证请求已失效"
|
||||
92
tests/test_passkey_challenge.py
Normal file
92
tests/test_passkey_challenge.py
Normal file
@@ -0,0 +1,92 @@
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
|
||||
from app.core.cache import TTLCache
|
||||
from app.helper.passkey_challenge import PasskeyChallengeStore
|
||||
|
||||
|
||||
def setup_function():
|
||||
PasskeyChallengeStore._cache.clear()
|
||||
|
||||
|
||||
def test_challenge_can_only_be_consumed_once():
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="authentication",
|
||||
user_id=1,
|
||||
)
|
||||
|
||||
challenge = PasskeyChallengeStore.consume(
|
||||
transaction_token=token,
|
||||
purpose="authentication",
|
||||
)
|
||||
|
||||
assert challenge is not None
|
||||
assert challenge.challenge == "server-challenge"
|
||||
assert challenge.user_id == 1
|
||||
assert (
|
||||
PasskeyChallengeStore.consume(
|
||||
transaction_token=token,
|
||||
purpose="authentication",
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_wrong_purpose_invalidates_transaction():
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="registration",
|
||||
user_id=1,
|
||||
)
|
||||
|
||||
assert (
|
||||
PasskeyChallengeStore.consume(
|
||||
transaction_token=token,
|
||||
purpose="authentication",
|
||||
)
|
||||
is None
|
||||
)
|
||||
assert (
|
||||
PasskeyChallengeStore.consume(
|
||||
transaction_token=token,
|
||||
purpose="registration",
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_expired_challenge_cannot_be_consumed(monkeypatch):
|
||||
expired_cache = TTLCache(region="expired_passkey_challenge", maxsize=1, ttl=0)
|
||||
monkeypatch.setattr(PasskeyChallengeStore, "_cache", expired_cache)
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="authentication",
|
||||
user_id=None,
|
||||
)
|
||||
|
||||
assert (
|
||||
PasskeyChallengeStore.consume(
|
||||
transaction_token=token,
|
||||
purpose="authentication",
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_concurrent_consumers_have_single_winner():
|
||||
token = PasskeyChallengeStore.issue(
|
||||
challenge="server-challenge",
|
||||
purpose="authentication",
|
||||
user_id=None,
|
||||
)
|
||||
|
||||
def consume():
|
||||
return PasskeyChallengeStore.consume(
|
||||
transaction_token=token,
|
||||
purpose="authentication",
|
||||
)
|
||||
|
||||
with ThreadPoolExecutor(max_workers=8) as executor:
|
||||
results = list(executor.map(lambda _: consume(), range(8)))
|
||||
|
||||
assert sum(result is not None for result in results) == 1
|
||||
Reference in New Issue
Block a user