feat(mail): enforce owned profile self-service
This commit is contained in:
@@ -2,14 +2,17 @@ from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import ANY, patch
|
||||
|
||||
from fastapi import HTTPException
|
||||
from sqlalchemy.dialects import postgresql
|
||||
|
||||
from govoplan_mail.backend import router
|
||||
from govoplan_mail.backend import mail_profiles
|
||||
from govoplan_mail.backend.mail_profiles import (
|
||||
EffectiveMailProfilePolicy,
|
||||
MailProfileError,
|
||||
get_mail_server_profile_for_actor,
|
||||
list_mail_server_profiles,
|
||||
mail_profile_visible_to_actor,
|
||||
)
|
||||
@@ -35,6 +38,8 @@ def _profile(
|
||||
name=profile_id,
|
||||
smtp_config={"host": "smtp.example.test"},
|
||||
imap_config={"host": "imap.example.test"},
|
||||
smtp_password_encrypted=None,
|
||||
imap_password_encrypted=None,
|
||||
)
|
||||
|
||||
|
||||
@@ -67,11 +72,15 @@ class _Session:
|
||||
def rollback(self):
|
||||
self.rollbacks += 1
|
||||
|
||||
def refresh(self, _value):
|
||||
return None
|
||||
|
||||
|
||||
class _Principal:
|
||||
tenant_id = "tenant-1"
|
||||
user = SimpleNamespace(id="user-1")
|
||||
group_ids = frozenset({"group-1"})
|
||||
api_key_id = None
|
||||
|
||||
def __init__(self, scopes):
|
||||
self._scopes = frozenset(scopes)
|
||||
@@ -81,6 +90,373 @@ class _Principal:
|
||||
|
||||
|
||||
class ProfileActorAuthorizationTests(unittest.TestCase):
|
||||
def test_profile_mutation_selector_is_owner_bounded_and_row_locked(self) -> None:
|
||||
statement = mail_profiles._mail_server_profile_mutation_statement( # noqa: SLF001
|
||||
tenant_id="tenant-1",
|
||||
profile_id="profile-1",
|
||||
owner_user_id="user-1",
|
||||
can_manage_tenant_profiles=False,
|
||||
can_manage_system_profiles=False,
|
||||
)
|
||||
|
||||
sql = str(statement.compile(dialect=postgresql.dialect()))
|
||||
self.assertIn("FOR UPDATE", sql)
|
||||
self.assertIn("mail_server_profiles.tenant_id =", sql)
|
||||
self.assertIn("mail_server_profiles.scope_type =", sql)
|
||||
self.assertIn("mail_server_profiles.scope_id =", sql)
|
||||
self.assertTrue(statement.get_execution_options()["populate_existing"])
|
||||
|
||||
def test_profile_mutation_selector_without_authority_cannot_lock_a_row(self) -> None:
|
||||
statement = mail_profiles._mail_server_profile_mutation_statement( # noqa: SLF001
|
||||
tenant_id="tenant-1",
|
||||
profile_id="profile-1",
|
||||
owner_user_id=None,
|
||||
can_manage_tenant_profiles=False,
|
||||
can_manage_system_profiles=False,
|
||||
)
|
||||
|
||||
sql = str(statement.compile(dialect=postgresql.dialect()))
|
||||
self.assertIn("mail_server_profiles.id IS NULL", sql)
|
||||
self.assertIn("FOR UPDATE", sql)
|
||||
|
||||
def test_self_service_profile_permissions_are_limited_to_own_user_scope(self) -> None:
|
||||
principal = _Principal(
|
||||
{"mail:profile:write_own", "mail:secret:manage_own"}
|
||||
)
|
||||
|
||||
router._require_profile_write_scope( # noqa: SLF001 - authorization seam
|
||||
principal, # type: ignore[arg-type]
|
||||
"user",
|
||||
"user-1",
|
||||
)
|
||||
router._require_profile_credentials_scope( # noqa: SLF001 - authorization seam
|
||||
principal, # type: ignore[arg-type]
|
||||
"user",
|
||||
"user-1",
|
||||
)
|
||||
|
||||
for scope_type, scope_id in (
|
||||
("user", "user-2"),
|
||||
("tenant", "tenant-1"),
|
||||
("group", "group-1"),
|
||||
("campaign", "campaign-1"),
|
||||
("system", None),
|
||||
):
|
||||
with self.subTest(scope_type=scope_type, scope_id=scope_id):
|
||||
with self.assertRaises(HTTPException) as write_denied:
|
||||
router._require_profile_write_scope( # noqa: SLF001
|
||||
principal, # type: ignore[arg-type]
|
||||
scope_type,
|
||||
scope_id,
|
||||
)
|
||||
self.assertEqual(write_denied.exception.status_code, 403)
|
||||
|
||||
with self.assertRaises(HTTPException) as secret_denied:
|
||||
router._require_profile_credentials_scope( # noqa: SLF001
|
||||
principal, # type: ignore[arg-type]
|
||||
scope_type,
|
||||
scope_id,
|
||||
)
|
||||
self.assertEqual(secret_denied.exception.status_code, 403)
|
||||
|
||||
def test_self_service_create_rejects_another_user_before_persistence(self) -> None:
|
||||
principal = _Principal(
|
||||
{
|
||||
"mail:profile:read",
|
||||
"mail:profile:write_own",
|
||||
"mail:secret:manage_own",
|
||||
}
|
||||
)
|
||||
payload = MailServerProfileCreateRequest.model_validate(
|
||||
{
|
||||
"name": "Other user's profile",
|
||||
"scope_type": "user",
|
||||
"scope_id": "user-2",
|
||||
"smtp": {
|
||||
"host": "smtp.example.test",
|
||||
"password": "not-persisted",
|
||||
},
|
||||
}
|
||||
)
|
||||
with (
|
||||
patch("govoplan_mail.backend.router.create_mail_server_profile") as create,
|
||||
self.assertRaises(HTTPException) as denied,
|
||||
):
|
||||
router.create_profile(
|
||||
payload,
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
session=_Session(), # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
self.assertEqual(denied.exception.status_code, 403)
|
||||
create.assert_not_called()
|
||||
|
||||
def test_self_service_update_hides_another_user_before_mutation(self) -> None:
|
||||
principal = _Principal(
|
||||
{"mail:profile:write_own", "mail:secret:manage_own"}
|
||||
)
|
||||
profile = _profile(
|
||||
"other-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-2",
|
||||
)
|
||||
with (
|
||||
patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
return_value=profile,
|
||||
),
|
||||
patch("govoplan_mail.backend.router.update_mail_server_profile") as update,
|
||||
self.assertRaises(HTTPException) as denied,
|
||||
):
|
||||
router.update_profile(
|
||||
profile.id,
|
||||
MailServerProfileUpdateRequest(name="Must not change"),
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
session=_Session(), # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
self.assertEqual(denied.exception.status_code, 404)
|
||||
update.assert_not_called()
|
||||
|
||||
def test_self_service_delete_hides_another_user_before_mutation(self) -> None:
|
||||
principal = _Principal(
|
||||
{"mail:profile:write_own", "mail:secret:manage_own"}
|
||||
)
|
||||
profile = _profile(
|
||||
"other-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-2",
|
||||
)
|
||||
with (
|
||||
patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
return_value=profile,
|
||||
),
|
||||
patch("govoplan_mail.backend.router.delete_mail_profile_credentials") as delete,
|
||||
self.assertRaises(HTTPException) as denied,
|
||||
):
|
||||
router.deactivate_profile(
|
||||
profile.id,
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
session=_Session(), # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
self.assertEqual(denied.exception.status_code, 404)
|
||||
delete.assert_not_called()
|
||||
|
||||
def test_mutation_lookup_returns_the_same_not_found_for_missing_and_non_owned(self) -> None:
|
||||
principal = _Principal({"mail:profile:write_own"})
|
||||
other_profile = _profile(
|
||||
"other-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-2",
|
||||
)
|
||||
cases = (
|
||||
(other_profile, None),
|
||||
(None, MailProfileError("Mail-server profile not found")),
|
||||
)
|
||||
for returned, failure in cases:
|
||||
with self.subTest(failure=failure is not None):
|
||||
kwargs = {"side_effect": failure} if failure else {"return_value": returned}
|
||||
with (
|
||||
patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
**kwargs,
|
||||
),
|
||||
self.assertRaises(HTTPException) as denied,
|
||||
):
|
||||
router._profile_for_mutation( # noqa: SLF001 - authorization seam
|
||||
_Session(), # type: ignore[arg-type]
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
profile_id="candidate-id",
|
||||
)
|
||||
|
||||
self.assertEqual(denied.exception.status_code, 404)
|
||||
self.assertEqual(denied.exception.detail, "Mail-server profile not found")
|
||||
|
||||
def test_broad_profile_admin_retains_cross_owner_mutation_access(self) -> None:
|
||||
principal = _Principal({"mail:profile:write"})
|
||||
profile = _profile(
|
||||
"other-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-2",
|
||||
)
|
||||
with patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
return_value=profile,
|
||||
) as get_profile:
|
||||
resolved = router._profile_for_mutation( # noqa: SLF001 - authorization seam
|
||||
_Session(), # type: ignore[arg-type]
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
profile_id=profile.id,
|
||||
)
|
||||
|
||||
self.assertIs(resolved, profile)
|
||||
get_profile.assert_called_once_with(
|
||||
ANY,
|
||||
tenant_id="tenant-1",
|
||||
profile_id=profile.id,
|
||||
for_update=True,
|
||||
mutation_owner_user_id=None,
|
||||
can_manage_tenant_profiles=True,
|
||||
can_manage_system_profiles=False,
|
||||
)
|
||||
|
||||
def test_self_service_transport_rebind_requires_own_secret_authority(self) -> None:
|
||||
principal = _Principal({"mail:profile:write_own"})
|
||||
profile = _profile(
|
||||
"own-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-1",
|
||||
)
|
||||
profile.smtp_config = {
|
||||
"host": "smtp.example.test",
|
||||
"port": 587,
|
||||
"security": "starttls",
|
||||
"timeout_seconds": 30,
|
||||
}
|
||||
profile.smtp_password_encrypted = "existing-ciphertext"
|
||||
payload = MailServerProfileUpdateRequest.model_validate(
|
||||
{"smtp": {"host": "smtp.attacker.test"}}
|
||||
)
|
||||
session = _Session()
|
||||
with (
|
||||
patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
return_value=profile,
|
||||
),
|
||||
patch("govoplan_mail.backend.router.update_mail_server_profile") as update,
|
||||
self.assertRaises(HTTPException) as denied,
|
||||
):
|
||||
router.update_profile(
|
||||
profile.id,
|
||||
payload,
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
session=session, # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
self.assertEqual(denied.exception.status_code, 403)
|
||||
self.assertEqual(session.rollbacks, 1)
|
||||
update.assert_not_called()
|
||||
|
||||
def test_self_service_imap_rebind_requires_own_secret_authority(self) -> None:
|
||||
principal = _Principal({"mail:profile:write_own"})
|
||||
profile = _profile(
|
||||
"own-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-1",
|
||||
)
|
||||
profile.imap_config = {
|
||||
"host": "imap.example.test",
|
||||
"port": 993,
|
||||
"security": "tls",
|
||||
"sent_folder": "auto",
|
||||
"timeout_seconds": 30,
|
||||
}
|
||||
profile.imap_password_encrypted = "existing-ciphertext"
|
||||
payload = MailServerProfileUpdateRequest.model_validate(
|
||||
{"imap": {"host": "imap.attacker.test"}}
|
||||
)
|
||||
with (
|
||||
patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
return_value=profile,
|
||||
),
|
||||
patch("govoplan_mail.backend.router.update_mail_server_profile") as update,
|
||||
self.assertRaises(HTTPException) as denied,
|
||||
):
|
||||
router.update_profile(
|
||||
profile.id,
|
||||
payload,
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
session=_Session(), # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
self.assertEqual(denied.exception.status_code, 403)
|
||||
update.assert_not_called()
|
||||
|
||||
def test_self_service_transport_rebind_is_allowed_with_own_secret_authority(self) -> None:
|
||||
principal = _Principal(
|
||||
{"mail:profile:write_own", "mail:secret:manage_own"}
|
||||
)
|
||||
profile = _profile(
|
||||
"own-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-1",
|
||||
)
|
||||
profile.smtp_config = {
|
||||
"host": "smtp.example.test",
|
||||
"port": 587,
|
||||
"security": "starttls",
|
||||
"timeout_seconds": 30,
|
||||
}
|
||||
profile.smtp_password_encrypted = "existing-ciphertext"
|
||||
payload = MailServerProfileUpdateRequest.model_validate(
|
||||
{"smtp": {"host": "smtp.allowed.test"}}
|
||||
)
|
||||
session = _Session()
|
||||
with (
|
||||
patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
return_value=profile,
|
||||
),
|
||||
patch(
|
||||
"govoplan_mail.backend.router.update_mail_server_profile",
|
||||
return_value=profile,
|
||||
) as update,
|
||||
patch("govoplan_mail.backend.router._record_mail_change"),
|
||||
patch(
|
||||
"govoplan_mail.backend.router._profile_response",
|
||||
return_value=profile,
|
||||
),
|
||||
):
|
||||
result = router.update_profile(
|
||||
profile.id,
|
||||
payload,
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
session=session, # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
self.assertIs(result, profile)
|
||||
self.assertEqual(session.commits, 1)
|
||||
update.assert_called_once()
|
||||
|
||||
def test_transport_endpoint_change_without_stored_secret_needs_only_write_authority(self) -> None:
|
||||
principal = _Principal({"mail:profile:write_own"})
|
||||
profile = _profile(
|
||||
"own-user-profile",
|
||||
scope_type="user",
|
||||
scope_id="user-1",
|
||||
)
|
||||
payload = MailServerProfileUpdateRequest.model_validate(
|
||||
{"smtp": {"host": "smtp.allowed.test"}}
|
||||
)
|
||||
session = _Session()
|
||||
with (
|
||||
patch(
|
||||
"govoplan_mail.backend.router.get_mail_server_profile",
|
||||
return_value=profile,
|
||||
),
|
||||
patch(
|
||||
"govoplan_mail.backend.router.update_mail_server_profile",
|
||||
return_value=profile,
|
||||
) as update,
|
||||
patch("govoplan_mail.backend.router._record_mail_change"),
|
||||
patch(
|
||||
"govoplan_mail.backend.router._profile_response",
|
||||
return_value=profile,
|
||||
),
|
||||
):
|
||||
router.update_profile(
|
||||
profile.id,
|
||||
payload,
|
||||
principal=principal, # type: ignore[arg-type]
|
||||
session=session, # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
self.assertEqual(session.commits, 1)
|
||||
update.assert_called_once()
|
||||
|
||||
def test_general_profile_list_hides_other_owner_contexts(self) -> None:
|
||||
profiles = [
|
||||
_profile("system", scope_type="system", scope_id=None, tenant_id=None),
|
||||
@@ -142,6 +518,63 @@ class ProfileActorAuthorizationTests(unittest.TestCase):
|
||||
|
||||
self.assertEqual([item.id for item in visible], ["inactive"])
|
||||
|
||||
def test_self_service_repair_visibility_is_limited_to_the_exact_owner(self) -> None:
|
||||
own_profile = _profile(
|
||||
"own-policy-invalid",
|
||||
scope_type="user",
|
||||
scope_id="user-1",
|
||||
)
|
||||
other_profile = _profile(
|
||||
"other-policy-invalid",
|
||||
scope_type="user",
|
||||
scope_id="user-2",
|
||||
)
|
||||
denied = EffectiveMailProfilePolicy(
|
||||
allowed_profile_id_sets=[{"different-profile"}]
|
||||
)
|
||||
session = _Session([own_profile, other_profile])
|
||||
|
||||
with patch(
|
||||
"govoplan_mail.backend.mail_profiles.effective_mail_profile_policy",
|
||||
return_value=denied,
|
||||
):
|
||||
ordinary_visible = list_mail_server_profiles(
|
||||
session, # type: ignore[arg-type]
|
||||
tenant_id="tenant-1",
|
||||
include_inactive=True,
|
||||
actor_user_id="user-1",
|
||||
actor_administrative_visibility=True,
|
||||
)
|
||||
repair_visible = list_mail_server_profiles(
|
||||
session, # type: ignore[arg-type]
|
||||
tenant_id="tenant-1",
|
||||
include_inactive=True,
|
||||
actor_user_id="user-1",
|
||||
actor_can_manage_own_profiles=True,
|
||||
actor_administrative_visibility=True,
|
||||
)
|
||||
resolved = get_mail_server_profile_for_actor(
|
||||
session, # type: ignore[arg-type]
|
||||
tenant_id="tenant-1",
|
||||
profile_id=own_profile.id,
|
||||
user_id="user-1",
|
||||
can_manage_own_profiles=True,
|
||||
administrative_visibility=True,
|
||||
)
|
||||
with self.assertRaises(MailProfileError):
|
||||
get_mail_server_profile_for_actor(
|
||||
session, # type: ignore[arg-type]
|
||||
tenant_id="tenant-1",
|
||||
profile_id=other_profile.id,
|
||||
user_id="user-1",
|
||||
can_manage_own_profiles=True,
|
||||
administrative_visibility=True,
|
||||
)
|
||||
|
||||
self.assertEqual(ordinary_visible, [])
|
||||
self.assertEqual([item.id for item in repair_visible], [own_profile.id])
|
||||
self.assertIs(resolved, own_profile)
|
||||
|
||||
def test_profile_admin_can_list_but_not_use_a_policy_denied_profile(self) -> None:
|
||||
profile = _profile("denied", scope_type="tenant", scope_id="tenant-1")
|
||||
session = _Session([profile])
|
||||
|
||||
Reference in New Issue
Block a user