Skip to content

Commit

Permalink
Merge pull request IQSS#4280 from IQSS/exp-4218
Browse files Browse the repository at this point in the history
ORCID v2 API upgrade
  • Loading branch information
kcondon authored Nov 17, 2017
2 parents c82628b + 11d4f0d commit 02d5a5a
Show file tree
Hide file tree
Showing 25 changed files with 848 additions and 230 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"factoryAlias":"oauth2",
"title":"ORCID",
"subtitle":"",
"factoryData":"type: orcid | userEndpoint: https://api.sandbox.orcid.org/v1.2/{ORCID}/orcid-profile | clientId: FIXME | clientSecret: FIXME",
"factoryData":"type: orcid | userEndpoint: https://api.sandbox.orcid.org/v2.0/{ORCID}/record | clientId: FIXME | clientSecret: FIXME",
"enabled":true
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"factoryAlias":"oauth2",
"title":"ORCID",
"subtitle":"",
"factoryData":"type: orcid | userEndpoint: https://api.orcid.org/v1.2/{ORCID}/orcid-profile | clientId: FIXME | clientSecret: FIXME",
"factoryData":"type: orcid | userEndpoint: https://api.orcid.org/v2.0/{ORCID}/record | clientId: FIXME | clientSecret: FIXME",
"enabled":true
}
8 changes: 0 additions & 8 deletions scripts/api/data/authentication-providers/base-oauth2.json

This file was deleted.

6 changes: 3 additions & 3 deletions scripts/api/data/authentication-providers/orcid-sandbox.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"id":"orcid-sandbox",
"id":"orcid-v2-sandbox",
"factoryAlias":"oauth2",
"title":"ORCID Sandbox",
"subtitle":"ORCiD - sandbox",
"factoryData":"type: orcid | userEndpoint: https://api.sandbox.orcid.org/v1.2/{ORCID}/orcid-profile | clientId: APP-HIV99BRM37FSWPH6 | clientSecret: ee844b70-f223-4f15-9b6f-4991bf8ed7f0",
"subtitle":"ORCiD - sandbox (v2)",
"factoryData":"type: orcid | userEndpoint: https://api.sandbox.orcid.org/v2.0/{ORCID}/person | clientId: APP-HIV99BRM37FSWPH6 | clientSecret: ee844b70-f223-4f15-9b6f-4991bf8ed7f0",
"enabled":true
}
4 changes: 3 additions & 1 deletion src/main/java/Bundle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ login.error=Error validating the username, email address, or password. Please tr
user.error.cannotChangePassword=Sorry, your password cannot be changed. Please contact your system administrator.
user.error.wrongPassword=Sorry, wrong password.
login.button=Log In with {0}
login.button.orcid=Create or Connect your ORCID
# authentication providers
auth.providers.title=Other options
auth.providers.tip=You can convert a Dataverse account to use one of the options above. <a href="{0}/{1}/user/account.html" target="_blank">Learn more</a>.
Expand All @@ -259,6 +260,7 @@ auth.providers.persistentUserIdName.orcid=ORCID iD
auth.providers.persistentUserIdName.github=ID
auth.providers.persistentUserIdTooltip.orcid=ORCID provides a persistent digital identifier that distinguishes you from other researchers.
auth.providers.persistentUserIdTooltip.github=GitHub assigns a unique number to every user.
auth.providers.orcid.insufficientScope=Dataverse was not granted the permission to read user data from ORCID.
# Friendly AuthenticationProvider names
authenticationProvider.name.builtin=Dataverse
authenticationProvider.name.null=(provider is unknown)
Expand Down Expand Up @@ -331,7 +333,7 @@ oauth2.convertAccount.success=Your Dataverse account is now associated with your

# oauth2/callback.xhtml
oauth2.callback.page.title=OAuth Callback
oauth2.callback.message=<strong>OAuth2 Error</strong> - Sorry, the identification process did not succeed.
oauth2.callback.message=<strong>Authentication Error</strong> - Dataverse could not authenticate your ORCID login. Please make sure you authorize your ORCID account to connect with Dataverse. For more details about the information being requested, see the <a href="{0}/{1}/user/account.html#orcid-log-in" title="ORCID Log In - Dataverse User Guide" target="_blank">User Guide</a>.

# tab on dataverseuser.xhtml
apitoken.title=API Token
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/edu/harvard/iq/dataverse/LoginPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public String login() {
}
authReq.setIpAddress( dvRequestService.getDataverseRequest().getSourceAddress() );
try {
AuthenticatedUser r = authSvc.authenticate(credentialsAuthProviderId, authReq);
AuthenticatedUser r = authSvc.getCreateAuthenticatedUser(credentialsAuthProviderId, authReq);
logger.log(Level.FINE, "User authenticated: {0}", r.getEmail());
session.setUser(r);

Expand Down
8 changes: 6 additions & 2 deletions src/main/java/edu/harvard/iq/dataverse/api/Admin.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@
import edu.harvard.iq.dataverse.ingest.IngestServiceBean;
import edu.harvard.iq.dataverse.userdata.UserListMaker;
import edu.harvard.iq.dataverse.userdata.UserListResult;
import edu.harvard.iq.dataverse.util.StringUtil;
import java.math.BigDecimal;
import java.util.Date;
import java.util.ResourceBundle;
import javax.inject.Inject;
Expand Down Expand Up @@ -1002,4 +1000,10 @@ public Response validatePassword(String password) {
.add("errors", errorArray)
);
}

@GET
@Path("/isOrcid")
public Response isOrcidEnabled() {
return authSvc.isOrcidEnabled() ? ok("Orcid is enabled") : ok("no orcid for you.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class AuthenticatedUserDisplayInfo extends RoleAssigneeDisplayInfo {
private String firstName;
private String position;

/**
/*
* @todo Shouldn't we persist the displayName too? It still exists on the
* authenticateduser table.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import javax.ejb.EJB;
import javax.ejb.EJBException;
import javax.ejb.Singleton;
import javax.inject.Named;
import javax.persistence.EntityManager;
import javax.persistence.NoResultException;
import javax.persistence.NonUniqueResultException;
Expand All @@ -63,6 +64,7 @@
*
* Register the providers in the {@link #startup()} method.
*/
@Named
@Singleton
public class AuthenticationServiceBean {
private static final Logger logger = Logger.getLogger(AuthenticationServiceBean.class.getName());
Expand Down Expand Up @@ -234,7 +236,12 @@ public void removeApiToken(AuthenticatedUser user){
em.remove(apiToken);
}
}
}
}

public boolean isOrcidEnabled() {
return oAuth2authenticationProviders.values().stream().anyMatch( s -> s.getId().toLowerCase().contains("orcid") );
}

/**
* Use with care! This method was written primarily for developers
* interested in API testing who want to:
Expand Down Expand Up @@ -318,7 +325,19 @@ public AuthenticatedUser getAuthenticatedUserByEmail( String email ) {
}
}

public AuthenticatedUser authenticate( String authenticationProviderId, AuthenticationRequest req ) throws AuthenticationFailedException {
/**
* Returns an {@link AuthenticatedUser} matching the passed provider id and the authentication request. If
* no such user exist, it is created and then returned.
*
* <strong>Invariant:</strong> upon successful return from this call, an {@link AuthenticatedUser} record
* matching the request and provider exists in the database.
*
* @param authenticationProviderId
* @param req
* @return The authenticated user for the passed provider id and authentication request.
* @throws AuthenticationFailedException
*/
public AuthenticatedUser getCreateAuthenticatedUser( String authenticationProviderId, AuthenticationRequest req ) throws AuthenticationFailedException {
AuthenticationProvider prv = getAuthenticationProvider(authenticationProviderId);
if ( prv == null ) throw new IllegalArgumentException("No authentication provider listed under id " + authenticationProviderId );
if ( ! (prv instanceof CredentialsAuthenticationProvider) ) {
Expand All @@ -334,18 +353,16 @@ public AuthenticatedUser authenticate( String authenticationProviderId, Authenti
user = userService.updateLastLogin(user);
}

/**
* @todo Why does a method called "authenticate" have the potential
* to call "createAuthenticatedUser"? Isn't the creation of a user a
* different action than authenticating?
*
* @todo Wouldn't this be more readable with if/else rather than
* ternary? (please)
*/
return ( user == null ) ?
AuthenticationServiceBean.this.createAuthenticatedUser(
new UserRecordIdentifier(authenticationProviderId, resp.getUserId()), resp.getUserId(), resp.getUserDisplayInfo(), true )
: (BuiltinAuthenticationProvider.PROVIDER_ID.equals(user.getAuthenticatedUserLookup().getAuthenticationProviderId())) ? user : updateAuthenticatedUser(user, resp.getUserDisplayInfo());
if ( user == null ) {
return createAuthenticatedUser(
new UserRecordIdentifier(authenticationProviderId, resp.getUserId()), resp.getUserId(), resp.getUserDisplayInfo(), true );
} else {
if (BuiltinAuthenticationProvider.PROVIDER_ID.equals(user.getAuthenticatedUserLookup().getAuthenticationProviderId())) {
return user;
} else {
return updateAuthenticatedUser(user, resp.getUserDisplayInfo());
}
}
} else {
throw new AuthenticationFailedException(resp, "Authentication Failed: " + resp.getMessage());
}
Expand Down Expand Up @@ -779,7 +796,7 @@ public AuthenticatedUser canLogInAsBuiltinUser(String username, String password)

String credentialsAuthProviderId = BuiltinAuthenticationProvider.PROVIDER_ID;
try {
AuthenticatedUser au = authenticate(credentialsAuthProviderId, authReq);
AuthenticatedUser au = getCreateAuthenticatedUser(credentialsAuthProviderId, authReq);
logger.fine("User authenticated:" + au.getEmail());
return au;
} catch (AuthenticationFailedException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
Expand Down Expand Up @@ -90,8 +91,6 @@ public String toString() {
protected String redirectUrl;
protected String scope;

public AbstractOAuth2AuthenticationProvider(){}

public abstract BaseApi<OAuth20Service> getApiInstance();

protected abstract ParsedUserResponse parseUserResponse( String responseBody );
Expand All @@ -111,6 +110,7 @@ public OAuth20Service getService(String state, String redirectUrl) {
public OAuth2UserRecord getUserRecord(String code, String state, String redirectUrl) throws IOException, OAuth2Exception {
OAuth20Service service = getService(state, redirectUrl);
OAuth2AccessToken accessToken = service.getAccessToken(code);

final String userEndpoint = getUserEndpoint(accessToken);

final OAuthRequest request = new OAuthRequest(Verb.GET, userEndpoint, service);
Expand All @@ -120,13 +120,13 @@ public OAuth2UserRecord getUserRecord(String code, String state, String redirect
final Response response = request.send();
int responseCode = response.getCode();
final String body = response.getBody();
logger.fine("In getUserRecord. Body: " + body);
logger.log(Level.FINE, "In getUserRecord. Body: {0}", body);

if ( responseCode == 200 ) {
final ParsedUserResponse parsed = parseUserResponse(body);
return new OAuth2UserRecord(getId(), parsed.userIdInProvider,
parsed.username,
accessToken.getAccessToken(),
OAuth2TokenData.from(accessToken),
parsed.displayInfo,
parsed.emails);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ public class OAuth2FirstLoginPage implements java.io.Serializable {
@EJB
AuthTestDataServiceBean authTestDataSvc;

@EJB
OAuth2TokenDataServiceBean oauth2Tokens;

@Inject
DataverseSession session;

Expand Down Expand Up @@ -99,7 +102,7 @@ public void init() throws IOException {
logger.fine("init called");

AbstractOAuth2AuthenticationProvider.DevOAuthAccountType devMode = systemConfig.getDevOAuthAccountType();
logger.fine("devMode: " + devMode);
logger.log(Level.FINE, "devMode: {0}", devMode);
if (!AbstractOAuth2AuthenticationProvider.DevOAuthAccountType.PRODUCTION.equals(devMode)) {
if (devMode.toString().startsWith("RANDOM")) {
Map<String, String> randomUser = authTestDataSvc.getRandomUser();
Expand Down Expand Up @@ -136,7 +139,8 @@ public void init() throws IOException {
}
String randomUsername = randomUser.get("username");
String eppn = randomUser.get("eppn");
String accessToken = "qwe-addssd-iiiiie";
OAuth2TokenData accessToken = new OAuth2TokenData();
accessToken.setAccessToken("qwe-addssd-iiiiie");
setNewUser(new OAuth2UserRecord(authProviderId, eppn, randomUsername, accessToken,
new AuthenticatedUserDisplayInfo(firstName, lastName, email, "myAffiliation", "myPosition"),
extraEmails));
Expand Down Expand Up @@ -185,7 +189,12 @@ public String createNewAccount() {
userNotificationService.sendNotification(user,
new Timestamp(new Date().getTime()),
UserNotification.Type.CREATEACC, null);


final OAuth2TokenData tokenData = newUser.getTokenData();
tokenData.setUser(user);
tokenData.setOauthProviderId(newUser.getServiceId());
oauth2Tokens.store(tokenData);

return "/dataverse.xhtml?faces-redirect=true";
}

Expand All @@ -196,13 +205,13 @@ public String convertExistingAccount() {
auReq.putCredential(creds.get(0).getTitle(), getUsername());
auReq.putCredential(creds.get(1).getTitle(), getPassword());
try {
AuthenticatedUser existingUser = authenticationSvc.authenticate(BuiltinAuthenticationProvider.PROVIDER_ID, auReq);
AuthenticatedUser existingUser = authenticationSvc.getCreateAuthenticatedUser(BuiltinAuthenticationProvider.PROVIDER_ID, auReq);
authenticationSvc.updateProvider(existingUser, newUser.getServiceId(), newUser.getIdInService());
builtinUserSvc.removeUser(existingUser.getUserIdentifier());

session.setUser(existingUser);
AuthenticationProvider authProvider = authenticationSvc.getAuthenticationProvider(newUser.getServiceId());
JsfHelper.addSuccessMessage(BundleUtil.getStringFromBundle("oauth2.convertAccount.success", Arrays.asList(authProvider.getInfo().getTitle())));
AuthenticationProvider newUserAuthProvider = authenticationSvc.getAuthenticationProvider(newUser.getServiceId());
JsfHelper.addSuccessMessage(BundleUtil.getStringFromBundle("oauth2.convertAccount.success", Arrays.asList(newUserAuthProvider.getInfo().getTitle())));

return "/dataverse.xhtml?faces-redirect=true";

Expand All @@ -212,22 +221,17 @@ public String convertExistingAccount() {
}
}

public String testAction() {
Logger.getLogger(OAuth2FirstLoginPage.class.getName()).log(Level.INFO, "testAction");
return "dataverse.xhtml";
}

public boolean isEmailAvailable() {
return authenticationSvc.isEmailAddressAvailable(getSelectedEmail());
}

/**
/*
* @todo This was copied from DataverseUserPage and modified so consider
* consolidating common code (DRY).
*/
public void validateUserName(FacesContext context, UIComponent toValidate, Object value) {
String userName = (String) value;
logger.fine("Validating username: " + userName);
logger.log(Level.FINE, "Validating username: {0}", userName);
boolean userNameFound = authenticationSvc.identifierExists(userName);
if (userNameFound) {
((UIInput) toValidate).setValid(false);
Expand All @@ -236,7 +240,7 @@ public void validateUserName(FacesContext context, UIComponent toValidate, Objec
}
}

/**
/*
* @todo This was copied from DataverseUserPage and modified so consider
* consolidating common code (DRY).
*/
Expand Down Expand Up @@ -336,11 +340,7 @@ public String getCreateFromWhereTip() {

public boolean isConvertFromBuiltinIsPossible() {
AuthenticationProvider builtinAuthProvider = authenticationSvc.getAuthenticationProvider(BuiltinAuthenticationProvider.PROVIDER_ID);
if (builtinAuthProvider != null) {
return true;
} else {
return false;
}
return builtinAuthProvider != null;
}

public String getSuggestConvertInsteadOfCreate() {
Expand Down Expand Up @@ -371,7 +371,7 @@ public List<String> getEmailsToPickFrom() {
}
}
}
logger.fine(emailsToPickFrom.size() + " emails to pick from: " + emailsToPickFrom);
logger.log(Level.FINE, "{0} emails to pick from: {1}", new Object[]{emailsToPickFrom.size(), emailsToPickFrom});
return emailsToPickFrom;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean;
import edu.harvard.iq.dataverse.authorization.UserRecordIdentifier;
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
import edu.harvard.iq.dataverse.settings.SettingsServiceBean;
import edu.harvard.iq.dataverse.util.StringUtil;
import java.io.BufferedReader;
import java.io.IOException;
Expand All @@ -26,8 +25,8 @@
import edu.harvard.iq.dataverse.util.SystemConfig;

/**
* Backing bean of the oauth2 login process. Used from the login page and the
* callback page.
* Backing bean of the oauth2 login process. Used from the login and the
* callback pages.
*
* @author michael
*/
Expand All @@ -45,6 +44,9 @@ public class OAuth2LoginBackingBean implements Serializable {

@EJB
AuthenticationServiceBean authenticationSvc;

@EJB
OAuth2TokenDataServiceBean oauth2Tokens;

@EJB
SystemConfig systemConfig;
Expand Down Expand Up @@ -91,7 +93,7 @@ public void exchangeCodeForToken() throws IOException {
oauthUser = idp.getUserRecord(code, state, getCallbackUrl());
UserRecordIdentifier idtf = oauthUser.getUserRecordIdentifier();
AuthenticatedUser dvUser = authenticationSvc.lookupUser(idtf);

if (dvUser == null) {
// need to create the user
newAccountPage.setNewUser(oauthUser);
Expand All @@ -100,6 +102,10 @@ public void exchangeCodeForToken() throws IOException {
} else {
// login the user and redirect to HOME of intended page (if any).
session.setUser(dvUser);
final OAuth2TokenData tokenData = oauthUser.getTokenData();
tokenData.setUser(dvUser);
tokenData.setOauthProviderId(idp.getId());
oauth2Tokens.store(tokenData);
String destination = redirectPage.orElse("/");
HttpServletResponse response = (HttpServletResponse) FacesContext.getCurrentInstance().getExternalContext().getResponse();
String prettyUrl = response.encodeRedirectURL(destination);
Expand Down
Loading

0 comments on commit 02d5a5a

Please sign in to comment.