issue-361: Reset user cookie after administrative password change

Cookies were not reset on administrative password change of a user
account. This allowed accounts with changed passwords to continue
authenticating.  Cookies are now reset on password changes, they are
validated on each page request, AND they will now expire 7 days after
generation.

7 files changed, 77 insertions, 26 deletions

diff --git a/src/main/java/com/gitblit/ConfigUserService.java b/src/main/java/com/gitblit/ConfigUserService.java
index 19e4736a..e8652252 100644
--- a/src/main/java/com/gitblit/ConfigUserService.java
+++ b/src/main/java/com/gitblit/ConfigUserService.java
@@ -272,6 +272,9 @@ public class ConfigUserService implements IUserService {
 			}

 			read();

 			originalUser = users.remove(username.toLowerCase());

+			if (originalUser != null) {

+				cookies.remove(originalUser.cookie);

+			}

 			users.put(model.username.toLowerCase(), model);

 			// null check on "final" teams because JSON-sourced UserModel

 			// can have a null teams object

diff --git a/src/main/java/com/gitblit/client/EditUserDialog.java b/src/main/java/com/gitblit/client/EditUserDialog.java
index 2936a29d..ab3ea67e 100644
--- a/src/main/java/com/gitblit/client/EditUserDialog.java
+++ b/src/main/java/com/gitblit/client/EditUserDialog.java
@@ -325,6 +325,9 @@ public class EditUserDialog extends JDialog {
 				return false;

 			}

 

+			// change the cookie

+			user.cookie = StringUtils.getSHA1(user.username + password);

+

 			String type = settings.get(Keys.realm.passwordStorage).getString("md5");

 			if (type.equalsIgnoreCase("md5")) {

 				// store MD5 digest of password

diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java
index eef675b2..cd4a258f 100644
--- a/src/main/java/com/gitblit/manager/AuthenticationManager.java
+++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java
@@ -22,6 +22,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
@@ -235,13 +236,18 @@ public class AuthenticationManager implements IAuthenticationManager {
 			return null;
 		}
 
+		UserModel user = null;
+
 		// try to authenticate by cookie
-		UserModel user = authenticate(httpRequest.getCookies());
-		if (user != null) {
-			flagWicketSession(AuthenticationType.COOKIE);
-			logger.debug(MessageFormat.format("{0} authenticated by cookie from {1}",
+		String cookie = getCookie(httpRequest);
+		if (!StringUtils.isEmpty(cookie)) {
+			user = userManager.getUserModel(cookie.toCharArray());
+			if (user != null) {
+				flagWicketSession(AuthenticationType.COOKIE);
+				logger.debug(MessageFormat.format("{0} authenticated by cookie from {1}",
 					user.username, httpRequest.getRemoteAddr()));
-			return user;
+				return user;
+			}
 		}
 
 		// try to authenticate by BASIC
@@ -272,26 +278,6 @@ public class AuthenticationManager implements IAuthenticationManager {
 		return null;
 	}
 
-	/**
-	 * Authenticate a user based on their cookie.
-	 *
-	 * @param cookies
-	 * @return a user object or null
-	 */
-	protected UserModel authenticate(Cookie[] cookies) {
-		if (settings.getBoolean(Keys.web.allowCookieAuthentication, true)) {
-			if (cookies != null && cookies.length > 0) {
-				for (Cookie cookie : cookies) {
-					if (cookie.getName().equals(Constants.NAME)) {
-						String value = cookie.getValue();
-						return userManager.getUserModel(value.toCharArray());
-					}
-				}
-			}
-		}
-		return null;
-	}
-
 	protected void flagWicketSession(AuthenticationType authenticationType) {
 		RequestCycle requestCycle = RequestCycle.get();
 		if (requestCycle != null) {
@@ -365,6 +351,28 @@ public class AuthenticationManager implements IAuthenticationManager {
 	}
 
 	/**
+	 * Returns the Gitlbit cookie in the request.
+	 *
+	 * @param request
+	 * @return the Gitblit cookie for the request or null if not found
+	 */
+	@Override
+	public String getCookie(HttpServletRequest request) {
+		if (settings.getBoolean(Keys.web.allowCookieAuthentication, true)) {
+			Cookie[] cookies = request.getCookies();
+			if (cookies != null && cookies.length > 0) {
+				for (Cookie cookie : cookies) {
+					if (cookie.getName().equals(Constants.NAME)) {
+						String value = cookie.getValue();
+						return value;
+					}
+				}
+			}
+		}
+		return null;
+	}
+
+	/**
 	 * Sets a cookie for the specified user.
 	 *
 	 * @param response
@@ -390,7 +398,8 @@ public class AuthenticationManager implements IAuthenticationManager {
 					} else {
 						// create real cookie
 						userCookie = new Cookie(Constants.NAME, cookie);
-						userCookie.setMaxAge(Integer.MAX_VALUE);
+						// expire the cookie in 7 days
+						userCookie.setMaxAge((int) TimeUnit.DAYS.toSeconds(7));
 					}
 				}
 				userCookie.setPath("/");
diff --git a/src/main/java/com/gitblit/manager/GitblitManager.java b/src/main/java/com/gitblit/manager/GitblitManager.java
index 7e788361..95d50ac1 100644
--- a/src/main/java/com/gitblit/manager/GitblitManager.java
+++ b/src/main/java/com/gitblit/manager/GitblitManager.java
@@ -634,6 +634,11 @@ public class GitblitManager implements IGitblit {
 	}
 
 	@Override
+	public String getCookie(HttpServletRequest request) {
+		return authenticationManager.getCookie(request);
+	}
+
+	@Override
 	public void setCookie(HttpServletResponse response, UserModel user) {
 		authenticationManager.setCookie(response, user);
 	}
diff --git a/src/main/java/com/gitblit/manager/IAuthenticationManager.java b/src/main/java/com/gitblit/manager/IAuthenticationManager.java
index 093f44d9..3007a303 100644
--- a/src/main/java/com/gitblit/manager/IAuthenticationManager.java
+++ b/src/main/java/com/gitblit/manager/IAuthenticationManager.java
@@ -56,6 +56,14 @@ public interface IAuthenticationManager extends IManager {
 	UserModel authenticate(String username, char[] password);
 
 	/**
+	 * Returns the Gitlbit cookie in the request.
+	 *
+	 * @param request
+	 * @return the Gitblit cookie for the request or null if not found
+	 */
+	String getCookie(HttpServletRequest request);
+
+	/**
 	 * Sets a cookie for the specified user.
 	 *
 	 * @param response
diff --git a/src/main/java/com/gitblit/wicket/pages/EditUserPage.java b/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
index 62a8ea5a..15c35fa6 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
@@ -154,6 +154,9 @@ public class EditUserPage extends RootSubPage {
 							return;

 						}

 

+						// change the cookie

+						userModel.cookie = StringUtils.getSHA1(userModel.username + password);

+

 						// Optionally store the password MD5 digest.

 						String type = app().settings().getString(Keys.realm.passwordStorage, "md5");

 						if (type.equalsIgnoreCase("md5")) {

diff --git a/src/main/java/com/gitblit/wicket/pages/SessionPage.java b/src/main/java/com/gitblit/wicket/pages/SessionPage.java
index d2fcfa0d..22ae6e2e 100644
--- a/src/main/java/com/gitblit/wicket/pages/SessionPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/SessionPage.java
@@ -16,6 +16,7 @@
 package com.gitblit.wicket.pages;

 

 import javax.servlet.http.HttpServletRequest;

+import javax.servlet.http.HttpServletResponse;

 

 import org.apache.wicket.PageParameters;

 import org.apache.wicket.markup.html.WebPage;

@@ -24,6 +25,7 @@ import org.apache.wicket.protocol.http.WebResponse;
 

 import com.gitblit.Keys;

 import com.gitblit.models.UserModel;

+import com.gitblit.utils.StringUtils;

 import com.gitblit.wicket.GitBlitWebApp;

 import com.gitblit.wicket.GitBlitWebSession;

 

@@ -53,6 +55,24 @@ public abstract class SessionPage extends WebPage {
 			// already have a session, refresh usermodel to pick up

 			// any changes to permissions or roles (issue-186)

 			UserModel user = app().users().getUserModel(session.getUser().username);

+

+			// validate cookie during session (issue-361)

+			if (app().settings().getBoolean(Keys.web.allowCookieAuthentication, true)) {

+				HttpServletRequest request = ((WebRequest) getRequestCycle().getRequest())

+						.getHttpServletRequest();

+				String requestCookie = app().authentication().getCookie(request);

+				if (!StringUtils.isEmpty(requestCookie) && !StringUtils.isEmpty(user.cookie)) {

+					if (!requestCookie.equals(user.cookie)) {

+						// cookie was changed during our session

+						HttpServletResponse response = ((WebResponse) getRequestCycle().getResponse())

+								.getHttpServletResponse();

+						app().authentication().logout(response, user);

+						session.setUser(null);

+						session.invalidateNow();

+						return;

+					}

+				}

+			}

 			session.setUser(user);

 			return;

 		}