From 78753bc22f140f863aa3fe56b1c59699ca3e2fa8 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Mon, 26 Sep 2011 22:29:07 -0400
Subject: [PATCH] Protect DownloadZipServlet with an AccessRestrictionFilter.
---
src/com/gitblit/DownloadZipServlet.java | 24 -------
docs/04_releases.mkd | 3
src/WEB-INF/web.xml | 38 +++++++++---
src/com/gitblit/DownloadZipFilter.java | 84 ++++++++++++++++++++++++++++
docs/00_index.mkd | 3
src/com/gitblit/AccessRestrictionFilter.java | 2
6 files changed, 118 insertions(+), 36 deletions(-)
diff --git a/docs/00_index.mkd b/docs/00_index.mkd
index 5cf8173..0da7f74 100644
--- a/docs/00_index.mkd
+++ b/docs/00_index.mkd
@@ -42,7 +42,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
issues, binaries, and sources @ [Google Code][googlecode]<br/>
sources @ [Github][gitbltsrc]
diff --git a/docs/04_releases.mkd b/docs/04_releases.mkd
index 242af41..b27371d 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 c5adadd..d557725 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 e309b59..25adc52 100644
--- a/src/com/gitblit/AccessRestrictionFilter.java
+++ b/src/com/gitblit/AccessRestrictionFilter.java
@@ -138,7 +138,7 @@
}
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 0000000..6145b12
--- /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 5f2a2a4..ed3aa55 100644
--- a/src/com/gitblit/DownloadZipServlet.java
+++ b/src/com/gitblit/DownloadZipServlet.java
@@ -25,20 +25,12 @@
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;
/**
* 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 @@
}
/**
- * 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 @@
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 @@
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('/', '_');
}
--
Gitblit v1.9.1