v1.8.13

Author: joeyorlando

engine/apps/api/serializers/user.py

@@ -222,12 +222,14 @@ def get_is_currently_oncall(self, obj: User) -> bool:
 class CurrentUserSerializer(UserSerializer):
     rbac_permissions = UserPermissionSerializer(read_only=True, many=True, source="permissions")
 
-    class Meta:
-        model = User
+    class Meta(UserSerializer.Meta):
         fields = UserSerializer.Meta.fields + [
             "rbac_permissions",
+            "google_oauth2_token_is_missing_scopes",
+        ]
+        read_only_fields = UserSerializer.Meta.read_only_fields + [
+            "google_oauth2_token_is_missing_scopes",
         ]
-        read_only_fields = UserSerializer.Meta.read_only_fields
 
 
 class UserHiddenFieldsSerializer(ListUserSerializer):

engine/apps/api/tests/test_auth.py

@@ -195,3 +195,30 @@ def test_google_complete_auth_redirect_ok(
 
     assert response.status_code == status.HTTP_302_FOUND
     assert response.url == "/a/grafana-oncall-app/users/me"
+
+
+@pytest.mark.django_db
+def test_complete_google_auth_redirect_error(
+    make_organization,
+    make_user_for_organization,
+    make_google_oauth2_token_for_user,
+):
+    organization = make_organization()
+    admin = make_user_for_organization(organization)
+    _, google_oauth2_token = make_google_oauth2_token_for_user(admin)
+
+    client = APIClient()
+    url = (
+        reverse("api-internal:complete-social-auth", kwargs={"backend": "google-oauth2"})
+        + f"?state={google_oauth2_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/tests/test_user.py

@@ -66,6 +66,7 @@ def test_current_user(
         "avatar_full": user.avatar_full_url,
         "has_google_oauth2_connected": False,
         "google_calendar_settings": None,
+        "google_oauth2_token_is_missing_scopes": False,
     }
 
     response = client.get(url, format="json", **make_user_auth_headers(user, token))

engine/apps/api/views/user.py

@@ -127,6 +127,7 @@ def get_serializer_context(self):
         return context
 
 
+@extend_schema(responses={status.HTTP_200_OK: CurrentUserSerializer})
 class CurrentUserView(APIView, CachedSchedulesContextMixin):
     authentication_classes = (MobileAppAuthTokenAuthentication, PluginAuthentication)
     permission_classes = (IsAuthenticated,)

engine/apps/google/client.py

@@ -104,25 +104,44 @@ def fetch_out_of_office_events(self) -> typing.List[GoogleCalendarEvent]:
             )
         except HttpError as e:
             if e.status_code == 403:
-                # this scenario can be encountered when, for some reason, the OAuth2 token that we have
+                # this scenario can be encountered when, the OAuth2 token that we have
                 # does not contain the https://www.googleapis.com/auth/calendar.events.readonly scope
                 # example error:
                 # <HttpError 403 when requesting https://www.googleapis.com/calendar/v3/calendars/primary/events?timeMin=2024-08-08T14%3A00%3A00%2B0000&timeMax=2024-09-07T14%3A00%3A00%2B0000&maxResults=250&singleEvents=true&orderBy=startTime&eventTypes=outOfOffice&alt=json returned "Request had insufficient authentication scopes.". Details: "[{'message': 'Insufficient Permission', 'domain': 'global', 'reason': 'insufficientPermissions'}]"> # noqa: E501
+                #
+                # this should really only occur for tokens granted prior to this commit (which wrote this comment).
+                # Before then we didn't handle the scenario where the Google oauth consent screen could potentially
+                # have checkboxes and users would have to actively check the checkbox to grant this scope. We now
+                # handle this scenario.
+                #
+                # References
+                # https://jpassing.com/2022/08/01/dealing-with-partial-consent-in-google-oauth-clients/
+                # https://raintank-corp.slack.com/archives/C05AMEGMLCT/p1723556508149689
+                # https://raintank-corp.slack.com/archives/C04JCU51NF8/p1723493330369349
                 logger.error(f"GoogleCalendarAPIClient - HttpError 403 when fetching out of office events: {e}")
                 raise GoogleCalendarUnauthorizedHTTPError(e)
 
             logger.error(f"GoogleCalendarAPIClient - HttpError when fetching out of office events: {e}")
             raise GoogleCalendarGenericHTTPError(e)
         except RefreshError as e:
-            # TODO: come back and solve this properly once we get better logging output
-            # it seems like right now we are seeing RefreshError in two different scenarios:
+            # we see RefreshError in two different scenarios:
             # 1. RefreshError('invalid_grant: Account has been deleted', {'error': 'invalid_grant', 'error_description': 'Account has been deleted'})
             # 2. RefreshError('invalid_grant: Token has been expired or revoked.', {'error': 'invalid_grant', 'error_description': 'Token has been expired or revoked.'})
+            #
             # https://stackoverflow.com/a/49024030/3902555
+            #
+            # in both of these cases the granted token is no longer good and we should delete it
+
+            try:
+                error_details = e.args[1]  # should be a dict like in the comment above
+            except IndexError:
+                error_details = None  # catch this just in case
+
+            error_description = error_details.get("error_description") if error_details else None
+
             logger.error(
                 f"GoogleCalendarAPIClient - RefreshError when fetching out of office events: {e} "
-                # NOTE: remove e.args after debugging how to dig into the error details
-                f"args={e.args}"
+                f"error_description={error_description}"
             )
             raise GoogleCalendarRefreshError(e)
 

engine/apps/google/constants.py

@@ -1,3 +1,5 @@
+from django.conf import settings
+
 EVENT_SUMMARY_IGNORE_KEYWORD = "#grafana-oncall-ignore"
 
 GOOGLE_CALENDAR_EVENT_DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S%z"
@@ -6,3 +8,4 @@
 """
 
 DAYS_IN_FUTURE_TO_CONSIDER_OUT_OF_OFFICE_EVENTS = 30
+REQUIRED_OAUTH_SCOPES = settings.SOCIAL_AUTH_GOOGLE_OAUTH2_SCOPE

engine/apps/google/tasks.py

@@ -1,6 +1,7 @@
 import logging
 
 from celery.utils.log import get_task_logger
+from django.db.models import Q
 
 from apps.google import constants
 from apps.google.client import (
@@ -39,15 +40,24 @@ def sync_out_of_office_calendar_events_for_user(google_oauth2_user_pk: int) -> N
     try:
         out_of_office_events = google_api_client.fetch_out_of_office_events()
     except GoogleCalendarUnauthorizedHTTPError:
-        # this happens because the user's access token is (somehow) missing the
-        # https://www.googleapis.com/auth/calendar.events.readonly scope
-        # they will need to reconnect their Google account and grant us the necessary scopes, retrying will not help
-        logger.exception(f"Failed to fetch out of office events for user {user_id} due to an unauthorized HTTP error")
-        # TODO: come back and solve this properly once we get better logging output
-        # user.reset_google_oauth2_settings()
+        # this means the user's current token is missing the required scopes, don't delete their token for now
+        # we'll notify them via the plugin UI and ask them to reauth and grant us the missing scope
+        logger.warning(
+            f"Failed to fetch out of office events for user {user_id} due to missing required scopes. "
+            "Safe to skip for now"
+        )
+        return
+    except GoogleCalendarRefreshError:
+        # in this scenarios there's really not much we can do with the refresh/access token that we
+        # have available. The user will need to re-connect with Google so lets delete their persisted token
+
+        logger.exception(
+            f"Failed to fetch out of office events for user {user_id} due to an invalid access and/or refresh token"
+        )
+        user.reset_google_oauth2_settings()
         return
-    except (GoogleCalendarRefreshError, GoogleCalendarGenericHTTPError):
-        logger.exception(f"Failed to fetch out of office events for user {user_id}")
+    except GoogleCalendarGenericHTTPError:
+        logger.exception(f"Failed to fetch out of office events for user {user_id} due to a generic HTTP error")
         return
 
     for out_of_office_event in out_of_office_events:
@@ -118,5 +128,16 @@ def sync_out_of_office_calendar_events_for_user(google_oauth2_user_pk: int) -> N
 
 @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True)
 def sync_out_of_office_calendar_events_for_all_users() -> None:
-    for google_oauth2_user in GoogleOAuth2User.objects.filter(user__organization__deleted_at__isnull=True):
+    # some existing tokens may not have all the required scopes, lets skip these
+    tokens_containing_required_scopes = GoogleOAuth2User.objects.filter(
+        *[Q(oauth_scope__contains=scope) for scope in constants.REQUIRED_OAUTH_SCOPES],
+        user__organization__deleted_at__isnull=True,
+    )
+
+    logger.info(
+        f"Google OAuth2 tokens with the required scopes - "
+        f"{tokens_containing_required_scopes.count()}/{GoogleOAuth2User.objects.count()}"
+    )
+
+    for google_oauth2_user in tokens_containing_required_scopes:
         sync_out_of_office_calendar_events_for_user.apply_async(args=(google_oauth2_user.pk,))

engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py

@@ -1,11 +1,13 @@
 import datetime
-from unittest.mock import patch
+from unittest.mock import call, patch
 
 import pytest
 from django.utils import timezone
+from google.auth.exceptions import RefreshError
 from googleapiclient.errors import HttpError
 
 from apps.google import constants, tasks
+from apps.google.models import GoogleOAuth2User
 from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb, ShiftSwapRequest
 
 
@@ -148,19 +150,61 @@ def __init__(self, reason=None, status=200) -> None:
 
 
 @patch("apps.google.client.build")
+@pytest.mark.parametrize(
+    "ErrorClass,http_status,should_reset_user_google_oauth2_settings,task_should_raise_exception",
+    [
+        (RefreshError, None, True, False),
+        (HttpError, 401, False, False),
+        (HttpError, 500, False, False),
+        (HttpError, 403, False, False),
+        (Exception, None, False, True),
+    ],
+)
 @pytest.mark.django_db
-def test_sync_out_of_office_calendar_events_for_user_httperror(mock_google_api_client_build, test_setup):
-    mock_response = MockResponse(reason="forbidden", status=403)
-    mock_google_api_client_build.return_value.events.return_value.list.return_value.execute.side_effect = HttpError(
-        resp=mock_response, content=b"error"
-    )
+def test_sync_out_of_office_calendar_events_for_user_error_scenarios(
+    mock_google_api_client_build,
+    ErrorClass,
+    http_status,
+    should_reset_user_google_oauth2_settings,
+    task_should_raise_exception,
+    test_setup,
+):
+    if ErrorClass == HttpError:
+        mock_response = MockResponse(reason="forbidden", status=http_status)
+        exception = ErrorClass(resp=mock_response, content=b"error")
+    elif ErrorClass == RefreshError:
+        exception = ErrorClass(
+            "invalid_grant: Token has been expired or revoked.",
+            {"error": "invalid_grant", "error_description": "Token has been expired or revoked."},
+        )
+    else:
+        exception = ErrorClass()
+
+    mock_google_api_client_build.return_value.events.return_value.list.return_value.execute.side_effect = exception
 
     google_oauth2_user, schedule = test_setup([])
     user = google_oauth2_user.user
 
-    tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk)
+    assert user.google_calendar_settings is not None
 
-    assert ShiftSwapRequest.objects.filter(beneficiary=user, schedule=schedule).count() == 0
+    if task_should_raise_exception:
+        with pytest.raises(ErrorClass):
+            tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk)
+    else:
+        tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk)
+
+        assert ShiftSwapRequest.objects.filter(beneficiary=user, schedule=schedule).count() == 0
+
+        user.refresh_from_db()
+
+        google_oauth2_user_count = GoogleOAuth2User.objects.filter(user=user).count()
+
+        if should_reset_user_google_oauth2_settings:
+            assert user.google_calendar_settings is None
+            assert google_oauth2_user_count == 0
+        else:
+            assert user.google_calendar_settings is not None
+            assert google_oauth2_user_count == 1
 
 
 @patch("apps.google.client.build")
@@ -374,14 +418,50 @@ def _fetch_shift_swap_requests():
     assert _fetch_shift_swap_requests().count() == 1
 
 
+REQUIRED_SCOPE_1 = "https://www.googleapis.com/test/foo"
+REQUIRED_SCOPE_2 = "https://www.googleapis.com/test/bar"
+
+
+@patch("apps.google.tasks.constants.REQUIRED_OAUTH_SCOPES", [REQUIRED_SCOPE_1, REQUIRED_SCOPE_2])
+@patch("apps.google.tasks.sync_out_of_office_calendar_events_for_user.apply_async")
+@pytest.mark.django_db
+def test_sync_out_of_office_calendar_events_for_all_users_only_called_for_tokens_having_all_required_scopes(
+    mock_sync_out_of_office_calendar_events_for_user,
+    make_organization_and_user,
+    make_user_for_organization,
+    make_google_oauth2_user_for_user,
+):
+    organization, user1 = make_organization_and_user()
+    user2 = make_user_for_organization(organization)
+    user3 = make_user_for_organization(organization)
+
+    missing_a_scope = f"{REQUIRED_SCOPE_1} foo_bar"
+    has_all_scopes = f"{REQUIRED_SCOPE_1} {REQUIRED_SCOPE_2} foo_bar"
+
+    _ = make_google_oauth2_user_for_user(user1, oauth_scope=missing_a_scope)
+    user2_google_oauth2_user = make_google_oauth2_user_for_user(user2, oauth_scope=has_all_scopes)
+    user3_google_oauth2_user = make_google_oauth2_user_for_user(user3, oauth_scope=has_all_scopes)
+
+    tasks.sync_out_of_office_calendar_events_for_all_users()
+
+    assert len(mock_sync_out_of_office_calendar_events_for_user.mock_calls) == 2
+    mock_sync_out_of_office_calendar_events_for_user.assert_has_calls(
+        [
+            call(args=(user2_google_oauth2_user.pk,)),
+            call(args=(user3_google_oauth2_user.pk,)),
+        ],
+        any_order=True,
+    )
+
+
 @patch("apps.google.tasks.sync_out_of_office_calendar_events_for_user.apply_async")
 @pytest.mark.django_db
-def test_sync_out_of_office_calendar_events_for_all_users(
+def test_sync_out_of_office_calendar_events_for_all_users_filters_out_users_from_deleted_organizations(
     mock_sync_out_of_office_calendar_events_for_user,
     make_organization_and_user,
     make_google_oauth2_user_for_user,
 ):
-    organization, user = make_organization_and_user()
+    _, user = make_organization_and_user()
     google_oauth2_user = make_google_oauth2_user_for_user(user)
 
     deleted_organization, deleted_user = make_organization_and_user()

engine/apps/google/tests/test_utils.py

@@ -0,0 +1,18 @@
+import pytest
+
+from apps.google import utils
+
+SCOPES_ALWAYS_GRANTED = (
+    "openid https://www.googleapis.com/auth/userinfo.profile https://www.googleapis.com/auth/userinfo.email"
+)
+
+
+@pytest.mark.parametrize(
+    "granted_scopes,expected",
+    (
+        (SCOPES_ALWAYS_GRANTED, False),
+        (f"{SCOPES_ALWAYS_GRANTED} https://www.googleapis.com/auth/calendar.events.readonly", True),
+    ),
+)
+def test_user_granted_all_required_scopes(granted_scopes, expected):
+    assert utils.user_granted_all_required_scopes(granted_scopes) == expected

engine/apps/google/utils.py

@@ -3,6 +3,14 @@
 from apps.google import constants
 
 
+def user_granted_all_required_scopes(user_granted_scopes: str) -> bool:
+    """
+    `user_granted_scopes` should be a space-separated string of scopes
+    """
+    granted_scopes = user_granted_scopes.split(" ")
+    return all(scope in granted_scopes for scope in constants.REQUIRED_OAUTH_SCOPES)
+
+
 def datetime_strftime(dt: datetime.datetime) -> str:
     return dt.strftime(constants.GOOGLE_CALENDAR_EVENT_DATETIME_FORMAT)
 

engine/apps/social_auth/exceptions.py

@@ -1,2 +1,5 @@
 class InstallMultiRegionSlackException(Exception):
     pass
+
+
+GOOGLE_AUTH_MISSING_GRANTED_SCOPE_ERROR = "missing_granted_scope"