Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Public ORCID login is available. #7025

Merged
merged 10 commits into from
Jul 20, 2020
Merged

Public ORCID login is available. #7025

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c10b516
Public ORCID login is available.
felker13 Jun 23, 2020
55c2c89
Fix for NPE in test.
felker13 Jun 29, 2020
8da201b
Oauth documentation is extended with ORCID public API option.
felker13 Jun 29, 2020
9d7c843
remove cost, link to ORCID APIs public, member #7025
pdurbin Jun 29, 2020
0bcd871
Merge pull request #1 from IQSS/7025-orcid-docs
felker13 Jun 30, 2020
8063067
Merge branch 'develop' into orcid-public-api
poikilotherm Jul 8, 2020
5a98e64
Refactor userEndpoint checks for Public ORCID scope. #7025
poikilotherm Jul 8, 2020
9475838
Convert ORCID OAuth2 provider test from JUnit4 to JUnit5. #7025
poikilotherm Jul 8, 2020
d588210
Add test case for public ORCID API endpoint to result in scope /authe…
poikilotherm Jul 8, 2020
298bfab
Merge pull request #2 from poikilotherm/7025-orcid-public
felker13 Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
0 ...tion/files/root/auth-providers/orcid.json → ...les/root/auth-providers/orcid-member.json
View file
Open in desktop
File renamed without changes.
8 changes: 8 additions & 0 deletions doc/sphinx-guides/source/_static/installation/files/root/auth-providers/orcid-public.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id":"orcid-public",
"factoryAlias":"oauth2",
"title":"ORCID",
"subtitle":"",
"factoryData":"type: orcid | userEndpoint: https://pub.orcid.org/v2.1/{ORCID}/person | clientId: FIXME | clientSecret: FIXME",
"enabled":true
}
7 changes: 4 additions & 3 deletions doc/sphinx-guides/source/installation/oauth2.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ Identity Provider Side
Obtain Client ID and Client Secret
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Before OAuth providers will release information about their users (first name, last name, etc.) to your Dataverse installation, you must request a "Client ID" and "Client Secret" from them. In the case of GitHub and Google, this is as simple as clicking a few buttons and there is no cost associated with using their authentication service. ORCID and Microsoft, on the other hand, do not have an automated system for requesting these credentials, and it is not free to use these authentication services.
Before OAuth providers will release information about their users (first name, last name, etc.) to your Dataverse installation, you must request a "Client ID" and "Client Secret" from them. In many cases you can use providers' automated system to request these credentials, but if not, contact the provider for assistance.

URLs to help you request a Client ID and Client Secret from the providers supported by Dataverse are provided below. For all of these providers, it's a good idea to request the Client ID and Client secret using a generic account, perhaps the one that's associated with the ``:SystemEmail`` you've configured for Dataverse, rather than your own personal Microsoft Azure AD, ORCID, GitHub, or Google account:

- ORCID: https://orcid.org/content/register-client-application-production-trusted-party
- ORCID: https://orcid.org/content/register-client-application-0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ORCID: https://orcid.org/content/register-client-application-0
- ORCID: https://orcid.org/content/register-client-application

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update the top comment of the PR to match the pull request template format in other PRs

This done so I'm moving this to code review.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I got the longer link with the "-0" at the end from https://orcid.org/organizations/integrators but I do think the shorter link looks nicer (and goes to the same place) so it would be a good change. @felker13 I don't have access to accept this change.

- Microsoft: https://docs.microsoft.com/en-us/azure/active-directory/develop/v1-protocols-oauth-code
- GitHub: https://github.com/settings/applications/new via https://developer.github.com/v3/oauth/
- Google: https://console.developers.google.com/projectselector/apis/credentials via https://developers.google.com/identity/protocols/OAuth2WebServer (pick "OAuth client ID")
Expand All @@ -51,7 +51,8 @@ As explained under "Auth Modes" in the :doc:`config` section, available authenti

We will ``POST`` a JSON file containing the Client ID and Client Secret to this ``authenticationProviders`` API endpoint to add another authentication provider. As a starting point, you'll want to download the JSON template file matching the provider you're setting up:

- :download:`orcid.json <../_static/installation/files/root/auth-providers/orcid.json>`
- :download:`orcid-public.json <../_static/installation/files/root/auth-providers/orcid-public.json>`
- :download:`orcid-member.json <../_static/installation/files/root/auth-providers/orcid-member.json>`
- :download:`github.json <../_static/installation/files/root/auth-providers/github.json>`
- :download:`google.json <../_static/installation/files/root/auth-providers/google.json>`
- :download:`microsoft.json <../_static/installation/files/root/auth-providers/microsoft.json>`
Expand Down
7 changes: 6 additions & 1 deletion ...main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/OrcidOAuth2AP.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ public class OrcidOAuth2AP extends AbstractOAuth2AuthenticationProvider {
public static final String PROVIDER_ID_SANDBOX = "orcid-sandbox";

public OrcidOAuth2AP(String clientId, String clientSecret, String userEndpoint) {
scope = Arrays.asList("/read-limited");

String s = null;
if(userEndpoint != null){
s = userEndpoint.startsWith("https://pub") ? "/authenticate" : "/read-limited";
}
scope = Arrays.asList(s);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could argue that this can be done more compact with only one if and without an extra variable allocation.

E. g. like

if(userEndpoint != null && userEndpoint.startsWith("https://pub")) {
    this.scope = Arrays.asList("/authenticate");
} else {
    this.scope = Arrays.asList("/read-limited");
}

Or you leave it to the code optimizer of the JVM... 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with letting the JVM optimize it. 😄

this.clientId = clientId;
this.clientSecret = clientSecret;
this.baseUserEndpoint = userEndpoint;
Expand Down