James Moger
2012-10-23 2bfb8ab137ac18b60cad0c375c7b9bef67499b94
Enforce strict order for permission determination

The order of permissions defined within a user or team is preserved
during read and write. This order is important for determining the
regex match used within the user or team object.

If the user is an admin or repository owner, then RW+
Else if user has an explicit permission, use that
Else check for the first regex match in user permissions
Else check for the HIGHEST permission from team memberships

If the team is an admin team, then RW+
Else if a team has an explicit permission, use that
Else check for the first regex match in team permissions
3 files modified
150 ■■■■■ changed files
src/com/gitblit/models/TeamModel.java 8 ●●●●● patch | view | raw | blame | history
src/com/gitblit/models/UserModel.java 22 ●●●●● patch | view | raw | blame | history
tests/com/gitblit/tests/PermissionsTest.java 120 ●●●●● patch | view | raw | blame | history
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);
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;
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");