From d97e52ef501a72fcf16aee02d7e79c91d123dfe6 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Fri, 24 Aug 2012 13:32:44 -0400
Subject: [PATCH] Implemented custom request handling for (un)authenticated sessions to workaround Wicket bugs

---
 src/com/gitblit/wicket/AuthorizationStrategy.java    |    5 +-
 docs/04_releases.mkd                                 |    1 
 src/com/gitblit/wicket/GitBlitWebSession.java        |   54 ++++++++++++++++++++++++++
 src/com/gitblit/wicket/pages/BasePage.java           |   27 ++++++++-----
 src/com/gitblit/wicket/pages/ChangePasswordPage.java |    2 
 src/com/gitblit/wicket/pages/RootPage.java           |    2 
 src/com/gitblit/wicket/pages/RepositoryPage.java     |   10 ++++-
 7 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/docs/04_releases.mkd b/docs/04_releases.mkd
index 074396c..21b47ba 100644
--- a/docs/04_releases.mkd
+++ b/docs/04_releases.mkd
@@ -11,6 +11,7 @@
 
 #### fixes
 
+- Bypass Wicket's inability to handle direct url addressing of a view-restricted, grouped repository for new, unauthenticated sessions (e.g. click link from email or rss feed without having an active Wicket session)
 - Fixed MailExecutor's failure to cope with mail server connection troubles resulting in 100% CPU usage
 - Fixed generated urls in Groovy *sendmail* hook script for grouped repositories
 - Fixed generated urls in RSS feeds for grouped repositories
diff --git a/src/com/gitblit/wicket/AuthorizationStrategy.java b/src/com/gitblit/wicket/AuthorizationStrategy.java
index 452215a..16a4ec8 100644
--- a/src/com/gitblit/wicket/AuthorizationStrategy.java
+++ b/src/com/gitblit/wicket/AuthorizationStrategy.java
@@ -16,7 +16,7 @@
 package com.gitblit.wicket;
 
 import org.apache.wicket.Component;
-import org.apache.wicket.RestartResponseAtInterceptPageException;
+import org.apache.wicket.RestartResponseException;
 import org.apache.wicket.authorization.IUnauthorizedComponentInstantiationListener;
 import org.apache.wicket.authorization.strategies.page.AbstractPageAuthorizationStrategy;
 
@@ -49,6 +49,7 @@
 			GitBlitWebSession session = GitBlitWebSession.get();
 			if (authenticateView && !session.isLoggedIn()) {
 				// authentication required
+				session.cacheRequest(pageClass);
 				return false;
 			}
 
@@ -78,7 +79,7 @@
 	@Override
 	public void onUnauthorizedInstantiation(Component component) {
 		if (component instanceof BasePage) {
-			throw new RestartResponseAtInterceptPageException(RepositoriesPage.class);
+			throw new RestartResponseException(RepositoriesPage.class);
 		}
 	}
 }
diff --git a/src/com/gitblit/wicket/GitBlitWebSession.java b/src/com/gitblit/wicket/GitBlitWebSession.java
index 2238660..7ecc05b 100644
--- a/src/com/gitblit/wicket/GitBlitWebSession.java
+++ b/src/com/gitblit/wicket/GitBlitWebSession.java
@@ -15,10 +15,16 @@
  */
 package com.gitblit.wicket;
 
+import java.util.Map;
 import java.util.TimeZone;
 
+import org.apache.wicket.Page;
+import org.apache.wicket.PageParameters;
+import org.apache.wicket.RedirectToUrlException;
 import org.apache.wicket.Request;
 import org.apache.wicket.Session;
+import org.apache.wicket.protocol.http.RequestUtils;
+import org.apache.wicket.protocol.http.WebRequestCycle;
 import org.apache.wicket.protocol.http.WebSession;
 import org.apache.wicket.protocol.http.request.WebClientInfo;
 
@@ -33,7 +39,9 @@
 	private UserModel user;
 
 	private String errorMessage;
-
+	
+	private String requestUrl;
+	
 	public GitBlitWebSession(Request request) {
 		super(request);
 	}
@@ -41,6 +49,46 @@
 	public void invalidate() {
 		super.invalidate();
 		user = null;
+	}
+	
+	/**
+	 * Cache the requested protected resource pending successful authentication.
+	 * 
+	 * @param pageClass
+	 */
+	public void cacheRequest(Class<? extends Page> pageClass) {
+		// build absolute url with correctly encoded parameters?!
+		Request req = WebRequestCycle.get().getRequest();
+		Map<String, ?> params = req.getRequestParameters().getParameters();
+		PageParameters pageParams = new PageParameters(params);
+		String relativeUrl = WebRequestCycle.get().urlFor(pageClass, pageParams).toString();
+		requestUrl = RequestUtils.toAbsolutePath(relativeUrl);
+		if (isTemporary())
+		{
+			// we must bind the temporary session into the session store
+			// so that we can re-use this session for reporting an error message
+			// on the redirected page and continuing the request after
+			// authentication.
+			bind();
+		}
+	}
+	
+	/**
+	 * Continue any cached request.  This is used when a request for a protected
+	 * resource is aborted/redirected pending proper authentication.  Gitblit
+	 * no longer uses Wicket's built-in mechanism for this because of Wicket's
+	 * failure to properly handle parameters with forward-slashes.  This is a
+	 * constant source of headaches with Wicket.
+	 *  
+	 * @return false if there is no cached request to process
+	 */
+	public boolean continueRequest() {
+		if (requestUrl != null) {
+			String url = requestUrl;
+			requestUrl = null;
+			throw new RedirectToUrlException(url);
+		}
+		return false;
 	}
 
 	public boolean isLoggedIn() {
@@ -53,6 +101,10 @@
 		}
 		return user.canAdmin;
 	}
+	
+	public String getUsername() {
+		return user == null ? "anonymous" : user.username;
+	}
 
 	public UserModel getUser() {
 		return user;
diff --git a/src/com/gitblit/wicket/pages/BasePage.java b/src/com/gitblit/wicket/pages/BasePage.java
index 82862ae..234c2a9 100644
--- a/src/com/gitblit/wicket/pages/BasePage.java
+++ b/src/com/gitblit/wicket/pages/BasePage.java
@@ -26,7 +26,7 @@
 import org.apache.wicket.Application;
 import org.apache.wicket.MarkupContainer;
 import org.apache.wicket.PageParameters;
-import org.apache.wicket.RestartResponseAtInterceptPageException;
+import org.apache.wicket.RedirectToUrlException;
 import org.apache.wicket.RestartResponseException;
 import org.apache.wicket.markup.html.CSSPackageResource;
 import org.apache.wicket.markup.html.WebPage;
@@ -35,9 +35,11 @@
 import org.apache.wicket.markup.html.link.ExternalLink;
 import org.apache.wicket.markup.html.panel.FeedbackPanel;
 import org.apache.wicket.markup.html.panel.Fragment;
+import org.apache.wicket.protocol.http.RequestUtils;
 import org.apache.wicket.protocol.http.WebRequest;
 import org.apache.wicket.protocol.http.WebResponse;
 import org.apache.wicket.protocol.http.servlet.ServletWebRequest;
+import org.apache.wicket.request.RequestParameters;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -137,7 +139,8 @@
 			// Set Cookie
 			WebResponse response = (WebResponse) getRequestCycle().getResponse();
 			GitBlit.self().setCookie(response, user);
-			continueToOriginalDestination();
+			
+			session.continueRequest();
 		}
 	}
 
@@ -229,7 +232,7 @@
 		// inject username into repository url if authentication is required
 		if (repository.accessRestriction.exceeds(AccessRestrictionType.NONE)
 				&& GitBlitWebSession.get().isLoggedIn()) {
-			String username = GitBlitWebSession.get().getUser().username;
+			String username = GitBlitWebSession.get().getUsername();
 			sb.insert(sb.indexOf("://") + 3, username + "@");
 		}
 		return sb.toString();
@@ -240,10 +243,13 @@
 	}
 
 	public void error(String message, boolean redirect) {
-		logger.error(message);
+		logger.error(message  + " for " + GitBlitWebSession.get().getUsername());
 		if (redirect) {
 			GitBlitWebSession.get().cacheErrorMessage(message);
-			throw new RestartResponseException(getApplication().getHomePage());
+			RequestParameters params = getRequest().getRequestParameters();
+			String relativeUrl = urlFor(RepositoriesPage.class, null).toString();
+			String absoluteUrl = RequestUtils.toAbsolutePath(relativeUrl);
+			throw new RedirectToUrlException(absoluteUrl);
 		} else {
 			super.error(message);
 		}
@@ -260,12 +266,13 @@
 	}
 
 	public void authenticationError(String message) {
-		logger.error(message);
-		if (GitBlitWebSession.get().isLoggedIn()) {
-			error(message, true);
-		} else {
-			throw new RestartResponseAtInterceptPageException(RepositoriesPage.class);
+		logger.error(getRequest().getURL() + " for " + GitBlitWebSession.get().getUsername());
+		if (!GitBlitWebSession.get().isLoggedIn()) {
+			// cache the request if we have not authenticated.
+			// the request will continue after authentication.
+			GitBlitWebSession.get().cacheRequest(getClass());
 		}
+		error(message, true);
 	}
 
 	/**
diff --git a/src/com/gitblit/wicket/pages/ChangePasswordPage.java b/src/com/gitblit/wicket/pages/ChangePasswordPage.java
index cbe732f..5e66300 100644
--- a/src/com/gitblit/wicket/pages/ChangePasswordPage.java
+++ b/src/com/gitblit/wicket/pages/ChangePasswordPage.java
@@ -56,7 +56,7 @@
 					GitBlit.getString(Keys.realm.userService, "users.conf")), true);
 		}
 		
-		setupPage(getString("gb.changePassword"), GitBlitWebSession.get().getUser().username);
+		setupPage(getString("gb.changePassword"), GitBlitWebSession.get().getUsername());
 
 		StatelessForm<Void> form = new StatelessForm<Void>("passwordForm") {
 
diff --git a/src/com/gitblit/wicket/pages/RepositoryPage.java b/src/com/gitblit/wicket/pages/RepositoryPage.java
index 6d33a14..19a5de2 100644
--- a/src/com/gitblit/wicket/pages/RepositoryPage.java
+++ b/src/com/gitblit/wicket/pages/RepositoryPage.java
@@ -151,7 +151,7 @@
 		if (showAdmin
 				|| GitBlitWebSession.get().isLoggedIn()
 				&& (model.owner != null && model.owner.equalsIgnoreCase(GitBlitWebSession.get()
-						.getUser().username))) {
+						.getUsername()))) {
 			pages.put("edit", new PageRegistration("gb.edit", EditRepositoryPage.class, params));
 		}
 		return pages;
@@ -198,7 +198,13 @@
 			RepositoryModel model = GitBlit.self().getRepositoryModel(
 					GitBlitWebSession.get().getUser(), repositoryName);
 			if (model == null) {
-				authenticationError(getString("gb.unauthorizedAccessForRepository") + " " + repositoryName);
+				if (GitBlit.self().hasRepository(repositoryName)) {
+					// has repository, but unauthorized
+					authenticationError(getString("gb.unauthorizedAccessForRepository") + " " + repositoryName);
+				} else {
+					// does not have repository
+					error(getString("gb.canNotLoadRepository") + " " + repositoryName, true);
+				}
 				return null;
 			}
 			m = model;
diff --git a/src/com/gitblit/wicket/pages/RootPage.java b/src/com/gitblit/wicket/pages/RootPage.java
index eaa2542..40f7aec 100644
--- a/src/com/gitblit/wicket/pages/RootPage.java
+++ b/src/com/gitblit/wicket/pages/RootPage.java
@@ -210,7 +210,7 @@
 				GitBlit.self().setCookie(response, user);
 			}
 
-			if (!continueToOriginalDestination()) {
+			if (!session.continueRequest()) {
 				PageParameters params = getPageParameters();
 				if (params == null) {
 					// redirect to this page

--
Gitblit v1.9.1