From a76796b3c67e47a0fa1dcadfd722d60b33821dab Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 3 Jan 2009 12:00:46 -0800 Subject: [PATCH] Create two magical groups 'Anonymous Users' and 'Registered Users' These groups represent everyone on Gerrit. Anonymous users is used for a user account when they are not yet signed in to the site. Registered users are used for any account which is signed in. Like the admin group, these groups are blessed by being marked in the system config table as the special groups. Since their membership is managed automatically by Gerrit we do not permit a group owner to edit the membership list in the web UI. We also don't show the membership, as the membership isn't stored in the database, its computed on the fly under the assumption that everyone is in either group if they have an Account.Id assigned. Signed-off-by: Shawn O. Pearce --- .../client/admin/AccountGroupDetail.java | 11 +++- .../client/admin/AccountGroupScreen.java | 23 +++++--- .../gerrit/client/reviewdb/SystemConfig.java | 8 +++ .../google/gerrit/server/GerritServer.java | 16 ++++++ .../gerrit/server/GroupAdminServiceImpl.java | 6 ++- .../com/google/gerrit/server/GroupCache.java | 54 +++++++++++++++++-- devutil/import_gerrit1.sql | 25 ++++++++- 7 files changed, 128 insertions(+), 15 deletions(-) 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);