From 7f70511e9a13f4801e4e941affad6fc7b579c79d Mon Sep 17 00:00:00 2001 From: James Moger Date: Wed, 10 Oct 2012 22:29:36 -0400 Subject: [PATCH] Support Team canAdmin, canCreate, and canFork (issue 36) --- src/com/gitblit/AccessRestrictionFilter.java | 2 +- src/com/gitblit/AuthenticationFilter.java | 2 +- src/com/gitblit/ConfigUserService.java | 24 ++++++ src/com/gitblit/FileUserService.java | 25 +++++++ src/com/gitblit/RpcFilter.java | 4 +- src/com/gitblit/RpcServlet.java | 4 +- src/com/gitblit/client/UsersTableModel.java | 2 +- src/com/gitblit/models/TeamModel.java | 3 + src/com/gitblit/models/UserModel.java | 75 +++++++++++++++++-- .../gitblit/wicket/AuthorizationStrategy.java | 2 +- .../gitblit/wicket/GitBlitWebApp.properties | 2 +- src/com/gitblit/wicket/GitBlitWebSession.java | 2 +- .../wicket/pages/EditRepositoryPage.java | 10 +-- .../gitblit/wicket/pages/EditTeamPage.html | 7 +- .../gitblit/wicket/pages/EditTeamPage.java | 4 + .../gitblit/wicket/pages/EditUserPage.html | 4 +- .../gitblit/wicket/pages/RepositoryPage.java | 2 +- src/com/gitblit/wicket/pages/UserPage.java | 2 +- .../wicket/panels/RepositoriesPanel.java | 2 +- src/com/gitblit/wicket/panels/UsersPanel.java | 2 +- test-users.conf | 1 + tests/com/gitblit/tests/PermissionsTest.java | 27 +++++++ 22 files changed, 178 insertions(+), 30 deletions(-) diff --git a/src/com/gitblit/AccessRestrictionFilter.java b/src/com/gitblit/AccessRestrictionFilter.java index aeb6835c..3a104813 100644 --- a/src/com/gitblit/AccessRestrictionFilter.java +++ b/src/com/gitblit/AccessRestrictionFilter.java @@ -156,7 +156,7 @@ public abstract class AccessRestrictionFilter extends AuthenticationFilter { return; } else { // check user access for request - if (user.canAdmin || canAccess(model, user, urlRequestType)) { + if (user.canAdmin() || canAccess(model, user, urlRequestType)) { // authenticated request permitted. // pass processing to the restricted servlet. newSession(authenticatedRequest, httpResponse); diff --git a/src/com/gitblit/AuthenticationFilter.java b/src/com/gitblit/AuthenticationFilter.java index 259991c9..4762c428 100644 --- a/src/com/gitblit/AuthenticationFilter.java +++ b/src/com/gitblit/AuthenticationFilter.java @@ -189,7 +189,7 @@ public abstract class AuthenticationFilter implements Filter { @Override public boolean isUserInRole(String role) { if (role.equals(Constants.ADMIN_ROLE)) { - return user.canAdmin; + return user.canAdmin(); } // Gitblit does not currently use actual roles in the traditional // servlet container sense. That is the reason this is marked diff --git a/src/com/gitblit/ConfigUserService.java b/src/com/gitblit/ConfigUserService.java index d2740094..82cd33e3 100644 --- a/src/com/gitblit/ConfigUserService.java +++ b/src/com/gitblit/ConfigUserService.java @@ -862,6 +862,24 @@ public class ConfigUserService implements IUserService { // write teams for (TeamModel model : teams.values()) { + // team roles + List roles = new ArrayList(); + if (model.canAdmin) { + roles.add(Constants.ADMIN_ROLE); + } + if (model.canFork) { + roles.add(Constants.FORK_ROLE); + } + if (model.canCreate) { + roles.add(Constants.CREATE_ROLE); + } + if (roles.size() == 0) { + // we do this to ensure that team record is written. + // Otherwise, StoredConfig might optimizes that record away. + roles.add(Constants.NO_ROLE); + } + config.setStringList(TEAM, model.name, ROLE, roles); + if (model.permissions == null) { // null check on "final" repositories because JSON-sourced TeamModel // can have a null repositories object @@ -982,6 +1000,12 @@ public class ConfigUserService implements IUserService { Set teamnames = config.getSubsections(TEAM); for (String teamname : teamnames) { TeamModel team = new TeamModel(teamname); + Set roles = new HashSet(Arrays.asList(config.getStringList( + TEAM, teamname, ROLE))); + team.canAdmin = roles.contains(Constants.ADMIN_ROLE); + team.canFork = roles.contains(Constants.FORK_ROLE); + team.canCreate = roles.contains(Constants.CREATE_ROLE); + team.addRepositoryPermissions(Arrays.asList(config.getStringList(TEAM, teamname, REPOSITORY))); team.addUsers(Arrays.asList(config.getStringList(TEAM, teamname, USER))); diff --git a/src/com/gitblit/FileUserService.java b/src/com/gitblit/FileUserService.java index c06266dc..d411b68f 100644 --- a/src/com/gitblit/FileUserService.java +++ b/src/com/gitblit/FileUserService.java @@ -780,6 +780,20 @@ public class FileUserService extends FileSettings implements IUserService { } else if (role.charAt(0) == '%') { postReceive.add(role.substring(1)); } else { + switch (role.charAt(0)) { + case '#': + // Permissions + if (role.equalsIgnoreCase(Constants.ADMIN_ROLE)) { + team.canAdmin = true; + } else if (role.equalsIgnoreCase(Constants.FORK_ROLE)) { + team.canFork = true; + } else if (role.equalsIgnoreCase(Constants.CREATE_ROLE)) { + team.canCreate = true; + } + break; + default: + repositories.add(role); + } repositories.add(role); } } @@ -1040,6 +1054,17 @@ public class FileUserService extends FileSettings implements IUserService { } } + // Permissions + if (model.canAdmin) { + roles.add(Constants.ADMIN_ROLE); + } + if (model.canFork) { + roles.add(Constants.FORK_ROLE); + } + if (model.canCreate) { + roles.add(Constants.CREATE_ROLE); + } + for (String role : roles) { sb.append(role); sb.append(','); diff --git a/src/com/gitblit/RpcFilter.java b/src/com/gitblit/RpcFilter.java index 4c0f03df..1de9fcc4 100644 --- a/src/com/gitblit/RpcFilter.java +++ b/src/com/gitblit/RpcFilter.java @@ -105,7 +105,7 @@ public class RpcFilter extends AuthenticationFilter { return; } else { // check user access for request - if (user.canAdmin || canAccess(user, requestType)) { + if (user.canAdmin() || canAccess(user, requestType)) { // authenticated request permitted. // pass processing to the restricted servlet. newSession(authenticatedRequest, httpResponse); @@ -140,7 +140,7 @@ public class RpcFilter extends AuthenticationFilter { case LIST_REPOSITORIES: return true; default: - return user.canAdmin; + return user.canAdmin(); } } } \ No newline at end of file diff --git a/src/com/gitblit/RpcServlet.java b/src/com/gitblit/RpcServlet.java index ff98ff55..2a6ba26c 100644 --- a/src/com/gitblit/RpcServlet.java +++ b/src/com/gitblit/RpcServlet.java @@ -73,10 +73,10 @@ public class RpcServlet extends JsonServlet { UserModel user = (UserModel) request.getUserPrincipal(); - boolean allowManagement = user != null && user.canAdmin + boolean allowManagement = user != null && user.canAdmin() && GitBlit.getBoolean(Keys.web.enableRpcManagement, false); - boolean allowAdmin = user != null && user.canAdmin + boolean allowAdmin = user != null && user.canAdmin() && GitBlit.getBoolean(Keys.web.enableRpcAdministration, false); Object result = null; diff --git a/src/com/gitblit/client/UsersTableModel.java b/src/com/gitblit/client/UsersTableModel.java index c05230ea..f635f7e8 100644 --- a/src/com/gitblit/client/UsersTableModel.java +++ b/src/com/gitblit/client/UsersTableModel.java @@ -102,7 +102,7 @@ public class UsersTableModel extends AbstractTableModel { case Display_Name: return model.displayName; case AccessLevel: - if (model.canAdmin) { + if (model.canAdmin()) { return "administrator"; } return ""; diff --git a/src/com/gitblit/models/TeamModel.java b/src/com/gitblit/models/TeamModel.java index 896adfe6..149c7659 100644 --- a/src/com/gitblit/models/TeamModel.java +++ b/src/com/gitblit/models/TeamModel.java @@ -41,6 +41,9 @@ public class TeamModel implements Serializable, Comparable { // field names are reflectively mapped in EditTeam page public String name; + public boolean canAdmin; + public boolean canFork; + public boolean canCreate; public final Set users = new HashSet(); // retained for backwards-compatibility with RPC clients @Deprecated diff --git a/src/com/gitblit/models/UserModel.java b/src/com/gitblit/models/UserModel.java index d8c2abe3..6fe8df2b 100644 --- a/src/com/gitblit/models/UserModel.java +++ b/src/com/gitblit/models/UserModel.java @@ -26,6 +26,7 @@ import com.gitblit.Constants.AccessPermission; import com.gitblit.Constants.AccessRestrictionType; import com.gitblit.Constants.AuthorizationControl; import com.gitblit.Constants.Unused; +import com.gitblit.utils.ArrayUtils; import com.gitblit.utils.StringUtils; /** @@ -80,7 +81,7 @@ public class UserModel implements Principal, Serializable, Comparable */ @Deprecated public boolean canAccessRepository(String repositoryName) { - return canAdmin || repositories.contains(repositoryName.toLowerCase()) + return canAdmin() || repositories.contains(repositoryName.toLowerCase()) || hasTeamAccess(repositoryName); } @@ -90,7 +91,7 @@ public class UserModel implements Principal, Serializable, Comparable boolean isOwner = !StringUtils.isEmpty(repository.owner) && repository.owner.equals(username); boolean allowAuthenticated = isAuthenticated && AuthorizationControl.AUTHENTICATED.equals(repository.authorizationControl); - return canAdmin || isOwner || repositories.contains(repository.name.toLowerCase()) + return canAdmin() || isOwner || repositories.contains(repository.name.toLowerCase()) || hasTeamAccess(repository.name) || allowAuthenticated; } @@ -177,7 +178,7 @@ public class UserModel implements Principal, Serializable, Comparable } public AccessPermission getRepositoryPermission(RepositoryModel repository) { - if (canAdmin || repository.isOwner(username) || repository.isUsersPersonalRepository(username)) { + if (canAdmin() || repository.isOwner(username) || repository.isUsersPersonalRepository(username)) { return AccessPermission.REWIND; } if (AuthorizationControl.AUTHENTICATED.equals(repository.authorizationControl) && isAuthenticated) { @@ -265,24 +266,84 @@ public class UserModel implements Principal, Serializable, Comparable // can not fork your own repository return false; } - if (canAdmin || repository.isOwner(username)) { + if (canAdmin() || repository.isOwner(username)) { return true; } if (!repository.allowForks) { return false; } - if (!isAuthenticated || !canFork) { + if (!isAuthenticated || !canFork()) { return false; } return canClone(repository); } public boolean canDelete(RepositoryModel model) { - return canAdmin || model.isUsersPersonalRepository(username); + return canAdmin() || model.isUsersPersonalRepository(username); } public boolean canEdit(RepositoryModel model) { - return canAdmin || model.isUsersPersonalRepository(username) || model.isOwner(username); + return canAdmin() || model.isUsersPersonalRepository(username) || model.isOwner(username); + } + + /** + * This returns true if the user has fork privileges or the user has fork + * privileges because of a team membership. + * + * @return true if the user can fork + */ + public boolean canFork() { + if (canFork) { + return true; + } + if (!ArrayUtils.isEmpty(teams)) { + for (TeamModel team : teams) { + if (team.canFork) { + return true; + } + } + } + return false; + } + + /** + * This returns true if the user has admin privileges or the user has admin + * privileges because of a team membership. + * + * @return true if the user can admin + */ + public boolean canAdmin() { + if (canAdmin) { + return true; + } + if (!ArrayUtils.isEmpty(teams)) { + for (TeamModel team : teams) { + if (team.canAdmin) { + return true; + } + } + } + return false; + } + + /** + * This returns true if the user has create privileges or the user has create + * privileges because of a team membership. + * + * @return true if the user can admin + */ + public boolean canCreate() { + if (canCreate) { + return true; + } + if (!ArrayUtils.isEmpty(teams)) { + for (TeamModel team : teams) { + if (team.canCreate) { + return true; + } + } + } + return false; } public boolean isTeamMember(String teamname) { diff --git a/src/com/gitblit/wicket/AuthorizationStrategy.java b/src/com/gitblit/wicket/AuthorizationStrategy.java index 16a4ec80..21bd1b70 100644 --- a/src/com/gitblit/wicket/AuthorizationStrategy.java +++ b/src/com/gitblit/wicket/AuthorizationStrategy.java @@ -60,7 +60,7 @@ public class AuthorizationStrategy extends AbstractPageAuthorizationStrategy imp if (authenticateAdmin) { // authenticate admin if (user != null) { - return user.canAdmin; + return user.canAdmin(); } return false; } else { diff --git a/src/com/gitblit/wicket/GitBlitWebApp.properties b/src/com/gitblit/wicket/GitBlitWebApp.properties index 97faf761..0c500648 100644 --- a/src/com/gitblit/wicket/GitBlitWebApp.properties +++ b/src/com/gitblit/wicket/GitBlitWebApp.properties @@ -329,7 +329,7 @@ gb.allowForks = allow forks gb.allowForksDescription = allow authorized users to fork this repository gb.forkedFrom = forked from gb.canFork = can fork -gb.canForkDescription = user is permitted to fork authorized repositories +gb.canForkDescription = can fork authorized repositories to a personal repository gb.myFork = view my fork gb.forksProhibited = forks prohibited gb.forksProhibitedWarning = this repository forbids forks diff --git a/src/com/gitblit/wicket/GitBlitWebSession.java b/src/com/gitblit/wicket/GitBlitWebSession.java index 0e1ae512..015d97ad 100644 --- a/src/com/gitblit/wicket/GitBlitWebSession.java +++ b/src/com/gitblit/wicket/GitBlitWebSession.java @@ -103,7 +103,7 @@ public final class GitBlitWebSession extends WebSession { if (user == null) { return false; } - return user.canAdmin; + return user.canAdmin(); } public String getUsername() { diff --git a/src/com/gitblit/wicket/pages/EditRepositoryPage.java b/src/com/gitblit/wicket/pages/EditRepositoryPage.java index f7427eb5..8176c28b 100644 --- a/src/com/gitblit/wicket/pages/EditRepositoryPage.java +++ b/src/com/gitblit/wicket/pages/EditRepositoryPage.java @@ -83,7 +83,7 @@ public class EditRepositoryPage extends RootSubPage { GitBlitWebSession session = GitBlitWebSession.get(); UserModel user = session.getUser(); - if (user != null && user.canCreate && !user.canAdmin) { + if (user != null && user.canCreate() && !user.canAdmin()) { // personal create permissions, inject personal repository path model.name = user.getPersonalPath() + "/"; model.projectPath = user.getPersonalPath(); @@ -120,7 +120,7 @@ public class EditRepositoryPage extends RootSubPage { final UserModel user = session.getUser() == null ? UserModel.ANONYMOUS : session.getUser(); if (isCreate) { - if (user.canAdmin) { + if (user.canAdmin()) { super.setupPage(getString("gb.newRepository"), ""); } else { super.setupPage(getString("gb.newRepository"), user.getDisplayName()); @@ -253,7 +253,7 @@ public class EditRepositoryPage extends RootSubPage { return; } - if (user.canCreate && !user.canAdmin) { + if (user.canCreate() && !user.canAdmin()) { // ensure repository name begins with the user's path if (!repositoryModel.name.startsWith(user.getPersonalPath())) { error(MessageFormat.format(getString("gb.illegalPersonalRepositoryLocation"), @@ -474,13 +474,13 @@ public class EditRepositoryPage extends RootSubPage { } if (isCreate) { // Create Repository - if (!user.canCreate && !user.canAdmin) { + if (!user.canCreate() && !user.canAdmin()) { // Only administrators or permitted users may create error(getString("gb.errorOnlyAdminMayCreateRepository"), true); } } else { // Edit Repository - if (user.canAdmin) { + if (user.canAdmin()) { // Admins can edit everything isAdmin = true; return; diff --git a/src/com/gitblit/wicket/pages/EditTeamPage.html b/src/com/gitblit/wicket/pages/EditTeamPage.html index 5a2edaee..30d2a94c 100644 --- a/src/com/gitblit/wicket/pages/EditTeamPage.html +++ b/src/com/gitblit/wicket/pages/EditTeamPage.html @@ -12,7 +12,10 @@

 

- + + + +

 

@@ -20,7 +23,7 @@

 

-
 
+
 
diff --git a/src/com/gitblit/wicket/pages/EditTeamPage.java b/src/com/gitblit/wicket/pages/EditTeamPage.java index 96bd188f..9cbccb59 100644 --- a/src/com/gitblit/wicket/pages/EditTeamPage.java +++ b/src/com/gitblit/wicket/pages/EditTeamPage.java @@ -27,6 +27,7 @@ import org.apache.wicket.PageParameters; import org.apache.wicket.behavior.SimpleAttributeModifier; import org.apache.wicket.extensions.markup.html.form.palette.Palette; import org.apache.wicket.markup.html.form.Button; +import org.apache.wicket.markup.html.form.CheckBox; import org.apache.wicket.markup.html.form.Form; import org.apache.wicket.markup.html.form.TextField; import org.apache.wicket.model.CompoundPropertyModel; @@ -222,6 +223,9 @@ public class EditTeamPage extends RootSubPage { // field names reflective match TeamModel fields form.add(new TextField("name")); + form.add(new CheckBox("canAdmin")); + form.add(new CheckBox("canFork")); + form.add(new CheckBox("canCreate")); form.add(users.setEnabled(editMemberships)); mailingLists = new Model(teamModel.mailingLists == null ? "" : StringUtils.flattenStrings(teamModel.mailingLists, " ")); diff --git a/src/com/gitblit/wicket/pages/EditUserPage.html b/src/com/gitblit/wicket/pages/EditUserPage.html index 77932079..9f178df8 100644 --- a/src/com/gitblit/wicket/pages/EditUserPage.html +++ b/src/com/gitblit/wicket/pages/EditUserPage.html @@ -17,8 +17,8 @@ - - + +

 

diff --git a/src/com/gitblit/wicket/pages/RepositoryPage.java b/src/com/gitblit/wicket/pages/RepositoryPage.java index 9048eba3..b7cade6f 100644 --- a/src/com/gitblit/wicket/pages/RepositoryPage.java +++ b/src/com/gitblit/wicket/pages/RepositoryPage.java @@ -248,7 +248,7 @@ public abstract class RepositoryPage extends BasePage { // user not allowed to fork or fork already exists or repo forbids forking add(new ExternalLink("forkLink", "").setVisible(false)); - if (user.canFork && !model.allowForks) { + if (user.canFork() && !model.allowForks) { // show forks prohibited indicator Fragment wc = new Fragment("forksProhibitedIndicator", "forksProhibitedFragment", this); Label lbl = new Label("forksProhibited", getString("gb.forksProhibited")); diff --git a/src/com/gitblit/wicket/pages/UserPage.java b/src/com/gitblit/wicket/pages/UserPage.java index e699d03f..d3e93c61 100644 --- a/src/com/gitblit/wicket/pages/UserPage.java +++ b/src/com/gitblit/wicket/pages/UserPage.java @@ -101,7 +101,7 @@ public class UserPage extends RootPage { add(new GravatarImage("gravatar", person, 210)); UserModel sessionUser = GitBlitWebSession.get().getUser(); - if (sessionUser != null && user.canCreate && sessionUser.equals(user)) { + if (sessionUser != null && user.canCreate() && sessionUser.equals(user)) { // user can create personal repositories add(new BookmarkablePageLink("newRepository", EditRepositoryPage.class)); } else { diff --git a/src/com/gitblit/wicket/panels/RepositoriesPanel.java b/src/com/gitblit/wicket/panels/RepositoriesPanel.java index 137fae6b..5678b092 100644 --- a/src/com/gitblit/wicket/panels/RepositoriesPanel.java +++ b/src/com/gitblit/wicket/panels/RepositoriesPanel.java @@ -92,7 +92,7 @@ public class RepositoriesPanel extends BasePanel { }.setVisible(GitBlit.getBoolean(Keys.git.cacheRepositoryList, true))); managementLinks.add(new BookmarkablePageLink("newRepository", EditRepositoryPage.class)); add(managementLinks); - } else if (showManagement && user != null && user.canCreate) { + } else if (showManagement && user != null && user.canCreate()) { // user can create personal repositories managementLinks = new Fragment("managementPanel", "personalLinks", this); managementLinks.add(new BookmarkablePageLink("newRepository", EditRepositoryPage.class)); diff --git a/src/com/gitblit/wicket/panels/UsersPanel.java b/src/com/gitblit/wicket/panels/UsersPanel.java index 6672c407..c5dba847 100644 --- a/src/com/gitblit/wicket/panels/UsersPanel.java +++ b/src/com/gitblit/wicket/panels/UsersPanel.java @@ -81,7 +81,7 @@ public class UsersPanel extends BasePanel { item.add(editLink); } - item.add(new Label("accesslevel", entry.canAdmin ? "administrator" : "")); + item.add(new Label("accesslevel", entry.canAdmin() ? "administrator" : "")); item.add(new Label("teams", entry.teams.size() > 0 ? ("" + entry.teams.size()) : "")); item.add(new Label("repositories", entry.repositories.size() > 0 ? ("" + entry.repositories.size()) : "")); diff --git a/test-users.conf b/test-users.conf index 5551e47a..4f947872 100644 --- a/test-users.conf +++ b/test-users.conf @@ -4,4 +4,5 @@ role = "#admin" role = "#notfederated" [team "admins"] + role = "#none" user = admin diff --git a/tests/com/gitblit/tests/PermissionsTest.java b/tests/com/gitblit/tests/PermissionsTest.java index 41ff5a63..2f47a489 100644 --- a/tests/com/gitblit/tests/PermissionsTest.java +++ b/tests/com/gitblit/tests/PermissionsTest.java @@ -2414,6 +2414,33 @@ public class PermissionsTest extends Assert { assertFalse("user CAN delete!", user.canDelete(repository)); assertFalse("user CAN edit!", user.canEdit(repository)); + } + + @Test + public void testAdminTeamInheritance() throws Exception { + UserModel user = new UserModel("test"); + TeamModel team = new TeamModel("team"); + team.canAdmin = true; + user.teams.add(team); + assertTrue("User did not inherit admin privileges", user.canAdmin()); + } + + @Test + public void testForkTeamInheritance() throws Exception { + UserModel user = new UserModel("test"); + TeamModel team = new TeamModel("team"); + team.canFork = true; + user.teams.add(team); + assertTrue("User did not inherit fork privileges", user.canFork()); + } + @Test + public void testCreateTeamInheritance() throws Exception { + UserModel user = new UserModel("test"); + TeamModel team = new TeamModel("team"); + team.canCreate= true; + user.teams.add(team); + assertTrue("User did not inherit create privileges", user.canCreate()); } + } -- 2.39.5