Protect DownloadZipServlet with an AccessRestrictionFilter.

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]

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
 

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> -->

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());

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);
+	}
+
+}

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('/', '_');
 			}