Skip to content

Commit eb777f5

Browse files
joeyorlandobrojd
andauthored
address Google OAuth2 issues where user didn't grant us the https://www.googleapis.com/auth/calendar.events.readonly` scope (#4802)
# What this PR does Follow up PR to #4792 Basically if when communicating with Google Calendar's API we encounter an HTTP 403, or the Google client throws a `google.auth.exceptions.RefreshError` this means one of three things: 1. the refresh token we have persisted for the user is missing the `https://www.googleapis.com/auth/calendar.events.readonly` scope (HTTP 403) 2. the Google user has been deleted (`google.auth.exceptions.RefreshError`) 3. the refresh token has expired (`google.auth.exceptions.RefreshError`) To prevent scenario 1 above from happening in the future we now will check that the token has been granted the required scopes. If the user doesn't grant us all the necessary scopes, we will show them an error message in the UI: https://www.loom.com/share/0055ef03192b4154b894c2221cecbd5f For tokens that were granted prior to this PR and which are missing the required scope, we will show the user a dismissible warning banner in the UI letting them know that they will need to reconnect their account and grant us the missing permissions (see [this second demo video](https://www.loom.com/share/bf2ee8b840864a64893165370a892bcd) showing this). ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --------- Co-authored-by: Dominik <[email protected]>
1 parent 29bd42c commit eb777f5

File tree

21 files changed

+671
-143
lines changed

21 files changed

+671
-143
lines changed

engine/apps/api/serializers/user.py

+5-3
Original file line numberDiff line numberDiff line change
@@ -222,12 +222,14 @@ def get_is_currently_oncall(self, obj: User) -> bool:
222222
class CurrentUserSerializer(UserSerializer):
223223
rbac_permissions = UserPermissionSerializer(read_only=True, many=True, source="permissions")
224224

225-
class Meta:
226-
model = User
225+
class Meta(UserSerializer.Meta):
227226
fields = UserSerializer.Meta.fields + [
228227
"rbac_permissions",
228+
"google_oauth2_token_is_missing_scopes",
229+
]
230+
read_only_fields = UserSerializer.Meta.read_only_fields + [
231+
"google_oauth2_token_is_missing_scopes",
229232
]
230-
read_only_fields = UserSerializer.Meta.read_only_fields
231233

232234

233235
class UserHiddenFieldsSerializer(ListUserSerializer):

engine/apps/api/tests/test_auth.py

+27
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,30 @@ def test_google_complete_auth_redirect_ok(
195195

196196
assert response.status_code == status.HTTP_302_FOUND
197197
assert response.url == "/a/grafana-oncall-app/users/me"
198+
199+
200+
@pytest.mark.django_db
201+
def test_complete_google_auth_redirect_error(
202+
make_organization,
203+
make_user_for_organization,
204+
make_google_oauth2_token_for_user,
205+
):
206+
organization = make_organization()
207+
admin = make_user_for_organization(organization)
208+
_, google_oauth2_token = make_google_oauth2_token_for_user(admin)
209+
210+
client = APIClient()
211+
url = (
212+
reverse("api-internal:complete-social-auth", kwargs={"backend": "google-oauth2"})
213+
+ f"?state={google_oauth2_token}"
214+
)
215+
216+
def _custom_do_complete(backend, *args, **kwargs):
217+
backend.strategy.session[REDIRECT_FIELD_NAME] = "some-url"
218+
return HttpResponse(status=status.HTTP_400_BAD_REQUEST)
219+
220+
with patch("apps.api.views.auth.do_complete", side_effect=_custom_do_complete):
221+
response = client.get(url)
222+
223+
assert response.status_code == status.HTTP_302_FOUND
224+
assert response.url == "some-url"

engine/apps/api/tests/test_user.py

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ def test_current_user(
6666
"avatar_full": user.avatar_full_url,
6767
"has_google_oauth2_connected": False,
6868
"google_calendar_settings": None,
69+
"google_oauth2_token_is_missing_scopes": False,
6970
}
7071

7172
response = client.get(url, format="json", **make_user_auth_headers(user, token))

engine/apps/api/views/user.py

+1
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ def get_serializer_context(self):
127127
return context
128128

129129

130+
@extend_schema(responses={status.HTTP_200_OK: CurrentUserSerializer})
130131
class CurrentUserView(APIView, CachedSchedulesContextMixin):
131132
authentication_classes = (MobileAppAuthTokenAuthentication, PluginAuthentication)
132133
permission_classes = (IsAuthenticated,)

engine/apps/google/client.py

+24-5
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,44 @@ def fetch_out_of_office_events(self) -> typing.List[GoogleCalendarEvent]:
104104
)
105105
except HttpError as e:
106106
if e.status_code == 403:
107-
# this scenario can be encountered when, for some reason, the OAuth2 token that we have
107+
# this scenario can be encountered when, the OAuth2 token that we have
108108
# does not contain the https://www.googleapis.com/auth/calendar.events.readonly scope
109109
# example error:
110110
# <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
111+
#
112+
# this should really only occur for tokens granted prior to this commit (which wrote this comment).
113+
# Before then we didn't handle the scenario where the Google oauth consent screen could potentially
114+
# have checkboxes and users would have to actively check the checkbox to grant this scope. We now
115+
# handle this scenario.
116+
#
117+
# References
118+
# https://jpassing.com/2022/08/01/dealing-with-partial-consent-in-google-oauth-clients/
119+
# https://raintank-corp.slack.com/archives/C05AMEGMLCT/p1723556508149689
120+
# https://raintank-corp.slack.com/archives/C04JCU51NF8/p1723493330369349
111121
logger.error(f"GoogleCalendarAPIClient - HttpError 403 when fetching out of office events: {e}")
112122
raise GoogleCalendarUnauthorizedHTTPError(e)
113123

114124
logger.error(f"GoogleCalendarAPIClient - HttpError when fetching out of office events: {e}")
115125
raise GoogleCalendarGenericHTTPError(e)
116126
except RefreshError as e:
117-
# TODO: come back and solve this properly once we get better logging output
118-
# it seems like right now we are seeing RefreshError in two different scenarios:
127+
# we see RefreshError in two different scenarios:
119128
# 1. RefreshError('invalid_grant: Account has been deleted', {'error': 'invalid_grant', 'error_description': 'Account has been deleted'})
120129
# 2. RefreshError('invalid_grant: Token has been expired or revoked.', {'error': 'invalid_grant', 'error_description': 'Token has been expired or revoked.'})
130+
#
121131
# https://stackoverflow.com/a/49024030/3902555
132+
#
133+
# in both of these cases the granted token is no longer good and we should delete it
134+
135+
try:
136+
error_details = e.args[1] # should be a dict like in the comment above
137+
except IndexError:
138+
error_details = None # catch this just in case
139+
140+
error_description = error_details.get("error_description") if error_details else None
141+
122142
logger.error(
123143
f"GoogleCalendarAPIClient - RefreshError when fetching out of office events: {e} "
124-
# NOTE: remove e.args after debugging how to dig into the error details
125-
f"args={e.args}"
144+
f"error_description={error_description}"
126145
)
127146
raise GoogleCalendarRefreshError(e)
128147

engine/apps/google/constants.py

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from django.conf import settings
2+
13
EVENT_SUMMARY_IGNORE_KEYWORD = "#grafana-oncall-ignore"
24

35
GOOGLE_CALENDAR_EVENT_DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S%z"
@@ -6,3 +8,4 @@
68
"""
79

810
DAYS_IN_FUTURE_TO_CONSIDER_OUT_OF_OFFICE_EVENTS = 30
11+
REQUIRED_OAUTH_SCOPES = settings.SOCIAL_AUTH_GOOGLE_OAUTH2_SCOPE

engine/apps/google/tasks.py

+30-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import logging
22

33
from celery.utils.log import get_task_logger
4+
from django.db.models import Q
45

56
from apps.google import constants
67
from apps.google.client import (
@@ -39,15 +40,24 @@ def sync_out_of_office_calendar_events_for_user(google_oauth2_user_pk: int) -> N
3940
try:
4041
out_of_office_events = google_api_client.fetch_out_of_office_events()
4142
except GoogleCalendarUnauthorizedHTTPError:
42-
# this happens because the user's access token is (somehow) missing the
43-
# https://www.googleapis.com/auth/calendar.events.readonly scope
44-
# they will need to reconnect their Google account and grant us the necessary scopes, retrying will not help
45-
logger.exception(f"Failed to fetch out of office events for user {user_id} due to an unauthorized HTTP error")
46-
# TODO: come back and solve this properly once we get better logging output
47-
# user.reset_google_oauth2_settings()
43+
# this means the user's current token is missing the required scopes, don't delete their token for now
44+
# we'll notify them via the plugin UI and ask them to reauth and grant us the missing scope
45+
logger.warning(
46+
f"Failed to fetch out of office events for user {user_id} due to missing required scopes. "
47+
"Safe to skip for now"
48+
)
49+
return
50+
except GoogleCalendarRefreshError:
51+
# in this scenarios there's really not much we can do with the refresh/access token that we
52+
# have available. The user will need to re-connect with Google so lets delete their persisted token
53+
54+
logger.exception(
55+
f"Failed to fetch out of office events for user {user_id} due to an invalid access and/or refresh token"
56+
)
57+
user.reset_google_oauth2_settings()
4858
return
49-
except (GoogleCalendarRefreshError, GoogleCalendarGenericHTTPError):
50-
logger.exception(f"Failed to fetch out of office events for user {user_id}")
59+
except GoogleCalendarGenericHTTPError:
60+
logger.exception(f"Failed to fetch out of office events for user {user_id} due to a generic HTTP error")
5161
return
5262

5363
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
118128

119129
@shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True)
120130
def sync_out_of_office_calendar_events_for_all_users() -> None:
121-
for google_oauth2_user in GoogleOAuth2User.objects.filter(user__organization__deleted_at__isnull=True):
131+
# some existing tokens may not have all the required scopes, lets skip these
132+
tokens_containing_required_scopes = GoogleOAuth2User.objects.filter(
133+
*[Q(oauth_scope__contains=scope) for scope in constants.REQUIRED_OAUTH_SCOPES],
134+
user__organization__deleted_at__isnull=True,
135+
)
136+
137+
logger.info(
138+
f"Google OAuth2 tokens with the required scopes - "
139+
f"{tokens_containing_required_scopes.count()}/{GoogleOAuth2User.objects.count()}"
140+
)
141+
142+
for google_oauth2_user in tokens_containing_required_scopes:
122143
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

+90-10
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import datetime
2-
from unittest.mock import patch
2+
from unittest.mock import call, patch
33

44
import pytest
55
from django.utils import timezone
6+
from google.auth.exceptions import RefreshError
67
from googleapiclient.errors import HttpError
78

89
from apps.google import constants, tasks
10+
from apps.google.models import GoogleOAuth2User
911
from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb, ShiftSwapRequest
1012

1113

@@ -148,19 +150,61 @@ def __init__(self, reason=None, status=200) -> None:
148150

149151

150152
@patch("apps.google.client.build")
153+
@pytest.mark.parametrize(
154+
"ErrorClass,http_status,should_reset_user_google_oauth2_settings,task_should_raise_exception",
155+
[
156+
(RefreshError, None, True, False),
157+
(HttpError, 401, False, False),
158+
(HttpError, 500, False, False),
159+
(HttpError, 403, False, False),
160+
(Exception, None, False, True),
161+
],
162+
)
151163
@pytest.mark.django_db
152-
def test_sync_out_of_office_calendar_events_for_user_httperror(mock_google_api_client_build, test_setup):
153-
mock_response = MockResponse(reason="forbidden", status=403)
154-
mock_google_api_client_build.return_value.events.return_value.list.return_value.execute.side_effect = HttpError(
155-
resp=mock_response, content=b"error"
156-
)
164+
def test_sync_out_of_office_calendar_events_for_user_error_scenarios(
165+
mock_google_api_client_build,
166+
ErrorClass,
167+
http_status,
168+
should_reset_user_google_oauth2_settings,
169+
task_should_raise_exception,
170+
test_setup,
171+
):
172+
if ErrorClass == HttpError:
173+
mock_response = MockResponse(reason="forbidden", status=http_status)
174+
exception = ErrorClass(resp=mock_response, content=b"error")
175+
elif ErrorClass == RefreshError:
176+
exception = ErrorClass(
177+
"invalid_grant: Token has been expired or revoked.",
178+
{"error": "invalid_grant", "error_description": "Token has been expired or revoked."},
179+
)
180+
else:
181+
exception = ErrorClass()
182+
183+
mock_google_api_client_build.return_value.events.return_value.list.return_value.execute.side_effect = exception
157184

158185
google_oauth2_user, schedule = test_setup([])
159186
user = google_oauth2_user.user
160187

161-
tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk)
188+
assert user.google_calendar_settings is not None
162189

163-
assert ShiftSwapRequest.objects.filter(beneficiary=user, schedule=schedule).count() == 0
190+
if task_should_raise_exception:
191+
with pytest.raises(ErrorClass):
192+
tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk)
193+
else:
194+
tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk)
195+
196+
assert ShiftSwapRequest.objects.filter(beneficiary=user, schedule=schedule).count() == 0
197+
198+
user.refresh_from_db()
199+
200+
google_oauth2_user_count = GoogleOAuth2User.objects.filter(user=user).count()
201+
202+
if should_reset_user_google_oauth2_settings:
203+
assert user.google_calendar_settings is None
204+
assert google_oauth2_user_count == 0
205+
else:
206+
assert user.google_calendar_settings is not None
207+
assert google_oauth2_user_count == 1
164208

165209

166210
@patch("apps.google.client.build")
@@ -374,14 +418,50 @@ def _fetch_shift_swap_requests():
374418
assert _fetch_shift_swap_requests().count() == 1
375419

376420

421+
REQUIRED_SCOPE_1 = "https://www.googleapis.com/test/foo"
422+
REQUIRED_SCOPE_2 = "https://www.googleapis.com/test/bar"
423+
424+
425+
@patch("apps.google.tasks.constants.REQUIRED_OAUTH_SCOPES", [REQUIRED_SCOPE_1, REQUIRED_SCOPE_2])
426+
@patch("apps.google.tasks.sync_out_of_office_calendar_events_for_user.apply_async")
427+
@pytest.mark.django_db
428+
def test_sync_out_of_office_calendar_events_for_all_users_only_called_for_tokens_having_all_required_scopes(
429+
mock_sync_out_of_office_calendar_events_for_user,
430+
make_organization_and_user,
431+
make_user_for_organization,
432+
make_google_oauth2_user_for_user,
433+
):
434+
organization, user1 = make_organization_and_user()
435+
user2 = make_user_for_organization(organization)
436+
user3 = make_user_for_organization(organization)
437+
438+
missing_a_scope = f"{REQUIRED_SCOPE_1} foo_bar"
439+
has_all_scopes = f"{REQUIRED_SCOPE_1} {REQUIRED_SCOPE_2} foo_bar"
440+
441+
_ = make_google_oauth2_user_for_user(user1, oauth_scope=missing_a_scope)
442+
user2_google_oauth2_user = make_google_oauth2_user_for_user(user2, oauth_scope=has_all_scopes)
443+
user3_google_oauth2_user = make_google_oauth2_user_for_user(user3, oauth_scope=has_all_scopes)
444+
445+
tasks.sync_out_of_office_calendar_events_for_all_users()
446+
447+
assert len(mock_sync_out_of_office_calendar_events_for_user.mock_calls) == 2
448+
mock_sync_out_of_office_calendar_events_for_user.assert_has_calls(
449+
[
450+
call(args=(user2_google_oauth2_user.pk,)),
451+
call(args=(user3_google_oauth2_user.pk,)),
452+
],
453+
any_order=True,
454+
)
455+
456+
377457
@patch("apps.google.tasks.sync_out_of_office_calendar_events_for_user.apply_async")
378458
@pytest.mark.django_db
379-
def test_sync_out_of_office_calendar_events_for_all_users(
459+
def test_sync_out_of_office_calendar_events_for_all_users_filters_out_users_from_deleted_organizations(
380460
mock_sync_out_of_office_calendar_events_for_user,
381461
make_organization_and_user,
382462
make_google_oauth2_user_for_user,
383463
):
384-
organization, user = make_organization_and_user()
464+
_, user = make_organization_and_user()
385465
google_oauth2_user = make_google_oauth2_user_for_user(user)
386466

387467
deleted_organization, deleted_user = make_organization_and_user()
+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import pytest
2+
3+
from apps.google import utils
4+
5+
SCOPES_ALWAYS_GRANTED = (
6+
"openid https://www.googleapis.com/auth/userinfo.profile https://www.googleapis.com/auth/userinfo.email"
7+
)
8+
9+
10+
@pytest.mark.parametrize(
11+
"granted_scopes,expected",
12+
(
13+
(SCOPES_ALWAYS_GRANTED, False),
14+
(f"{SCOPES_ALWAYS_GRANTED} https://www.googleapis.com/auth/calendar.events.readonly", True),
15+
),
16+
)
17+
def test_user_granted_all_required_scopes(granted_scopes, expected):
18+
assert utils.user_granted_all_required_scopes(granted_scopes) == expected

engine/apps/google/utils.py

+8
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@
33
from apps.google import constants
44

55

6+
def user_granted_all_required_scopes(user_granted_scopes: str) -> bool:
7+
"""
8+
`user_granted_scopes` should be a space-separated string of scopes
9+
"""
10+
granted_scopes = user_granted_scopes.split(" ")
11+
return all(scope in granted_scopes for scope in constants.REQUIRED_OAUTH_SCOPES)
12+
13+
614
def datetime_strftime(dt: datetime.datetime) -> str:
715
return dt.strftime(constants.GOOGLE_CALENDAR_EVENT_DATETIME_FORMAT)
816

engine/apps/social_auth/exceptions.py

+3
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
class InstallMultiRegionSlackException(Exception):
22
pass
3+
4+
5+
GOOGLE_AUTH_MISSING_GRANTED_SCOPE_ERROR = "missing_granted_scope"

0 commit comments

Comments
 (0)