Browse Source

fix(hc): Change to AuthProvider.get_auth_provider (#55416)

AuthProvider has `unique=True` on its `organization_id` foreign key, so
it should have always been impossible for `get_auth_providers` to return
more than one object. Deprecate `get_auth_providers` and replace it with
a new method that returns an optional RpcAuthProvider.
Ryan Skonnord 1 year ago
parent
commit
8e17420dec

+ 2 - 3
src/sentry/api/endpoints/organization_auth_provider_details.py

@@ -29,13 +29,12 @@ class OrganizationAuthProviderDetailsEndpoint(OrganizationEndpoint):
         :pparam string organization_slug: the organization short name
         :auth: required
         """
-        auth_providers = auth_service.get_auth_providers(organization_id=organization.id)
-        if not auth_providers:
+        auth_provider = auth_service.get_auth_provider(organization_id=organization.id)
+        if not auth_provider:
             # This is a valid state where org does not have an auth provider
             # configured, make sure we respond with a 20x
             return Response(status=status.HTTP_204_NO_CONTENT)
 
-        auth_provider = auth_providers[0]
         return Response(
             serialize(
                 auth_provider,

+ 2 - 2
src/sentry/api/endpoints/organization_details.py

@@ -203,8 +203,8 @@ class OrganizationSerializer(BaseOrganizationSerializer):
 
     def _has_sso_enabled(self):
         org = self.context["organization"]
-        org_auth_providers = auth_service.get_auth_providers(organization_id=org.id)
-        return len(org_auth_providers) > 0
+        org_auth_provider = auth_service.get_auth_provider(organization_id=org.id)
+        return org_auth_provider is not None
 
     def validate_relayPiiConfig(self, value):
         organization = self.context["organization"]

+ 2 - 2
src/sentry/api/endpoints/organization_member/requests/join.py

@@ -72,8 +72,8 @@ class OrganizationJoinRequestEndpoint(OrganizationEndpoint):
 
         # users can already join organizations with SSO enabled without an invite
         # so they should join that way and not through a request to the admins
-        providers = auth_service.get_auth_providers(organization_id=organization.id)
-        if len(providers) != 0:
+        provider = auth_service.get_auth_provider(organization_id=organization.id)
+        if provider is not None:
             return Response(status=403)
 
         ip_address = request.META["REMOTE_ADDR"]

+ 2 - 2
src/sentry/scim/endpoints/members.py

@@ -119,10 +119,10 @@ def _scim_member_serializer_with_expansion(organization):
     care about this and rely on the behavior of setting "active" to false
     to delete a member.
     """
-    auth_providers = auth_service.get_auth_providers(organization_id=organization.id)
+    auth_provider = auth_service.get_auth_provider(organization_id=organization.id)
     expand = ["active"]
 
-    if any(ap.provider == ACTIVE_DIRECTORY_PROVIDER_NAME for ap in auth_providers):
+    if auth_provider and auth_provider.provider == ACTIVE_DIRECTORY_PROVIDER_NAME:
         expand = []
     return OrganizationMemberSCIMSerializer(expand=expand)
 

+ 3 - 3
src/sentry/scim/endpoints/utils.py

@@ -88,13 +88,13 @@ class SCIMQueryParamSerializer(serializers.Serializer):
 
 
 class OrganizationSCIMPermission(OrganizationPermission):
-    def has_object_permission(self, request: Request, view, organization: Organization):
+    def has_object_permission(self, request: Request, view, organization: Organization) -> bool:
         result = super().has_object_permission(request, view, organization)
         # The scim endpoints should only be used in conjunction with a SAML2 integration
         if not result:
             return result
-        providers = auth_service.get_auth_providers(organization_id=organization.id)
-        return any(p.flags.scim_enabled for p in providers)
+        provider = auth_service.get_auth_provider(organization_id=organization.id)
+        return provider is not None and provider.flags.scim_enabled
 
 
 class OrganizationSCIMMemberPermission(OrganizationSCIMPermission):

+ 11 - 5
src/sentry/services/hybrid_cloud/auth/impl.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import base64
-from typing import Any, List, Mapping
+from typing import Any, List, Mapping, Optional
 
 from django.contrib.auth.models import AnonymousUser
 from django.db import router, transaction
@@ -222,10 +222,16 @@ class DatabaseBackedAuthService(AuthService):
         )
 
     def get_auth_providers(self, organization_id: int) -> List[RpcAuthProvider]:
-        return [
-            serialize_auth_provider(auth_provider)
-            for auth_provider in AuthProvider.objects.filter(organization_id=organization_id)
-        ]
+        # DEPRECATED. TODO: Delete after usages are removed from getsentry.
+        auth_provider = self.get_auth_provider(organization_id)
+        return [auth_provider] if auth_provider else []
+
+    def get_auth_provider(self, organization_id: int) -> Optional[RpcAuthProvider]:
+        try:
+            auth_provider = AuthProvider.objects.get(organization_id=organization_id)
+        except AuthProvider.DoesNotExist:
+            return None
+        return serialize_auth_provider(auth_provider)
 
     def change_scim(
         self, *, user_id: int, provider_id: int, enabled: bool, allow_unlinked: bool

+ 6 - 2
src/sentry/services/hybrid_cloud/auth/service.py

@@ -75,9 +75,13 @@ class AuthService(RpcService):
     @rpc_method
     @abc.abstractmethod
     def get_auth_providers(self, *, organization_id: int) -> List[RpcAuthProvider]:
+        """DEPRECATED. TODO: Delete after usages are removed from getsentry."""
+
+    @rpc_method
+    @abc.abstractmethod
+    def get_auth_provider(self, *, organization_id: int) -> Optional[RpcAuthProvider]:
         """
-        This method returns a list of auth providers for an org
-        :return:
+        This method returns the auth provider for an org, if one exists
         """
         pass
 

+ 3 - 3
src/sentry/web/frontend/organization_auth_settings.py

@@ -214,13 +214,13 @@ class OrganizationAuthSettingsView(ControlSiloOrganizationView):
         return self.respond("sentry/organization-auth-provider-settings.html", context)
 
     def handle(self, request: Request, organization: RpcOrganization) -> HttpResponseBase:  # type: ignore[override]
-        providers = auth_service.get_auth_providers(organization_id=organization.id)
-        if providers:
+        provider = auth_service.get_auth_provider(organization_id=organization.id)
+        if provider:
             # if the org has SSO set up already, allow them to modify the existing provider
             # regardless if the feature flag is set up. This allows orgs who might no longer
             # have the SSO feature to be able to turn it off
             return self.handle_existing_provider(
-                request=request, organization=organization, auth_provider=providers[0]
+                request=request, organization=organization, auth_provider=provider
             )
 
         if request.method == "POST":