From 2bfb8ab137ac18b60cad0c375c7b9bef67499b94 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Tue, 23 Oct 2012 17:35:42 -0400
Subject: [PATCH] Enforce strict order for permission determination

---
 src/com/gitblit/models/TeamModel.java        |    8 +-
 tests/com/gitblit/tests/PermissionsTest.java |  120 +++++++++++++++++++++++++++++++++++++++
 src/com/gitblit/models/UserModel.java        |   22 ++++---
 3 files changed, 137 insertions(+), 13 deletions(-)

diff --git a/src/com/gitblit/models/TeamModel.java b/src/com/gitblit/models/TeamModel.java
index 6410eb4..7d557db 100644
--- a/src/com/gitblit/models/TeamModel.java
+++ b/src/com/gitblit/models/TeamModel.java
@@ -19,8 +19,8 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -51,7 +51,7 @@
 	// retained for backwards-compatibility with RPC clients
 	@Deprecated
 	public final Set<String> repositories = new HashSet<String>();
-	public final Map<String, AccessPermission> permissions = new HashMap<String, AccessPermission>();
+	public final Map<String, AccessPermission> permissions = new LinkedHashMap<String, AccessPermission>();
 	public final Set<String> mailingLists = new HashSet<String>();
 	public final List<String> preReceiveScripts = new ArrayList<String>();
 	public final List<String> postReceiveScripts = new ArrayList<String>();
@@ -191,6 +191,8 @@
 					AccessPermission p = permissions.get(key);
 					if (p != null) {
 						permission = p;
+						// take first match
+						break;
 					}
 				}
 			}
@@ -198,7 +200,7 @@
 		return permission;
 	}
 	
-	private boolean canAccess(RepositoryModel repository, AccessRestrictionType ifRestriction, AccessPermission requirePermission) {
+	protected boolean canAccess(RepositoryModel repository, AccessRestrictionType ifRestriction, AccessPermission requirePermission) {
 		if (repository.accessRestriction.atLeast(ifRestriction)) {
 			AccessPermission permission = getRepositoryPermission(repository);
 			return permission.atLeast(requirePermission);
diff --git a/src/com/gitblit/models/UserModel.java b/src/com/gitblit/models/UserModel.java
index 6cc0778..d7bc293 100644
--- a/src/com/gitblit/models/UserModel.java
+++ b/src/com/gitblit/models/UserModel.java
@@ -19,8 +19,8 @@
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -60,7 +60,7 @@
 	// retained for backwards-compatibility with RPC clients
 	@Deprecated
 	public final Set<String> repositories = new HashSet<String>();
-	public final Map<String, AccessPermission> permissions = new HashMap<String, AccessPermission>();
+	public final Map<String, AccessPermission> permissions = new LinkedHashMap<String, AccessPermission>();
 	public final Set<TeamModel> teams = new HashSet<TeamModel>();
 
 	// non-persisted fields
@@ -217,8 +217,8 @@
 			return AccessPermission.REWIND;
 		}
 		
-		// determine best permission available based on user's personal permissions
-		// and the permissions of teams of which the user belongs
+		// explicit user permission OR user regex match is used
+		// if that fails, then the best team permission is used
 		AccessPermission permission = AccessPermission.NONE;
 		if (permissions.containsKey(repository.name.toLowerCase())) {
 			// exact repository permission specified, use it
@@ -232,17 +232,21 @@
 				if (StringUtils.matchesIgnoreCase(repository.name, key)) {
 					AccessPermission p = permissions.get(key);
 					if (p != null) {
+						// take first match
 						permission = p;
+						break;
 					}
 				}
 			}
 		}
 		
-		for (TeamModel team : teams) {
-			AccessPermission p = team.getRepositoryPermission(repository);
-			if (permission == null || p.exceeds(permission)) {
-				// use team permission
-				permission = p;
+		if (AccessPermission.NONE.equals(permission)) {
+			for (TeamModel team : teams) {
+				AccessPermission p = team.getRepositoryPermission(repository);
+				if (p.exceeds(permission)) {
+					// use highest team permission
+					permission = p;
+				}
 			}
 		}
 		return permission;
diff --git a/tests/com/gitblit/tests/PermissionsTest.java b/tests/com/gitblit/tests/PermissionsTest.java
index c0e406d..befd360 100644
--- a/tests/com/gitblit/tests/PermissionsTest.java
+++ b/tests/com/gitblit/tests/PermissionsTest.java
@@ -2393,7 +2393,7 @@
 	}
 	
 	@Test
-	public void testWildcardMatching() throws Exception {
+	public void testRegexMatching() throws Exception {
 		RepositoryModel repository = new RepositoryModel("ubercool/_my-r/e~po.git", null, null, new Date());
 		repository.authorizationControl = AuthorizationControl.NAMED;
 		repository.accessRestriction = AccessRestrictionType.VIEW;
@@ -2415,8 +2415,126 @@
 		assertFalse("user CAN delete!", user.canDelete(repository));
 		assertFalse("user CAN edit!", user.canEdit(repository));
 	}
+
+	@Test
+	public void testRegexIncludeCommonExcludePersonal() throws Exception {
+		
+		UserModel user = new UserModel("test");
+		user.setRepositoryPermission("[^~].*", AccessPermission.CLONE);
+
+		// common
+		RepositoryModel common = new RepositoryModel("ubercool/_my-r/e~po.git", null, null, new Date());
+		common.authorizationControl = AuthorizationControl.NAMED;
+		common.accessRestriction = AccessRestrictionType.VIEW;
+		
+		assertTrue("user DOES NOT HAVE a repository permission!", user.hasRepositoryPermission(common.name));
+		assertTrue("user CAN NOT view!", user.canView(common));
+		assertTrue("user CAN NOT clone!", user.canClone(common));
+		assertFalse("user CAN push!", user.canPush(common));
+		
+		assertFalse("user CAN create ref!", user.canCreateRef(common));
+		assertFalse("user CAN delete ref!", user.canDeleteRef(common));
+		assertFalse("user CAN rewind ref!", user.canRewindRef(common));
+
+		assertFalse("user CAN fork!", user.canFork(common));
+		
+		assertFalse("user CAN delete!", user.canDelete(common));
+		assertFalse("user CAN edit!", user.canEdit(common));
+
+		// personal
+		RepositoryModel personal = new RepositoryModel("~ubercool/_my-r/e~po.git", null, null, new Date());
+		personal.authorizationControl = AuthorizationControl.NAMED;
+		personal.accessRestriction = AccessRestrictionType.VIEW;
+		
+		assertFalse("user HAS a repository permission!", user.hasRepositoryPermission(personal.name));
+		assertFalse("user CAN NOT view!", user.canView(personal));
+		assertFalse("user CAN NOT clone!", user.canClone(personal));
+		assertFalse("user CAN push!", user.canPush(personal));
+		
+		assertFalse("user CAN create ref!", user.canCreateRef(personal));
+		assertFalse("user CAN delete ref!", user.canDeleteRef(personal));
+		assertFalse("user CAN rewind ref!", user.canRewindRef(personal));
+
+		assertFalse("user CAN fork!", user.canFork(personal));
+		
+		assertFalse("user CAN delete!", user.canDelete(personal));
+		assertFalse("user CAN edit!", user.canEdit(personal));
+	}
 	
 	@Test
+	public void testRegexMatching2() throws Exception {
+		RepositoryModel personal = new RepositoryModel("~ubercool/_my-r/e~po.git", null, null, new Date());
+		personal.authorizationControl = AuthorizationControl.NAMED;
+		personal.accessRestriction = AccessRestrictionType.VIEW;
+
+		UserModel user = new UserModel("test");
+		// permit all repositories excluding all personal rpeositories
+		user.setRepositoryPermission("[^~].*", AccessPermission.CLONE);
+		// permitall  ~ubercool repositories
+		user.setRepositoryPermission("~ubercool/.*", AccessPermission.CLONE);
+		
+		// personal
+		assertTrue("user DOES NOT HAVE a repository permission!", user.hasRepositoryPermission(personal.name));
+		assertTrue("user CAN NOT view!", user.canView(personal));
+		assertTrue("user CAN NOT clone!", user.canClone(personal));
+		assertFalse("user CAN push!", user.canPush(personal));
+		
+		assertFalse("user CAN create ref!", user.canCreateRef(personal));
+		assertFalse("user CAN delete ref!", user.canDeleteRef(personal));
+		assertFalse("user CAN rewind ref!", user.canRewindRef(personal));
+
+		assertFalse("user CAN fork!", user.canFork(personal));
+		
+		assertFalse("user CAN delete!", user.canDelete(personal));
+		assertFalse("user CAN edit!", user.canEdit(personal));
+	}
+	
+	@Test
+	public void testRegexOrder() throws Exception {
+		RepositoryModel personal = new RepositoryModel("~ubercool/_my-r/e~po.git", null, null, new Date());
+		personal.authorizationControl = AuthorizationControl.NAMED;
+		personal.accessRestriction = AccessRestrictionType.VIEW;
+
+		UserModel user = new UserModel("test");
+		user.setRepositoryPermission(".*", AccessPermission.PUSH);
+		user.setRepositoryPermission("~ubercool/.*", AccessPermission.CLONE);
+		
+		// has PUSH access because first match is PUSH permission 
+		assertTrue("user HAS a repository permission!", user.hasRepositoryPermission(personal.name));
+		assertTrue("user CAN NOT view!", user.canView(personal));
+		assertTrue("user CAN NOT clone!", user.canClone(personal));
+		assertTrue("user CAN NOT push!", user.canPush(personal));
+		
+		assertFalse("user CAN create ref!", user.canCreateRef(personal));
+		assertFalse("user CAN delete ref!", user.canDeleteRef(personal));
+		assertFalse("user CAN rewind ref!", user.canRewindRef(personal));
+
+		assertFalse("user CAN fork!", user.canFork(personal));
+		
+		assertFalse("user CAN delete!", user.canDelete(personal));
+		assertFalse("user CAN edit!", user.canEdit(personal));
+				
+		user.permissions.clear();
+		user.setRepositoryPermission("~ubercool/.*", AccessPermission.CLONE);
+		user.setRepositoryPermission(".*", AccessPermission.PUSH);
+		
+		// has CLONE access because first match is CLONE permission
+		assertTrue("user HAS a repository permission!", user.hasRepositoryPermission(personal.name));
+		assertTrue("user CAN NOT view!", user.canView(personal));
+		assertTrue("user CAN NOT clone!", user.canClone(personal));
+		assertFalse("user CAN push!", user.canPush(personal));
+				
+		assertFalse("user CAN create ref!", user.canCreateRef(personal));
+		assertFalse("user CAN delete ref!", user.canDeleteRef(personal));
+		assertFalse("user CAN rewind ref!", user.canRewindRef(personal));
+
+		assertFalse("user CAN fork!", user.canFork(personal));
+				
+		assertFalse("user CAN delete!", user.canDelete(personal));
+		assertFalse("user CAN edit!", user.canEdit(personal));
+	}
+
+	@Test
 	public void testAdminTeamInheritance() throws Exception {
 		UserModel user = new UserModel("test");
 		TeamModel team = new TeamModel("team");

--
Gitblit v1.9.1