Reworked slack login pipeline errors (#2526)

Related to https://github.com/grafana/oncall/issues/313

---------

Co-authored-by: Yulia Shanyrova <yulia.shanyrova@grafana.com>

CHANGELOG.md

@@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Deprecate `/oncall` Slack command, update direct paging functionality by @vadimkerr ([#2537](https://github.com/grafana/oncall/pull/2537))
 - Change plugin version to drop the `v` prefix. ([#2540](https://github.com/grafana/oncall/pull/2540))
 
+### Fixed
+
+- Fixed rendering of slack connection errors ([#2526](https://github.com/grafana/oncall/pull/2526))
+
 ## v1.3.13 (2023-07-17)
 
 ### Changed

engine/apps/api/tests/test_auth.py

@@ -0,0 +1,70 @@
+from unittest.mock import patch
+
+import pytest
+from django.contrib.auth import REDIRECT_FIELD_NAME
+from django.http import HttpResponse
+from django.urls import reverse
+from rest_framework import status
+from rest_framework.test import APIClient
+
+from apps.auth_token.constants import SLACK_AUTH_TOKEN_NAME
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "backend_name,expected_url",
+    (
+        ("slack-login", "/a/grafana-oncall-app/users/me"),
+        ("slack-install-free", "/a/grafana-oncall-app/chat-ops"),
+    ),
+)
+def test_complete_slack_auth_redirect_ok(
+    make_organization,
+    make_user_for_organization,
+    make_slack_token_for_user,
+    backend_name,
+    expected_url,
+):
+    organization = make_organization()
+    admin = make_user_for_organization(organization)
+    _, slack_token = make_slack_token_for_user(admin)
+
+    client = APIClient()
+    url = (
+        reverse("api-internal:complete-slack-auth", kwargs={"backend": backend_name})
+        + f"?{SLACK_AUTH_TOKEN_NAME}={slack_token}"
+    )
+
+    with patch("apps.api.views.auth.do_complete") as mock_do_complete:
+        mock_do_complete.return_value = None
+        response = client.get(url)
+
+    assert response.status_code == status.HTTP_302_FOUND
+    assert response.url == expected_url
+
+
+@pytest.mark.django_db
+def test_complete_slack_auth_redirect_error(
+    make_organization,
+    make_user_for_organization,
+    make_slack_token_for_user,
+):
+    organization = make_organization()
+    admin = make_user_for_organization(organization)
+    _, slack_token = make_slack_token_for_user(admin)
+
+    client = APIClient()
+    url = (
+        reverse("api-internal:complete-slack-auth", kwargs={"backend": "slack-login"})
+        + f"?{SLACK_AUTH_TOKEN_NAME}={slack_token}"
+    )
+
+    def _custom_do_complete(backend, *args, **kwargs):
+        backend.strategy.session[REDIRECT_FIELD_NAME] = "some-url"
+        return HttpResponse(status=status.HTTP_400_BAD_REQUEST)
+
+    with patch("apps.api.views.auth.do_complete", side_effect=_custom_do_complete):
+        response = client.get(url)
+
+    assert response.status_code == status.HTTP_302_FOUND
+    assert response.url == "some-url"

engine/apps/api/views/auth.py

@@ -3,7 +3,7 @@ from urllib.parse import urljoin
 
 from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME
-from django.http import HttpResponseRedirect
+from django.http import HttpResponse, HttpResponseRedirect
 from django.views.decorators.cache import never_cache
 from django.views.decorators.csrf import csrf_exempt
 from rest_framework.decorators import api_view, authentication_classes
@@ -49,7 +49,7 @@ def overridden_complete_slack_auth(request, backend, *args, **kwargs):
         # if this was a user login/linking account, redirect to profile
         redirect_to = "/a/grafana-oncall-app/users/me"
 
-    do_complete(
+    result = do_complete(
         request.backend,
         _do_login,
         user=request.user,
@@ -59,6 +59,13 @@ def overridden_complete_slack_auth(request, backend, *args, **kwargs):
         **kwargs,
     )
 
-    # We build the frontend url using org url since multiple stacks could be connected to one backend.
-    return_to = urljoin(request.user.organization.grafana_url, redirect_to)
+    # handle potential errors in the strategy pipeline
+    return_to = None
+    if isinstance(result, HttpResponse):
+        # check if there was a redirect set in the session
+        return_to = request.backend.strategy.session.get(REDIRECT_FIELD_NAME)
+
+    if return_to is None:
+        # We build the frontend url using org url since multiple stacks could be connected to one backend.
+        return_to = urljoin(request.user.organization.grafana_url, redirect_to)
     return HttpResponseRedirect(return_to)

engine/apps/social_auth/pipeline.py

@@ -3,17 +3,14 @@ from urllib.parse import urljoin
 
 from django.apps import apps
 from django.conf import settings
+from django.contrib.auth import REDIRECT_FIELD_NAME
 from django.http import HttpResponse
 from rest_framework import status
 from social_core.exceptions import AuthForbidden
 
 from apps.slack.tasks import populate_slack_channels_for_team, populate_slack_usergroups_for_team
 from apps.social_auth.exceptions import InstallMultiRegionSlackException
-from common.constants.slack_auth import (
-    REDIRECT_AFTER_SLACK_INSTALL,
-    SLACK_AUTH_SLACK_USER_ALREADY_CONNECTED_ERROR,
-    SLACK_AUTH_WRONG_WORKSPACE_ERROR,
-)
+from common.constants.slack_auth import SLACK_AUTH_SLACK_USER_ALREADY_CONNECTED_ERROR, SLACK_AUTH_WRONG_WORKSPACE_ERROR
 from common.insight_log import ChatOpsEvent, ChatOpsTypePlug, write_chatops_insight_log
 from common.oncall_gateway import check_slack_installation_possible, create_slack_connector
 
@@ -41,26 +38,24 @@ def connect_user_to_slack(response, backend, strategy, user, organization, *args
     slack_team_identity = organization.slack_team_identity
     slack_user_id = response["authed_user"]["id"]
 
+    redirect_to = "/a/grafana-oncall-app/users/me/"
+    base_url_to_redirect = urljoin(organization.grafana_url, redirect_to)
+
     if slack_team_identity is None:
         # means that organization doesn't have slack integration, so user cannot connect their account to slack
         return HttpResponse(status=status.HTTP_400_BAD_REQUEST)
+
     if slack_team_identity.slack_id != response["team"]["id"]:
         # means that user authed in another slack workspace that is not connected to their organization
         # change redirect url to show user error message and save it in session param
-        url = urljoin(
-            strategy.session[REDIRECT_AFTER_SLACK_INSTALL],
-            f"?page=users&slack_error={SLACK_AUTH_WRONG_WORKSPACE_ERROR}",
-        )
-        strategy.session[REDIRECT_AFTER_SLACK_INSTALL] = url
+        url = base_url_to_redirect + f"?slack_error={SLACK_AUTH_WRONG_WORKSPACE_ERROR}"
+        strategy.session[REDIRECT_FIELD_NAME] = url
         return HttpResponse(status=status.HTTP_400_BAD_REQUEST)
 
     if organization.users.filter(slack_user_identity__slack_id=slack_user_id).exists():
         # means that slack user has already been connected to another user in current organization
-        url = urljoin(
-            strategy.session[REDIRECT_AFTER_SLACK_INSTALL],
-            f"?page=users&slack_error={SLACK_AUTH_SLACK_USER_ALREADY_CONNECTED_ERROR}",
-        )
-        strategy.session[REDIRECT_AFTER_SLACK_INSTALL] = url
+        url = base_url_to_redirect + f"?slack_error={SLACK_AUTH_SLACK_USER_ALREADY_CONNECTED_ERROR}"
+        strategy.session[REDIRECT_FIELD_NAME] = url
         return HttpResponse(status=status.HTTP_400_BAD_REQUEST)
 
     slack_user_identity, _ = SlackUserIdentity.objects.get_or_create(

engine/conftest.py

@@ -44,7 +44,7 @@ from apps.api.permissions import (
     LegacyAccessControlRole,
     RBACPermission,
 )
-from apps.auth_token.models import ApiAuthToken, PluginAuthToken
+from apps.auth_token.models import ApiAuthToken, PluginAuthToken, SlackAuthToken
 from apps.base.models.user_notification_policy_log_record import (
     UserNotificationPolicyLogRecord,
     listen_for_usernotificationpolicylogrecord_model_save,
@@ -210,6 +210,14 @@ def make_mobile_app_auth_token_for_user():
     return _make_mobile_app_auth_token_for_user
 
 
+@pytest.fixture
+def make_slack_token_for_user():
+    def _make_slack_token_for_user(user):
+        return SlackAuthToken.create_auth_token(organization=user.organization, user=user)
+
+    return _make_slack_token_for_user
+
+
 @pytest.fixture
 def make_public_api_token():
     def _make_public_api_token(user, organization, name="test_api_token"):

grafana-plugin/src/containers/Alerts/Alerts.tsx

@@ -68,15 +68,14 @@ export default function Alerts() {
   if (!showSlackInstallAlert && !showBannerTeam() && !showMismatchWarning() && !showChannelWarnings()) {
     return null;
   }
-
   return (
     <div className={cx('alerts-container', { 'alerts-container--legacy': !isTopNavbar() })}>
       {showSlackInstallAlert && (
         <Alert
           className={cx('alert')}
           onRemove={handleCloseInstallSlackAlert}
-          severity="warning"
-          title="Slack integration warning"
+          severity="error"
+          title="Slack integration error"
         >
           {getSlackMessage(
             showSlackInstallAlert,