Skip to content

Commit 8445a5b

Browse files
committed
ref(typing): Let security emails take a service account
capture_security_activity() and generate_security_email() take a RequestPrincipal as the actor. The log line records acting_user_id(actor), so a service account logs no actor id instead of a ServiceAccountId; the email template already renders a blank address for callers with no user. Drops organization_auth_tokens.py and user_authenticator_details.py from the request.user allowlist.
1 parent cf654d3 commit 8445a5b

4 files changed

Lines changed: 10 additions & 7 deletions

File tree

‎src/sentry/security/emails.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
from datetime import datetime
55
from typing import TYPE_CHECKING, Any, Protocol
66

7-
from django.contrib.auth.models import AnonymousUser
87
from django.utils import timezone
98

9+
from sentry.auth.principal import RequestPrincipal
1010
from sentry.users.services.user.model import RpcUser
1111
from sentry.utils.email import MessageBuilder
1212

@@ -32,7 +32,7 @@ def email(self) -> str: ...
3232
def generate_security_email(
3333
account: User | RpcUser | SecurityEmailAccount,
3434
type: str,
35-
actor: AnonymousUser | User | RpcUser,
35+
actor: RequestPrincipal,
3636
ip_address: str,
3737
context: Mapping[str, Any] | None = None,
3838
current_datetime: datetime | None = None,

‎src/sentry/security/utils.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from django.contrib.auth.models import AnonymousUser
99
from django.utils import timezone
1010

11+
from sentry.auth.principal import RequestPrincipal, acting_user_id
1112
from sentry.organizations.services.organization.model import RpcOrganization
1213
from sentry.sentry_apps.services.app.model import RpcSentryApp
1314
from sentry.users.services.user.model import RpcUser
@@ -26,7 +27,7 @@
2627
def capture_security_activity(
2728
account: User | RpcUser | AnonymousUser | SecurityEmailAccount,
2829
type: str, # FIXME: "type" is a built-in function, so this isn't a great name
29-
actor: User | RpcUser | AnonymousUser,
30+
actor: RequestPrincipal,
3031
ip_address: str,
3132
context: Mapping[str, Any] | None = None,
3233
send_email: bool = True,
@@ -37,7 +38,11 @@ def capture_security_activity(
3738

3839
assert not isinstance(account, AnonymousUser)
3940

40-
logger_context = {"ip_address": ip_address, "user_id": account.id, "actor_id": actor.id}
41+
logger_context = {
42+
"ip_address": ip_address,
43+
"user_id": account.id,
44+
"actor_id": acting_user_id(actor),
45+
}
4146

4247
if type == "mfa-removed" or type == "mfa-added":
4348
assert context is not None

‎tests/sentry/auth/test_principal.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ def _is_request_user_type_check(node: ast.AST) -> bool:
277277
# Lower these as callers move off. Exact, so the baseline can't drift back up.
278278
REQUEST_USER_ID_BASELINE = 133
279279
REQUEST_USER_TYPE_CHECK_BASELINE = 29
280-
REQUEST_USER_ALLOWLIST_BASELINE = 17
280+
REQUEST_USER_ALLOWLIST_BASELINE = 15
281281

282282

283283
def test_request_user_id_uses_only_go_down() -> None:

‎tools/mypy_helpers/request_user_allowlist.txt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
src/sentry/api/bases/organizationmember.py
2-
src/sentry/api/endpoints/organization_auth_tokens.py
32
src/sentry/api/endpoints/organization_events.py
43
src/sentry/api/endpoints/organization_events_stats.py
54
src/sentry/api/endpoints/organization_releases.py
@@ -13,5 +12,4 @@ src/sentry/data_export/endpoints/data_export.py
1312
src/sentry/issues/endpoints/group_integration_details.py
1413
src/sentry/notebooks/endpoints/base.py
1514
src/sentry/seer/endpoints/organization_agent_token.py
16-
src/sentry/users/api/endpoints/user_authenticator_details.py
1715
src/sentry/web/client_config.py

0 commit comments

Comments
 (0)