diff --git a/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupDetail.java b/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupDetail.java index a70c826424..632505fda3 100644 --- a/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupDetail.java +++ b/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupDetail.java @@ -33,19 +33,28 @@ public class AccountGroupDetail { protected AccountGroup group; protected List members; protected AccountGroup ownerGroup; + protected boolean autoGroup; public AccountGroupDetail() { } public void load(final ReviewDb db, final AccountInfoCacheFactory acc, - final AccountGroup g) throws OrmException { + final AccountGroup g, final boolean isAuto) throws OrmException { group = g; if (group.getId().equals(group.getOwnerGroupId())) { ownerGroup = group; } else { ownerGroup = db.accountGroups().get(group.getOwnerGroupId()); } + autoGroup = isAuto; + if (!autoGroup) { + loadMembers(db, acc); + } + } + + private void loadMembers(final ReviewDb db, final AccountInfoCacheFactory acc) + throws OrmException { members = db.accountGroupMembers().byGroup(group.getId()).toList(); for (final AccountGroupMember m : members) { acc.want(m.getAccountId()); diff --git a/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java b/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java index e8933453f1..6c6d01c219 100644 --- a/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java +++ b/appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java @@ -31,6 +31,7 @@ import com.google.gwt.user.client.ui.ClickListener; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.FocusListenerAdapter; import com.google.gwt.user.client.ui.Label; +import com.google.gwt.user.client.ui.Panel; import com.google.gwt.user.client.ui.SourcesTableEvents; import com.google.gwt.user.client.ui.SuggestBox; import com.google.gwt.user.client.ui.TableListener; @@ -59,6 +60,7 @@ public class AccountGroupScreen extends AccountScreen { private TextArea descTxt; private Button saveDesc; + private Panel memberPanel; private Button addMember; private TextBox nameTxtBox; private SuggestBox nameTxt; @@ -241,10 +243,12 @@ public class AccountGroupScreen extends AccountScreen { } }); - add(memberHdr); - add(addPanel); - add(members); - add(delMember); + memberPanel = new FlowPanel(); + memberPanel.add(memberHdr); + memberPanel.add(addPanel); + memberPanel.add(members); + memberPanel.add(delMember); + add(memberPanel); } private void display(final AccountGroupDetail result) { @@ -252,9 +256,14 @@ public class AccountGroupScreen extends AccountScreen { groupNameTxt.setText(result.group.getName()); ownerTxt.setText(result.ownerGroup.getName()); descTxt.setText(result.group.getDescription()); - accounts = result.accounts; - members.display(result.members); - members.finishDisplay(true); + if (result.autoGroup) { + memberPanel.setVisible(false); + } else { + memberPanel.setVisible(true); + accounts = result.accounts; + members.display(result.members); + members.finishDisplay(true); + } } void doAddNew() { diff --git a/appjar/src/main/java/com/google/gerrit/client/reviewdb/SystemConfig.java b/appjar/src/main/java/com/google/gerrit/client/reviewdb/SystemConfig.java index d598ec4bf4..a413de91d6 100644 --- a/appjar/src/main/java/com/google/gerrit/client/reviewdb/SystemConfig.java +++ b/appjar/src/main/java/com/google/gerrit/client/reviewdb/SystemConfig.java @@ -91,6 +91,14 @@ public final class SystemConfig { @Column public AccountGroup.Id adminGroupId; + /** Identity of the anonymous group, which permits anyone. */ + @Column + public AccountGroup.Id anonymousGroupId; + + /** Identity of the registered users group, which permits anyone. */ + @Column + public AccountGroup.Id registeredGroupId; + protected SystemConfig() { } } diff --git a/appjar/src/main/java/com/google/gerrit/server/GerritServer.java b/appjar/src/main/java/com/google/gerrit/server/GerritServer.java index 0a9d72b18d..a9f00b48c7 100644 --- a/appjar/src/main/java/com/google/gerrit/server/GerritServer.java +++ b/appjar/src/main/java/com/google/gerrit/server/GerritServer.java @@ -158,11 +158,27 @@ public class GerritServer { admin.setDescription("Gerrit Site Administrators"); c.accountGroups().insert(Collections.singleton(admin)); + final AccountGroup anonymous = + new AccountGroup(new AccountGroup.NameKey("Anonymous Users"), + new AccountGroup.Id(c.nextAccountGroupId())); + anonymous.setDescription("Any user, signed-in or not"); + anonymous.setOwnerGroupId(admin.getId()); + c.accountGroups().insert(Collections.singleton(anonymous)); + + final AccountGroup registered = + new AccountGroup(new AccountGroup.NameKey("Registered Users"), + new AccountGroup.Id(c.nextAccountGroupId())); + registered.setDescription("Any signed-in user"); + registered.setOwnerGroupId(admin.getId()); + c.accountGroups().insert(Collections.singleton(registered)); + final SystemConfig s = SystemConfig.create(); s.xsrfPrivateKey = SignedToken.generateRandomKey(); s.accountPrivateKey = SignedToken.generateRandomKey(); s.sshdPort = 29418; s.adminGroupId = admin.getId(); + s.anonymousGroupId = anonymous.getId(); + s.registeredGroupId = registered.getId(); c.systemConfig().insert(Collections.singleton(s)); } diff --git a/appjar/src/main/java/com/google/gerrit/server/GroupAdminServiceImpl.java b/appjar/src/main/java/com/google/gerrit/server/GroupAdminServiceImpl.java index 94a7d13815..c050baed6b 100644 --- a/appjar/src/main/java/com/google/gerrit/server/GroupAdminServiceImpl.java +++ b/appjar/src/main/java/com/google/gerrit/server/GroupAdminServiceImpl.java @@ -108,7 +108,8 @@ public class GroupAdminServiceImpl extends BaseServiceImplementation implements } final AccountGroupDetail d = new AccountGroupDetail(); - d.load(db, new AccountInfoCacheFactory(db), group); + final boolean isAuto = groupCache.isAutoGroup(group.getId()); + d.load(db, new AccountInfoCacheFactory(db), group, isAuto); return d; } }); @@ -181,6 +182,9 @@ public class GroupAdminServiceImpl extends BaseServiceImplementation implements run(callback, new Action() { public AccountGroupDetail run(ReviewDb db) throws OrmException, Failure { assertAmGroupOwner(db, groupId); + if (groupCache.isAutoGroup(groupId)) { + throw new Failure(new NameAlreadyUsedException()); + } final Account a = findAccount(db, nameOrEmail); final AccountGroupMember.Key key = new AccountGroupMember.Key(a.getId(), groupId); diff --git a/appjar/src/main/java/com/google/gerrit/server/GroupCache.java b/appjar/src/main/java/com/google/gerrit/server/GroupCache.java index 940b70c02c..44b814ced0 100644 --- a/appjar/src/main/java/com/google/gerrit/server/GroupCache.java +++ b/appjar/src/main/java/com/google/gerrit/server/GroupCache.java @@ -32,6 +32,8 @@ import java.util.Set; public class GroupCache { private final SchemaFactory schema; private AccountGroup.Id adminGroupId; + private AccountGroup.Id anonymousGroupId; + private AccountGroup.Id registeredGroupId; private final LinkedHashMap> byAccount = new LinkedHashMap>(16, 0.75f, true) { @@ -45,6 +47,39 @@ public class GroupCache { protected GroupCache(final SchemaFactory rdf, final SystemConfig cfg) { schema = rdf; adminGroupId = cfg.adminGroupId; + anonymousGroupId = cfg.anonymousGroupId; + registeredGroupId = cfg.registeredGroupId; + } + + /** + * Is this group membership managed automatically by Gerrit? + * + * @param groupId the group to test. + * @return true if Gerrit handles this group membership automatically; false + * if it can be manually managed. + */ + public boolean isAutoGroup(final AccountGroup.Id groupId) { + return isAnonymousUsers(groupId) || isRegisteredUsers(groupId); + } + + /** + * Does this group designate the magical 'Anonymous Users' group? + * + * @param groupId the group to test. + * @return true if this is the magical 'Anonymous' group; false otherwise. + */ + public boolean isAnonymousUsers(final AccountGroup.Id groupId) { + return anonymousGroupId.equals(groupId); + } + + /** + * Does this group designate the magical 'Registered Users' group? + * + * @param groupId the group to test. + * @return true if this is the magical 'Registered' group; false otherwise. + */ + public boolean isRegisteredUsers(final AccountGroup.Id groupId) { + return registeredGroupId.equals(groupId); } /** @@ -67,8 +102,10 @@ public class GroupCache { */ public boolean isInGroup(final Account.Id accountId, final AccountGroup.Id groupId) { - final Set m = getGroups(accountId); - return m.contains(groupId); + if (isAnonymousUsers(groupId) || isRegisteredUsers(groupId)) { + return true; + } + return getGroups(accountId).contains(groupId); } /** @@ -88,6 +125,9 @@ public class GroupCache { * @param m the account-group pairing that was just inserted. */ public void notifyGroupAdd(final AccountGroupMember m) { + if (isAutoGroup(m.getAccountGroupId())) { + return; + } synchronized (byAccount) { final Set e = byAccount.get(m.getAccountId()); if (e != null) { @@ -104,6 +144,9 @@ public class GroupCache { * @param m the account-group pairing that was just deleted. */ public void notifyGroupDelete(final AccountGroupMember m) { + if (isAutoGroup(m.getAccountGroupId())) { + return; + } synchronized (byAccount) { final Set e = byAccount.get(m.getAccountId()); if (e != null) { @@ -137,15 +180,16 @@ public class GroupCache { accountId)) { m.add(g.getAccountGroupId()); } - m = Collections.unmodifiableSet(m); } finally { db.close(); } } catch (OrmException e) { - m = Collections.emptySet(); + m.clear(); } + m.add(anonymousGroupId); + m.add(registeredGroupId); synchronized (byAccount) { - byAccount.put(accountId, m); + byAccount.put(accountId, Collections.unmodifiableSet(m)); } return m; } diff --git a/devutil/import_gerrit1.sql b/devutil/import_gerrit1.sql index 20d1365b33..f05026f14f 100644 --- a/devutil/import_gerrit1.sql +++ b/devutil/import_gerrit1.sql @@ -155,6 +155,23 @@ INSERT INTO account_groups g.name FROM gerrit1.account_groups g; +INSERT INTO account_groups +(group_id, + description, + name) VALUES +(nextval('account_group_id'), +'Any user, signed-in or not', +'Anonymous Users'); + +INSERT INTO account_groups +(group_id, + description, + name) VALUES +(nextval('account_group_id'), +'Any signed-in user', +'Registered Users'); + + DELETE FROM account_group_members; INSERT INTO account_group_members (account_id, @@ -171,7 +188,13 @@ INSERT INTO account_group_members UPDATE system_config SET admin_group_id = (SELECT group_id FROM account_groups - WHERE name = 'admin'); + WHERE name = 'admin') +,anonymous_group_id = (SELECT group_id + FROM account_groups + WHERE name = 'Anonymous Users') +,registered_group_id = (SELECT group_id + FROM account_groups + WHERE name = 'Registered Users'); UPDATE account_groups SET owner_group_id = (SELECT admin_group_id FROM system_config);