]> source.dussan.org Git - gitblit.git/commitdiff
Protect DownloadZipServlet with an AccessRestrictionFilter.
authorJames Moger <james.moger@gitblit.com>
Tue, 27 Sep 2011 02:29:07 +0000 (22:29 -0400)
committerJames Moger <james.moger@gitblit.com>
Tue, 27 Sep 2011 02:29:07 +0000 (22:29 -0400)
docs/00_index.mkd
docs/04_releases.mkd
src/WEB-INF/web.xml
src/com/gitblit/AccessRestrictionFilter.java
src/com/gitblit/DownloadZipFilter.java [new file with mode: 0644]
src/com/gitblit/DownloadZipServlet.java

index 5cf8173507e1b15c710e917c6bce8df9651a7a25..0da7f742bb8a790f3d9857ff542b24685500378d 100644 (file)
@@ -42,7 +42,8 @@ Gitblit requires a Java 6 Runtime Environment (JRE) or a Java 6 Development Kit
 - updated: MarkdownPapers 1.1.1\r
 - updated: Wicket 1.4.18\r
 - updated: JGit 1.1.0\r
-- fixed: syndication urls for WAR builds\r
+- fixed: syndication urls for WAR deployments\r
+- fixed: authentication for zip downloads\r
 \r
 issues, binaries, and sources @ [Google Code][googlecode]<br/>\r
 sources @ [Github][gitbltsrc]\r
index 242af41902576119cc0f775952da5744b9c35562..b27371d2957fe6f2a9b2ffa2a5ce0d822c02cdc2 100644 (file)
@@ -17,7 +17,8 @@
 - updated: MarkdownPapers 1.1.1\r
 - updated: Wicket 1.4.18\r
 - updated: JGit 1.1.0\r
-- fixed: syndication urls for WAR builds\r
+- fixed: syndication urls for WAR deployments\r
+- fixed: authentication for zip downloads\r
 \r
 ### Older Releases\r
 \r
index c5adadd96065fade07616f5f72b90979ece75aa5..d557725955ccf6e142b263c4b78ad811b539cdfe 100644 (file)
                <url-pattern>/zip/*</url-pattern>\r
        </servlet-mapping>\r
        \r
+       \r
+       <!-- Federation Servlet\r
+                <url-pattern> MUST match: \r
+                       * com.gitblit.Constants.FEDERATION_PATH          \r
+                       * Wicket Filter ignorePaths parameter -->\r
+       <servlet>\r
+               <servlet-name>FederationServlet</servlet-name>\r
+               <servlet-class>com.gitblit.FederationServlet</servlet-class>            \r
+       </servlet>\r
+       <servlet-mapping>\r
+               <servlet-name>FederationServlet</servlet-name>\r
+               <url-pattern>/federation/*</url-pattern>\r
+       </servlet-mapping>      \r
+       \r
 \r
        <!-- Git Access Restriction Filter\r
                 <url-pattern> MUST match: \r
                <url-pattern>/feed/*</url-pattern>\r
        </filter-mapping>\r
        \r
-       <!-- Federation Servlet\r
+       \r
+       <!-- Download Zip Restriction Filter\r
                 <url-pattern> MUST match: \r
-                       * com.gitblit.Constants.FEDERATION_PATH          \r
+                       * DownloadZipServlet\r
+                       * com.gitblit.Constants.ZIP_PATH\r
                        * Wicket Filter ignorePaths parameter -->\r
-       <servlet>\r
-               <servlet-name>FederationServlet</servlet-name>\r
-               <servlet-class>com.gitblit.FederationServlet</servlet-class>            \r
-       </servlet>\r
-       <servlet-mapping>\r
-               <servlet-name>FederationServlet</servlet-name>\r
-               <url-pattern>/federation/*</url-pattern>\r
-       </servlet-mapping>\r
-\r
+       <filter>\r
+               <filter-name>ZipFilter</filter-name>\r
+               <filter-class>com.gitblit.DownloadZipFilter</filter-class>\r
+       </filter>\r
+       <filter-mapping>\r
+               <filter-name>ZipFilter</filter-name>\r
+               <url-pattern>/zip/*</url-pattern>\r
+       </filter-mapping>\r
                \r
        <!-- Wicket Filter -->\r
     <filter>\r
                * GitFilter <url-pattern>\r
                * GitServlet <url-pattern>\r
                * com.gitblit.Constants.GIT_PATH\r
+               * Zipfilter <url-pattern>\r
                * ZipServlet <url-pattern>\r
                * com.gitblit.Constants.ZIP_PATH\r
                * FederationServlet <url-pattern> -->\r
index e309b5957f9f52e38c96b7311d114df6789ef232..25adc5255c46c69b3d3dbd9914d9abcae6237421 100644 (file)
@@ -138,7 +138,7 @@ public abstract class AccessRestrictionFilter implements Filter {
                }\r
                String fullUrl = url + (StringUtils.isEmpty(params) ? "" : ("?" + params));\r
 \r
-               String repository = extractRepositoryName(url);\r
+               String repository = extractRepositoryName(fullUrl);\r
 \r
                // Determine if the request URL is restricted\r
                String fullSuffix = fullUrl.substring(repository.length());\r
diff --git a/src/com/gitblit/DownloadZipFilter.java b/src/com/gitblit/DownloadZipFilter.java
new file mode 100644 (file)
index 0000000..6145b12
--- /dev/null
@@ -0,0 +1,84 @@
+/*\r
+ * Copyright 2011 gitblit.com.\r
+ *\r
+ * Licensed under the Apache License, Version 2.0 (the "License");\r
+ * you may not use this file except in compliance with the License.\r
+ * You may obtain a copy of the License at\r
+ *\r
+ *     http://www.apache.org/licenses/LICENSE-2.0\r
+ *\r
+ * Unless required by applicable law or agreed to in writing, software\r
+ * distributed under the License is distributed on an "AS IS" BASIS,\r
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\r
+ * See the License for the specific language governing permissions and\r
+ * limitations under the License.\r
+ */\r
+package com.gitblit;\r
+\r
+import com.gitblit.Constants.AccessRestrictionType;\r
+import com.gitblit.models.RepositoryModel;\r
+import com.gitblit.models.UserModel;\r
+\r
+/**\r
+ * The DownloadZipFilter is an AccessRestrictionFilter which ensures that zip\r
+ * requests for view-restricted repositories have proper authentication\r
+ * credentials and are authorized.\r
+ * \r
+ * @author James Moger\r
+ * \r
+ */\r
+public class DownloadZipFilter extends AccessRestrictionFilter {\r
+\r
+       /**\r
+        * Extract the repository name from the url.\r
+        * \r
+        * @param url\r
+        * @return repository name\r
+        */\r
+       @Override\r
+       protected String extractRepositoryName(String url) {\r
+               int a = url.indexOf("r=");\r
+               String repository = url.substring(a + 2);\r
+               if (repository.indexOf('&') > -1) {\r
+                       repository = repository.substring(0, repository.indexOf('&'));\r
+               }\r
+               return repository;\r
+       }\r
+\r
+       /**\r
+        * Analyze the url and returns the action of the request.\r
+        * \r
+        * @param url\r
+        * @return action of the request\r
+        */\r
+       @Override\r
+       protected String getUrlRequestAction(String url) {\r
+               return "DOWNLOAD";\r
+       }\r
+\r
+       /**\r
+        * Determine if the repository requires authentication.\r
+        * \r
+        * @param repository\r
+        * @return true if authentication required\r
+        */\r
+       @Override\r
+       protected boolean requiresAuthentication(RepositoryModel repository) {\r
+               return repository.accessRestriction.atLeast(AccessRestrictionType.VIEW);\r
+       }\r
+\r
+       /**\r
+        * Determine if the user can access the repository and perform the specified\r
+        * action.\r
+        * \r
+        * @param repository\r
+        * @param user\r
+        * @param action\r
+        * @return true if user may execute the action on the repository\r
+        */\r
+       @Override\r
+       protected boolean canAccess(RepositoryModel repository, UserModel user, String action) {\r
+               return user.canAccessRepository(repository.name);\r
+       }\r
+\r
+}\r
index 5f2a2a4b7365db20cacf27525cfdfe9d27b9d370..ed3aa553e78846f31d4a997e2cfe0d71200440ff 100644 (file)
@@ -25,8 +25,6 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.slf4j.Logger;\r
 import org.slf4j.LoggerFactory;\r
 \r
-import com.gitblit.Constants.AccessRestrictionType;\r
-import com.gitblit.models.RepositoryModel;\r
 import com.gitblit.utils.JGitUtils;\r
 import com.gitblit.utils.StringUtils;\r
 \r
@@ -34,12 +32,6 @@ import com.gitblit.utils.StringUtils;
  * Streams out a zip file from the specified repository for any tree path at any\r
  * revision.\r
  * \r
- * Unlike the GitServlet and the SyndicationServlet, this servlet is not\r
- * protected by an AccessRestrictionFilter. It performs its own authorization\r
- * check, but it does not perform any authentication. The assumption is that\r
- * requests to this servlet are made via the web ui and not by direct url\r
- * access. Unauthorized requests fail with a standard 403 (FORBIDDEN) code.\r
- * \r
  * @author James Moger\r
  * \r
  */\r
@@ -72,7 +64,7 @@ public class DownloadZipServlet extends HttpServlet {
        }\r
 \r
        /**\r
-        * Performs the authorization and zip streaming of the specified elements.\r
+        * Creates a zip stream from the repository of the requested data.\r
         * \r
         * @param request\r
         * @param response\r
@@ -86,8 +78,8 @@ public class DownloadZipServlet extends HttpServlet {
                        logger.warn("Zip downloads are disabled");\r
                        response.sendError(HttpServletResponse.SC_FORBIDDEN);\r
                        return;\r
-\r
                }\r
+               \r
                String repository = request.getParameter("r");\r
                String basePath = request.getParameter("p");\r
                String objectId = request.getParameter("h");\r
@@ -98,18 +90,6 @@ public class DownloadZipServlet extends HttpServlet {
                                name = name.substring(name.lastIndexOf('/') + 1);\r
                        }\r
 \r
-                       // check roles first\r
-                       boolean authorized = request.isUserInRole(Constants.ADMIN_ROLE);\r
-                       authorized |= request.isUserInRole(repository);\r
-\r
-                       if (!authorized) {\r
-                               RepositoryModel model = GitBlit.self().getRepositoryModel(repository);\r
-                               if (model.accessRestriction.atLeast(AccessRestrictionType.VIEW)) {\r
-                                       logger.warn("Unauthorized access via zip servlet for " + model.name);\r
-                                       response.sendError(HttpServletResponse.SC_FORBIDDEN);\r
-                                       return;\r
-                               }\r
-                       }\r
                        if (!StringUtils.isEmpty(basePath)) {\r
                                name += "-" + basePath.replace('/', '_');\r
                        }\r