From: James Moger <james.moger@gitblit.com>
Date: Fri, 11 Nov 2011 22:59:15 +0000 (-0500)
Subject: Revised user access checks to account for repository ownership.
X-Git-Tag: v0.7.0~1
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=efe8ecb216b0e2f2f1dceb26c4f21dcec1fb497c;p=gitblit.git

Revised user access checks to account for repository ownership.

Repository owners no longer have to be explicitly selected to grant
them access to Git, feeds, and zip downloads. Idea from Github/dadalar.
---

diff --git a/src/com/gitblit/AuthenticationFilter.java b/src/com/gitblit/AuthenticationFilter.java
index 277b220b..caa8a074 100644
--- a/src/com/gitblit/AuthenticationFilter.java
+++ b/src/com/gitblit/AuthenticationFilter.java
@@ -171,7 +171,7 @@ public abstract class AuthenticationFilter implements Filter {
 			super(req);
 			user = new UserModel("anonymous");
 		}
-		
+
 		UserModel getUser() {
 			return user;
 		}
@@ -190,6 +190,9 @@ public abstract class AuthenticationFilter implements Filter {
 			if (role.equals(Constants.ADMIN_ROLE)) {
 				return user.canAdmin;
 			}
+			// Gitblit does not currently use actual roles in the traditional
+			// servlet container sense.  That is the reason this is marked
+			// deprecated, but I may want to revisit this.
 			return user.canAccessRepository(role);
 		}
 
diff --git a/src/com/gitblit/DownloadZipFilter.java b/src/com/gitblit/DownloadZipFilter.java
index 6145b125..c308cbbb 100644
--- a/src/com/gitblit/DownloadZipFilter.java
+++ b/src/com/gitblit/DownloadZipFilter.java
@@ -78,7 +78,7 @@ public class DownloadZipFilter extends AccessRestrictionFilter {
 	 */
 	@Override
 	protected boolean canAccess(RepositoryModel repository, UserModel user, String action) {
-		return user.canAccessRepository(repository.name);
+		return user.canAccessRepository(repository);
 	}
 
 }
diff --git a/src/com/gitblit/GitBlit.java b/src/com/gitblit/GitBlit.java
index 8db72af1..bc356676 100644
--- a/src/com/gitblit/GitBlit.java
+++ b/src/com/gitblit/GitBlit.java
@@ -555,7 +555,7 @@ public class GitBlit implements ServletContextListener {
 			return null;
 		}
 		if (model.accessRestriction.atLeast(AccessRestrictionType.VIEW)) {
-			if (user != null && user.canAccessRepository(model.name)) {
+			if (user != null && user.canAccessRepository(model)) {
 				return model;
 			}
 			return null;
diff --git a/src/com/gitblit/GitFilter.java b/src/com/gitblit/GitFilter.java
index 8127ffae..a7f0fe74 100644
--- a/src/com/gitblit/GitFilter.java
+++ b/src/com/gitblit/GitFilter.java
@@ -110,7 +110,7 @@ public class GitFilter extends AccessRestrictionFilter {
 		}
 		boolean readOnly = repository.isFrozen;
 		if (readOnly || repository.accessRestriction.atLeast(AccessRestrictionType.PUSH)) {
-			boolean authorizedUser = user.canAccessRepository(repository.name);
+			boolean authorizedUser = user.canAccessRepository(repository);
 			if (action.equals(gitReceivePack)) {
 				// Push request
 				if (!readOnly && authorizedUser) {
diff --git a/src/com/gitblit/SyndicationFilter.java b/src/com/gitblit/SyndicationFilter.java
index 9c7a8630..d6dd1f2d 100644
--- a/src/com/gitblit/SyndicationFilter.java
+++ b/src/com/gitblit/SyndicationFilter.java
@@ -76,7 +76,7 @@ public class SyndicationFilter extends AccessRestrictionFilter {
 	 */
 	@Override
 	protected boolean canAccess(RepositoryModel repository, UserModel user, String action) {
-		return user.canAccessRepository(repository.name);
+		return user.canAccessRepository(repository);
 	}
 
 }
diff --git a/src/com/gitblit/models/UserModel.java b/src/com/gitblit/models/UserModel.java
index fcf2b263..dadc44e7 100644
--- a/src/com/gitblit/models/UserModel.java
+++ b/src/com/gitblit/models/UserModel.java
@@ -20,6 +20,8 @@ import java.security.Principal;
 import java.util.HashSet;
 import java.util.Set;
 
+import com.gitblit.utils.StringUtils;
+
 /**
  * UserModel is a serializable model class that represents a user and the user's
  * restricted repository memberships. Instances of UserModels are also used as
@@ -43,10 +45,24 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
 		this.username = username;
 	}
 
+	/**
+	 * This method does not take into consideration Ownership where the
+	 * administrator has not explicitly granted access to the owner.
+	 * 
+	 * @param repositoryName
+	 * @return
+	 */
+	@Deprecated
 	public boolean canAccessRepository(String repositoryName) {
 		return canAdmin || repositories.contains(repositoryName.toLowerCase());
 	}
 
+	public boolean canAccessRepository(RepositoryModel repository) {
+		boolean isOwner = !StringUtils.isEmpty(repository.owner)
+				&& repository.owner.equals(username);
+		return canAdmin || isOwner || repositories.contains(repository.name.toLowerCase());
+	}
+
 	public void addRepository(String name) {
 		repositories.add(name.toLowerCase());
 	}
diff --git a/tests/com/gitblit/tests/GitBlitTest.java b/tests/com/gitblit/tests/GitBlitTest.java
index 1d20ced9..669b25ac 100644
--- a/tests/com/gitblit/tests/GitBlitTest.java
+++ b/tests/com/gitblit/tests/GitBlitTest.java
@@ -52,9 +52,10 @@ public class GitBlitTest extends TestCase {
 		model.canAdmin = false;
 		assertFalse("Admin should not have #admin!", model.canAdmin);
 		String repository = GitBlitSuite.getHelloworldRepository().getDirectory().getName();
-		assertFalse("Admin can still access repository!", model.canAccessRepository(repository));
+		RepositoryModel repositoryModel = GitBlit.self().getRepositoryModel(model, repository);
+		assertFalse("Admin can still access repository!", model.canAccessRepository(repositoryModel));
 		model.addRepository(repository);
-		assertTrue("Admin can't access repository!", model.canAccessRepository(repository));
+		assertTrue("Admin can't access repository!", model.canAccessRepository(repositoryModel));
 		assertEquals(GitBlit.self().getRepositoryModel(model, "pretend"), null);
 		assertNotNull(GitBlit.self().getRepositoryModel(model, repository));
 		assertTrue(GitBlit.self().getRepositoryModels(model).size() > 0);