From efe8ecb216b0e2f2f1dceb26c4f21dcec1fb497c Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Fri, 11 Nov 2011 17:59:15 -0500
Subject: [PATCH] Revised user access checks to account for repository ownership.
---
tests/com/gitblit/tests/GitBlitTest.java | 5 +++--
src/com/gitblit/GitFilter.java | 2 +-
src/com/gitblit/GitBlit.java | 2 +-
src/com/gitblit/SyndicationFilter.java | 2 +-
src/com/gitblit/AuthenticationFilter.java | 5 ++++-
src/com/gitblit/DownloadZipFilter.java | 2 +-
src/com/gitblit/models/UserModel.java | 16 ++++++++++++++++
7 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/com/gitblit/AuthenticationFilter.java b/src/com/gitblit/AuthenticationFilter.java
index 277b220..caa8a07 100644
--- a/src/com/gitblit/AuthenticationFilter.java
+++ b/src/com/gitblit/AuthenticationFilter.java
@@ -171,7 +171,7 @@
super(req);
user = new UserModel("anonymous");
}
-
+
UserModel getUser() {
return user;
}
@@ -190,6 +190,9 @@
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 6145b12..c308cbb 100644
--- a/src/com/gitblit/DownloadZipFilter.java
+++ b/src/com/gitblit/DownloadZipFilter.java
@@ -78,7 +78,7 @@
*/
@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 8db72af..bc35667 100644
--- a/src/com/gitblit/GitBlit.java
+++ b/src/com/gitblit/GitBlit.java
@@ -555,7 +555,7 @@
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 8127ffa..a7f0fe7 100644
--- a/src/com/gitblit/GitFilter.java
+++ b/src/com/gitblit/GitFilter.java
@@ -110,7 +110,7 @@
}
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 9c7a863..d6dd1f2 100644
--- a/src/com/gitblit/SyndicationFilter.java
+++ b/src/com/gitblit/SyndicationFilter.java
@@ -76,7 +76,7 @@
*/
@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 fcf2b26..dadc44e 100644
--- a/src/com/gitblit/models/UserModel.java
+++ b/src/com/gitblit/models/UserModel.java
@@ -20,6 +20,8 @@
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 @@
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 1d20ced..669b25a 100644
--- a/tests/com/gitblit/tests/GitBlitTest.java
+++ b/tests/com/gitblit/tests/GitBlitTest.java
@@ -52,9 +52,10 @@
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);
--
Gitblit v1.9.1