Skip to content

Commit

Permalink
autoset emailconfirmed timestamp for Shib users, rm check for stale t…
Browse files Browse the repository at this point in the history
…okens #5663

For Shib users we now set the emailconfirmed timestamp on login.
(The guides say we do this already but are wrong. It was only
being set on account creation.)

For Shib users, I also prevent "check for your welcome email to
verify your address" from being shown in the in-app
welcome/new account notification.

I put in a check to make sure Shib users never get a "verify your
email address" email notification.

Finally, I removed the hasNoStaleVerificationTokens check from the
hasVerifiedEmail method. We've never worried about if there are
stale verification tokens in the database or not and this check
was preventing "Verified" from being shown, even when the user has
a timestamp (the timestamp being the way we know if an email is
verified or not).
  • Loading branch information
pdurbin committed Apr 7, 2022
1 parent 3669666 commit 7fccaa7
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 8 deletions.
7 changes: 7 additions & 0 deletions doc/release-notes/5663-shib-confirm-email.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
For Shib users we now set the emailconfirmed timestamp on login. (The guides say we do this already but are wrong. It was only being set on account creation.)

For Shib users, I also prevent "check for your welcome email to verify your address" from being shown in the in-app welcome/new account notification.

I put in a check to make sure Shib users never get a "verify your email address" email notification.

Finally, I removed the hasNoStaleVerificationTokens check from the hasVerifiedEmail method. We've never worried about if there are stale verification tokens in the database or not and this check was preventing "Verified" from being shown, even when the user has a timestamp (the timestamp being the way we know if an email is verified or not).
2 changes: 1 addition & 1 deletion doc/sphinx-guides/source/admin/user-administration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ The app will send a standard welcome email with a URL the user can click, which,

Should users' URL token expire, they will see a "Verify Email" button on the account information page to send another URL.

Sysadmins can determine which users have verified their email addresses by looking for the presence of the value ``emailLastConfirmed`` in the JSON output from listing users (see :ref:`admin` section of Native API in the API Guide). As mentioned in the :doc:`/user/account` section of the User Guide, the email addresses for Shibboleth users are re-confirmed on every login.
Sysadmins can determine which users have verified their email addresses by looking for the presence of the value ``emailLastConfirmed`` in the JSON output from listing users (see :ref:`admin` section of Native API in the API Guide). As mentioned in the :doc:`/user/account` section of the User Guide, the email addresses for Shibboleth users are re-confirmed on every login (so their welcome email does not contain a URL to click for this purpose).

Deleting an API Token
---------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,9 +636,8 @@ public AuthenticatedUser createAuthenticatedUser(UserRecordIdentifier userRecord
authenticatedUser.setAuthenticatedUserLookup(auusLookup);

if (ShibAuthenticationProvider.PROVIDER_ID.equals(auusLookup.getAuthenticationProviderId())) {
Timestamp emailConfirmedNow = new Timestamp(new Date().getTime());
// Email addresses for Shib users are confirmed by the Identity Provider.
authenticatedUser.setEmailConfirmed(emailConfirmedNow);
authenticatedUser.updateEmailConfirmedToNow();
authenticatedUser = save(authenticatedUser);
} else {
/* @todo Rather than creating a token directly here it might be
Expand All @@ -665,6 +664,7 @@ public boolean identifierExists( String idtf ) {

public AuthenticatedUser updateAuthenticatedUser(AuthenticatedUser user, AuthenticatedUserDisplayInfo userDisplayInfo) {
user.applyDisplayInfo(userDisplayInfo);
user.updateEmailConfirmedToNow();
actionLogSvc.log( new ActionLogRecord(ActionLogRecord.ActionType.Auth, "updateUser")
.setInfo(user.getIdentifier()));
return update(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,8 @@ public boolean isDisplayIdentifier() {
return false;
}

// We don't override "isEmailVerified" because we're using timestamps
// ("emailconfirmed" on the "authenticateduser" table) to know if
// Shib users have confirmed/verified their email or not.

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2TokenData;
import edu.harvard.iq.dataverse.userdata.UserUtil;
import edu.harvard.iq.dataverse.authorization.providers.oauth2.impl.OrcidOAuth2AP;
import edu.harvard.iq.dataverse.authorization.providers.shib.ShibAuthenticationProvider;
import edu.harvard.iq.dataverse.util.BundleUtil;
import static edu.harvard.iq.dataverse.util.StringUtil.nonEmpty;
import edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder;
import java.io.Serializable;
import java.sql.Timestamp;
import java.util.Date;
import java.util.List;
import java.util.Objects;
import javax.json.Json;
Expand Down Expand Up @@ -193,6 +195,13 @@ public void applyDisplayInfo( AuthenticatedUserDisplayInfo inf ) {
}
}

// For Shib users, set "email confirmed" timestamp on login.
public void updateEmailConfirmedToNow() {
if (ShibAuthenticationProvider.PROVIDER_ID.equals(this.getAuthenticatedUserLookup().getAuthenticationProviderId())) {
Timestamp emailConfirmedNow = new Timestamp(new Date().getTime());
this.setEmailConfirmed(emailConfirmedNow);
}
}

//For User List Admin dashboard
@Transient
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,16 @@ public class ConfirmEmailServiceBean {
*/
public boolean hasVerifiedEmail(AuthenticatedUser user) {
boolean hasTimestamp = user.getEmailConfirmed() != null;
boolean hasNoStaleVerificationTokens = this.findSingleConfirmEmailDataByUser(user) == null;
boolean isVerifiedByAuthProvider = authenticationService.lookupProvider(user).isEmailVerified();

return (hasTimestamp && hasNoStaleVerificationTokens) || isVerifiedByAuthProvider;
// Note: In practice, we are relying on hasTimestamp to know if an email
// has been confirmed/verified or not. We have switched the Shib code to automatically
// overwrite the "confirm email" timestamp on login. So hasTimeStamp will be enough.
// If we ever want to get away from using "confirmed email" timestamps for Shib users
// we can make use of the isVerifiedByAuthProvider boolean. Currently,
// isVerifiedByAuthProvider is set to false in the super class and nothing
// is overridden in the shib auth provider (or any auth provider) but we could override
// isVerifiedByAuthProvider in the Shib auth provider and have it return true.
return hasTimestamp || isVerifiedByAuthProvider;
}

/**
Expand Down Expand Up @@ -128,6 +134,11 @@ private void sendLinkOnEmailChange(AuthenticatedUser aUser, String confirmationU
userNotification.setType(UserNotification.Type.CONFIRMEMAIL);
String subject = MailUtil.getSubjectTextBasedOnNotification(userNotification, null);
logger.fine("sending email to " + toAddress + " with this subject: " + subject);
if (ShibAuthenticationProvider.PROVIDER_ID.equals(aUser.getAuthenticatedUserLookup().getAuthenticationProviderId())) {
// Shib users have "emailconfirmed" timestamp set on login.
logger.info("Returning early to prevent an email confirmation link from being sent to Shib user " + aUser.getUserIdentifier() + ".");
return;
}
mailService.sendSystemEmail(toAddress, subject, messageBody);
} catch (Exception ex) {
/**
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/propertyFiles/Bundle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ wasReturnedByReviewer=, was returned by the curator of
# TODO: Confirm that "toReview" can be deleted.
toReview=Don't forget to publish it or send it back to the contributor!
# Bundle file editors, please note that "notification.welcome" is used in a unit test.
notification.welcome=Welcome to {0}! Get started by adding or finding data. Have questions? Check out the {1}. Want to test out Dataverse features? Use our {2}. Also, check for your welcome email to verify your address.
notification.welcome=Welcome to {0}! Get started by adding or finding data. Have questions? Check out the {1}. Want to test out Dataverse features? Use our {2}.
notification.welcomeConfirmEmail=Also, check for your welcome email to verify your address.
notification.demoSite=Demo Site
notification.requestFileAccess=File access requested for dataset: {0} was made by {1} ({2}).
notification.grantFileAccess=Access granted for files in dataset: {0}.
Expand Down
2 changes: 2 additions & 0 deletions src/main/webapp/dataverseuser.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@
<a href="https://demo.dataverse.org">#{bundle['notification.demoSite']}</a>
</o:param>
</h:outputFormat>
<!--Only show message about an email confirmation link to non-Shib users (Shib users can't edit their accounts)-->
<h:outputFormat value=" #{bundle['notification.welcomeConfirmEmail']}" rendered="#{DataverseUserPage.accountDetailsEditable}"/>
</ui:fragment>
<ui:fragment rendered="#{item.type == 'CREATEDV'}">
<span class="icon-dataverse text-icon-inline text-muted"></span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ public void testWelcomeInAppNotification(TestInfo testInfo) {
"LibraScholar",
"<a href=\"http://guides.dataverse.org/en/4.3/user/index.html\">User Guide</a>",
"<a href=\"https://demo.dataverse.org\">Demo Site</a>"
));
))
+ " " + BundleUtil.getStringFromBundle("notification.welcomeConfirmEmail");
log.fine("message: " + message);
assertEquals("Welcome to LibraScholar! Get started by adding or finding data. "
+ "Have questions? Check out the <a href=\"http://guides.dataverse.org/en/4.3/user/index.html\">User Guide</a>."
Expand Down

0 comments on commit 7fccaa7

Please sign in to comment.