fix(api): scope TOTP oversight to the caller's workspace

The second-factor listing was instance-wide and the three mutating endpoints never checked that the target Account belongs to the caller's Workspace, so an owner or admin could see and reset another tenant's account. The listing now resolves the Workspace members and the mutations answer 404 for a foreign account, which does not disclose that it exists.

Also fixes the TOTP write path on Postgres: six write points passed timezone-aware datetimes into timestamp-without-time-zone columns, which asyncpg rejects, so every enroll, confirm, revoke and recovery-code consumption raised a 500 there. SQLite tolerated the same values.

The panel derives its oversight view from the API authorization instead of the length of the account list, which would have hidden it in a single-member Workspace.
This commit is contained in:
Hyu
2026-09-28 22:38:30 +08:00
parent 06ca7a93a9
commit e14e277424
12 changed files with 208 additions and 27 deletions
@@ -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/<target_account_uuid>', 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:
+27 -10
View File
@@ -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)
@@ -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()
@@ -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);
+1 -1
View File
@@ -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:
+1 -1
View File
@@ -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:
+1 -1
View File
@@ -2331,7 +2331,7 @@ const jaJP = {
totpStatusDisabled: '未設定',
totpCodesRemaining: 'リカバリーコード残り {{count}} 個',
totpManagerSectionDesc:
'オーナーと管理者はすべてのアカウントの二段階認証を確認・解除できます。',
'オーナーと管理者はこのワークスペースのアカウントの二段階認証を確認・解除できます。',
revokeTotp: '再バインド',
totpAdminResetTitle: '{{user}} の二段階認証を再バインド',
totpAdminResetDesc:
+1 -1
View File
@@ -2339,7 +2339,7 @@ const ruRU = {
totpStatusDisabled: 'Не включено',
totpCodesRemaining: 'Осталось кодов восстановления: {{count}}',
totpManagerSectionDesc:
'Владельцы и администраторы могут просматривать и сбрасывать второй фактор любого аккаунта.',
'Владельцы и администраторы могут просматривать и сбрасывать второй фактор аккаунтов этого рабочего пространства.',
revokeTotp: 'Перепривязать',
totpAdminResetTitle:
'Перепривязать двухфакторную аутентификацию для {{user}}',
+1 -1
View File
@@ -2268,7 +2268,7 @@ const thTH = {
totpStatusDisabled: 'ยังไม่เปิดใช้',
totpCodesRemaining: 'เหลือรหัสกู้คืน {{count}} รหัส',
totpManagerSectionDesc:
'เจ้าของและผู้ดูแลสามารถตรวจสอบและรีเซ็ตการยืนยันสองขั้นตอนของบัญชีใดก็ได้',
'เจ้าของและผู้ดูแลสามารถตรวจสอบและรีเซ็ตการยืนยันสองขั้นตอนของบัญชีในพื้นที่ทำงานนี้',
revokeTotp: 'ผูกใหม่',
totpAdminResetTitle: 'ผูกการยืนยันสองขั้นตอนใหม่ให้ {{user}}',
totpAdminResetDesc:
+1 -1
View File
@@ -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:
+1 -1
View File
@@ -2175,7 +2175,7 @@ const zhHans = {
totpRecoveryCodesRegenerated: '已生成新的恢复代码',
totpStatusDisabled: '未启用',
totpCodesRemaining: '剩余 {{count}} 个恢复代码',
totpManagerSectionDesc: '所有者和管理员可以查看并重置任意账户的两步验证。',
totpManagerSectionDesc: '所有者和管理员可以查看并重置本工作区账户的两步验证。',
revokeTotp: '重新绑定',
totpAdminResetTitle: '重新绑定 {{user}} 的两步验证',
totpAdminResetDesc:
+1 -1
View File
@@ -2172,7 +2172,7 @@ const zhHant = {
totpRecoveryCodesRegenerated: '已產生新的恢復代碼',
totpStatusDisabled: '未啟用',
totpCodesRemaining: '剩餘 {{count}} 個恢復代碼',
totpManagerSectionDesc: '擁有者與管理員可以檢視並重設任意帳號的兩步驗證。',
totpManagerSectionDesc: '擁有者與管理員可以檢視並重設本工作區帳號的兩步驗證。',
revokeTotp: '重新綁定',
totpAdminResetTitle: '重新綁定 {{user}} 的兩步驗證',
totpAdminResetDesc: