diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index e500e4acb4..e259048ba2 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -11,35 +11,13 @@ users. Gerrit comes with the following system groups: -* Administrators * Anonymous Users * Change Owner -* Non-Interactive Users * Project Owners * Registered Users The system groups are assigned special access and membership management -privileges. The identity of these groups is set in the `system_config` -table within the database, so the groups can be renamed after installation -if desired. - - -[[administrators]] -=== Administrators - -This is the Gerrit "root" identity. - -Users in the 'Administrators' group can perform any action under -the Admin menu, to any group or project, without further validation -or any other access controls. In most installations only those -users who have direct filesystem and database access would be -placed into this group. - -Membership in the 'Administrators' group does not imply any other -access rights. Administrators do not automatically get code review -approval or submit rights in projects. This is a feature designed -to permit administrative users to otherwise access Gerrit as any -other normal user would, without needing two different accounts. +privileges. [[anonymous_users]] @@ -57,21 +35,6 @@ to grant `Read` access to this group as Gerrit requires an account identity for all other operations. -[[non-interactive_users]] -=== Non-Interactive Users - -This is an internal user group, members of this group are not expected -to perform interactive operations on the Gerrit web front-end. - -However, sometimes such a user may need a separate thread pool in -order to prevent it from grabbing threads from the interactive users. - -These users live in a second thread pool, which separates operations -made by the non-interactive users from the ones made by the interactive -users. This ensures that the interactive users can keep working when -resources are tight. - - [[project_owners]] === Project Owners @@ -120,6 +83,59 @@ Registered users are always permitted to make and publish comments on any change in any project they have `Read` access to. +== Predefined Groups + +Predefined groups differs from system groups by the fact that they +exist in the ACCOUNT_GROUPS table (like normal groups) but predefined groups +are created on Gerrit site initialization and unique UUIDs are assigned +to those groups. These UUIDs are different on different Gerrit sites. + +Gerrit comes with two predefined groups: + +* Administrators +* Non-Interactive Users + + +[[administrators]] +=== Administrators + +This is the Gerrit "root" identity. The capability +link:access-control.html#capability_administrateServer['Administrate Server'] +is assigned to this predefined group on Gerrit site creation. + +Users in the 'Administrators' group can perform any action under +the Admin menu, to any group or project, without further validation +or any other access controls. In most installations only those +users who have direct filesystem and database access would be +placed into this group. + +Membership in the 'Administrators' group does not imply any other +access rights. Administrators do not automatically get code review +approval or submit rights in projects. This is a feature designed +to permit administrative users to otherwise access Gerrit as any +other normal user would, without needing two different accounts. + + +[[non-interactive_users]] +=== Non-Interactive Users + +This is the Gerrit "batch" identity. The capabilities +link:access-control.html#capability_priority['Priority BATCH'] and +link:access-control.html#capability_streamEvents['Stream Events'] +are assigned to this predefined group on Gerrit site creation. + +The members of this group are not expected to perform interactive +operations on the Gerrit web front-end. + +However, sometimes such a user may need a separate thread pool in +order to prevent it from grabbing threads from the interactive users. + +These users live in a second thread pool, which separates operations +made by the non-interactive users from the ones made by the interactive +users. This ensures that the interactive users can keep working when +resources are tight. + + == Account Groups Account groups contain a list of zero or more user account members, diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt index f5f01c015c..b0e8098b8b 100644 --- a/Documentation/rest-api-accounts.txt +++ b/Documentation/rest-api-accounts.txt @@ -269,7 +269,7 @@ Retrieves the HTTP password of an account. Content-Type: application/json;charset=UTF-8 )]}' - "ETxgpih8xrNs" + "Qmxlc21ydCB1YmVyIGFsbGVzIGluIGRlciBXZWx0IQ" ---- If the account does not have an HTTP password the response is `404 Not Found`. diff --git a/ReleaseNotes/ReleaseNotes-2.10.txt b/ReleaseNotes/ReleaseNotes-2.10.txt index ebf51d2c6c..85aa848f14 100644 --- a/ReleaseNotes/ReleaseNotes-2.10.txt +++ b/ReleaseNotes/ReleaseNotes-2.10.txt @@ -76,8 +76,6 @@ Change Screen ^^^^^^^^^^^^^ -* Remove 'send email' checkbox from reply box on change screen. - * Do not linkify trailing dot or comma in messages. + As linkifying trailing dots and trailing commas does more harm than diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java index c6b6282e4a..8cba72e7fb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java @@ -45,7 +45,7 @@ public class PutHttpPassword implements RestModifyView { public boolean generate; } - private static final int LEN = 12; + private static final int LEN = 31; private static final SecureRandom rng; static { @@ -126,8 +126,8 @@ public class PutHttpPassword implements RestModifyView { rng.nextBytes(rand); byte[] enc = Base64.encodeBase64(rand, false); - StringBuilder r = new StringBuilder(LEN); - for (int i = 0; i < LEN; i++) { + StringBuilder r = new StringBuilder(enc.length); + for (int i = 0; i < enc.length; i++) { if (enc[i] == '=') { break; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 585c072567..9f03450cad 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -70,6 +70,7 @@ import com.google.gerrit.reviewdb.client.PatchSetInfo; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RevId; +import com.google.gerrit.reviewdb.client.SubmoduleSubscription; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalCopier; import com.google.gerrit.server.ApprovalsUtil; @@ -607,6 +608,17 @@ public class ReceiveCommits { break; case DELETE: + ResultSet submoduleSubscriptions = null; + Branch.NameKey projRef = new Branch.NameKey(project.getNameKey(), + c.getRefName()); + try { + submoduleSubscriptions = + db.submoduleSubscriptions().bySuperProject(projRef); + db.submoduleSubscriptions().delete(submoduleSubscriptions); + } catch (OrmException e) { + log.error("Cannot delete submodule subscription(s) of branch " + + projRef + ": " + submoduleSubscriptions, e); + } break; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java index 2e41894926..2b73ff972f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.git; import com.google.common.base.Strings; +import com.google.common.collect.Lists; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Account; @@ -233,23 +234,36 @@ public class SubmoduleOp { } // update subscribers of this module + List incorrectSubscriptions = Lists.newLinkedList(); for (final SubmoduleSubscription s : subscribers) { - if (!updatedSubscribers.add(s.getSuperProject())) { - log.error("Possible circular subscription involving " - + s.toString()); - } else { + try { + if (!updatedSubscribers.add(s.getSuperProject())) { + log.error("Possible circular subscription involving " + s); + } else { Map modules = new HashMap<>(1); - modules.put(updatedBranch, mergedCommit); + modules.put(updatedBranch, mergedCommit); Map paths = new HashMap<>(1); - paths.put(updatedBranch, s.getPath()); - - try { + paths.put(updatedBranch, s.getPath()); updateGitlinks(s.getSuperProject(), myRw, modules, paths, msgbuf); - } catch (SubmoduleException e) { - throw e; } + } catch (SubmoduleException e) { + log.warn("Cannot update gitlinks for " + s + " due to " + e.getMessage()); + incorrectSubscriptions.add(s); + } catch (Exception e) { + log.error("Cannot update gitlinks for " + s, e); + } + } + + if (!incorrectSubscriptions.isEmpty()) { + try { + schema.submoduleSubscriptions().delete(incorrectSubscriptions); + log.info("Deleted incorrect submodule subscription(s) " + + incorrectSubscriptions); + } catch (OrmException e) { + log.error("Cannot delete submodule subscription(s) " + + incorrectSubscriptions, e); } } } @@ -366,8 +380,8 @@ public class SubmoduleOp { // Recursive call: update subscribers of the subscriber updateSuperProjects(subscriber, recRw, commitId, msgbuf.toString()); } catch (IOException e) { - logAndThrowSubmoduleException("Cannot update gitlinks for " - + subscriber.get(), e); + throw new SubmoduleException("Cannot update gitlinks for " + + subscriber.get(), e); } finally { if (recRw != null) { recRw.release(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailHeader.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailHeader.java index d4dce703bb..46e151b2dd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailHeader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailHeader.java @@ -64,23 +64,47 @@ public abstract class EmailHeader { return false; } + static boolean needsQuotedPrintableWithinPhrase(final int cp) { + switch (cp) { + case '!': + case '*': + case '+': + case '-': + case '/': + case '=': + case '_': + return false; + default: + if (('a' <= cp && cp <= 'z') + || ('A' <= cp && cp <= 'Z') + || ('0' <= cp && cp <= '9')) { + return false; + } else { + return true; + } + } + } + static java.lang.String quotedPrintable(java.lang.String value) throws UnsupportedEncodingException { final StringBuilder r = new StringBuilder(); - final byte[] encoded = value.getBytes("UTF-8"); r.append("=?UTF-8?Q?"); - for (byte b : encoded) { - if (b == ' ') { + for (int i = 0; i < value.length(); i++) { + final int cp = value.codePointAt(i); + if (cp == ' ') { r.append('_'); - } else if (b == ',' || b == '=' || b == '"' || b == '_' || b < ' ' || '~' <= b) { - r.append('='); - r.append(Integer.toHexString((b >>> 4) & 0x0f).toUpperCase()); - r.append(Integer.toHexString(b & 0x0f).toUpperCase()); + } else if (needsQuotedPrintableWithinPhrase(cp)) { + byte[] buf = new java.lang.String(Character.toChars(cp)).getBytes("UTF-8"); + for (byte b: buf) { + r.append('='); + r.append(Integer.toHexString((b >>> 4) & 0x0f).toUpperCase()); + r.append(Integer.toHexString(b & 0x0f).toUpperCase()); + } } else { - r.append((char) b); + r.append(Character.toChars(cp)); } } r.append("?="); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java index 14069bd7c6..162a97dea5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java @@ -19,12 +19,14 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.client.SubmoduleSubscription; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.project.DeleteBranch.Input; import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -92,6 +94,9 @@ public class DeleteBranch implements RestModifyView{ case FORCED: referenceUpdated.fire(rsrc.getNameKey(), u); hooks.doRefUpdatedHook(rsrc.getBranchKey(), u, identifiedUser.get().getAccount()); + ResultSet submoduleSubscriptions = + dbProvider.get().submoduleSubscriptions().bySuperProject(rsrc.getBranchKey()); + dbProvider.get().submoduleSubscriptions().delete(submoduleSubscriptions); break; case REJECTED_CURRENT_BRANCH: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java index 3a5f55d901..62d625f0c9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.project; +import static com.google.gerrit.common.data.PermissionRule.Action.ALLOW; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Function; @@ -140,7 +141,7 @@ public class ProjectState { if (owner != null) { for (PermissionRule rule : owner.getRules()) { GroupReference ref = rule.getGroup(); - if (ref.getUUID() != null) { + if (rule.getAction() == ALLOW && ref.getUUID() != null) { groups.add(ref.getUUID()); } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java index b5c03e73bd..f4f989fc96 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java @@ -14,13 +14,10 @@ package com.google.gerrit.server.git; +import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.capture; -import static org.easymock.EasyMock.createNiceMock; -import static org.easymock.EasyMock.createStrictMock; import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; import com.google.gerrit.common.ChangeHooks; @@ -41,6 +38,8 @@ import com.google.gwtorm.server.StandardKeyEncoder; import com.google.inject.Provider; import org.easymock.Capture; +import org.easymock.EasyMock; +import org.easymock.IMocksControl; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.dircache.DirCacheBuilder; import org.eclipse.jgit.dircache.DirCacheEntry; @@ -75,6 +74,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { private static final String newLine = System.getProperty("line.separator"); + private IMocksControl mockMaker; private SchemaFactory schemaFactory; private SubmoduleSubscriptionAccess subscriptions; private ReviewDb schema; @@ -89,23 +89,22 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { public void setUp() throws Exception { super.setUp(); - schemaFactory = createStrictMock(SchemaFactory.class); - schema = createStrictMock(ReviewDb.class); - subscriptions = createStrictMock(SubmoduleSubscriptionAccess.class); - urlProvider = createStrictMock(Provider.class); - repoManager = createStrictMock(GitRepositoryManager.class); - gitRefUpdated = createStrictMock(GitReferenceUpdated.class); - changeHooks = createNiceMock(ChangeHooks.class); + mockMaker = EasyMock.createStrictControl(); + schemaFactory = mockMaker.createMock(SchemaFactory.class); + schema = mockMaker.createMock(ReviewDb.class); + subscriptions = mockMaker.createMock(SubmoduleSubscriptionAccess.class); + urlProvider = mockMaker.createMock(Provider.class); + repoManager = mockMaker.createMock(GitRepositoryManager.class); + gitRefUpdated = mockMaker.createMock(GitReferenceUpdated.class); + changeHooks = mockMaker.createMock(ChangeHooks.class); } private void doReplay() { - replay(schemaFactory, schema, subscriptions, urlProvider, repoManager, - gitRefUpdated); + mockMaker.replay(); } private void doVerify() { - verify(schemaFactory, schema, subscriptions, urlProvider, repoManager, - gitRefUpdated); + mockMaker.verify(); } /** @@ -640,6 +639,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { Capture ruCapture = new Capture<>(); gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()), capture(ruCapture)); + changeHooks.doRefUpdatedHook(eq(targetBranchNameKey), + anyObject(RefUpdate.class), EasyMock.isNull()); expect(schema.submoduleSubscriptions()).andReturn(subscriptions); final ResultSet emptySubscriptions = @@ -743,6 +744,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { Capture ruCapture = new Capture<>(); gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()), capture(ruCapture)); + changeHooks.doRefUpdatedHook(eq(targetBranchNameKey), + anyObject(RefUpdate.class), EasyMock.isNull()); expect(schema.submoduleSubscriptions()).andReturn(subscriptions); final ResultSet incorrectSubscriptions = diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/AddressTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/AddressTest.java index d8ff54328f..c77e0f6925 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/AddressTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/AddressTest.java @@ -130,6 +130,12 @@ public class AddressTest { assertEquals("=?UTF-8?Q?A_=E2=82=AC_B?= ", format("A \u20ac B", "a@a")); } + @Test + public void testToHeaderString_NameEmail7() { + assertEquals("=?UTF-8?Q?A_=E2=82=AC_B_=28Code_Review=29?= ", + format("A \u20ac B (Code Review)", "a@a")); + } + @Test public void testToHeaderString_Email1() { assertEquals("a@a", format(null, "a@a")); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index bbb9be477a..6f159fccca 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -79,11 +79,23 @@ public class RefControlTest { public void testOwnerProject() { allow(local, OWNER, ADMIN, "refs/*"); - ProjectControl uBlah = util.user(local, DEVS); - ProjectControl uAdmin = util.user(local, DEVS, ADMIN); + assertAdminsAreOwnersAndDevsAreNot(); + } - assertFalse("not owner", uBlah.isOwner()); - assertTrue("is owner", uAdmin.isOwner()); + @Test + public void testDenyOwnerProject() { + allow(local, OWNER, ADMIN, "refs/*"); + deny(local, OWNER, DEVS, "refs/*"); + + assertAdminsAreOwnersAndDevsAreNot(); + } + + @Test + public void testBlockOwnerProject() { + allow(local, OWNER, ADMIN, "refs/*"); + block(local, OWNER, DEVS, "refs/*"); + + assertAdminsAreOwnersAndDevsAreNot(); } @Test @@ -527,4 +539,12 @@ public class RefControlTest { assertFalse("u can vote -2", range.contains(-2)); assertFalse("u can vote +2", range.contains(2)); } + + private void assertAdminsAreOwnersAndDevsAreNot() { + ProjectControl uBlah = util.user(local, DEVS); + ProjectControl uAdmin = util.user(local, DEVS, ADMIN); + + assertFalse("not owner", uBlah.isOwner()); + assertTrue("is owner", uAdmin.isOwner()); + } }