From: James Moger Date: Fri, 24 Aug 2012 17:32:44 +0000 (-0400) Subject: Implemented custom request handling for (un)authenticated sessions to workaround... X-Git-Tag: v1.1.0~2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d97e52ef501a72fcf16aee02d7e79c91d123dfe6;p=gitblit.git Implemented custom request handling for (un)authenticated sessions to workaround Wicket bugs --- diff --git a/docs/04_releases.mkd b/docs/04_releases.mkd index 074396c7..21b47bab 100644 --- a/docs/04_releases.mkd +++ b/docs/04_releases.mkd @@ -11,6 +11,7 @@ If you are updating from an earlier release AND you have indexed branches with t #### 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 452215a7..16a4ec80 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 @@ public class AuthorizationStrategy extends AbstractPageAuthorizationStrategy imp GitBlitWebSession session = GitBlitWebSession.get(); if (authenticateView && !session.isLoggedIn()) { // authentication required + session.cacheRequest(pageClass); return false; } @@ -78,7 +79,7 @@ public class AuthorizationStrategy extends AbstractPageAuthorizationStrategy imp @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 2238660f..7ecc05b2 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 @@ public final class GitBlitWebSession extends WebSession { private UserModel user; private String errorMessage; - + + private String requestUrl; + public GitBlitWebSession(Request request) { super(request); } @@ -42,6 +50,46 @@ public final class GitBlitWebSession extends WebSession { super.invalidate(); user = null; } + + /** + * Cache the requested protected resource pending successful authentication. + * + * @param pageClass + */ + public void cacheRequest(Class pageClass) { + // build absolute url with correctly encoded parameters?! + Request req = WebRequestCycle.get().getRequest(); + Map 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() { return user != null; @@ -53,6 +101,10 @@ public final class GitBlitWebSession extends WebSession { } 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 82862ae9..234c2a94 100644 --- a/src/com/gitblit/wicket/pages/BasePage.java +++ b/src/com/gitblit/wicket/pages/BasePage.java @@ -26,7 +26,7 @@ import javax.servlet.http.HttpServletRequest; 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.BookmarkablePageLink; 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 @@ public abstract class BasePage extends WebPage { // Set Cookie WebResponse response = (WebResponse) getRequestCycle().getResponse(); GitBlit.self().setCookie(response, user); - continueToOriginalDestination(); + + session.continueRequest(); } } @@ -229,7 +232,7 @@ public abstract class BasePage extends WebPage { // 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 abstract class BasePage extends WebPage { } 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 abstract class BasePage extends WebPage { } 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 cbe732ff..5e663006 100644 --- a/src/com/gitblit/wicket/pages/ChangePasswordPage.java +++ b/src/com/gitblit/wicket/pages/ChangePasswordPage.java @@ -56,7 +56,7 @@ public class ChangePasswordPage extends RootSubPage { GitBlit.getString(Keys.realm.userService, "users.conf")), true); } - setupPage(getString("gb.changePassword"), GitBlitWebSession.get().getUser().username); + setupPage(getString("gb.changePassword"), GitBlitWebSession.get().getUsername()); StatelessForm form = new StatelessForm("passwordForm") { diff --git a/src/com/gitblit/wicket/pages/RepositoryPage.java b/src/com/gitblit/wicket/pages/RepositoryPage.java index 6d33a147..19a5de2c 100644 --- a/src/com/gitblit/wicket/pages/RepositoryPage.java +++ b/src/com/gitblit/wicket/pages/RepositoryPage.java @@ -151,7 +151,7 @@ public abstract class RepositoryPage extends BasePage { 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 @@ public abstract class RepositoryPage extends BasePage { 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 eaa25425..40f7aec4 100644 --- a/src/com/gitblit/wicket/pages/RootPage.java +++ b/src/com/gitblit/wicket/pages/RootPage.java @@ -210,7 +210,7 @@ public abstract class RootPage extends BasePage { GitBlit.self().setCookie(response, user); } - if (!continueToOriginalDestination()) { + if (!session.continueRequest()) { PageParameters params = getPageParameters(); if (params == null) { // redirect to this page