From 6515ba3b94afca6d8121e8b862247844a1c4a688 Mon Sep 17 00:00:00 2001 From: Hyu Date: Mon, 28 Sep 2026 23:53:01 +0800 Subject: [PATCH] fix(api): serve the passkey and password routes for the authenticated Account Passkeys and passwords belong to the Account, but those routes required a Workspace -- which the WebUI never sends for them -- and then resolved the caller by looking the login name up as an email address. Every Account whose login name is not its email answered 404, so the passkey panel could neither add nor list a key. The routes now authenticate with the account token and address the Account the caller authenticated as, matching the self-service second-factor routes in the same file. /space/bind-authorize-url carried the same Workspace requirement. The login page also surfaced a TypeError instead of the passkey login failure message when an assertion was rejected. --- .../pkg/api/http/controller/groups/user.py | 80 ++++------ .../api/test_passkey_account_scope.py | 139 ++++++++++++++++++ web/src/app/login/page.tsx | 6 + 3 files changed, 175 insertions(+), 50 deletions(-) create mode 100644 tests/integration/api/test_passkey_account_scope.py diff --git a/src/langbot/pkg/api/http/controller/groups/user.py b/src/langbot/pkg/api/http/controller/groups/user.py index 365f5964e..ee2ee7dc9 100644 --- a/src/langbot/pkg/api/http/controller/groups/user.py +++ b/src/langbot/pkg/api/http/controller/groups/user.py @@ -267,8 +267,8 @@ class UserRouterGroup(group.RouterGroup): return self.success(data={'user': user_email}) - @self.route('/change-password', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) - async def _(user_email: str) -> str: + @self.route('/change-password', methods=['POST'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account) -> str: # Check if password change is allowed allow_modify_login_info = self.ap.instance_config.data.get('system', {}).get( 'allow_modify_login_info', True @@ -282,13 +282,13 @@ class UserRouterGroup(group.RouterGroup): new_password = json_data['new_password'] try: - await self.ap.user_service.change_password(user_email, current_password, new_password) + await self.ap.user_service.change_password(account.normalized_email, current_password, new_password) except argon2.exceptions.VerifyMismatchError: return self.http_status(400, -1, 'Current password is incorrect') except ValueError as e: return self.http_status(400, -1, str(e)) - return self.success(data={'user': user_email}) + return self.success(data={'user': account.user}) # Space OAuth endpoints (redirect flow) @@ -323,19 +323,19 @@ class UserRouterGroup(group.RouterGroup): except ValueError as e: return self.fail(1, str(e)) - @self.route('/space/bind-authorize-url', methods=['GET'], auth_type=group.AuthType.USER_TOKEN) - async def _(request_context: RequestContext) -> str: + @self.route('/space/bind-authorize-url', methods=['GET'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account) -> str: """Issue an account-bound, one-time Space OAuth redirect.""" redirect_uri = quart.request.args.get('redirect_uri', '') if not redirect_uri: return self.fail(1, 'Missing redirect_uri parameter') - if not request_context.account_uuid: + if not account.uuid: return self.http_status(403, 'account_required', 'An Account is required') try: redirect_uri = self._validate_space_redirect_uri(redirect_uri, bind=True) state = await self.ap.user_service.issue_space_oauth_state( 'bind', - account_uuid=request_context.account_uuid, + account_uuid=account.uuid, ) authorize_url = self.ap.space_service.get_oauth_authorize_url(redirect_uri, state) return self.success(data={'authorize_url': authorize_url}) @@ -507,8 +507,8 @@ class UserRouterGroup(group.RouterGroup): capabilities['passkey_supported'] = True return self.success(data={'initialized': True, **capabilities}) - @self.route('/set-password', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) - async def _(user_email: str) -> str: + @self.route('/set-password', methods=['POST'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account) -> str: """Set password for Space account (first time) or change password""" # Check if modifying login info is allowed allow_modify_login_info = self.ap.instance_config.data.get('system', {}).get( @@ -524,13 +524,9 @@ class UserRouterGroup(group.RouterGroup): if not new_password: return self.http_status(400, -1, 'New password is required') - user_obj = await self.ap.user_service.get_user_by_email(user_email) - if user_obj is None: - return self.http_status(404, -1, 'User not found') - try: - await self.ap.user_service.set_password(user_email, new_password, current_password) - return self.success(data={'user': user_email}) + await self.ap.user_service.set_password(account.normalized_email, new_password, current_password) + return self.success(data={'user': account.user}) except ValueError as e: return self.http_status(400, -1, str(e)) except argon2.exceptions.VerifyMismatchError: @@ -595,8 +591,12 @@ class UserRouterGroup(group.RouterGroup): except Exception: raise - @self.route('/passkey/register/options', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) - async def _(user_email: str) -> str: + # Passkeys belong to the Account, not to a Workspace: these routes are + # ACCOUNT_TOKEN scoped for the same reason as the second-factor routes + # below. Resolving the caller through the login name would also miss every + # Account whose login name is not its email. + @self.route('/passkey/register/options', methods=['POST'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account) -> str: """Generate WebAuthn registration options for current account.""" allow_modify_login_info = self.ap.instance_config.data.get('system', {}).get( 'allow_modify_login_info', True @@ -604,16 +604,12 @@ class UserRouterGroup(group.RouterGroup): if not allow_modify_login_info: return self.http_status(403, -1, 'Modifying login info is disabled') - user_obj = await self.ap.user_service.get_user_by_email(user_email) - if user_obj is None: - return self.http_status(404, -1, 'User not found') - json_data = (await quart.request.json) or {} origin, rp_id = self._extract_origin_and_rp_id(json_data) try: options, challenge_token = await self.ap.user_service.generate_passkey_registration_options( - account_uuid=user_obj.uuid, + account_uuid=account.uuid, rp_id=rp_id, origin=origin, rp_name='LangBot', @@ -622,8 +618,8 @@ class UserRouterGroup(group.RouterGroup): except Exception as e: return self.fail(1, str(e)) - @self.route('/passkey/register/verify', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) - async def _(user_email: str) -> str: + @self.route('/passkey/register/verify', methods=['POST'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account) -> str: """Verify WebAuthn registration response and save credential.""" allow_modify_login_info = self.ap.instance_config.data.get('system', {}).get( 'allow_modify_login_info', True @@ -631,10 +627,6 @@ class UserRouterGroup(group.RouterGroup): if not allow_modify_login_info: return self.http_status(403, -1, 'Modifying login info is disabled') - user_obj = await self.ap.user_service.get_user_by_email(user_email) - if user_obj is None: - return self.http_status(404, -1, 'User not found') - json_data = await quart.request.json challenge_token = json_data.get('challenge_token') credential = json_data.get('credential') or json_data.get('response') @@ -700,14 +692,10 @@ class UserRouterGroup(group.RouterGroup): except Exception as e: return self.fail(1, str(e)) - @self.route('/passkeys', methods=['GET'], auth_type=group.AuthType.USER_TOKEN) - async def _(user_email: str) -> str: + @self.route('/passkeys', methods=['GET'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account) -> str: """List registered passkeys for the current user.""" - user_obj = await self.ap.user_service.get_user_by_email(user_email) - if user_obj is None: - return self.http_status(404, -1, 'User not found') - - passkeys = await self.ap.user_service.get_user_passkeys(user_obj.uuid) + passkeys = await self.ap.user_service.get_user_passkeys(account.uuid) return self.success( data=[ { @@ -723,8 +711,8 @@ class UserRouterGroup(group.RouterGroup): ] ) - @self.route('/passkey/', methods=['PATCH'], auth_type=group.AuthType.USER_TOKEN) - async def _(user_email: str, passkey_uuid: str) -> str: + @self.route('/passkey/', methods=['PATCH'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account, passkey_uuid: str) -> str: """Rename a registered passkey.""" allow_modify_login_info = self.ap.instance_config.data.get('system', {}).get( 'allow_modify_login_info', True @@ -732,17 +720,13 @@ class UserRouterGroup(group.RouterGroup): if not allow_modify_login_info: return self.http_status(403, -1, 'Modifying login info is disabled') - user_obj = await self.ap.user_service.get_user_by_email(user_email) - if user_obj is None: - return self.http_status(404, -1, 'User not found') - json_data = await quart.request.json name = (json_data.get('name') or '').strip() if not name: return self.fail(1, 'Passkey name cannot be empty') updated = await self.ap.user_service.rename_user_passkey( - account_uuid=user_obj.uuid, + account_uuid=account.uuid, passkey_uuid=passkey_uuid, new_name=name, ) @@ -750,8 +734,8 @@ class UserRouterGroup(group.RouterGroup): return self.http_status(404, -1, 'Passkey not found') return self.success(data={'uuid': updated.uuid, 'name': updated.name}) - @self.route('/passkey/', methods=['DELETE'], auth_type=group.AuthType.USER_TOKEN) - async def _(user_email: str, passkey_uuid: str) -> str: + @self.route('/passkey/', methods=['DELETE'], auth_type=group.AuthType.ACCOUNT_TOKEN) + async def _(account, passkey_uuid: str) -> str: """Delete/revoke a registered passkey.""" allow_modify_login_info = self.ap.instance_config.data.get('system', {}).get( 'allow_modify_login_info', True @@ -759,12 +743,8 @@ class UserRouterGroup(group.RouterGroup): if not allow_modify_login_info: return self.http_status(403, -1, 'Modifying login info is disabled') - user_obj = await self.ap.user_service.get_user_by_email(user_email) - if user_obj is None: - return self.http_status(404, -1, 'User not found') - deleted = await self.ap.user_service.delete_user_passkey( - account_uuid=user_obj.uuid, + account_uuid=account.uuid, passkey_uuid=passkey_uuid, ) if not deleted: diff --git a/tests/integration/api/test_passkey_account_scope.py b/tests/integration/api/test_passkey_account_scope.py new file mode 100644 index 000000000..8f6ce38bc --- /dev/null +++ b/tests/integration/api/test_passkey_account_scope.py @@ -0,0 +1,139 @@ +"""Account scoping for the passkey and password routes. + +A passkey belongs to the Account, so these routes must answer for the Account the +caller authenticated as. They previously required a Workspace (which the WebUI +never sends for them) and then resolved the Account by looking the login name up +as an email address, so every Account whose login name is not its email answered +``404`` and the passkey feature was unreachable. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import AsyncMock, Mock + +import pytest +import quart + +from langbot.pkg.api.http.controller.groups.user import UserRouterGroup + +pytestmark = pytest.mark.integration + +ACCOUNT_UUID = '1905ad7c-168f-49f4-893e-e43614e4c4bf' +LOGIN_NAME = 'dev' +ACCOUNT_EMAIL = 'dev@local.test' + + +@pytest.fixture +async def account_api(): + # The login name deliberately differs from the email: that is what made the + # email lookup miss the authenticated Account. + account = SimpleNamespace( + uuid=ACCOUNT_UUID, + user=LOGIN_NAME, + normalized_email=ACCOUNT_EMAIL, + password='', + account_type='space', + ) + application = Mock() + application.deployment = SimpleNamespace(multi_workspace_enabled=False) + application.instance_config.data = {'system': {'allow_modify_login_info': True}} + application.persistence_mgr = SimpleNamespace(tenant_uow=None) + application.user_service.get_authenticated_account = AsyncMock(return_value=account) + application.user_service.get_user_by_email = AsyncMock(return_value=None) + application.user_service.get_user_passkeys = AsyncMock(return_value=[]) + application.user_service.generate_passkey_registration_options = AsyncMock( + return_value=({'challenge': 'probe'}, 'challenge-token') + ) + application.user_service.verify_and_save_passkey_registration = AsyncMock( + return_value=SimpleNamespace(uuid='credential-uuid', name='probe', created_at=None) + ) + application.user_service.rename_user_passkey = AsyncMock( + return_value=SimpleNamespace(uuid='credential-uuid', name='renamed') + ) + application.user_service.delete_user_passkey = AsyncMock(return_value=True) + application.user_service.set_password = AsyncMock() + application.user_service.change_password = AsyncMock() + + quart_app = quart.Quart(__name__) + router = UserRouterGroup(application, quart_app) + await router.initialize() + return application, quart_app.test_client() + + +def _headers() -> dict[str, str]: + # The WebUI sends an account session and no Workspace for these routes. + return {'Authorization': 'Bearer account-token'} + + +async def test_passkey_list_answers_without_a_workspace(account_api): + application, client = account_api + + response = await client.get('/api/v1/user/passkeys', headers=_headers()) + + assert response.status_code == 200, await response.get_data(as_text=True) + application.user_service.get_user_passkeys.assert_awaited_once_with(ACCOUNT_UUID) + application.user_service.get_user_by_email.assert_not_awaited() + + +async def test_passkey_registration_options_bind_the_authenticated_account(account_api): + application, client = account_api + + response = await client.post( + '/api/v1/user/passkey/register/options', + json={'origin': 'http://localhost:3000'}, + headers=_headers(), + ) + + assert response.status_code == 200, await response.get_data(as_text=True) + assert (await response.get_json())['data']['challenge_token'] == 'challenge-token' + options_call = application.user_service.generate_passkey_registration_options.await_args + assert options_call.kwargs['account_uuid'] == ACCOUNT_UUID + assert options_call.kwargs['rp_id'] == 'localhost' + application.user_service.get_user_by_email.assert_not_awaited() + + +async def test_passkey_verify_and_revoke_address_the_authenticated_account(account_api): + application, client = account_api + + verified = await client.post( + '/api/v1/user/passkey/register/verify', + json={'challenge_token': 'challenge-token', 'credential': {'id': 'cred'}}, + headers=_headers(), + ) + renamed = await client.patch( + '/api/v1/user/passkey/credential-uuid', + json={'name': 'renamed'}, + headers=_headers(), + ) + deleted = await client.delete('/api/v1/user/passkey/credential-uuid', headers=_headers()) + + assert verified.status_code == 200, await verified.get_data(as_text=True) + assert renamed.status_code == 200, await renamed.get_data(as_text=True) + assert deleted.status_code == 200, await deleted.get_data(as_text=True) + assert application.user_service.rename_user_passkey.await_args.kwargs['account_uuid'] == ACCOUNT_UUID + assert application.user_service.delete_user_passkey.await_args.kwargs['account_uuid'] == ACCOUNT_UUID + application.user_service.get_user_by_email.assert_not_awaited() + + +async def test_password_routes_target_the_authenticated_account(account_api): + application, client = account_api + + set_password = await client.post( + '/api/v1/user/set-password', + json={'new_password': 'new-secret'}, + headers=_headers(), + ) + change_password = await client.post( + '/api/v1/user/change-password', + json={'current_password': 'old-secret', 'new_password': 'new-secret'}, + headers=_headers(), + ) + + assert set_password.status_code == 200, await set_password.get_data(as_text=True) + assert change_password.status_code == 200, await change_password.get_data(as_text=True) + # The service resolves the row by email, so the route hands it the email of the + # account the token identified -- never the login name it was given. + assert application.user_service.set_password.await_args.args[0] == ACCOUNT_EMAIL + assert application.user_service.change_password.await_args.args[0] == ACCOUNT_EMAIL + application.user_service.get_user_by_email.assert_not_awaited() diff --git a/web/src/app/login/page.tsx b/web/src/app/login/page.tsx index d0134bb4c..d342909ed 100644 --- a/web/src/app/login/page.tsx +++ b/web/src/app/login/page.tsx @@ -212,6 +212,12 @@ export default function Login() { ); const authResp = await startAuthentication({ optionsJSON: options }); const res = await httpClient.verifyPasskeyAuth(challenge_token, authResp); + // A rejected assertion answers with an error payload and no session, so a + // missing token means the login failed rather than that the user cancelled. + if (!res?.token) { + toast.error(t('common.passkeyLoginFailed')); + return; + } if (await finishLogin(res.token, res.user)) { toast.success(t('common.passkeyLoginSuccess')); }