[ENG-11829] - Add revoke endpoint request for ORCID

admin/users/views.py

@@ -213,12 +213,13 @@ class UserGDPRDeleteView(UserMixin, View):
     permission_required = 'osf.change_osfuser'
 
     def post(self, request, *args, **kwargs):
+        user = self.get_object()
         try:
-            user = self.get_object()
             user.gdpr_delete()
             user.save()
         except UserStateError as e:
             messages.warning(request, str(e))
+            return redirect(self.get_success_url())
 
         messages.success(request, f'User {user._id} was successfully GDPR deleted')
 

framework/auth/cas.py

@@ -8,6 +8,9 @@
 from lxml import etree
 import requests
 
+import logging
+
+from framework import sentry
 from framework.auth import authenticate, external_first_login_authenticate
 from framework.auth.core import get_user, generate_verification_key
 from framework.auth.utils import print_cas_log, LogLevel
@@ -289,6 +292,29 @@ def make_response_from_ticket(ticket, service_url):
             if tos_checked_via_cas:
                 user_updates['accepted_terms_of_service'] = timezone.now()
                 print_cas_log(f'CAS TOS consent checked: {user.guids.first()._id}, {user.username}', LogLevel.INFO)
+            orcid_id = cas_resp.attributes.get('orcidId')
+            orcid_access_token = cas_resp.attributes.get('orcidAccessToken')
+            sentry.log_message(
+                f'CAS response ORCID attributes: user=[{user._id}], orcidId=[{orcid_id}], '
+                f'orcidAccessToken=[{"present" if orcid_access_token else "missing"}]',
+                level=logging.INFO,
+            )
+            if orcid_id and orcid_access_token:
+                from osf.models.external import ExternalAccount
+                account, created = ExternalAccount.objects.update_or_create(
+                    provider='orcid',
+                    provider_id=orcid_id,
+                    defaults={
+                        'provider_name': 'ORCID',
+                        'oauth_key': orcid_access_token,
+                    },
+                )
+                sentry.log_message(
+                    f'ORCID external account {"created" if created else "updated"}: '
+                    f'user=[{user._id}], account=[{account._id}], provider_id=[{orcid_id}]',
+                    level=logging.INFO,
+                )
+                user.external_accounts.add(account)
             # if we successfully authenticate and a verification key is present, invalidate it
             if user.verification_key:
                 user_updates['verification_key'] = None

framework/auth/tasks.py

@@ -54,6 +54,7 @@ def update_affiliation_for_orcid_sso_users(user_id, orcid_id):
         logger.error(error_message)
         sentry.log_message(error_message)
         return
+
     institution = check_institution_affiliation(orcid_id)
     if institution:
         logger.info(f'Eligible institution affiliation has been found for ORCiD SSO user: '

osf/models/user.py

@@ -12,6 +12,7 @@
 # OSF imports
 import itsdangerous
 import pytz
+import requests
 from dirtyfields import DirtyFieldsMixin
 
 from django.conf import settings
@@ -2131,6 +2132,45 @@ def _clear_identifying_information(self):
         '''
         This method ensures a user's info is deleted during a GDPR delete
         '''
+        orcid_accounts = self.external_accounts.filter(provider='orcid')
+        sentry.log_message(
+            f'[GDPR delete; _clear_identifying_information] user={self._id}: '
+            f'found {orcid_accounts.count()} ORCID account(s) to revoke',
+            level=logging.INFO,
+        )
+        for account in orcid_accounts:
+            sentry.log_message(
+                f'[GDPR delete] user={self._id}: revoking ORCID account={account._id} '
+                f'via {website_settings.ORCID_OAUTH_REVOKE_URL}',
+                level=logging.INFO,
+            )
+            try:
+                response = requests.post(
+                    website_settings.ORCID_OAUTH_REVOKE_URL,
+                    data={
+                        'client_id': website_settings.ORCID_OAUTH_CLIENT_ID,
+                        'client_secret': website_settings.ORCID_OAUTH_CLIENT_SECRET,
+                        'token': account.oauth_key,
+                    },
+                    timeout=5,
+                )
+                sentry.log_message(
+                    f'[GDPR delete] user={self._id}: ORCID account={account._id} revoked, '
+                    f'status_code={response.status_code}, response_text={response.text}, response={response}',
+                    level=logging.INFO,
+                )
+                response.raise_for_status()
+            except requests.exceptions.RequestException as e:
+                sentry.log_message(
+                    f'[GDPR delete] Failed to revoke ORCID token for user {self._id}: {e}',
+                    level=logging.ERROR,
+                )
+                sentry.log_exception(e)
+                raise UserStateError(
+                    'Unable to revoke this user\'s ORCID access right now because ORCID\'s '
+                    'service could not be reached. Please try the GDPR delete again later.'
+                )
+
         # This doesn't remove identifying info, but ensures other users can't see the deleted user's profile etc.
         self.deactivate_account()
 
@@ -2166,6 +2206,7 @@ def _clear_identifying_information(self):
                 account.profile_url = None
                 account.save()
             self.external_accounts.clear()
+
         self.external_identity = {}
         self.deleted = timezone.now()
 

osf_tests/test_user.py

@@ -12,6 +12,8 @@
 from unittest import mock
 import itsdangerous
 import pytest
+import requests
+import responses
 from importlib import import_module
 
 from framework.auth.exceptions import ExpiredTokenError, InvalidTokenError, ChangePasswordError
@@ -2242,6 +2244,76 @@ def test_can_gdpr_delete(self, user):
         assert user.is_disabled
         assert user.deleted is not None
 
+    @responses.activate
+    def test_gdpr_delete_revokes_orcid_token(self, user):
+        responses.add(
+            responses.POST,
+            settings.ORCID_OAUTH_REVOKE_URL,
+            status=200,
+        )
+        account = ExternalAccountFactory(provider='orcid', oauth_key='fake-orcid-token')
+        user.external_accounts.add(account)
+
+        user.gdpr_delete()
+
+        assert len(responses.calls) == 1
+        request_body = responses.calls[0].request.body
+        assert f'token={account.oauth_key}' in request_body
+        assert f'client_id={settings.ORCID_OAUTH_CLIENT_ID}' in request_body
+        assert f'client_secret={settings.ORCID_OAUTH_CLIENT_SECRET}' in request_body
+        account.reload()
+        assert account.oauth_key is None
+
+    @responses.activate
+    def test_gdpr_delete_no_orcid_account_no_revoke_call(self, user):
+        responses.add(
+            responses.POST,
+            settings.ORCID_OAUTH_REVOKE_URL,
+            status=200,
+        )
+        user.external_accounts.add(ExternalAccountFactory(provider='github'))
+
+        user.gdpr_delete()
+
+        assert len(responses.calls) == 0
+
+    @responses.activate
+    def test_gdpr_delete_orcid_revoke_failure_blocks_delete(self, user):
+        responses.add(
+            responses.POST,
+            settings.ORCID_OAUTH_REVOKE_URL,
+            body=requests.exceptions.ConnectionError('boom'),
+        )
+        account = ExternalAccountFactory(provider='orcid', oauth_key='fake-orcid-token')
+        user.external_accounts.add(account)
+
+        with pytest.raises(UserStateError):
+            user.gdpr_delete()
+
+        assert len(responses.calls) == 1
+        # Nothing else should have been touched: fail fast, before any other clearing happens.
+        assert user.deleted is None
+        assert not user.is_disabled
+        assert user.external_accounts.exists()
+        account.reload()
+        assert account.oauth_key == 'fake-orcid-token'
+
+    @responses.activate
+    def test_gdpr_delete_orcid_revoke_http_error_blocks_delete(self, user):
+        responses.add(
+            responses.POST,
+            settings.ORCID_OAUTH_REVOKE_URL,
+            status=500,
+        )
+        account = ExternalAccountFactory(provider='orcid', oauth_key='fake-orcid-token')
+        user.external_accounts.add(account)
+
+        with pytest.raises(UserStateError):
+            user.gdpr_delete()
+
+        assert len(responses.calls) == 1
+        assert user.deleted is None
+
     def test_can_gdpr_delete_personal_nodes(self, user):
 
         user.gdpr_delete()

tests/test_cas_authentication.py

@@ -6,9 +6,10 @@
 
 from framework.auth import cas
 
+from osf.models import ExternalAccount
 from tests.base import OsfTestCase, fake
 from tests.utils import run_celery_tasks
-from osf_tests.factories import UserFactory
+from osf_tests.factories import ExternalAccountFactory, UserFactory
 
 
 def make_successful_response(user):
@@ -32,6 +33,18 @@ def make_successful_response_with_tos_consent(user):
     )
 
 
+def make_successful_response_with_orcid_attrs(user, orcid_id=None, access_token=None):
+    return cas.CasResponse(
+        authenticated=True,
+        user=user._id,
+        attributes={
+            'accessToken': fake.md5(),
+            'orcidId': orcid_id or fake.numerify('####-####-####-####'),
+            'orcidAccessToken': access_token or fake.md5(),
+        }
+    )
+
+
 def make_failure_response():
     return cas.CasResponse(
         authenticated=False,
@@ -261,6 +274,53 @@ def test_make_response_from_ticket_success_with_tos_consent(self, mock_service_v
         assert mock_service_validate.call_count == 1
         assert mock_get_user_from_cas_resp.call_count == 1
 
+    @mock.patch('framework.auth.cas.get_user_from_cas_resp')
+    @mock.patch('framework.auth.cas.CasClient.service_validate')
+    def test_make_response_from_ticket_success_with_orcid_attrs(self, mock_service_validate, mock_get_user_from_cas_resp):
+        orcid_id = fake.numerify('####-####-####-####')
+        access_token = fake.md5()
+        mock_service_validate.return_value = make_successful_response_with_orcid_attrs(
+            self.user, orcid_id=orcid_id, access_token=access_token
+        )
+        mock_get_user_from_cas_resp.return_value = (self.user, None, 'authenticate')
+        ticket = fake.md5()
+        service_url = 'http://localhost:5000/'
+        resp = cas.make_response_from_ticket(ticket, service_url)
+        assert resp.status_code == 302
+        account = ExternalAccount.objects.get(provider='orcid', provider_id=orcid_id)
+        assert account.oauth_key == access_token
+        assert account in self.user.external_accounts.all()
+
+    @mock.patch('framework.auth.cas.get_user_from_cas_resp')
+    @mock.patch('framework.auth.cas.CasClient.service_validate')
+    def test_make_response_from_ticket_updates_existing_orcid_account(self, mock_service_validate, mock_get_user_from_cas_resp):
+        orcid_id = fake.numerify('####-####-####-####')
+        existing_account = ExternalAccountFactory(provider='orcid', provider_id=orcid_id, oauth_key='old-token')
+        self.user.external_accounts.add(existing_account)
+        new_access_token = fake.md5()
+        mock_service_validate.return_value = make_successful_response_with_orcid_attrs(
+            self.user, orcid_id=orcid_id, access_token=new_access_token
+        )
+        mock_get_user_from_cas_resp.return_value = (self.user, None, 'authenticate')
+        ticket = fake.md5()
+        service_url = 'http://localhost:5000/'
+        resp = cas.make_response_from_ticket(ticket, service_url)
+        assert resp.status_code == 302
+        assert ExternalAccount.objects.filter(provider='orcid', provider_id=orcid_id).count() == 1
+        existing_account.reload()
+        assert existing_account.oauth_key == new_access_token
+
+    @mock.patch('framework.auth.cas.get_user_from_cas_resp')
+    @mock.patch('framework.auth.cas.CasClient.service_validate')
+    def test_make_response_from_ticket_no_orcid_attrs_no_account_created(self, mock_service_validate, mock_get_user_from_cas_resp):
+        mock_service_validate.return_value = make_successful_response(self.user)
+        mock_get_user_from_cas_resp.return_value = (self.user, None, 'authenticate')
+        ticket = fake.md5()
+        service_url = 'http://localhost:5000/'
+        resp = cas.make_response_from_ticket(ticket, service_url)
+        assert resp.status_code == 302
+        assert not ExternalAccount.objects.filter(provider='orcid').exists()
+
     @mock.patch('framework.auth.cas.get_user_from_cas_resp')
     @mock.patch('framework.auth.cas.CasClient.service_validate')
     def test_make_response_from_ticket_failure(self, mock_service_validate, mock_get_user_from_cas_resp):

website/settings/defaults.py

@@ -796,6 +796,10 @@ class CeleryConfig:
 ORCID_RECORD_EMPLOYMENT_PATH = '/employments'
 ORCID_RECORD_EDUCATION_PATH = '/educations'
 
+ORCID_OAUTH_CLIENT_ID = os.environ.get('ORCID_OAUTH_CLIENT_ID', 'changeme')
+ORCID_OAUTH_CLIENT_SECRET = os.environ.get('ORCID_OAUTH_CLIENT_SECRET', 'changeme')
+ORCID_OAUTH_REVOKE_URL = os.environ.get('ORCID_OAUTH_REVOKE_URL', 'https://sandbox.orcid.org/oauth/revoke')
+
 # Source: https://github.com/maxd/fake_email_validator/blob/master/config/fake_domains.list
 BLACKLISTED_DOMAINS = [
     '0-mail.com',