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 277b220b3ea6e59807ea192919c7eb94f774804c..caa8a0749956f9d1b2c270752d494411da9b3a1d 100644 (file)
--- a/src/com/gitblit/AuthenticationFilter.java
+++ b/src/com/gitblit/AuthenticationFilter.java
@@ -171,7 +171,7 @@ public abstract class AuthenticationFilter implements Filter {
                        super(req);\r
                        user = new UserModel("anonymous");\r
                }\r
-               \r
+\r
                UserModel getUser() {\r
                        return user;\r
                }\r
@@ -190,6 +190,9 @@ public abstract class AuthenticationFilter implements Filter {
                        if (role.equals(Constants.ADMIN_ROLE)) {\r
                                return user.canAdmin;\r
                        }\r
+                       // Gitblit does not currently use actual roles in the traditional\r
+                       // servlet container sense.  That is the reason this is marked\r
+                       // deprecated, but I may want to revisit this.\r
                        return user.canAccessRepository(role);\r
                }\r
 \r

diff --git a/src/com/gitblit/DownloadZipFilter.java b/src/com/gitblit/DownloadZipFilter.java
index 6145b12584a3e3b512234b2ec35631baf017f2df..c308cbbb35d2ac7b4a280fedd8bdfad48dceaae2 100644 (file)
--- a/src/com/gitblit/DownloadZipFilter.java
+++ b/src/com/gitblit/DownloadZipFilter.java
@@ -78,7 +78,7 @@ public class DownloadZipFilter extends AccessRestrictionFilter {
         */\r
        @Override\r
        protected boolean canAccess(RepositoryModel repository, UserModel user, String action) {\r
-               return user.canAccessRepository(repository.name);\r
+               return user.canAccessRepository(repository);\r
        }\r
 \r
 }\r

diff --git a/src/com/gitblit/GitBlit.java b/src/com/gitblit/GitBlit.java
index 8db72af1703b0f94bd8b7d009967870baa483c16..bc356676202a7b85218657b13da2ddc681a54d15 100644 (file)
--- a/src/com/gitblit/GitBlit.java
+++ b/src/com/gitblit/GitBlit.java
@@ -555,7 +555,7 @@ public class GitBlit implements ServletContextListener {
                        return null;\r
                }\r
                if (model.accessRestriction.atLeast(AccessRestrictionType.VIEW)) {\r
-                       if (user != null && user.canAccessRepository(model.name)) {\r
+                       if (user != null && user.canAccessRepository(model)) {\r
                                return model;\r
                        }\r
                        return null;\r

diff --git a/src/com/gitblit/GitFilter.java b/src/com/gitblit/GitFilter.java
index 8127ffae974a1c1b7e757c712d9db7312a5712c5..a7f0fe741abcb4f9104dc74e72d831c60f254c47 100644 (file)
--- a/src/com/gitblit/GitFilter.java
+++ b/src/com/gitblit/GitFilter.java
@@ -110,7 +110,7 @@ public class GitFilter extends AccessRestrictionFilter {
                }\r
                boolean readOnly = repository.isFrozen;\r
                if (readOnly || repository.accessRestriction.atLeast(AccessRestrictionType.PUSH)) {\r
-                       boolean authorizedUser = user.canAccessRepository(repository.name);\r
+                       boolean authorizedUser = user.canAccessRepository(repository);\r
                        if (action.equals(gitReceivePack)) {\r
                                // Push request\r
                                if (!readOnly && authorizedUser) {\r

diff --git a/src/com/gitblit/SyndicationFilter.java b/src/com/gitblit/SyndicationFilter.java
index 9c7a86300ed115e0fc8e218cafe387241d551818..d6dd1f2d9b48e553306ca8b305a5a158ef886dc9 100644 (file)
--- a/src/com/gitblit/SyndicationFilter.java
+++ b/src/com/gitblit/SyndicationFilter.java
@@ -76,7 +76,7 @@ public class SyndicationFilter extends AccessRestrictionFilter {
         */\r
        @Override\r
        protected boolean canAccess(RepositoryModel repository, UserModel user, String action) {\r
-               return user.canAccessRepository(repository.name);\r
+               return user.canAccessRepository(repository);\r
        }\r
 \r
 }\r

diff --git a/src/com/gitblit/models/UserModel.java b/src/com/gitblit/models/UserModel.java
index fcf2b263b55da0582a747a6d29622b5fce0bc543..dadc44e7f8fad31aff6e2cca745ccca6bc3a6c4a 100644 (file)
--- 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;\r
 import java.util.Set;\r
 \r
+import com.gitblit.utils.StringUtils;\r
+\r
 /**\r
  * UserModel is a serializable model class that represents a user and the user's\r
  * restricted repository memberships. Instances of UserModels are also used as\r
@@ -43,10 +45,24 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
                this.username = username;\r
        }\r
 \r
+       /**\r
+        * This method does not take into consideration Ownership where the\r
+        * administrator has not explicitly granted access to the owner.\r
+        * \r
+        * @param repositoryName\r
+        * @return\r
+        */\r
+       @Deprecated\r
        public boolean canAccessRepository(String repositoryName) {\r
                return canAdmin || repositories.contains(repositoryName.toLowerCase());\r
        }\r
 \r
+       public boolean canAccessRepository(RepositoryModel repository) {\r
+               boolean isOwner = !StringUtils.isEmpty(repository.owner)\r
+                               && repository.owner.equals(username);\r
+               return canAdmin || isOwner || repositories.contains(repository.name.toLowerCase());\r
+       }\r
+\r
        public void addRepository(String name) {\r
                repositories.add(name.toLowerCase());\r
        }\r

diff --git a/tests/com/gitblit/tests/GitBlitTest.java b/tests/com/gitblit/tests/GitBlitTest.java
index 1d20ced92e6de46ee44b0aa3d80dd83fe037ebc7..669b25acfe27d678a8e1c5fcbff568ed28b6f573 100644 (file)
--- 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;\r
                assertFalse("Admin should not have #admin!", model.canAdmin);\r
                String repository = GitBlitSuite.getHelloworldRepository().getDirectory().getName();\r
-               assertFalse("Admin can still access repository!", model.canAccessRepository(repository));\r
+               RepositoryModel repositoryModel = GitBlit.self().getRepositoryModel(model, repository);\r
+               assertFalse("Admin can still access repository!", model.canAccessRepository(repositoryModel));\r
                model.addRepository(repository);\r
-               assertTrue("Admin can't access repository!", model.canAccessRepository(repository));\r
+               assertTrue("Admin can't access repository!", model.canAccessRepository(repositoryModel));\r
                assertEquals(GitBlit.self().getRepositoryModel(model, "pretend"), null);\r
                assertNotNull(GitBlit.self().getRepositoryModel(model, repository));\r
                assertTrue(GitBlit.self().getRepositoryModels(model).size() > 0);\r