Просмотр исходного кода

fix(hybridcloud) Get login working in devserver (#53684)

- Add more routes to webpack configuration so those paths forward to
control silo.
- Correct return value of user_service.get_by_username to serialize
users. Returning ORM models doesn't work across the RPC layer.
- Update utils.auth.find_users() to use the ORM directly. Authentication
takes place on control silo so we can use the ORM directly. Using an RPC
service would require adding `password` to the `RpcUser` interface. I
don't think this is sound and want to avoid it. The `find_users()`
method is used in a few other places in the application and those call
sites need to be reworked as they are currently wrong.
Mark Story 1 год назад
Родитель
Сommit
23b5fcecc6

+ 1 - 2
src/sentry/api/fields/user.py

@@ -7,7 +7,6 @@ from rest_framework import serializers
 from sentry.models.user import User
 from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.services.hybrid_cloud.user.service import user_service
-from sentry.utils.auth import find_users
 
 
 class UserField(serializers.Field):
@@ -24,6 +23,6 @@ class UserField(serializers.Field):
                 return user
 
         try:
-            return find_users(data)[0]
+            return user_service.get_by_username(username=data)[0]
         except IndexError:
             raise serializers.ValidationError("Unable to find user")

+ 2 - 2
src/sentry/models/actor.py

@@ -183,7 +183,6 @@ class ActorTuple(namedtuple("Actor", "id type")):
     @classmethod
     def from_actor_identifier(cls, actor_identifier: int | str | None) -> ActorTuple | None:
         from sentry.models import Team, User
-        from sentry.utils.auth import find_users
 
         """
         Returns an Actor tuple corresponding to a User or Team associated with
@@ -217,7 +216,8 @@ class ActorTuple(namedtuple("Actor", "id type")):
             return cls(int(actor_identifier[5:]), Team)
 
         try:
-            return cls(find_users(actor_identifier)[0].id, User)
+            user = user_service.get_by_username(username=actor_identifier)[0]
+            return cls(user.id, User)
         except IndexError as e:
             raise serializers.ValidationError(f"Unable to resolve actor identifier: {e}")
 

+ 15 - 8
src/sentry/search/utils.py

@@ -20,6 +20,9 @@ from typing import (
 from django.db import DataError, connections, router
 from django.utils import timezone
 
+from sentry.services.hybrid_cloud.user.model import RpcUser
+from sentry.services.hybrid_cloud.user.serial import serialize_rpc_user
+
 if TYPE_CHECKING:
     from sentry.api.event_search import SearchFilter
 
@@ -37,8 +40,8 @@ from sentry.models import (
 )
 from sentry.models.group import STATUS_QUERY_CHOICES
 from sentry.search.base import ANY
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.types.group import SUBSTATUS_UPDATE_CHOICES
-from sentry.utils.auth import find_users
 
 
 class InvalidQuery(Exception):
@@ -312,7 +315,7 @@ def get_date_params(value: str, from_field: str, to_field: str) -> dict[str, Uni
     return result
 
 
-def parse_team_value(projects: Sequence[Project], value: Sequence[str], user: User) -> Team:
+def parse_team_value(projects: Sequence[Project], value: Sequence[str]) -> Team:
     return Team.objects.filter(
         slug__iexact=value[1:], projectteam__project__in=projects
     ).first() or Team(id=0)
@@ -331,30 +334,34 @@ def get_teams_for_users(projects: Sequence[Project], users: Sequence[User]) -> l
     return list(teams)
 
 
-def parse_actor_value(projects: Sequence[Project], value: str, user: User) -> Union[User, Team]:
+def parse_actor_value(
+    projects: Sequence[Project], value: str, user: RpcUser | User
+) -> Union[RpcUser, Team]:
     if value.startswith("#"):
-        return parse_team_value(projects, value, user)
+        return parse_team_value(projects, value)
     return parse_user_value(value, user)
 
 
 def parse_actor_or_none_value(
     projects: Sequence[Project], value: str, user: User
-) -> Optional[Union[User, Team]]:
+) -> Optional[Union[RpcUser, Team]]:
     if value == "none":
         return None
     return parse_actor_value(projects, value, user)
 
 
-def parse_user_value(value: str, user: User) -> User:
+def parse_user_value(value: str, user: User | RpcUser) -> RpcUser:
     if value == "me":
+        if isinstance(user, User):
+            return serialize_rpc_user(user)
         return user
 
     try:
-        return find_users(value)[0]
+        return user_service.get_by_username(username=value)[0]
     except IndexError:
         # XXX(dcramer): hacky way to avoid showing any results when
         # an invalid user is entered
-        return User(id=0)
+        return serialize_rpc_user(User(id=0))
 
 
 class LatestReleaseOrders(Enum):

+ 2 - 2
src/sentry/services/hybrid_cloud/user/impl.py

@@ -95,12 +95,12 @@ class DatabaseBackedUserService(UserService):
         try:
             # First, assume username is an iexact match for username
             user = qs.get(username__iexact=username)
-            return [user]
+            return [serialize_rpc_user(user)]
         except User.DoesNotExist:
             # If not, we can take a stab at guessing it's an email address
             if "@" in username:
                 # email isn't guaranteed unique
-                return list(qs.filter(email__iexact=username))
+                return [serialize_rpc_user(u) for u in qs.filter(email__iexact=username)]
         return []
 
     def get_organizations(

+ 15 - 4
src/sentry/utils/auth.py

@@ -17,7 +17,6 @@ from django.utils.http import url_has_allowed_host_and_scheme
 from sentry import options
 from sentry.models import Organization, User
 from sentry.services.hybrid_cloud.organization import RpcOrganization
-from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.utils import metrics
 from sentry.utils.http import absolute_uri
 
@@ -251,9 +250,21 @@ def find_users(
     Return a list of users that match a username
     and falling back to email
     """
-    return user_service.get_by_username(
-        username=username, with_valid_password=with_valid_password, is_active=is_active
-    )
+    queryset = User.objects.filter()
+    if is_active is not None:
+        queryset = queryset.filter(is_active=is_active)
+    if with_valid_password:
+        queryset = queryset.exclude(password="!")
+    try:
+        # First try username case insenstive match on username.
+        user = queryset.get(username__iexact=username)
+        return [user]
+    except User.DoesNotExist:
+        # If not, we can take a stab at guessing it's an email address
+        if "@" in username:
+            # email isn't guaranteed unique
+            return list(queryset.filter(email__iexact=username))
+        return []
 
 
 def login(

+ 1 - 0
src/sentry/web/frontend/auth_login.py

@@ -69,6 +69,7 @@ class AdditionalContext:
 additional_context = AdditionalContext()
 
 
+# TODO(hybridcloud) Make this view control silo only.
 class AuthLoginView(BaseView):
     auth_required = False
 

+ 3 - 1
tests/sentry/api/endpoints/test_organization_releases.py

@@ -2233,7 +2233,9 @@ class ReleaseSerializerWithProjectsTest(TestCase):
         )
         result = serializer.validated_data
         assert result["version"] == self.version
-        assert result["owner"] == self.user
+        assert result["owner"]
+        assert result["owner"].id == self.user.id
+        assert result["owner"].username == self.user.username
         assert result["ref"] == self.ref
         assert result["url"] == self.url
         assert result["dateReleased"] == datetime(1000, 10, 10, 6, 6, tzinfo=pytz.UTC)

+ 3 - 1
tests/sentry/api/endpoints/test_project_releases.py

@@ -730,7 +730,9 @@ class ReleaseSerializerTest(TestCase):
 
         result = serializer.validated_data
         assert result["version"] == self.version
-        assert result["owner"] == self.user
+        assert result["owner"]
+        assert result["owner"].id == self.user.id
+        assert result["owner"].username == self.user.username
         assert result["ref"] == self.ref
         assert result["url"] == self.url
         assert result["dateReleased"] == datetime(1000, 10, 10, 6, 6, tzinfo=pytz.UTC)

+ 7 - 3
tests/sentry/api/test_issue_search.py

@@ -361,14 +361,18 @@ class ConvertActorOrNoneValueTest(TestCase):
 @region_silo_test
 class ConvertUserValueTest(TestCase):
     def test_me(self):
-        assert convert_user_value(["me"], [self.project], self.user, None) == [self.user]
+        result = convert_user_value(["me"], [self.project], self.user, None)
+        assert result[0].id == self.user.id
+        assert result[0].username == self.user.username
 
     def test_specified_user(self):
         user = self.create_user()
-        assert convert_user_value([user.username], [self.project], self.user, None) == [user]
+        result = convert_user_value([user.username], [self.project], self.user, None)
+        assert result[0].id == user.id
+        assert result[0].username == user.username
 
     def test_invalid_user(self):
-        assert convert_user_value(["fake-user"], [], None, None)[0].id == 0
+        assert convert_user_value(["fake-user"], [], self.user, None)[0].id == 0
 
 
 @region_silo_test(stable=True)

+ 23 - 10
tests/sentry/search/test_utils.py

@@ -4,7 +4,7 @@ import pytest
 from django.utils import timezone
 from freezegun import freeze_time
 
-from sentry.models import EventUser, GroupStatus, Release, Team, User
+from sentry.models import EventUser, GroupStatus, Release, Team
 from sentry.search.base import ANY
 from sentry.search.utils import (
     DEVICE_CLASS,
@@ -16,6 +16,9 @@ from sentry.search.utils import (
     parse_query,
     tokenize_query,
 )
+from sentry.services.hybrid_cloud.user.model import RpcUser
+from sentry.services.hybrid_cloud.user.serial import serialize_rpc_user
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.testutils import TestCase
 from sentry.testutils.silo import control_silo_test, region_silo_test
 
@@ -133,8 +136,18 @@ def test_get_numeric_field_value_invalid():
 
 @region_silo_test(stable=True)
 class ParseQueryTest(TestCase):
+    @property
+    def rpc_user(self):
+        return user_service.get_user(user_id=self.user.id)
+
+    @property
+    def current_rpc_user(self):
+        # This doesn't include useremails. Used in filters
+        # where the current user is passed back
+        return serialize_rpc_user(self.user)
+
     def parse_query(self, query):
-        return parse_query([self.project], query, self.user, None)
+        return parse_query([self.project], query, self.user, [])
 
     def test_simple(self):
         result = self.parse_query("foo bar")
@@ -335,7 +348,7 @@ class ParseQueryTest(TestCase):
 
     def test_assigned_me(self):
         result = self.parse_query("assigned:me")
-        assert result == {"assigned_to": self.user, "tags": {}, "query": ""}
+        assert result == {"assigned_to": self.current_rpc_user, "tags": {}, "query": ""}
 
     def test_assigned_none(self):
         result = self.parse_query("assigned:none")
@@ -343,11 +356,11 @@ class ParseQueryTest(TestCase):
 
     def test_assigned_email(self):
         result = self.parse_query(f"assigned:{self.user.email}")
-        assert result == {"assigned_to": self.user, "tags": {}, "query": ""}
+        assert result == {"assigned_to": self.rpc_user, "tags": {}, "query": ""}
 
     def test_assigned_unknown_user(self):
         result = self.parse_query("assigned:fake@example.com")
-        assert isinstance(result["assigned_to"], User)
+        assert isinstance(result["assigned_to"], RpcUser)
         assert result["assigned_to"].id == 0
 
     def test_assigned_valid_team(self):
@@ -367,11 +380,11 @@ class ParseQueryTest(TestCase):
 
     def test_bookmarks_me(self):
         result = self.parse_query("bookmarks:me")
-        assert result == {"bookmarked_by": self.user, "tags": {}, "query": ""}
+        assert result == {"bookmarked_by": self.current_rpc_user, "tags": {}, "query": ""}
 
     def test_bookmarks_email(self):
         result = self.parse_query(f"bookmarks:{self.user.email}")
-        assert result == {"bookmarked_by": self.user, "tags": {}, "query": ""}
+        assert result == {"bookmarked_by": self.rpc_user, "tags": {}, "query": ""}
 
     def test_bookmarks_unknown_user(self):
         result = self.parse_query("bookmarks:fake@example.com")
@@ -589,7 +602,7 @@ class ParseQueryTest(TestCase):
 
     def test_assigned_or_suggested_me(self):
         result = self.parse_query("assigned_or_suggested:me")
-        assert result == {"assigned_or_suggested": self.user, "tags": {}, "query": ""}
+        assert result == {"assigned_or_suggested": self.current_rpc_user, "tags": {}, "query": ""}
 
     def test_assigned_or_suggested_none(self):
         result = self.parse_query("assigned_or_suggested:none")
@@ -601,11 +614,11 @@ class ParseQueryTest(TestCase):
 
     def test_owner_email(self):
         result = self.parse_query(f"assigned_or_suggested:{self.user.email}")
-        assert result == {"assigned_or_suggested": self.user, "tags": {}, "query": ""}
+        assert result == {"assigned_or_suggested": self.rpc_user, "tags": {}, "query": ""}
 
     def test_assigned_or_suggested_unknown_user(self):
         result = self.parse_query("assigned_or_suggested:fake@example.com")
-        assert isinstance(result["assigned_or_suggested"], User)
+        assert isinstance(result["assigned_or_suggested"], RpcUser)
         assert result["assigned_or_suggested"].id == 0
 
     def test_owner_valid_team(self):

Некоторые файлы не были показаны из-за большого количества измененных файлов