diff --git a/src/langbot/pkg/api/http/controller/groups/user.py b/src/langbot/pkg/api/http/controller/groups/user.py index 240d7a213..365f5964e 100644 --- a/src/langbot/pkg/api/http/controller/groups/user.py +++ b/src/langbot/pkg/api/http/controller/groups/user.py @@ -914,22 +914,52 @@ class UserRouterGroup(group.RouterGroup): if access.membership.role not in ('owner', 'admin'): raise PermissionError('Only Workspace owners and admins may manage other Accounts') + async def _require_target_in_workspace( + request_context: RequestContext, + target_account_uuid: str, + ) -> bool: + """Report whether the target Account belongs to the caller's Workspace. + + A cross-Workspace Account is reported as absent, so the response never + confirms another tenant's Account existence. + """ + + try: + await self.ap.workspace_collaboration_service.resolve_account_workspace( + target_account_uuid, + request_context.workspace_uuid, + ) + except Exception: + return False + return True + @self.route('/totp/accounts', methods=['GET'], auth_type=group.AuthType.USER_TOKEN) async def _(request_context: RequestContext) -> str: - """List the second-factor state of every Account for owners/admins. + """List the second-factor state of the caller's Workspace Accounts. - Oversight is deliberately instance-wide: managing the second factor - of any Account is an owner/admin responsibility and is not scoped to - the caller's Workspace. + Oversight is a Workspace responsibility: owners and admins answer for + the Accounts that belong to their own Workspace and must never see + another tenant's Accounts. """ try: - await _require_workspace_manager(request_context) + access = await self.ap.workspace_collaboration_service.resolve_account_workspace( + request_context.account_uuid, + request_context.workspace_uuid, + ) + if access.membership.role not in ('owner', 'admin'): + raise PermissionError('Only Workspace owners and admins may manage other Accounts') + members = await self.ap.workspace_collaboration_service.list_members( + request_context.workspace_uuid, + access.membership, + ) except PermissionError as e: return self.http_status(403, 'permission_denied', str(e)) except Exception: return self.http_status(403, 'permission_denied', 'Not permitted') - accounts = await self.ap.totp_service.list_account_states() + accounts = await self.ap.totp_service.list_account_states( + account_uuids=[member.membership.account_uuid for member in members], + ) return self.success(data={'accounts': accounts}) @self.route('/totp/accounts/', methods=['DELETE'], auth_type=group.AuthType.USER_TOKEN) @@ -948,6 +978,9 @@ class UserRouterGroup(group.RouterGroup): except Exception: return self.http_status(403, 'permission_denied', 'Not permitted') + if not await _require_target_in_workspace(request_context, target_account_uuid): + return self.http_status(404, 'account_not_found', 'Account not found') + revoked = await self.ap.totp_service.revoke_for_account(target_account_uuid) if not revoked: return self.http_status(404, 'totp_not_enrolled', 'TOTP is not enabled for that Account') @@ -978,6 +1011,9 @@ class UserRouterGroup(group.RouterGroup): except Exception: return self.http_status(403, 'permission_denied', 'Not permitted') + if not await _require_target_in_workspace(request_context, target_account_uuid): + return self.http_status(404, 'account_not_found', 'Account not found') + target = await self.ap.totp_service.get_account(target_account_uuid) if target is None: return self.http_status(404, 'account_not_found', 'Account not found') @@ -1022,6 +1058,9 @@ class UserRouterGroup(group.RouterGroup): if not isinstance(code, str) or not code: return self.fail(1, 'Missing code parameter') + if not await _require_target_in_workspace(request_context, target_account_uuid): + return self.http_status(404, 'account_not_found', 'Account not found') + try: result = await self.ap.totp_service.confirm_enrollment(target_account_uuid, code) except totp_module.TotpNotEnrolledError as e: diff --git a/src/langbot/pkg/api/http/service/totp.py b/src/langbot/pkg/api/http/service/totp.py index 5995e91ab..8fd4a1157 100644 --- a/src/langbot/pkg/api/http/service/totp.py +++ b/src/langbot/pkg/api/http/service/totp.py @@ -49,6 +49,17 @@ _TOTP_MAX_ATTEMPTS = 5 _RECOVERY_ALPHABET = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789' +def _utc_now_naive() -> datetime.datetime: + """UTC now without tzinfo, matching the credential ``timestamp`` columns. + + The TOTP tables store ``timestamp without time zone``. asyncpg rejects an + offset-aware value for such a column (SQLite accepts it, which is why this + only surfaced on PostgreSQL), so every write converts at the boundary. + """ + + return datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None) + + class TotpError(ValueError): """Base class for TOTP second-factor failures.""" @@ -363,7 +374,7 @@ class TotpService: await session.execute( sqlalchemy.update(totp_entity.TotpCredential) .where(totp_entity.TotpCredential.account_uuid == account_uuid) - .values(disabled_at=datetime.datetime.now(datetime.timezone.utc)) + .values(disabled_at=_utc_now_naive()) ) await session.execute( sqlalchemy.delete(totp_entity.TotpRecoveryCode).where( @@ -409,7 +420,7 @@ class TotpService: counter = self._verify_counter(credential, code) codes = _generate_recovery_codes() - now = datetime.datetime.now(datetime.timezone.utc) + now = _utc_now_naive() async with self._session_factory()() as session: async with session.begin(): @@ -479,7 +490,7 @@ class TotpService: if not await self.verify_code(account_uuid, code, allow_recovery=True): raise TotpInvalidCodeError('Invalid TOTP code') - now = datetime.datetime.now(datetime.timezone.utc) + now = _utc_now_naive() async with self._session_factory()() as session: async with session.begin(): await session.execute( @@ -497,11 +508,16 @@ class TotpService: # ------------------------------------------------------------------ # owner/admin oversight # ------------------------------------------------------------------ - async def list_account_states(self) -> list[dict[str, typing.Any]]: - """Second-factor state for every Account, for owner/admin oversight. + async def list_account_states( + self, + *, + account_uuids: typing.Sequence[str], + ) -> list[dict[str, typing.Any]]: + """Second-factor state for the Accounts a Workspace owner/admin may see. - Oversight is instance-wide by design: an owner/admin may manage the - second factor of any Account. + Oversight belongs to the Workspace, so the caller supplies the member + Account UUIDs of its own Workspace: an owner/admin must never observe or + manage another tenant's Accounts. Only state is returned: no secret and no recovery-code material. """ @@ -525,6 +541,7 @@ class TotpService: credential.last_used_at, sqlalchemy.func.coalesce(unused_codes.c.unused, 0), ) + .where(user.User.uuid.in_(tuple(account_uuids))) .outerjoin( credential, sqlalchemy.and_( @@ -558,7 +575,7 @@ class TotpService: outstanding recovery codes are destroyed. """ - now = datetime.datetime.now(datetime.timezone.utc) + now = _utc_now_naive() async with self._session_factory()() as session: async with session.begin(): result = await session.execute( @@ -630,7 +647,7 @@ class TotpService: raise if consume_counter: - now = datetime.datetime.now(datetime.timezone.utc) + now = _utc_now_naive() async with self._session_factory()() as session: async with session.begin(): await session.execute( @@ -680,7 +697,7 @@ class TotpService: totp_entity.TotpRecoveryCode.id == matched_id, totp_entity.TotpRecoveryCode.used_at.is_(None), ) - .values(used_at=datetime.datetime.now(datetime.timezone.utc)) + .values(used_at=_utc_now_naive()) ) await session.commit() return bool(result.rowcount) diff --git a/tests/integration/api/test_totp_workspace_scope.py b/tests/integration/api/test_totp_workspace_scope.py new file mode 100644 index 000000000..460dcbbb2 --- /dev/null +++ b/tests/integration/api/test_totp_workspace_scope.py @@ -0,0 +1,123 @@ +"""Workspace scoping for the owner/admin second-factor oversight endpoints. + +An owner or admin answers for the Accounts of its own Workspace. The oversight +endpoints must never list, revoke, or re-bind another tenant's Account. +""" + +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 +from langbot.pkg.workspace.errors import WorkspaceNotFoundError + +pytestmark = pytest.mark.integration + +WORKSPACE_UUID = '11111111-1111-4111-8111-111111111111' +MEMBER_UUID = 'member-account' +FOREIGN_UUID = 'foreign-account' + + +def _access(account_uuid: str) -> SimpleNamespace: + return SimpleNamespace( + workspace=SimpleNamespace(uuid=WORKSPACE_UUID), + membership=SimpleNamespace( + uuid=f'membership-{account_uuid}', + account_uuid=account_uuid, + role='owner', + projection_revision=1, + ), + execution=SimpleNamespace(instance_uuid='instance-a', placement_generation=1), + ) + + +async def _resolve(account_uuid: str, _workspace_uuid: str | None) -> SimpleNamespace: + # The collaboration service hides Accounts that are not members here. + if account_uuid == FOREIGN_UUID: + raise WorkspaceNotFoundError('Workspace not found') + return _access(account_uuid) + + +@pytest.fixture +async def totp_admin_api(): + accounts = {'member-token': SimpleNamespace(uuid=MEMBER_UUID, user='member@example.com')} + 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( + side_effect=lambda token: accounts[token] + ) + application.workspace_collaboration_service.resolve_account_workspace = AsyncMock( + side_effect=_resolve + ) + application.workspace_collaboration_service.list_members = AsyncMock( + return_value=[SimpleNamespace(membership=SimpleNamespace(account_uuid=MEMBER_UUID))] + ) + application.totp_service.list_account_states = AsyncMock( + return_value=[{'account_uuid': MEMBER_UUID, 'user': 'member', 'enabled': False}] + ) + application.totp_service.revoke_for_account = AsyncMock(return_value=True) + application.totp_service.get_account = AsyncMock( + return_value=SimpleNamespace(uuid=FOREIGN_UUID, user='foreign@example.com') + ) + application.totp_service.begin_enrollment = AsyncMock() + application.totp_service.confirm_enrollment = 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]: + return {'Authorization': 'Bearer member-token', 'X-Workspace-Id': WORKSPACE_UUID} + + +async def test_account_list_covers_only_the_callers_workspace(totp_admin_api): + application, client = totp_admin_api + + response = await client.get('/api/v1/user/totp/accounts', headers=_headers()) + + assert response.status_code == 200 + payload = await response.get_json() + assert [item['account_uuid'] for item in payload['data']['accounts']] == [MEMBER_UUID] + application.totp_service.list_account_states.assert_awaited_once_with( + account_uuids=[MEMBER_UUID] + ) + + +async def test_foreign_account_cannot_be_revoked(totp_admin_api): + application, client = totp_admin_api + + response = await client.delete( + f'/api/v1/user/totp/accounts/{FOREIGN_UUID}', + headers=_headers(), + ) + + assert response.status_code == 404 + assert (await response.get_json())['code'] == 'account_not_found' + application.totp_service.revoke_for_account.assert_not_awaited() + + +async def test_foreign_account_cannot_be_re_bound(totp_admin_api): + application, client = totp_admin_api + + enroll = await client.post( + f'/api/v1/user/totp/accounts/{FOREIGN_UUID}/enroll', + headers=_headers(), + ) + confirm = await client.post( + f'/api/v1/user/totp/accounts/{FOREIGN_UUID}/enroll/confirm', + json={'code': '123456'}, + headers=_headers(), + ) + + assert enroll.status_code == 404 + assert confirm.status_code == 404 + application.totp_service.begin_enrollment.assert_not_awaited() + application.totp_service.confirm_enrollment.assert_not_awaited() diff --git a/web/src/app/home/components/account-settings-dialog/AccountSettingsPanel.tsx b/web/src/app/home/components/account-settings-dialog/AccountSettingsPanel.tsx index ea12d95f4..1839815ce 100644 --- a/web/src/app/home/components/account-settings-dialog/AccountSettingsPanel.tsx +++ b/web/src/app/home/components/account-settings-dialog/AccountSettingsPanel.tsx @@ -131,9 +131,11 @@ export default function AccountSettingsPanel({ async function loadTotpAccounts() { try { const res = await httpClient.getTotpAccounts(); - const list = res.accounts || []; - setIsManager(list.length > 1); - setTotpRows(list); + // The role rule lives in the API: owners and admins receive the Workspace + // list, everyone else receives 403. Deriving manager state from the list + // length would hide oversight in a single-member Workspace. + setIsManager(true); + setTotpRows(res.accounts || []); } catch { // A non-manager receives 403 here; the panel keeps working on own state. setIsManager(false); diff --git a/web/src/i18n/locales/en-US.ts b/web/src/i18n/locales/en-US.ts index e42a1d99f..5d29d623c 100644 --- a/web/src/i18n/locales/en-US.ts +++ b/web/src/i18n/locales/en-US.ts @@ -2316,7 +2316,7 @@ const enUS = { totpStatusDisabled: 'Not enabled', totpCodesRemaining: '{{count}} recovery codes left', totpManagerSectionDesc: - 'Owners and admins can review and revoke the second factor of any Account.', + 'Owners and admins can review and revoke the second factor of Accounts in this Workspace.', revokeTotp: 'Re-bind', totpAdminResetTitle: 'Re-bind two-factor authentication for {{user}}', totpAdminResetDesc: diff --git a/web/src/i18n/locales/es-ES.ts b/web/src/i18n/locales/es-ES.ts index b641f221a..80876513f 100644 --- a/web/src/i18n/locales/es-ES.ts +++ b/web/src/i18n/locales/es-ES.ts @@ -2367,7 +2367,7 @@ const esES = { totpStatusDisabled: 'No activada', totpCodesRemaining: 'Quedan {{count}} códigos de recuperación', totpManagerSectionDesc: - 'Los propietarios y administradores pueden revisar y restablecer el segundo factor de cualquier cuenta.', + 'Los propietarios y administradores pueden revisar y restablecer el segundo factor de las cuentas de este espacio de trabajo.', revokeTotp: 'Reasignar', totpAdminResetTitle: 'Reasignar la verificación en dos pasos de {{user}}', totpAdminResetDesc: diff --git a/web/src/i18n/locales/ja-JP.ts b/web/src/i18n/locales/ja-JP.ts index e2d123bf1..0d8746171 100644 --- a/web/src/i18n/locales/ja-JP.ts +++ b/web/src/i18n/locales/ja-JP.ts @@ -2331,7 +2331,7 @@ const jaJP = { totpStatusDisabled: '未設定', totpCodesRemaining: 'リカバリーコード残り {{count}} 個', totpManagerSectionDesc: - 'オーナーと管理者はすべてのアカウントの二段階認証を確認・解除できます。', + 'オーナーと管理者はこのワークスペースのアカウントの二段階認証を確認・解除できます。', revokeTotp: '再バインド', totpAdminResetTitle: '{{user}} の二段階認証を再バインド', totpAdminResetDesc: diff --git a/web/src/i18n/locales/ru-RU.ts b/web/src/i18n/locales/ru-RU.ts index bebd8a4ce..eca8e3767 100644 --- a/web/src/i18n/locales/ru-RU.ts +++ b/web/src/i18n/locales/ru-RU.ts @@ -2339,7 +2339,7 @@ const ruRU = { totpStatusDisabled: 'Не включено', totpCodesRemaining: 'Осталось кодов восстановления: {{count}}', totpManagerSectionDesc: - 'Владельцы и администраторы могут просматривать и сбрасывать второй фактор любого аккаунта.', + 'Владельцы и администраторы могут просматривать и сбрасывать второй фактор аккаунтов этого рабочего пространства.', revokeTotp: 'Перепривязать', totpAdminResetTitle: 'Перепривязать двухфакторную аутентификацию для {{user}}', diff --git a/web/src/i18n/locales/th-TH.ts b/web/src/i18n/locales/th-TH.ts index beedf3e89..99b465e5c 100644 --- a/web/src/i18n/locales/th-TH.ts +++ b/web/src/i18n/locales/th-TH.ts @@ -2268,7 +2268,7 @@ const thTH = { totpStatusDisabled: 'ยังไม่เปิดใช้', totpCodesRemaining: 'เหลือรหัสกู้คืน {{count}} รหัส', totpManagerSectionDesc: - 'เจ้าของและผู้ดูแลสามารถตรวจสอบและรีเซ็ตการยืนยันสองขั้นตอนของบัญชีใดก็ได้', + 'เจ้าของและผู้ดูแลสามารถตรวจสอบและรีเซ็ตการยืนยันสองขั้นตอนของบัญชีในพื้นที่ทำงานนี้', revokeTotp: 'ผูกใหม่', totpAdminResetTitle: 'ผูกการยืนยันสองขั้นตอนใหม่ให้ {{user}}', totpAdminResetDesc: diff --git a/web/src/i18n/locales/vi-VN.ts b/web/src/i18n/locales/vi-VN.ts index e31d0cced..5e3dbfc60 100644 --- a/web/src/i18n/locales/vi-VN.ts +++ b/web/src/i18n/locales/vi-VN.ts @@ -2302,7 +2302,7 @@ const viVN = { totpStatusDisabled: 'Chưa bật', totpCodesRemaining: 'Còn {{count}} mã khôi phục', totpManagerSectionDesc: - 'Chủ sở hữu và quản trị viên có thể xem và đặt lại yếu tố thứ hai của bất kỳ tài khoản nào.', + 'Chủ sở hữu và quản trị viên có thể xem và đặt lại yếu tố thứ hai của các tài khoản trong không gian làm việc này.', revokeTotp: 'Liên kết lại', totpAdminResetTitle: 'Liên kết lại xác thực hai bước cho {{user}}', totpAdminResetDesc: diff --git a/web/src/i18n/locales/zh-Hans.ts b/web/src/i18n/locales/zh-Hans.ts index 53e1e5925..d206bea70 100644 --- a/web/src/i18n/locales/zh-Hans.ts +++ b/web/src/i18n/locales/zh-Hans.ts @@ -2175,7 +2175,7 @@ const zhHans = { totpRecoveryCodesRegenerated: '已生成新的恢复代码', totpStatusDisabled: '未启用', totpCodesRemaining: '剩余 {{count}} 个恢复代码', - totpManagerSectionDesc: '所有者和管理员可以查看并重置任意账户的两步验证。', + totpManagerSectionDesc: '所有者和管理员可以查看并重置本工作区账户的两步验证。', revokeTotp: '重新绑定', totpAdminResetTitle: '重新绑定 {{user}} 的两步验证', totpAdminResetDesc: diff --git a/web/src/i18n/locales/zh-Hant.ts b/web/src/i18n/locales/zh-Hant.ts index 40dee9b93..c6dd57b15 100644 --- a/web/src/i18n/locales/zh-Hant.ts +++ b/web/src/i18n/locales/zh-Hant.ts @@ -2172,7 +2172,7 @@ const zhHant = { totpRecoveryCodesRegenerated: '已產生新的恢復代碼', totpStatusDisabled: '未啟用', totpCodesRemaining: '剩餘 {{count}} 個恢復代碼', - totpManagerSectionDesc: '擁有者與管理員可以檢視並重設任意帳號的兩步驗證。', + totpManagerSectionDesc: '擁有者與管理員可以檢視並重設本工作區帳號的兩步驗證。', revokeTotp: '重新綁定', totpAdminResetTitle: '重新綁定 {{user}} 的兩步驗證', totpAdminResetDesc: