Skip to content

Commit

Permalink
Split "DataverseSession.setUser" into "logIn" and "logOut" methods. r…
Browse files Browse the repository at this point in the history
…emoved "setUser" method. Updated callers.
  • Loading branch information
lbownik committed Jan 29, 2025
1 parent e0f2454 commit f76ddc8
Show file tree
Hide file tree
Showing 42 changed files with 86 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public void initBreadcrumbs(DvObject dvObject, String subPage) {

public String logout() {
samlSessionRegistry.unregister(dataverseSession);
dataverseSession.setUser(null);
dataverseSession.logOut();
dataverseSession.setStatusDismissed(false);

String redirectPage = navigationWrapper.getPageFromContext();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package edu.harvard.iq.dataverse;

import static edu.harvard.iq.dataverse.persistence.ActionLogRecord.ActionType.SessionManagement;
import static edu.harvard.iq.dataverse.persistence.ActionLogRecord.ActionType.SessionManagement;
import static java.util.Objects.requireNonNull;

import java.io.Serializable;
import java.util.Locale;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;

Expand Down Expand Up @@ -160,12 +162,18 @@ private String getBrowserLanguage() {
}

// -------------------- SETTERS --------------------

public void setUser(User aUser) {
logSvc.log(
new ActionLogRecord(SessionManagement, (aUser == null) ? "logout" : "login")
.setUserIdentifier((aUser != null) ? aUser.getIdentifier() : (user != null ? user.getIdentifier() : "")));
this.user = aUser != null ? aUser : GuestUser.get();

public void logIn(final User user) {
requireNonNull(user);
this.logSvc.log(new ActionLogRecord(SessionManagement, "login")
.setUserIdentifier(user.getIdentifier()));
this.user = user;
}

public void logOut() {
this.logSvc.log(new ActionLogRecord(SessionManagement, "logout")
.setUserIdentifier(this.user.getIdentifier()));
this.user = GuestUser.get();
}

public void setLocaleCode(String localeCode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public String login() {
try {
AuthenticatedUser r = authSvc.getUpdateAuthenticatedUser(credentialsAuthProviderId, authReq);
logger.log(Level.FINE, "User authenticated: {0}", r.getEmail());
session.setUser(r);
session.logIn(r);

if ("dataverse.xhtml".equals(redirectPage)) {
redirectPage = redirectToRoot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public String confirmAndConvertAccount() {

private void logInUserAndSetShibAttributes(AuthenticatedUser au) {
au.setShibIdentityProvider(shibIdp);
session.setUser(au);
session.logIn(au);
logger.debug("Groups for user " + au.getId() + " (" + au.getIdentifier() + "): " + getGroups(au));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,7 @@ public Response submitDatasetVersionToArchive(@PathParam("id") String dsid, @Pat

try {
AuthenticatedUser au = findAuthenticatedUserOrDie();
session.setUser(au);
session.logIn(au);
Dataset ds = findDatasetOrDie(dsid);

DatasetVersion dv = datasetversionService.findByFriendlyVersionNumber(ds.getId(), versionNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private FilterLogIn logInUserByTokenIfNeeded(HttpServletRequest request) {
if (!systemConfig.isReadonlyMode()) {
user = userService.updateLastApiUseTime(user);
}
session.setUser(user);
session.logIn(user);
return FilterLogIn.TOKEN_LOG_IN;
}
}
Expand All @@ -94,7 +94,7 @@ private enum FilterLogIn {
NONE((r, ds) -> { }),
TOKEN_LOG_IN((r, ds) -> {
r.getSession().invalidate();
ds.setUser(null);
ds.logOut();
});

private final BiConsumer<HttpServletRequest, DataverseSession> logout;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ public String save() {

// Authenticated user registered. Save the new bulitin, and log in.
builtinUserService.save(builtinUser);
session.setUser(au);
session.logIn(au);
/**
* @todo Move this to
* AuthenticationServiceBean.createAuthenticatedUser
Expand Down Expand Up @@ -452,7 +452,7 @@ public String save() {
} catch (ConfirmEmailException ex) {
logger.log(Level.INFO, "Unable to send email confirmation link to user id {0}", savedUser.getId());
}
session.setUser(currentUser);
session.logIn(currentUser);
JsfHelper.addFlashSuccessMessage(BundleUtil.getStringFromBundle("confirmEmail.changed", currentUser.getEmail(), expTime));
} else {
JsfHelper.addFlashSuccessMessage(msg.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public String createNewAccount() {
final AuthenticatedUser user = authenticationSvc.createAuthenticatedUser(newUser.toUserRecordIdentifier(),
getUsername(), newAuthenticatedUserDisplayInfo, true, preferredNotificationsLanguage).getOrNull();

session.setUser(user);
session.logIn(user);
userNotificationService.sendNotificationWithEmail(user, new Timestamp(new Date().getTime()), NotificationType.CREATEACC,
null, NotificationObjectType.AUTHENTICATED_USER);
consentService.executeActionsAndSaveAcceptedConsents(consents, user);
Expand Down Expand Up @@ -269,7 +269,7 @@ public String convertExistingAccount() {
authenticationSvc.updateProvider(existingUser, newUser.getServiceId(), newUser.getIdInService());
builtinUserSvc.removeUser(existingUser.getUserIdentifier());

session.setUser(existingUser);
session.logIn(existingUser);
AuthenticationProvider newUserAuthProvider = authenticationSvc.getAuthenticationProvider(newUser.getServiceId());
JsfHelper.addFlashSuccessMessage(
BundleUtil.getStringFromBundle("oauth2.convertAccount.success", newUserAuthProvider.getInfo().getTitle()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void exchangeCodeForToken() throws IOException {

} else {
// login the user and redirect to HOME of intended page (if any).
session.setUser(dvUser);
session.logIn(dvUser);

if (!systemConfig.isReadonlyMode()) {
final OAuth2TokenData tokenData = oauthUser.getTokenData();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ private void login(HttpServletRequest request, HttpServletResponse response) thr
return;
}
AuthenticatedUser updatedUser = updateResult.get();
session.setUser(updatedUser);
session.logIn(updatedUser);
String relayState = request.getParameter("RelayState");
response.sendRedirect(StringUtils.isNotBlank(relayState)
? relayState
Expand Down Expand Up @@ -247,7 +247,7 @@ private AuthenticatedUser findUserForLogout(String issuer, String nameId) {
private void clearActiveSessionsForUser(AuthenticatedUser user) {
List<DataverseSession> sessions = samlSessionRegistry.unregister(user);
for (DataverseSession session : sessions) {
session.setUser(null);
session.logOut();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public String confirmAndConvertAccount() {
}
authenticationService.updateAuthenticatedUser(authenticatedUser,
new AuthenticatedUserDisplayInfo(userData.getName(), userData.getSurname(), userData.getEmail(), null, null));
dataverseSession.setUser(authenticatedUser);
dataverseSession.logIn(authenticatedUser);
consentService.executeActionsAndSaveAcceptedConsents(consents, authenticatedUser);
logger.info("Local account validated and successfully converted to a Saml account. The old account username was "
+ builtinUsername);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public String init() {
confirmEmailData = confirmEmailExecResponse.getConfirmEmailData();
if (confirmEmailData != null) {
user = confirmEmailData.getAuthenticatedUser();
session.setUser(user);
session.logIn(user);
JsfHelper.addFlashSuccessMessage(BundleUtil.getStringFromBundle("confirmEmail.details.success"));
return "/dataverse.xhtml?faces-redirect=true";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public String resetPassword() {
response.getMessageDetail()));
String builtinAuthProviderId = BuiltinAuthenticationProvider.PROVIDER_ID;
AuthenticatedUser au = authSvc.lookupUser(builtinAuthProviderId, user.getUserName());
session.setUser(au);
session.logIn(au);
consentService.executeActionsAndSaveAcceptedConsents(consents, au);
return "/dataverse.xhtml?alias=" + dataverseDao.findRootDataverse().getAlias() + "faces-redirect=true";
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public String init() {
PrivateUrlRedirectData privateUrlRedirectData = privateUrlService.getPrivateUrlRedirectDataFromToken(token);
String datasetPageToBeRedirectedTo = privateUrlRedirectData.getDatasetPageToBeRedirectedTo() + "&faces-redirect=true";
PrivateUrlUser privateUrlUser = privateUrlRedirectData.getPrivateUrlUser();
session.setUser(privateUrlUser);
session.logIn(privateUrlUser);
logger.info("Redirecting PrivateUrlUser '" + privateUrlUser.getIdentifier() + "' to " + datasetPageToBeRedirectedTo);
return datasetPageToBeRedirectedTo;
} catch (Exception ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ public void emailGetsSentToSystem_whenUserIsLoggedIn_andDataAreProvided()
assertThat(this.dialog.getFormHeader())
.isEqualTo("Contact the Repository's Support");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -302,7 +302,7 @@ public void emailGetsSentToSystemAndUser_whenUserIsLoggedIn_andDataAreProvided_w
assertThat(this.dialog.getFormHeader())
.isEqualTo("Contact the Repository's Support");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -368,7 +368,7 @@ public void emailGetsSentToDataverseContact_whenUserIsLoggedIn_andDataAreProvide
.containsExactly(SYSTEM_SUPPORT, DATAVERSE_CONTACT);
assertThat(this.dialog.getFormHeader()).isEqualTo("Send E-mail");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -408,7 +408,7 @@ public void emailGetsSentToDataverseConctactAndUser_whenUserIsLoggedIn_andDataAr
.containsExactly(SYSTEM_SUPPORT, DATAVERSE_CONTACT);
assertThat(this.dialog.getFormHeader()).isEqualTo("Send E-mail");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -459,7 +459,7 @@ public void emailGetsSentToDatasetContact_whenUserIsLoggedIn_andDataAreProvided(
.containsExactly(SYSTEM_SUPPORT, DATAVERSE_CONTACT, DATASET_CONTACT);
assertThat(this.dialog.getFormHeader()).isEqualTo("Send E-mail");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -498,7 +498,7 @@ public void emailGetsSentToDatasetConctactAndUser_whenUserIsLoggedIn_andDataAreP
.containsExactly(SYSTEM_SUPPORT, DATAVERSE_CONTACT, DATASET_CONTACT);
assertThat(this.dialog.getFormHeader()).isEqualTo("Send E-mail");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -549,7 +549,7 @@ public void emailGetsSentToFileDatasetContact_whenUserIsLoggedIn_andDataAreProvi
.containsExactly(SYSTEM_SUPPORT, DATAVERSE_CONTACT, DATASET_CONTACT);
assertThat(this.dialog.getFormHeader()).isEqualTo("Send E-mail");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -588,7 +588,7 @@ public void emailGetsSentToFileDatasetConctactAndUser_whenUserIsLoggedIn_andData
.containsExactly(SYSTEM_SUPPORT, DATAVERSE_CONTACT, DATASET_CONTACT);
assertThat(this.dialog.getFormHeader()).isEqualTo("Send E-mail");

this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

assertThat(this.dialog.isLoggedIn()).isTrue();
assertThat(this.dialog.loggedInUserEmail()).isEqualTo(USER_EMAIL);
Expand Down Expand Up @@ -633,7 +633,7 @@ public void emailGetsSentToFileDatasetConctactAndUser_whenUserIsLoggedIn_andData
@Test
public void exceedingCombinedAttachmentSizeLimit_failsValidation()
throws Exception {
this.dataverseSession.setUser(this.authenticatedUser);
this.dataverseSession.logIn(this.authenticatedUser);

this.dialog.handleFileUpload(newEvent(attachment2));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void shouldCheckEmbargoRestriction_userWithPermissions() {
// given
Dataset dataset = datasetDao.find(57L);
dataset.setEmbargoDate(Date.from(Instant.now().plus(1, ChronoUnit.DAYS)));
dataverseSession.setUser(authenticationService.getAdminUser());
dataverseSession.logIn(authenticationService.getAdminUser());

// when&then
Assertions.assertFalse(embargoAccess.isRestrictedByEmbargo(dataset));
Expand All @@ -53,7 +53,7 @@ public void shouldCheckEmbargoRestriction_userWithoutPermissions() {
// given
Dataset dataset = datasetDao.find(57L);
dataset.setEmbargoDate(Date.from(Instant.now().plus(1, ChronoUnit.DAYS)));
dataverseSession.setUser(GuestUser.get());
dataverseSession.logIn(GuestUser.get());

// when&then
Assertions.assertTrue(embargoAccess.isRestrictedByEmbargo(dataset));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void shouldDoNothingIfUserIsAlreadyLogged() throws IOException, ServletEx
// then
verify(authenticationService, never()).lookupUser(nullable(String.class));
verify(userService, never()).updateLastApiUseTime(nullable(AuthenticatedUser.class));
verify(dataverseSession, never()).setUser(nullable(AuthenticatedUser.class));
verify(dataverseSession, never()).logIn(nullable(AuthenticatedUser.class));
verify(httpSession, never()).invalidate();
}

Expand All @@ -91,7 +91,7 @@ public void shouldLogAndLogoutUserWhenKeyIsPassedAndUserIsNotAlreadyLoggedIn()
// then
verify(authenticationService, times(1)).lookupUser(anyString());
verify(userService, times(1)).updateLastApiUseTime(any(AuthenticatedUser.class));
verify(dataverseSession, times(1)).setUser(any(AuthenticatedUser.class));
verify(dataverseSession, times(1)).logIn(any(AuthenticatedUser.class));
verify(httpSession, times(1)).invalidate();
}

Expand All @@ -108,7 +108,7 @@ public void shouldNotUpdateLastApiUseTimeWhenLogIsByApiInReadonlyMode()
filter.doFilter(request, null, filterChain);

// then
verify(dataverseSession, times(1)).setUser(any(AuthenticatedUser.class));
verify(dataverseSession, times(1)).logIn(any(AuthenticatedUser.class));
verify(userService, never()).updateLastApiUseTime(any(AuthenticatedUser.class));
}

Expand All @@ -125,7 +125,7 @@ public void shouldDoNothingWhenNoKeyPassedAndUserIsNotAlreadyLoggedIn()
// then
verify(authenticationService, times(1)).lookupUser(nullable(String.class));
verify(userService, never()).updateLastApiUseTime(any(AuthenticatedUser.class));
verify(dataverseSession, never()).setUser(any(AuthenticatedUser.class));
verify(dataverseSession, never()).logIn(any(AuthenticatedUser.class));
verify(httpSession, never()).invalidate();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class DashboardUsersServiceIT extends WebappArquillianDeployment {

@BeforeEach
public void setUp() {
dataverseSession.setUser(authenticationServiceBean.getAdminUser());
dataverseSession.logIn(authenticationServiceBean.getAdminUser());
}

@Test
Expand All @@ -57,7 +57,7 @@ public void shouldRevokeAllRolesForUser() {
public void shouldRevokeAllRolesForUser_withPermissionsException() {
// given
AuthenticatedUser user = authenticationServiceBean.findByID(2L);
dataverseSession.setUser(authenticationServiceBean.findByID(4L));
dataverseSession.logIn(authenticationServiceBean.findByID(4L));

// when & then
assertThrows(PermissionException.class, () -> dashboardUsersService.revokeAllRolesForUser(user.getId()));
Expand Down Expand Up @@ -93,7 +93,7 @@ public void shouldChangeSuperuserStatus_grantSuperuserStatus() {
public void shouldChangeSuperuserStatus_revokeSuperuserStatus_withPermissionException() {
// given
AuthenticatedUser user = authenticationServiceBean.findByID(2L);
dataverseSession.setUser(authenticationServiceBean.findByID(4L));
dataverseSession.logIn(authenticationServiceBean.findByID(4L));

// when & then
assertThrows(PermissionException.class, () -> dashboardUsersService.changeSuperuserStatus(user.getId()));
Expand All @@ -103,7 +103,7 @@ public void shouldChangeSuperuserStatus_revokeSuperuserStatus_withPermissionExce
public void shouldChangeSuperuserStatus_grantSuperuserStatus_withPermissionException() {
// given
AuthenticatedUser user = authenticationServiceBean.findByID(3L);
dataverseSession.setUser(authenticationServiceBean.findByID(4L));
dataverseSession.logIn(authenticationServiceBean.findByID(4L));

// when & then
assertThrows(PermissionException.class, () -> dashboardUsersService.changeSuperuserStatus(user.getId()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public class FilePermissionsServiceIT extends WebappArquillianDeployment {

@BeforeEach
public void initBefore() {
dataverseSession.setUser(authenticationService.getAuthenticatedUser("superuser"));
dataverseSession.logIn(authenticationService.getAuthenticatedUser("superuser"));
}


Expand Down Expand Up @@ -131,7 +131,7 @@ public void assignFileDownloadRole() {
@Transactional(TransactionMode.ROLLBACK)
public void assignFileDownloadRole__MISSING_MANAGE_DATASET_PERMISSIONS() {
// given
dataverseSession.setUser(GuestUser.get());
dataverseSession.logIn(GuestUser.get());
AuthenticatedUser user = authenticationService.getAuthenticatedUser("superuser");
DataFile datafile = dataFileService.find(53L);

Expand Down Expand Up @@ -172,7 +172,7 @@ public void revokeFileDownloadRole() {
@Transactional(TransactionMode.ROLLBACK)
public void revokeFileDownloadRole__MISSING_MANAGE_DATASET_PERMISSIONS() {
// given
dataverseSession.setUser(GuestUser.get());
dataverseSession.logIn(GuestUser.get());
DataFile datafile = dataFileService.find(53L);
AuthenticatedUser user = authenticationService.getAuthenticatedUser("filedownloader");

Expand Down
Loading

0 comments on commit f76ddc8

Please sign in to comment.