Skip to content

Commit e700072

Browse files
simonkagwiCopilottobiasmcnulty
authored
Improve publish flow when user's refresh token has expired or they don't have permission to the doc. (#93)
* Improve publish flow when user's refresh token has expired or they don't have permission to the doc. * Update apps/publish_mdm/consumers.py Co-authored-by: Copilot <[email protected]> * Improve the error message displayed when a user doesn't have permission to the doc. Co-authored-by: Tobias McNulty <[email protected]> * Update a test. --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Tobias McNulty <[email protected]>
1 parent 494bc03 commit e700072

File tree

5 files changed

+308
-13
lines changed

5 files changed

+308
-13
lines changed

apps/publish_mdm/consumers.py

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,23 @@
11
import json
22
import pprint
33
import traceback
4+
from urllib.parse import urlencode
45

56
import structlog
67
from channels.generic.websocket import WebsocketConsumer
78
from django.conf import settings
9+
from django.contrib.auth import REDIRECT_FIELD_NAME
810
from django.template.loader import render_to_string
9-
from gspread.exceptions import SpreadsheetNotFound
11+
from django.urls import reverse
12+
from google.auth.exceptions import RefreshError
13+
from gspread.exceptions import APIError, SpreadsheetNotFound
1014
from gspread.utils import extract_id_from_url
1115
from requests import Response
1216

1317
from apps.publish_mdm.models import FormTemplate
1418

1519
from .etl.load import PublishTemplateEvent, publish_form_template
20+
from .utils import get_login_url
1621

1722
logger = structlog.getLogger(__name__)
1823

@@ -58,15 +63,86 @@ def receive(self, text_data):
5863
logger.exception("Error publishing form")
5964
tbe = traceback.TracebackException.from_exception(exc=e, compact=True)
6065
message = "".join(tbe.format())
61-
summary = None
62-
if isinstance(e, SpreadsheetNotFound):
63-
summary = self.get_google_picker(form_template_id=event_data.get("form_template"))
66+
summary = self.get_error_summary(e, event_data)
6467
# If the error is from ODK Central, format the error message for easier reading
6568
if len(e.args) >= 2 and isinstance(e.args[1], Response):
6669
data = e.args[1].json()
6770
message = f"ODK Central error:\n\n{pprint.pformat(data)}\n\n{message}"
6871
self.send_message(message, error=True, error_summary=summary)
6972

73+
def get_error_summary(self, exc: Exception, event_data: dict):
74+
"""For some exceptions, add a helpful message or instructions that will be
75+
displayed above the traceback.
76+
"""
77+
if isinstance(exc, SpreadsheetNotFound):
78+
# User has not authorized us to access the file using their credentials.
79+
# Display a message to that effect and a button for them to give us access
80+
# using the Google Picker
81+
return self.get_google_picker(form_template_id=event_data.get("form_template"))
82+
83+
error_message = None
84+
button = None
85+
86+
if (is_refresh_error := isinstance(exc, RefreshError)) or (
87+
# gspread raises an APIError, catches it, then does `raise PermissionError from ...`
88+
isinstance(exc, PermissionError) and isinstance(exc.__context__, APIError)
89+
):
90+
if (
91+
is_refresh_error
92+
or "Request had insufficient authentication scopes"
93+
in exc.__context__.error["message"]
94+
):
95+
# Either an expired/invalid refresh token, or the user did not
96+
# check the checkbox to give us access to their Google Drive files
97+
# when they first logged in. Ask them to log in again
98+
error_message = (
99+
"Sorry, you need to log in again to be able to publish. "
100+
"Please click the button below."
101+
)
102+
form_template = FormTemplate.objects.get(id=event_data.get("form_template"))
103+
publish_url = reverse(
104+
"publish_mdm:form-template-publish",
105+
args=[
106+
form_template.project.organization.slug,
107+
form_template.project.id,
108+
form_template.id,
109+
],
110+
)
111+
# Add a link that will log them out then redirect to the login page.
112+
# User will be taken through the OAuth flow again then redirected
113+
# back to the publish page
114+
logout_url = reverse("account_logout")
115+
login_url = get_login_url(publish_url)
116+
querystring = urlencode({REDIRECT_FIELD_NAME: login_url})
117+
button = {
118+
"href": f"{logout_url}?{querystring}",
119+
"text": "Log in again",
120+
}
121+
elif "The caller does not have permission" in exc.__context__.error["message"]:
122+
# User does not have access to the file in Google Sheets.
123+
# Display instructions on how to confirm if they have access
124+
error_message = (
125+
"Unfortunately, we could not access the form in Google Sheets. "
126+
'Click the button below to open the spreadsheet and request access.'
127+
'<br><br>'
128+
'Within the spreadsheet, you or someone else with access will need to click '
129+
'<strong>Share</strong> and confirm the Google user '
130+
f'<strong>{self.scope["user"].email}</strong> appears in the list of '
131+
'people with access.'
132+
'<br><br>'
133+
"When done, return to this page and click "
134+
"<strong>Publish next version</strong> again."
135+
)
136+
form_template = FormTemplate.objects.get(id=event_data.get("form_template"))
137+
button = {"href": form_template.template_url, "text": "Open spreadsheet"}
138+
139+
if error_message or button:
140+
context = {
141+
"error_message": error_message,
142+
"button": button,
143+
}
144+
return render_to_string("publish_mdm/ws/form_template_error_summary.html", context)
145+
70146
def publish_form_template(self, event_data: dict):
71147
"""Publish a form template to ODK Central and stream progress to the browser."""
72148
# Parse the event data and raise an error if it's invalid

apps/publish_mdm/utils.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,41 @@
11
import os
2+
from urllib.parse import urlsplit, urlunsplit
23

34
from django.conf import settings
5+
from django.contrib.auth import REDIRECT_FIELD_NAME
6+
from django.http import QueryDict
7+
from django.shortcuts import resolve_url
48

59

610
def get_secret(key):
711
"""Get a value either from the SECRETS setting (populated from a file) or
812
from environment variables.
913
"""
1014
return settings.SECRETS.get(key, os.getenv(key))
15+
16+
17+
def get_login_url(
18+
next, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME, force_oauth_flow=True
19+
):
20+
"""
21+
Get a login URL that would redirect the user to the login page, passing the
22+
given 'next' page.
23+
24+
Like django.contrib.auth.views.redirect_to_login, but adds force_oauth_flow
25+
and returns a URL instead of a HttpResponseRedirect object.
26+
27+
force_oauth_flow will force the user to go through the Google OAuth flow
28+
even if they had logged in before.
29+
"""
30+
resolved_url = resolve_url(login_url or settings.LOGIN_URL)
31+
32+
login_url_parts = list(urlsplit(resolved_url))
33+
if redirect_field_name or force_oauth_flow:
34+
querystring = QueryDict(login_url_parts[3], mutable=True)
35+
if force_oauth_flow:
36+
querystring["auth_params"] = "prompt=select_account consent"
37+
if redirect_field_name:
38+
querystring[redirect_field_name] = next
39+
login_url_parts[3] = querystring.urlencode(safe="/")
40+
41+
return urlunsplit(login_url_parts)

apps/publish_mdm/views.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,16 @@
11
import json
2-
from urllib.parse import urlencode
32

43
import structlog
54
from django.conf import settings
65
from django.contrib import messages
7-
from django.contrib.auth import logout, REDIRECT_FIELD_NAME
6+
from django.contrib.auth import logout
87
from django.contrib.auth.decorators import login_required
9-
from django.contrib.auth.views import redirect_to_login
108
from django.contrib.postgres.aggregates import ArrayAgg
119
from django.db import models, transaction
1210
from django.db.models import OuterRef, Q, Subquery, Value
1311
from django.db.models.functions import Collate, Lower, NullIf
1412
from django.http import HttpRequest, HttpResponse
15-
from django.shortcuts import get_object_or_404, redirect, render, resolve_url
13+
from django.shortcuts import get_object_or_404, redirect, render
1614
from django.utils.html import mark_safe
1715
from django.utils.timezone import localdate
1816
from django_tables2.config import RequestConfig
@@ -89,6 +87,7 @@
8987
FormTemplateTable,
9088
FormTemplateVersionTable,
9189
)
90+
from .utils import get_login_url
9291

9392

9493
logger = structlog.getLogger(__name__)
@@ -230,9 +229,7 @@ def form_template_publish(
230229
# OAuth consent flow again
231230
logout(request)
232231
messages.error(request, "Sorry, you need to log in again to be able to publish.")
233-
querystring = urlencode({"auth_params": "prompt=select_account consent"})
234-
login_url = f"{resolve_url(settings.LOGIN_URL)}?{querystring}"
235-
return redirect_to_login(request.path, login_url, REDIRECT_FIELD_NAME)
232+
return redirect(get_login_url(request.path))
236233

237234
form_template: FormTemplate = get_object_or_404(
238235
request.odk_project.form_templates, pk=form_template_id
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<div id="form-access-error" class="mb-1">
2+
<p class="mb-1">{{ error_message|safe }}</p>
3+
{% if button %}
4+
<a href="{{ button.href }}"
5+
type="button"
6+
class="btn btn-outline px-2 py-1 inline-block"
7+
{% if button.text == "Open spreadsheet" %}target="_blank"{% endif %}>{{ button.text }}</a>
8+
{% endif %}
9+
</div>

0 commit comments

Comments
 (0)