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')); }