Merge pull request #10623 from DSpace/backport-10599-to-dspace-7_x

[Port dspace-7_x] Fix login endpoints not automatically closing their Context
This commit is contained in:
Tim Donohue
2025-04-23 11:12:18 -05:00
committed by GitHub
4 changed files with 81 additions and 40 deletions

View File

@@ -45,7 +45,7 @@ public class CustomLogoutHandler implements LogoutHandler {
try {
Context context = ContextUtil.obtainContext(httpServletRequest);
restAuthenticationService.invalidateAuthenticationData(httpServletRequest, httpServletResponse, context);
context.commit();
context.complete();
} catch (Exception e) {
log.error("Unable to logout", e);

View File

@@ -85,7 +85,7 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
} else {
// Otherwise, this is a new login & we need to attempt authentication
log.debug("Request to authenticate new login");
return authenticateNewLogin(authentication);
return authenticateNewLogin(context, authentication);
}
}
@@ -107,56 +107,44 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
* If login is successful, returns a NEW Authentication class containing the logged in EPerson and their list of
* GrantedAuthority objects. If login fails, a BadCredentialsException is thrown. If no valid login found implicit
* or explicit, then null is returned.
*
* @param context The current DSpace context
* @param authentication Authentication class to attempt authentication.
* @return new Authentication class containing logged-in user information or null
*/
private Authentication authenticateNewLogin(Authentication authentication) {
Context newContext = null;
private Authentication authenticateNewLogin(Context context, Authentication authentication) {
Authentication output = null;
if (authentication != null) {
try {
newContext = new Context();
String name = authentication.getName();
String password = Objects.toString(authentication.getCredentials(), null);
String name = authentication.getName();
String password = Objects.toString(authentication.getCredentials(), null);
int implicitStatus = authenticationService.authenticateImplicit(newContext, null, null, null, request);
int implicitStatus = authenticationService.authenticateImplicit(context, null, null, null, request);
if (implicitStatus == AuthenticationMethod.SUCCESS) {
log.info(LogHelper.getHeader(newContext, "login", "type=implicit"));
output = createAuthentication(newContext);
} else {
int authenticateResult = authenticationService
.authenticate(newContext, name, password, null, request);
if (AuthenticationMethod.SUCCESS == authenticateResult) {
if (implicitStatus == AuthenticationMethod.SUCCESS) {
log.info(LogHelper.getHeader(context, "login", "type=implicit"));
output = createAuthentication(context);
} else {
int authenticateResult = authenticationService.authenticate(context, name, password, null, request);
if (AuthenticationMethod.SUCCESS == authenticateResult) {
log.info(LogHelper
.getHeader(newContext, "login", "type=explicit"));
log.info(LogHelper.getHeader(context, "login", "type=explicit"));
output = createAuthentication(newContext);
output = createAuthentication(context);
for (PostLoggedInAction action : postLoggedInActions) {
try {
action.loggedIn(newContext);
} catch (Exception ex) {
log.error("An error occurs performing post logged in action", ex);
}
for (PostLoggedInAction action : postLoggedInActions) {
try {
action.loggedIn(context);
} catch (Exception ex) {
log.error("An error occurs performing post logged in action", ex);
}
}
} else {
log.info(LogHelper.getHeader(newContext, "failed_login", "email="
+ name + ", result="
+ authenticateResult));
throw new BadCredentialsException("Login failed");
}
}
} finally {
if (newContext != null && newContext.isValid()) {
try {
newContext.complete();
} catch (SQLException e) {
log.error(e.getMessage() + " occurred while trying to close", e);
}
} else {
log.info(LogHelper.getHeader(context, "failed_login", "email="
+ name + ", result="
+ authenticateResult));
throw new BadCredentialsException("Login failed");
}
}
}

View File

@@ -10,11 +10,18 @@ package org.dspace.app.rest.security;
import static org.dspace.authenticate.OidcAuthenticationBean.OIDC_AUTH_ATTRIBUTE;
import java.io.IOException;
import java.util.ArrayList;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.dspace.core.Utils;
import org.dspace.services.ConfigurationService;
import org.dspace.services.factory.DSpaceServicesFactory;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
@@ -27,6 +34,11 @@ import org.springframework.security.core.AuthenticationException;
public class OidcLoginFilter extends StatelessLoginFilter {
private static final Logger log = LogManager.getLogger(OidcLoginFilter.class);
private final ConfigurationService configurationService = DSpaceServicesFactory.getInstance()
.getConfigurationService();
public OidcLoginFilter(String url, AuthenticationManager authenticationManager,
RestAuthenticationService restAuthenticationService) {
super(url, authenticationManager, restAuthenticationService);
@@ -44,7 +56,45 @@ public class OidcLoginFilter extends StatelessLoginFilter {
protected void successfulAuthentication(HttpServletRequest req, HttpServletResponse res, FilterChain chain,
Authentication auth) throws IOException, ServletException {
restAuthenticationService.addAuthenticationDataForUser(req, res, (DSpaceAuthentication) auth, true);
chain.doFilter(req, res);
redirectAfterSuccess(req, res);
}
/**
* After successful login, redirect to the DSpace URL specified by this OIDC
* request (in the "redirectUrl" request parameter). If that 'redirectUrl' is
* not valid or trusted for this DSpace site, then return a 400 error.
* @param request
* @param response
* @throws IOException
*/
private void redirectAfterSuccess(HttpServletRequest request, HttpServletResponse response) throws IOException {
// Get redirect URL from request parameter
String redirectUrl = request.getParameter("redirectUrl");
// If redirectUrl unspecified, default to the configured UI
if (StringUtils.isEmpty(redirectUrl)) {
redirectUrl = configurationService.getProperty("dspace.ui.url");
}
// Validate that the redirectURL matches either the server or UI hostname. It
// *cannot* be an arbitrary URL.
String redirectHostName = Utils.getHostName(redirectUrl);
String serverHostName = Utils.getHostName(configurationService.getProperty("dspace.server.url"));
ArrayList<String> allowedHostNames = new ArrayList<>();
allowedHostNames.add(serverHostName);
String[] allowedUrls = configurationService.getArrayProperty("rest.cors.allowed-origins");
for (String url : allowedUrls) {
allowedHostNames.add(Utils.getHostName(url));
}
if (StringUtils.equalsAnyIgnoreCase(redirectHostName, allowedHostNames.toArray(new String[0]))) {
log.debug("OIDC redirecting to " + redirectUrl);
response.sendRedirect(redirectUrl);
} else {
log.error("Invalid OIDC redirectURL=" + redirectUrl + ". URL doesn't match hostname of server or UI!");
response.sendError(HttpServletResponse.SC_BAD_REQUEST,
"Invalid redirectURL! Must match server or ui hostname.");
}
}
}

View File

@@ -84,6 +84,9 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
String token = loginJWTTokenHandler.createTokenForEPerson(context, request,
authentication.getPreviousLoginDate());
context.commit();
// Close the Context, because the DSpaceRequestContextFilter is not called for requests that trigger
// the authentication filters (filters that extend AbstractAuthenticationProcessingFilter)
context.close();
// Add newly generated auth token to the response
addTokenToResponse(request, response, token, addCookie);