Fix/eng 11829 2 - #11885
Conversation
292149b to
4c48222
Compare
| sentry.log_message( | ||
| f'CAS response ORCID attributes: user=[{user._id}], orcidId=[{orcid_id}], ' | ||
| f'orcidAccessToken=[{"present" if access_token else "missing"}]', | ||
| level=logging.INFO, | ||
| ) |
There was a problem hiding this comment.
Only log message to sentry if access token or refresh token are missing, level is warning
| # }, | ||
| # ... | ||
| # } | ||
| external_identity_access_token = DateTimeAwareJSONField(default=dict, blank=True) |
There was a problem hiding this comment.
external_identity_tokens
| access_token = cas_resp.attributes.get('orcidAccessToken', None) | ||
| refresh_token = cas_resp.attributes.get('orcidRefreshToken', None) |
There was a problem hiding this comment.
Move into if external_credential:
| external_credential['id'], | ||
| access_token, | ||
| refresh_token, | ||
| ) |
| f'found {len(orcid_tokens)} ORCID token(s) to revoke', | ||
| level=logging.INFO, | ||
| ) | ||
| for orcid_id, orcid_token in orcid_tokens.items(): |
There was a problem hiding this comment.
We need to check if there is an external_identity but there is no matching external_identity_tokens, we need to fail the GDPR delete with relevant error messsage.
| 'client_secret': website_settings.ORCID_OAUTH_CLIENT_SECRET, | ||
| 'token': orcid_token, | ||
| }, | ||
| timeout=5, |
There was a problem hiding this comment.
Future, we probably want a retry just in case
| ) | ||
| sentry.log_message( | ||
| f'[GDPR delete] user={self._id}: ORCID id={orcid_id} revoked, ' | ||
| f'status_code={response.status_code}, response_text={response.text}, response={response}', |
There was a problem hiding this comment.
we probably don't need the full response
| f'status_code={response.status_code}, response_text={response.text}, response={response}', | ||
| level=logging.INFO, | ||
| ) | ||
| response.raise_for_status() |
There was a problem hiding this comment.
Add unit tests for success, invalid client_id/secret, invalid token, empty token
| 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}', |
There was a problem hiding this comment.
Add the orcid id just in case user has mulitple
| '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.' |
There was a problem hiding this comment.
rephrase to a more general message like fail to revoke
f757b97 to
eebddd6
Compare
2 participants
Ticket
Purpose
Changes
Side Effects
QE Notes
CE Notes
Documentation