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

framework/auth/cas.py

@@ -289,6 +289,19 @@ 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')
+            if orcid_id and orcid_access_token:
+                from osf.models.external import ExternalAccount
+                account, _ = ExternalAccount.objects.update_or_create(
+                    provider='orcid',
+                    provider_id=orcid_id,
+                    defaults={
+                        'provider_name': 'ORCID',
+                        'oauth_key': orcid_access_token,
+                    },
+                )
+                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
@@ -2154,6 +2155,21 @@ def _clear_identifying_information(self):
         self.social = {}
         self.unclaimed_records = {}
         self.notifications_configured = {}
+        # Revoke ORCID OAuth grant before scrubbing the token below (best-effort, does not block deletion)
+        orcid_accounts = self.external_accounts.filter(provider='orcid')
+        for account in orcid_accounts:
+            try:
+                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,
+                )
+            except requests.exceptions.RequestException as e:
+                logger.warning(f'Failed to revoke ORCID token for user {self._id}: {e}')
         # Scrub all external accounts
         if self.external_accounts.exists():
             logger.info('Clearing identifying information from external accounts...')
@@ -2166,6 +2182,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,57 @@ 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_does_not_block_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)
+
+        user.gdpr_delete()
+
+        assert len(responses.calls) == 1
+        assert user.deleted is not None
+        assert not user.external_accounts.exists()
+        account.reload()
+        assert account.oauth_key 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',