Skip to content

Commit

Permalink
v1.8.13
Browse files Browse the repository at this point in the history
  • Loading branch information
joeyorlando authored Aug 15, 2024
2 parents ad6c49b + eb777f5 commit ce9c7db
Show file tree
Hide file tree
Showing 21 changed files with 671 additions and 143 deletions.
8 changes: 5 additions & 3 deletions engine/apps/api/serializers/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
27 changes: 27 additions & 0 deletions engine/apps/api/tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
1 change: 1 addition & 0 deletions engine/apps/api/tests/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions engine/apps/api/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,)
Expand Down
29 changes: 24 additions & 5 deletions engine/apps/google/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 3 additions & 0 deletions engine/apps/google/constants.py
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -6,3 +8,4 @@
"""

DAYS_IN_FUTURE_TO_CONSIDER_OUT_OF_OFFICE_EVENTS = 30
REQUIRED_OAUTH_SCOPES = settings.SOCIAL_AUTH_GOOGLE_OAUTH2_SCOPE
39 changes: 30 additions & 9 deletions engine/apps/google/tasks.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,))
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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()
Expand Down
18 changes: 18 additions & 0 deletions engine/apps/google/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions engine/apps/google/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 3 additions & 0 deletions engine/apps/social_auth/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
class InstallMultiRegionSlackException(Exception):
pass


GOOGLE_AUTH_MISSING_GRANTED_SCOPE_ERROR = "missing_granted_scope"
Loading

0 comments on commit ce9c7db

Please sign in to comment.