From 78753bc22f140f863aa3fe56b1c59699ca3e2fa8 Mon Sep 17 00:00:00 2001 From: James Moger Date: Mon, 26 Sep 2011 22:29:07 -0400 Subject: Protect DownloadZipServlet with an AccessRestrictionFilter. --- docs/00_index.mkd | 3 +- docs/04_releases.mkd | 3 +- src/WEB-INF/web.xml | 38 +++++++++---- src/com/gitblit/AccessRestrictionFilter.java | 2 +- src/com/gitblit/DownloadZipFilter.java | 84 ++++++++++++++++++++++++++++ src/com/gitblit/DownloadZipServlet.java | 24 +------- 6 files changed, 118 insertions(+), 36 deletions(-) create mode 100644 src/com/gitblit/DownloadZipFilter.java 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]
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 @@ /zip/* + + + + FederationServlet + com.gitblit.FederationServlet + + + FederationServlet + /federation/* + + - - FederationServlet - com.gitblit.FederationServlet - - - FederationServlet - /federation/* - - + + ZipFilter + com.gitblit.DownloadZipFilter + + + ZipFilter + /zip/* + @@ -118,6 +133,7 @@ * GitFilter * GitServlet * com.gitblit.Constants.GIT_PATH + * Zipfilter * ZipServlet * com.gitblit.Constants.ZIP_PATH * FederationServlet --> 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('/', '_'); } -- cgit v1.2.3