Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ The `ViewerContext` contextvar carries the current viewer's identity. When the v
- `acting_user_id(request.user)`: the user behind the call (an agent's is the user it acts for), or None. Handle None.
- `has_user(request.user)`: whether there is one; narrows to `User | RpcUser`.
- `UserRequired` (403): raise it when code needs a user and there is none.
- `set_request_principal(request, user, auth)`: the only place `request.user` and `request.auth` are set.

Personal state (bookmarks, "is this mine?") needs a user; "who did this" fields record None. Never look rows up by a possibly-None user id: `filter(user_id=None)` matches rows with no user.

Expand Down
8 changes: 4 additions & 4 deletions src/sentry/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
)
from sentry.apidocs.hooks import HTTP_METHOD_NAME
from sentry.auth import access
from sentry.auth.principal import set_request_principal
from sentry.auth.scope_declaration import bind_endpoint_scope_declaration
from sentry.auth.staff import has_staff_option
from sentry.hybridcloud.apigateway.cell_request_resolvers import CellRequestResolver
Expand Down Expand Up @@ -439,10 +440,9 @@ def initialize_request(self, request: HttpRequest, *args: Any, **kwargs: Any) ->
# If our request is being made via our internal API client, we need to
# stitch back on auth and user information
if getattr(request, "__from_api_client__", False):
if rv.auth is None:
rv.auth = orig_auth
if rv.user is None:
rv.user = orig_user # type: ignore[unreachable] # the request here is partially initialized
auth = orig_auth if rv.auth is None else rv.auth
user = orig_user if rv.user is None else rv.user
set_request_principal(rv, user, auth)
return rv

def has_pagination(self, response: Response) -> bool:
Expand Down
15 changes: 10 additions & 5 deletions src/sentry/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
from django.urls import resolve
from rest_framework.test import APIRequestFactory, force_authenticate

from sentry.auth.principal import set_request_principal
from sentry.auth.superuser import Superuser
from sentry.viewer_context import get_viewer_context, viewer_context_scope

__all__ = ("ApiClient", "ApiError", "client")

Expand All @@ -30,7 +32,12 @@ class ApiClient:

ApiError: TypeAlias = ApiError

def request(
def request(self, *args, **kwargs):
# The called view sets its own caller on the viewer context; keep it out of ours.
with viewer_context_scope(get_viewer_context()):
return self._request(*args, **kwargs)

def _request(
self,
method,
path,
Expand Down Expand Up @@ -65,8 +72,7 @@ def request(
mock_request.__from_api_client__ = True

if request:
mock_request.auth = getattr(request, "auth", None)
mock_request.user = request.user
set_request_principal(mock_request, request.user, getattr(request, "auth", None))

if is_sudo is None:
mock_request.is_sudo = lambda: request.is_sudo()
Expand All @@ -79,8 +85,7 @@ def request(
else:
mock_request.superuser = Superuser(mock_request)
else:
mock_request.auth = auth
mock_request.user = user or AnonymousUser()
set_request_principal(mock_request, user or AnonymousUser(), auth)
mock_request.is_sudo = lambda: is_sudo
mock_request.session = {}
mock_request.superuser = Superuser(mock_request)
Expand Down
7 changes: 4 additions & 3 deletions src/sentry/api/endpoints/auth_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from sentry.api.validators import AuthVerifyValidator
from sentry.api.validators.auth import MISSING_PASSWORD_OR_U2F_CODE
from sentry.auth.authenticators.u2f import U2fInterface
from sentry.auth.principal import set_request_principal
from sentry.auth.providers.saml2.provider import handle_saml_single_logout
from sentry.auth.services.auth.impl import promote_request_rpc_user
from sentry.auth.superuser import SUPERUSER_ORG_ID, SuperuserAccessSerializer
Expand Down Expand Up @@ -238,7 +239,7 @@ def post(self, request: Request) -> Response:
status=status.HTTP_403_FORBIDDEN,
)

request.user = request._request.user
set_request_principal(request, request._request.user, request.auth)

return self.get(request)

Expand Down Expand Up @@ -356,7 +357,7 @@ def put(self, request: Request) -> Response:
serializer.validated_data["superuserReason"],
)

request.user = request._request.user
set_request_principal(request, request._request.user, request.auth)

return self.get(request)

Expand All @@ -377,7 +378,7 @@ def delete(self, request: Request, *args, **kwargs) -> Response:

# For signals to work here, we must promote the request.user to a full user object
logout(request._request)
request.user = AnonymousUser()
set_request_principal(request, AnonymousUser(), request.auth)

# Force cookies to be deleted
response = Response()
Expand Down
51 changes: 45 additions & 6 deletions src/sentry/auth/principal.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@
from typing import Any, Protocol, TypeIs

from django.contrib.auth.models import AnonymousUser
from django.http.request import HttpRequest
from rest_framework.request import Request

from sentry.auth.services.auth.model import AuthenticatedToken
from sentry.types.superuser import to_viewer_context_superuser
from sentry.users.models.user import User
from sentry.users.services.user.model import RpcUser
from sentry.viewer_context import Actor, ActorType
from sentry.utils.auth import is_user_from_viewer_context
from sentry.viewer_context import Actor, ActorType, set_viewer_context_actor


class RequestPrincipal(Protocol):
Expand Down Expand Up @@ -54,24 +59,58 @@ def resolve_actor(user: Any, auth: Any) -> Actor | None:
"""The actor behind an authenticated ``(user, auth)`` pair, or None if there is none."""
from sentry.seer.agent_token import is_agent_auth # circular: imported at app setup

superuser = (
to_viewer_context_superuser(auth.superuser)
if isinstance(auth, AuthenticatedToken)
else None
)
if is_agent_auth(auth):
return Actor(type=ActorType.AGENT, delegated_user_id=auth.user_id)
return Actor(type=ActorType.AGENT, delegated_user_id=auth.user_id, superuser=superuser)
user_id = acting_user_id(user)
if user_id is not None:
return Actor(type=ActorType.USER, actor_id=user_id)
return Actor(type=ActorType.USER, actor_id=user_id, superuser=superuser)
return None


def set_request_principal(request: HttpRequest | Request, user: Any, auth: Any) -> None:
"""
Set ``request.user`` and ``request.auth``.

The only place they're set, so what lands on ``request.user`` is decided here.
"""
request.user = user
request.auth = auth


_drf_authenticate = vars(Request)["_authenticate"]


def _authenticate(self: Request) -> None:
# DRF sets the authenticator's result itself; route it through the chokepoint.
_drf_authenticate(self)
set_request_principal(self, self.user, self.auth)
# Endpoints with their own authentication classes run them after the viewer-context
# middleware, so update the caller it recorded. A credential with no caller of its own,
# e.g. a signed service call, keeps the one from the viewer-context header.
# A user loaded from a signed viewer context keeps its actor too, e.g. an agent.
if is_user_from_viewer_context(self):
return
if (actor := resolve_actor(self.user, self.auth)) is not None:
set_viewer_context_actor(actor)
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved


def _anonymous_display_name(self: AnonymousUser) -> str:
return ""


def patch_request_principal() -> None:
"""
Give Django's ``AnonymousUser`` the ``RequestPrincipal`` interface.
Route DRF authentication through ``set_request_principal`` and give Django's
``AnonymousUser`` the ``RequestPrincipal`` interface.

Patched rather than subclassed because Django, DRF and our own code construct it in
many places. ``tools/mypy_helpers/plugin.py`` declares the same members.
Patched rather than subclassed because Django, DRF and our own code construct these
in many places. ``tools/mypy_helpers/plugin.py`` declares the same members.
"""
Request._authenticate = _authenticate # type: ignore[attr-defined]
AnonymousUser.is_sentry_app = False
AnonymousUser.get_display_name = _anonymous_display_name # type: ignore[method-assign]
8 changes: 4 additions & 4 deletions src/sentry/auth/services/auth/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,17 @@ def applied_to_request[T: HttpRequest](self, request: T) -> Generator[T]:
isolated, controlled way. This method allows for a context handling an RPC or inter silo behavior to assume
the correct user and auth context provided explicitly in a context.
"""
from sentry.auth.principal import set_request_principal # imported at app setup

old_user = request.user
old_auth = request.auth

request.user = self._get_user() # type: ignore[assignment] # RpcUser vs. User
request.auth = self.auth
set_request_principal(request, self._get_user(), self.auth)

try:
yield request
finally:
request.user = old_user
request.auth = old_auth
set_request_principal(request, old_user, old_auth)


class RpcAuthProviderFlags(RpcModel):
Expand Down
13 changes: 8 additions & 5 deletions src/sentry/middleware/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
UserAuthTokenAuthentication,
ViewerContextAuthentication,
)
from sentry.auth.principal import set_request_principal
from sentry.users.models.userip import UserIP
from sentry.utils.auth import AuthUserPasswordExpired, logger

Expand Down Expand Up @@ -68,7 +69,7 @@ def process_request(self, request: Request) -> None:
if request.path.startswith("/api/0/internal/rpc/"):
# Avoid doing RPC authentication when we're already
# in an RPC request.
request.user, request.auth = AnonymousUser(), None
set_request_principal(request, AnonymousUser(), None)
return

auth = get_authorization_header(request).split()
Expand All @@ -83,10 +84,12 @@ def process_request(self, request: Request) -> None:
except AuthenticationFailed:
result = None
if result:
request.user, request.auth = result
set_request_principal(request, *result)
else:
# default to anonymous user and use IP ratelimit
request.user, request.auth = SimpleLazyObject(lambda: get_user(request)), None # type:ignore[assignment]
set_request_principal(
request, SimpleLazyObject(lambda: get_user(request)), None
)
return

# Try viewer context authentication (signed headers from trusted services)
Expand All @@ -95,11 +98,11 @@ def process_request(self, request: Request) -> None:
except AuthenticationFailed:
result = None
if result:
request.user, request.auth = result
set_request_principal(request, *result)
return

# default to anonymous user and use IP ratelimit
request.user, request.auth = SimpleLazyObject(lambda: get_user(request)), None # type:ignore[assignment]
set_request_principal(request, SimpleLazyObject(lambda: get_user(request)), None)

def process_exception(
self, request: HttpRequest, exception: Exception
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/testutils/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
from sentry import auth
from sentry.api.serializers.models.dashboard import DATASET_SOURCES
from sentry.auth.authenticators.totp import TotpInterface
from sentry.auth.principal import set_request_principal
from sentry.auth.provider import Provider
from sentry.auth.providers.dummy import DummyProvider
from sentry.auth.providers.saml2.activedirectory.apps import (
Expand Down Expand Up @@ -297,8 +298,7 @@ def make_request(

# order matters here, session -> user -> other things
request.session = self.session
request.auth = auth
request.user = user or AnonymousUser()
set_request_principal(request, user or AnonymousUser(), auth)
# must happen after request.user/request.session is populated
request.superuser = Superuser(request)
request.staff = Staff(request)
Expand Down Expand Up @@ -332,7 +332,7 @@ def login_as(
request = self.make_request()
with assume_test_silo_mode(SiloMode.CONTROL):
login(request, user)
request.user = user
set_request_principal(request, user, request.auth)

if organization_ids is None:
organization_ids = set()
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/testutils/requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from rest_framework.request import Request
from rest_framework.views import APIView

from sentry.auth.principal import set_request_principal
from sentry.middleware.auth import AuthenticationMiddleware
from sentry.middleware.placeholder import placeholder_get_response
from sentry.testutils.factories import Factories
Expand All @@ -29,8 +30,7 @@ def wrapper(*args, **kwds) -> tuple[HttpRequest, User] | None:
login(request, user)
AuthenticationMiddleware(placeholder_get_response).process_request(request)
else:
request.user = user
request.auth = None
set_request_principal(request, user, None)
cache.clear()
return result

Expand Down
11 changes: 11 additions & 0 deletions src/sentry/viewer_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,17 @@ def set_viewer_context_superuser(
)


def set_viewer_context_actor(actor: Actor) -> None:
"""Replace the caller, e.g. when an endpoint authenticates after the middleware did."""
ctx = get_viewer_context()
if ctx is None or ctx.actor == actor:
return
if actor.superuser is None and dataclasses.replace(ctx.actor, superuser=None) == actor:
# Same caller: keep the elevation the access checks granted.
return
_viewer_context_var.set(dataclasses.replace(ctx, actor=actor))


def set_viewer_context_organization(organization_id: int) -> None:
"""Update the current ``ViewerContext`` with a resolved organization id."""
ctx = get_viewer_context()
Expand Down
3 changes: 2 additions & 1 deletion src/sentry/web/frontend/auth_logout.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.http.response import HttpResponseBase
from django.utils.http import url_has_allowed_host_and_scheme

from sentry.auth.principal import set_request_principal
from sentry.utils import auth
from sentry.web.frontend.base import BaseView, control_silo_view

Expand All @@ -23,5 +24,5 @@ def get(self, request: HttpRequest) -> HttpResponseBase:

def post(self, request: HttpRequest) -> HttpResponseBase:
logout(request)
request.user = AnonymousUser()
set_request_principal(request, AnonymousUser(), request.auth)
return self._redirect(request)
20 changes: 20 additions & 0 deletions tests/sentry/api/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@

from sentry.api.client import ApiClient
from sentry.testutils.cases import TestCase
from sentry.viewer_context import (
Actor,
ActorType,
ViewerContext,
get_viewer_context,
viewer_context_scope,
)


class ClientParameterHandlingTest(TestCase):
Expand Down Expand Up @@ -42,3 +49,16 @@ def test_mixed_parameters_in_query_string(self, mock_resolve):
actual_queries = dict(request.GET.lists())

assert actual_queries == expected_queries


class ClientViewerContextTest(TestCase):
@patch("sentry.api.client.resolve")
def test_view_caller_does_not_leak(self, mock_resolve):
mock_view = Mock(return_value=JsonResponse({}))
mock_resolve.return_value = (mock_view, (), {})
outer = ViewerContext(actor=Actor(type=ActorType.UNKNOWN))
other = self.create_user()

with viewer_context_scope(outer):
ApiClient().get(user=other, path="/test/")
assert get_viewer_context() == outer
Loading
Loading