Protect DownloadZipServlet with an AccessRestrictionFilter.

6 files changed, 118 insertions, 36 deletions

diff --git a/docs/00_index.mkd b/docs/00_index.mkd
index 5cf81735..0da7f742 100644
--- a/docs/00_index.mkd
+++ b/docs/00_index.mkd
@@ -42,7 +42,8 @@ Gitblit requires a Java 6 Runtime Environment (JRE) or a Java 6 Development Kit
 - updated: MarkdownPapers 1.1.1

 - updated: Wicket 1.4.18

 - updated: JGit 1.1.0

-- fixed: syndication urls for WAR builds

+- fixed: syndication urls for WAR deployments

+- fixed: authentication for zip downloads

 

 issues, binaries, and sources @ [Google Code][googlecode]<br/>

 sources @ [Github][gitbltsrc]

diff --git a/docs/04_releases.mkd b/docs/04_releases.mkd
index 242af419..b27371d2 100644
--- a/docs/04_releases.mkd
+++ b/docs/04_releases.mkd
@@ -17,7 +17,8 @@
 - updated: MarkdownPapers 1.1.1

 - updated: Wicket 1.4.18

 - updated: JGit 1.1.0

-- fixed: syndication urls for WAR builds

+- fixed: syndication urls for WAR deployments

+- fixed: authentication for zip downloads

 

 ### Older Releases

 

diff --git a/src/WEB-INF/web.xml b/src/WEB-INF/web.xml
index c5adadd9..d5577259 100644
--- a/src/WEB-INF/web.xml
+++ b/src/WEB-INF/web.xml
@@ -55,6 +55,20 @@
 		<url-pattern>/zip/*</url-pattern>

 	</servlet-mapping>

 	

+	

+	<!-- Federation Servlet

+		 <url-pattern> MUST match: 

+		 	* com.gitblit.Constants.FEDERATION_PATH		 

+			* Wicket Filter ignorePaths parameter -->

+	<servlet>

+		<servlet-name>FederationServlet</servlet-name>

+		<servlet-class>com.gitblit.FederationServlet</servlet-class>		

+	</servlet>

+	<servlet-mapping>

+		<servlet-name>FederationServlet</servlet-name>

+		<url-pattern>/federation/*</url-pattern>

+	</servlet-mapping>	

+	

 

 	<!-- Git Access Restriction Filter

 		 <url-pattern> MUST match: 

@@ -85,19 +99,20 @@
 		<url-pattern>/feed/*</url-pattern>

 	</filter-mapping>

 	

-	<!-- Federation Servlet

+	

+	<!-- Download Zip Restriction Filter

 		 <url-pattern> MUST match: 

-		 	* com.gitblit.Constants.FEDERATION_PATH		 

+			* DownloadZipServlet

+			* com.gitblit.Constants.ZIP_PATH

 			* Wicket Filter ignorePaths parameter -->

-	<servlet>

-		<servlet-name>FederationServlet</servlet-name>

-		<servlet-class>com.gitblit.FederationServlet</servlet-class>		

-	</servlet>

-	<servlet-mapping>

-		<servlet-name>FederationServlet</servlet-name>

-		<url-pattern>/federation/*</url-pattern>

-	</servlet-mapping>

-

+	<filter>

+		<filter-name>ZipFilter</filter-name>

+		<filter-class>com.gitblit.DownloadZipFilter</filter-class>

+	</filter>

+	<filter-mapping>

+		<filter-name>ZipFilter</filter-name>

+		<url-pattern>/zip/*</url-pattern>

+	</filter-mapping>

 		

 	<!-- Wicket Filter -->

     <filter>

@@ -118,6 +133,7 @@
              	* GitFilter <url-pattern>

              	* GitServlet <url-pattern>

              	* com.gitblit.Constants.GIT_PATH

+             	* Zipfilter <url-pattern>

              	* ZipServlet <url-pattern>

              	* com.gitblit.Constants.ZIP_PATH

              	* FederationServlet <url-pattern> -->

diff --git a/src/com/gitblit/AccessRestrictionFilter.java b/src/com/gitblit/AccessRestrictionFilter.java
index e309b595..25adc525 100644
--- a/src/com/gitblit/AccessRestrictionFilter.java
+++ b/src/com/gitblit/AccessRestrictionFilter.java
@@ -138,7 +138,7 @@ public abstract class AccessRestrictionFilter implements Filter {
 		}

 		String fullUrl = url + (StringUtils.isEmpty(params) ? "" : ("?" + params));

 

-		String repository = extractRepositoryName(url);

+		String repository = extractRepositoryName(fullUrl);

 

 		// Determine if the request URL is restricted

 		String fullSuffix = fullUrl.substring(repository.length());

diff --git a/src/com/gitblit/DownloadZipFilter.java b/src/com/gitblit/DownloadZipFilter.java
new file mode 100644
index 00000000..6145b125
--- /dev/null
+++ b/src/com/gitblit/DownloadZipFilter.java
@@ -0,0 +1,84 @@
+/*

+ * Copyright 2011 gitblit.com.

+ *

+ * Licensed under the Apache License, Version 2.0 (the "License");

+ * you may not use this file except in compliance with the License.

+ * You may obtain a copy of the License at

+ *

+ *     http://www.apache.org/licenses/LICENSE-2.0

+ *

+ * Unless required by applicable law or agreed to in writing, software

+ * distributed under the License is distributed on an "AS IS" BASIS,

+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

+ * See the License for the specific language governing permissions and

+ * limitations under the License.

+ */

+package com.gitblit;

+

+import com.gitblit.Constants.AccessRestrictionType;

+import com.gitblit.models.RepositoryModel;

+import com.gitblit.models.UserModel;

+

+/**

+ * The DownloadZipFilter is an AccessRestrictionFilter which ensures that zip

+ * requests for view-restricted repositories have proper authentication

+ * credentials and are authorized.

+ * 

+ * @author James Moger

+ * 

+ */

+public class DownloadZipFilter extends AccessRestrictionFilter {

+

+	/**

+	 * Extract the repository name from the url.

+	 * 

+	 * @param url

+	 * @return repository name

+	 */

+	@Override

+	protected String extractRepositoryName(String url) {

+		int a = url.indexOf("r=");

+		String repository = url.substring(a + 2);

+		if (repository.indexOf('&') > -1) {

+			repository = repository.substring(0, repository.indexOf('&'));

+		}

+		return repository;

+	}

+

+	/**

+	 * Analyze the url and returns the action of the request.

+	 * 

+	 * @param url

+	 * @return action of the request

+	 */

+	@Override

+	protected String getUrlRequestAction(String url) {

+		return "DOWNLOAD";

+	}

+

+	/**

+	 * Determine if the repository requires authentication.

+	 * 

+	 * @param repository

+	 * @return true if authentication required

+	 */

+	@Override

+	protected boolean requiresAuthentication(RepositoryModel repository) {

+		return repository.accessRestriction.atLeast(AccessRestrictionType.VIEW);

+	}

+

+	/**

+	 * Determine if the user can access the repository and perform the specified

+	 * action.

+	 * 

+	 * @param repository

+	 * @param user

+	 * @param action

+	 * @return true if user may execute the action on the repository

+	 */

+	@Override

+	protected boolean canAccess(RepositoryModel repository, UserModel user, String action) {

+		return user.canAccessRepository(repository.name);

+	}

+

+}

diff --git a/src/com/gitblit/DownloadZipServlet.java b/src/com/gitblit/DownloadZipServlet.java
index 5f2a2a4b..ed3aa553 100644
--- a/src/com/gitblit/DownloadZipServlet.java
+++ b/src/com/gitblit/DownloadZipServlet.java
@@ -25,8 +25,6 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.slf4j.Logger;

 import org.slf4j.LoggerFactory;

 

-import com.gitblit.Constants.AccessRestrictionType;

-import com.gitblit.models.RepositoryModel;

 import com.gitblit.utils.JGitUtils;

 import com.gitblit.utils.StringUtils;

 

@@ -34,12 +32,6 @@ import com.gitblit.utils.StringUtils;
  * Streams out a zip file from the specified repository for any tree path at any

  * revision.

  * 

- * Unlike the GitServlet and the SyndicationServlet, this servlet is not

- * protected by an AccessRestrictionFilter. It performs its own authorization

- * check, but it does not perform any authentication. The assumption is that

- * requests to this servlet are made via the web ui and not by direct url

- * access. Unauthorized requests fail with a standard 403 (FORBIDDEN) code.

- * 

  * @author James Moger

  * 

  */

@@ -72,7 +64,7 @@ public class DownloadZipServlet extends HttpServlet {
 	}

 

 	/**

-	 * Performs the authorization and zip streaming of the specified elements.

+	 * Creates a zip stream from the repository of the requested data.

 	 * 

 	 * @param request

 	 * @param response

@@ -86,8 +78,8 @@ public class DownloadZipServlet extends HttpServlet {
 			logger.warn("Zip downloads are disabled");

 			response.sendError(HttpServletResponse.SC_FORBIDDEN);

 			return;

-

 		}

+		

 		String repository = request.getParameter("r");

 		String basePath = request.getParameter("p");

 		String objectId = request.getParameter("h");

@@ -98,18 +90,6 @@ public class DownloadZipServlet extends HttpServlet {
 				name = name.substring(name.lastIndexOf('/') + 1);

 			}

 

-			// check roles first

-			boolean authorized = request.isUserInRole(Constants.ADMIN_ROLE);

-			authorized |= request.isUserInRole(repository);

-

-			if (!authorized) {

-				RepositoryModel model = GitBlit.self().getRepositoryModel(repository);

-				if (model.accessRestriction.atLeast(AccessRestrictionType.VIEW)) {

-					logger.warn("Unauthorized access via zip servlet for " + model.name);

-					response.sendError(HttpServletResponse.SC_FORBIDDEN);

-					return;

-				}

-			}

 			if (!StringUtils.isEmpty(basePath)) {

 				name += "-" + basePath.replace('/', '_');

 			}