From ec7ed84b04cd3981ae01b104bd52fc010f31e6a7 Mon Sep 17 00:00:00 2001 From: James Moger Date: Thu, 25 Sep 2014 09:06:39 -0400 Subject: [PATCH] Restrict Gitblit cookie to the context path --- .../manager/AuthenticationManager.java | 37 ++++++++++++++++++- .../com/gitblit/manager/GitblitManager.java | 12 ++++++ .../manager/IAuthenticationManager.java | 22 +++++++++++ .../wicket/pages/ChangePasswordPage.java | 5 ++- .../com/gitblit/wicket/pages/LogoutPage.java | 3 +- .../com/gitblit/wicket/pages/RootPage.java | 5 ++- .../com/gitblit/wicket/pages/SessionPage.java | 10 +++-- 7 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java index d1b1af0a..bc1857bc 100644 --- a/src/main/java/com/gitblit/manager/AuthenticationManager.java +++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java @@ -454,7 +454,20 @@ public class AuthenticationManager implements IAuthenticationManager { * @param user */ @Override + @Deprecated public void setCookie(HttpServletResponse response, UserModel user) { + setCookie(null, response, user); + } + + /** + * Sets a cookie for the specified user. + * + * @param request + * @param response + * @param user + */ + @Override + public void setCookie(HttpServletRequest request, HttpServletResponse response, UserModel user) { if (settings.getBoolean(Keys.web.allowCookieAuthentication, true)) { GitBlitWebSession session = GitBlitWebSession.get(); boolean standardLogin = session.authenticationType.isStandard(); @@ -477,7 +490,13 @@ public class AuthenticationManager implements IAuthenticationManager { userCookie.setMaxAge((int) TimeUnit.DAYS.toSeconds(7)); } } - userCookie.setPath("/"); + String path = "/"; + if (request != null) { + if (!StringUtils.isEmpty(request.getContextPath())) { + path = request.getContextPath(); + } + } + userCookie.setPath(path); response.addCookie(userCookie); } } @@ -486,11 +505,25 @@ public class AuthenticationManager implements IAuthenticationManager { /** * Logout a user. * + * @param response * @param user */ @Override + @Deprecated public void logout(HttpServletResponse response, UserModel user) { - setCookie(response, null); + setCookie(null, response, null); + } + + /** + * Logout a user. + * + * @param request + * @param response + * @param user + */ + @Override + public void logout(HttpServletRequest request, HttpServletResponse response, UserModel user) { + setCookie(request, response, null); } /** diff --git a/src/main/java/com/gitblit/manager/GitblitManager.java b/src/main/java/com/gitblit/manager/GitblitManager.java index 08853a66..88fa804e 100644 --- a/src/main/java/com/gitblit/manager/GitblitManager.java +++ b/src/main/java/com/gitblit/manager/GitblitManager.java @@ -736,15 +736,27 @@ public class GitblitManager implements IGitblit { } @Override + @Deprecated public void setCookie(HttpServletResponse response, UserModel user) { authenticationManager.setCookie(response, user); } @Override + public void setCookie(HttpServletRequest request, HttpServletResponse response, UserModel user) { + authenticationManager.setCookie(request, response, user); + } + + @Override + @Deprecated public void logout(HttpServletResponse response, UserModel user) { authenticationManager.logout(response, user); } + @Override + public void logout(HttpServletRequest request, HttpServletResponse response, UserModel user) { + authenticationManager.logout(request, response, user); + } + @Override public boolean supportsCredentialChanges(UserModel user) { return authenticationManager.supportsCredentialChanges(user); diff --git a/src/main/java/com/gitblit/manager/IAuthenticationManager.java b/src/main/java/com/gitblit/manager/IAuthenticationManager.java index 2665b439..3600b325 100644 --- a/src/main/java/com/gitblit/manager/IAuthenticationManager.java +++ b/src/main/java/com/gitblit/manager/IAuthenticationManager.java @@ -85,16 +85,38 @@ public interface IAuthenticationManager extends IManager { * @param user * @since 1.4.0 */ + @Deprecated void setCookie(HttpServletResponse response, UserModel user); + /** + * Sets a cookie for the specified user. + * + * @param request + * @param response + * @param user + * @since 1.6.1 + */ + void setCookie(HttpServletRequest request, HttpServletResponse response, UserModel user); + /** * Logout a user. * * @param user * @since 1.4.0 */ + @Deprecated void logout(HttpServletResponse response, UserModel user); + /** + * Logout a user. + * + * @param request + * @param response + * @param user + * @since 1.6.1 + */ + void logout(HttpServletRequest request, HttpServletResponse response, UserModel user); + /** * Does the user service support changes to credentials? * diff --git a/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java b/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java index 4c8d3a1d..a6aca22d 100644 --- a/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java +++ b/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java @@ -23,6 +23,7 @@ import org.apache.wicket.markup.html.form.PasswordTextField; import org.apache.wicket.markup.html.form.StatelessForm; import org.apache.wicket.model.IModel; import org.apache.wicket.model.Model; +import org.apache.wicket.protocol.http.WebRequest; import org.apache.wicket.protocol.http.WebResponse; import com.gitblit.GitBlitException; @@ -99,8 +100,10 @@ public class ChangePasswordPage extends RootSubPage { try { app().gitblit().reviseUser(user.username, user); if (app().settings().getBoolean(Keys.web.allowCookieAuthentication, false)) { + WebRequest request = (WebRequest) getRequestCycle().getRequest(); WebResponse response = (WebResponse) getRequestCycle().getResponse(); - app().authentication().setCookie(response.getHttpServletResponse(), user); + app().authentication().setCookie(request.getHttpServletRequest(), + response.getHttpServletResponse(), user); } } catch (GitBlitException e) { error(e.getMessage()); diff --git a/src/main/java/com/gitblit/wicket/pages/LogoutPage.java b/src/main/java/com/gitblit/wicket/pages/LogoutPage.java index 27542bd0..a8ae7d0f 100644 --- a/src/main/java/com/gitblit/wicket/pages/LogoutPage.java +++ b/src/main/java/com/gitblit/wicket/pages/LogoutPage.java @@ -27,7 +27,8 @@ public class LogoutPage extends BasePage { super(); GitBlitWebSession session = GitBlitWebSession.get(); UserModel user = session.getUser(); - app().authentication().logout(((WebResponse) getResponse()).getHttpServletResponse(), user); + app().authentication().logout(((WebRequest) getRequest()).getHttpServletRequest(), + ((WebResponse) getResponse()).getHttpServletResponse(), user); session.invalidate(); /* diff --git a/src/main/java/com/gitblit/wicket/pages/RootPage.java b/src/main/java/com/gitblit/wicket/pages/RootPage.java index b1c3639d..6a933b76 100644 --- a/src/main/java/com/gitblit/wicket/pages/RootPage.java +++ b/src/main/java/com/gitblit/wicket/pages/RootPage.java @@ -46,6 +46,7 @@ import org.apache.wicket.markup.repeater.data.DataView; import org.apache.wicket.markup.repeater.data.ListDataProvider; import org.apache.wicket.model.IModel; import org.apache.wicket.model.Model; +import org.apache.wicket.protocol.http.WebRequest; import org.apache.wicket.protocol.http.WebResponse; import com.gitblit.Constants; @@ -269,8 +270,10 @@ public abstract class RootPage extends BasePage { // Set Cookie if (app().settings().getBoolean(Keys.web.allowCookieAuthentication, false)) { + WebRequest request = (WebRequest) getRequestCycle().getRequest(); WebResponse response = (WebResponse) getRequestCycle().getResponse(); - app().authentication().setCookie(response.getHttpServletResponse(), user); + app().authentication().setCookie(request.getHttpServletRequest(), + response.getHttpServletResponse(), user); } if (!session.continueRequest()) { diff --git a/src/main/java/com/gitblit/wicket/pages/SessionPage.java b/src/main/java/com/gitblit/wicket/pages/SessionPage.java index 7a58175f..7717854b 100644 --- a/src/main/java/com/gitblit/wicket/pages/SessionPage.java +++ b/src/main/java/com/gitblit/wicket/pages/SessionPage.java @@ -58,9 +58,11 @@ public abstract class SessionPage extends WebPage { if (user == null || user.disabled) { // user was deleted/disabled during session + HttpServletRequest request = ((WebRequest) getRequestCycle().getRequest()) + .getHttpServletRequest(); HttpServletResponse response = ((WebResponse) getRequestCycle().getResponse()) .getHttpServletResponse(); - app().authentication().logout(response, user); + app().authentication().logout(request, response, user); session.setUser(null); session.invalidateNow(); return; @@ -76,7 +78,7 @@ public abstract class SessionPage extends WebPage { // cookie was changed during our session HttpServletResponse response = ((WebResponse) getRequestCycle().getResponse()) .getHttpServletResponse(); - app().authentication().logout(response, user); + app().authentication().logout(request, response, user); session.setUser(null); session.invalidateNow(); return; @@ -99,8 +101,10 @@ public abstract class SessionPage extends WebPage { session.setUser(user); // Set Cookie + WebRequest request = (WebRequest) getRequestCycle().getRequest(); WebResponse response = (WebResponse) getRequestCycle().getResponse(); - app().authentication().setCookie(response.getHttpServletResponse(), user); + app().authentication().setCookie(request.getHttpServletRequest(), + response.getHttpServletResponse(), user); session.continueRequest(); } -- 2.39.5