From b5702867e8327c019b4da37d52b069b67427de1e Mon Sep 17 00:00:00 2001 From: apiyo Date: Mon, 17 Oct 2022 10:57:34 +0300 Subject: [PATCH 1/3] Ask users to provide a new google oauth credential if current one is invalid --- onadata/libs/utils/api_export_tools.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/onadata/libs/utils/api_export_tools.py b/onadata/libs/utils/api_export_tools.py index b59cb78205..1512a3fdce 100644 --- a/onadata/libs/utils/api_export_tools.py +++ b/onadata/libs/utils/api_export_tools.py @@ -627,6 +627,7 @@ def generate_google_web_flow(request): def _get_google_credential(request): credential = None + storage = None if request.user.is_authenticated: try: storage = TokenStorageModel.objects.get(id=request.user) @@ -636,6 +637,11 @@ def _get_google_credential(request): elif request.session.get("access_token"): credential = Credentials(token=request.session["access_token"]) + if credential and not credential.valid: + # We need the user to provide a new crential + storage.delete() + credential = None + if not credential: google_flow = generate_google_web_flow(request) authorization_url, _state = google_flow.authorization_url( From 7e3e35c59863d52b41e063be2d9d6ce93f31c7df Mon Sep 17 00:00:00 2001 From: apiyo Date: Mon, 17 Oct 2022 14:35:39 +0300 Subject: [PATCH 2/3] Add test case that check that invalid tokens are deleted --- .../libs/tests/utils/test_api_export_tools.py | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/onadata/libs/tests/utils/test_api_export_tools.py b/onadata/libs/tests/utils/test_api_export_tools.py index 53c162ea10..683adb7f50 100644 --- a/onadata/libs/tests/utils/test_api_export_tools.py +++ b/onadata/libs/tests/utils/test_api_export_tools.py @@ -5,6 +5,8 @@ from collections import OrderedDict, defaultdict import mock +import datetime +from google.oauth2.credentials import Credentials from celery.backends.rpc import BacklogLimitExceeded from django.http import Http404 from django.test.utils import override_settings @@ -13,12 +15,14 @@ from onadata.apps.logger.models import XForm from onadata.apps.main.tests.test_base import TestBase +from onadata.apps.main.models import TokenStorageModel from onadata.apps.viewer.models.export import Export, ExportConnectionError from onadata.libs.exceptions import ServiceUnavailable from onadata.libs.utils.api_export_tools import ( get_async_response, process_async_export, response_for_format, - get_metadata_format + get_metadata_format, + _get_google_credential ) from onadata.libs.utils.async_status import SUCCESSFUL, status_msg @@ -40,6 +44,35 @@ def _create_old_export(self, xform, export_type, options, filename=None): self.export = Export.objects.filter( xform=xform, export_type=export_type)[0] + def test_get_google_credentials(self): + """ + Test create_async_export deletes credential when invalid + """ + request = self.factory.get('/') + request.user = self.user + request.query_params = {} + request.data = {} + credential = { + "refresh_token": "refresh-token", + "token_uri": "https://oauth2.googleapis.com/token", + "client_id": "client-id", + "client_secret": "client-secret", + "scopes": ["https://www.googleapis.com/auth/drive.file"], + "expiry": datetime.datetime(2016, 8, 18, 12, 43, 30, 316792) + } + t = TokenStorageModel(id=self.user, + credential=Credentials(**credential, token=None)) + t.save() + response = _get_google_credential(request) + + self.assertEqual(response.status_code, 302) + self.assertEqual( + response.url[:71], + 'https://accounts.google.com/o/oauth2/auth?response_type=code&client_id=' + ) + with self.assertRaises(TokenStorageModel.DoesNotExist): + TokenStorageModel.objects.get(id=self.user) + # pylint: disable=invalid-name def test_process_async_export_creates_new_export(self): """ From 9ee1ee9111f4b9c21ec0a66cef01859910389a3f Mon Sep 17 00:00:00 2001 From: apiyo Date: Mon, 17 Oct 2022 17:00:18 +0300 Subject: [PATCH 3/3] Try to refresh invalid google credentials before deleting --- .../libs/tests/utils/test_api_export_tools.py | 39 +++++++++++++++---- onadata/libs/utils/api_export_tools.py | 15 +++++-- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/onadata/libs/tests/utils/test_api_export_tools.py b/onadata/libs/tests/utils/test_api_export_tools.py index 683adb7f50..9754fab960 100644 --- a/onadata/libs/tests/utils/test_api_export_tools.py +++ b/onadata/libs/tests/utils/test_api_export_tools.py @@ -32,6 +32,15 @@ class TestApiExportTools(TestBase): Test api_export_tools. """ + google_credential = { + "refresh_token": "refresh-token", + "token_uri": "https://oauth2.googleapis.com/token", + "client_id": "client-id", + "client_secret": "client-secret", + "scopes": ["https://www.googleapis.com/auth/drive.file"], + "expiry": datetime.datetime(2016, 8, 18, 12, 43, 30, 316792) + } + def _create_old_export(self, xform, export_type, options, filename=None): options = OrderedDict(sorted(options.items())) Export( @@ -52,17 +61,11 @@ def test_get_google_credentials(self): request.user = self.user request.query_params = {} request.data = {} - credential = { - "refresh_token": "refresh-token", - "token_uri": "https://oauth2.googleapis.com/token", - "client_id": "client-id", - "client_secret": "client-secret", - "scopes": ["https://www.googleapis.com/auth/drive.file"], - "expiry": datetime.datetime(2016, 8, 18, 12, 43, 30, 316792) - } + credential = self.google_credential t = TokenStorageModel(id=self.user, credential=Credentials(**credential, token=None)) t.save() + self.assertFalse(t.credential.valid) response = _get_google_credential(request) self.assertEqual(response.status_code, 302) @@ -73,6 +76,26 @@ def test_get_google_credentials(self): with self.assertRaises(TokenStorageModel.DoesNotExist): TokenStorageModel.objects.get(id=self.user) + def test_get_google_credentials_valid(self): + """ + Test create_async_export does not get rid of valid credential + """ + + request = self.factory.get('/') + request.user = self.user + request.query_params = {} + request.data = {} + self.google_credential['expiry'] = \ + datetime.datetime.utcnow() + datetime.timedelta(seconds=300) + credential = self.google_credential + t = TokenStorageModel(id=self.user, + credential=Credentials(**credential, token="token")) + t.save() + self.assertTrue(t.credential.valid) + credential = _get_google_credential(request) + + self.assertEqual(credential.to_json(), t.credential.to_json()) + # pylint: disable=invalid-name def test_process_async_export_creates_new_export(self): """ diff --git a/onadata/libs/utils/api_export_tools.py b/onadata/libs/utils/api_export_tools.py index 1512a3fdce..3bd5dd5935 100644 --- a/onadata/libs/utils/api_export_tools.py +++ b/onadata/libs/utils/api_export_tools.py @@ -7,6 +7,10 @@ import sys from datetime import datetime +from google.auth.transport.requests import Request +from google.auth.exceptions import RefreshError +from google.oauth2.credentials import Credentials # noqa + from django.conf import settings from django.http import Http404, HttpResponseRedirect from django.shortcuts import get_object_or_404 @@ -15,7 +19,6 @@ import six from celery.backends.rpc import BacklogLimitExceeded from celery.result import AsyncResult -from google.oauth2.credentials import Credentials # noqa from kombu.exceptions import OperationalError from rest_framework import exceptions, status from rest_framework.response import Response @@ -638,9 +641,13 @@ def _get_google_credential(request): credential = Credentials(token=request.session["access_token"]) if credential and not credential.valid: - # We need the user to provide a new crential - storage.delete() - credential = None + try: + credential.refresh(Request()) + storage.credential = credential + storage.save() + except RefreshError: + storage.delete() + credential = None if not credential: google_flow = generate_google_web_flow(request)