prevent session fixation for external authentication

  + use request instead of session to flag authentication status
    and user, for external authentication types

1 files changed, 25 insertions, 10 deletions

diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java
index 38e45a6f..51aa2213 100644
--- a/src/main/java/com/gitblit/manager/AuthenticationManager.java
+++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java
@@ -194,6 +194,14 @@ public class AuthenticationManager implements IAuthenticationManager {
 	 */
 	@Override
 	public UserModel authenticate(HttpServletRequest httpRequest, boolean requiresCertificate) {
+
+		// Check if this request has already been authenticated, and trust that instead of re-processing
+		String reqAuthUser = (String) httpRequest.getAttribute(Constants.ATTRIB_AUTHUSER);
+		if (!StringUtils.isEmpty(reqAuthUser)) {
+			logger.warn("Called servlet authenticate when request is already authenticated.");
+			return userManager.getUserModel(reqAuthUser);
+		}
+
 		// try to authenticate by servlet container principal
 		if (!requiresCertificate) {
 			Principal principal = httpRequest.getUserPrincipal();
@@ -204,7 +212,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 					UserModel user = userManager.getUserModel(username);
 					if (user != null) {
 						// existing user
-						flagSession(httpRequest, AuthenticationType.CONTAINER);
+						flagRequest(httpRequest, AuthenticationType.CONTAINER, user.username);
 						logger.debug(MessageFormat.format("{0} authenticated by servlet container principal from {1}",
 								user.username, httpRequest.getRemoteAddr()));
 						return validateAuthentication(user, AuthenticationType.CONTAINER);
@@ -239,7 +247,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 						}
 						
 						userManager.updateUserModel(user);
-						flagSession(httpRequest, AuthenticationType.CONTAINER);
+						flagRequest(httpRequest, AuthenticationType.CONTAINER, user.username);
 						logger.debug(MessageFormat.format("{0} authenticated and created by servlet container principal from {1}",
 								user.username, httpRequest.getRemoteAddr()));
 						return validateAuthentication(user, AuthenticationType.CONTAINER);
@@ -260,7 +268,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 			UserModel user = userManager.getUserModel(model.username);
 			X509Metadata metadata = HttpUtils.getCertificateMetadata(httpRequest);
 			if (user != null) {
-				flagSession(httpRequest, AuthenticationType.CERTIFICATE);
+				flagRequest(httpRequest, AuthenticationType.CERTIFICATE, user.username);
 				logger.debug(MessageFormat.format("{0} authenticated by client certificate {1} from {2}",
 						user.username, metadata.serialNumber, httpRequest.getRemoteAddr()));
 				return validateAuthentication(user, AuthenticationType.CERTIFICATE);
@@ -282,7 +290,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 		if (!StringUtils.isEmpty(cookie)) {
 			user = userManager.getUserModel(cookie.toCharArray());
 			if (user != null) {
-				flagSession(httpRequest, AuthenticationType.COOKIE);
+				flagRequest(httpRequest, AuthenticationType.COOKIE, user.username);
 				logger.debug(MessageFormat.format("{0} authenticated by cookie from {1}",
 					user.username, httpRequest.getRemoteAddr()));
 				return validateAuthentication(user, AuthenticationType.COOKIE);
@@ -304,7 +312,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 				char[] password = values[1].toCharArray();
 				user = authenticate(username, password);
 				if (user != null) {
-					flagSession(httpRequest, AuthenticationType.CREDENTIALS);
+					flagRequest(httpRequest, AuthenticationType.CREDENTIALS, user.username);
 					logger.debug(MessageFormat.format("{0} authenticated by BASIC request header from {1}",
 							user.username, httpRequest.getRemoteAddr()));
 					return validateAuthentication(user, AuthenticationType.CREDENTIALS);
@@ -423,8 +431,9 @@ public class AuthenticationManager implements IAuthenticationManager {
 		return user;
 	}
 
-	protected void flagSession(HttpServletRequest httpRequest, AuthenticationType authenticationType) {
-		httpRequest.getSession().setAttribute(Constants.AUTHENTICATION_TYPE, authenticationType);
+	protected void flagRequest(HttpServletRequest httpRequest, AuthenticationType authenticationType, String authedUsername) {
+		httpRequest.setAttribute(Constants.ATTRIB_AUTHUSER,  authedUsername);
+		httpRequest.setAttribute(Constants.ATTRIB_AUTHTYPE,  authenticationType);
 	}
 
 	/**
@@ -545,9 +554,15 @@ public class AuthenticationManager implements IAuthenticationManager {
 	@Override
 	public void setCookie(HttpServletRequest request, HttpServletResponse response, UserModel user) {
 		if (settings.getBoolean(Keys.web.allowCookieAuthentication, true)) {
-			HttpSession session = request.getSession();
-			AuthenticationType authenticationType = (AuthenticationType) session.getAttribute(Constants.AUTHENTICATION_TYPE);
-			boolean standardLogin = authenticationType.isStandard();
+			boolean standardLogin = true;
+
+			if (null != request) {
+				// Pull the auth type from the request, it is set there if container managed
+				AuthenticationType authenticationType = (AuthenticationType) request.getAttribute(Constants.ATTRIB_AUTHTYPE);
+
+				if (null != authenticationType)
+					standardLogin = authenticationType.isStandard();
+			}
 
 			if (standardLogin) {
 				Cookie userCookie;