Refactor ORCID OAuth2 and OAuth2TokenData

[poikilotherm]

This is related to #5974, #5689 and a successor to #5279.
This is kind of security related, but just as a precaution.

---

OAuth2TokenData

While hacking on #5974 and OIDC stuff, I came across OAuth2TokenData. Everyone that logs in via some OAuth2 provider (no matter which provider) has its token stored in the database.

This data is stored, but never read. Nowhere. It had been introduced due to ORCID support:

dataverse/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2TokenData.java

Lines 21 to 26 in 51739f6

	* Persisting token data is a requirement for ORCID according to
	* https://members.orcid.org/api/news/xsd-20-update which says "Store full
	* responses from token exchange: access tokens, refresh tokens, scope, scope
	* expiry to indicate an iD has been authenticated and with what scope" but we
	* don't know how long responses need to be stored. There is no such requirement
	* to store responses for any other OAuth provider.

That link is dead, find an updated checklist here.

Storing this information might lead to a data breach, as a hacker that gains access to these tokens can use it to read information from the provider (in the limits of the scope and readonly of course). However, this should be avoided: it's not used, eats DB storage and might pose a risk.

This leads to the next section:

ORCID API usage

Currently, when you want to authenticate to Dataverse using ORCID provider, you have some things you need to attack:

1. Become an ORCID member (paying $$$). Non-members cannot get access to the members API. Dataverse is using the Members API exclusively.
2. Once being a member, move through a manual process of sandbox environments, showcases etc etc. Very tedious. Very long. Exhausting. Been through it twice now. No fun.

I really wonder why Dataverse should do this in the first place. Currently we are not using any kind of Members only stuff in Dataverse (see API comparison for details on what you can do with it), we are just using it for auth and reading public data.

Email addresses are private by default in ORCID and you cannot access them either via Public API or Member API.

The Public API on the other hand:

• Sufficient for auth and reading public things.
• No need to become an ORCID member
• Automated process to register your client app
• No obligations for us to store tokens, be compliant with the UI stuff or anything else from the checklist linked above.

(I should note: public API credentials are tied to an individual, not an org. Non-moveable. Meh.)

(tl;dr)
So my question is: shouldn't we refactor, remove OAuth2TokenData and switch to Public API`?
(At the same time one could check for #5689, the ORCID API 3.0 compatibility.)

---

Mentions:

• @TaniaSchlatter because this is UI related
• @scolapasta because IIRC he's one of the sec guys
• @pameyer because he was engaged in some of the other issues
• @michbarsinai because he wrote much of that code back in 2017
• @pdurbin because I call him the Chief Community Officer and he also did much coding on this stuff

[michbarsinai]

That was a long time ago, but IIRC the idea was to start some ORCID infrastucture that will allow later to add published datasets to the ORCID record of the users, if there are any.

…

On 31 Oct 2019, at 17:59, Oliver Bertuch ***@***.***> wrote: This is related to #5974 <#5974>, #5689 <#5689> and a successor to #5279 <#5279>. This is kind of security related, but just as a precaution. OAuth2TokenData While hacking on #5974 <#5974> and OIDC stuff, I came across OAuth2TokenData. Everyone that logs in via some OAuth2 provider (no matter which provider) has its token stored in the database. This data is stored, but never read. Nowhere. It had been introduced due to ORCID support: https://github.com/IQSS/dataverse/blob/51739f61fedfd4f8cc1e3d547b45eb5ff0801c25/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2TokenData.java#L21-L26 <https://github.com/IQSS/dataverse/blob/51739f61fedfd4f8cc1e3d547b45eb5ff0801c25/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2TokenData.java#L21-L26> That link is dead, find an updated checklist here <https://members.orcid.org/api/member-api-credentials-check-list>. Storing this information might lead to a data breach, as a hacker that gains access to these tokens can use it to read information from the provider (in the limits of the scope and readonly of course). However, this should be avoided: it's not used, eats DB storage and might pose a risk. This leads to the next section: ORCID API usage Currently, when you want to authenticate to Dataverse using ORCID provider, you have some things you need to attack: Become an ORCID member. Non-members cannot get access to the members API. Dataverse is using the Members API exclusively. Once being a member, move through a manual process of sandbox environments, showcases etc etc. Very tedious. Very long. Exhausting. Been through it twice now. No fun. I really wonder why Dataverse should do this in the first place. Currently we are not using any kind of Members only stuff in Dataverse (see API comparison <https://orcid.org/organizations/integrators/API> for details on what you can do with it), we are just using it for auth and reading public data. Email addresses are private by default in ORCID and you cannot access them either via Public API or Member API. The Public API <http://members.orcid.org/api/about-public-api> on the other hand: Sufficient for auth and reading public things. No need to become an ORCID member Automated process to register your client app No obligations for us to store tokens, be compliant with the UI stuff or anything else from the checklist linked above. So my question is: shouldn't we refactor? Mentions: @TaniaSchlatter <https://github.com/TaniaSchlatter> because this is UI related @scolapasta <https://github.com/scolapasta> because IIRC he's one the sec guys @pameyer <https://github.com/pameyer> because he was engaged in some of the other issues @michbarsinai <https://github.com/michbarsinai> because he wrote much of that code back in 2017 @pdurbin <https://github.com/pdurbin> because I call him the Chief Community Officer and he also did much coding on this stuff — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6329?email_source=notifications&email_token=AAOZIJDSNFKHKP5LOIY267TQRL6GHA5CNFSM4JHMO5V2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HV2TRFQ>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOZIJHZLGZXQGH3Q32PTWTQRL6GHANCNFSM4JHMO5VQ>.

[cmbz]

To focus on the most important features and bugs, we are closing issues created before 2020 (version 5.0) that are not new feature requests with the label 'Type: Feature'.

If you created this issue and you feel the team should revisit this decision, please reopen the issue and leave a comment.