Merge branch 'stable-3.3'

* stable-3.3:
  e2e-tests: Add a CreateBranch scenario
  Adapt CreateChange and DeleteChange to perform load testing
  Adapt CloneUsingBothProtocols to perform load testing
  Set version to 3.1.13-SNAPSHOT
  Set version to 3.0.17-SNAPSHOT
  Set version to 2.15.23-SNAPSHOT
  Set version to 3.2.8-SNAPSHOT
  Set version to 3.3.3-SNAPSHOT
  Set version to 3.3.2
  Set version to 3.2.7
  Set version to 2.16.27-SNAPSHOT
  Set version to 3.1.12
  Set version to 3.0.16
  Set version to 2.16.26
  Do not remove Change-Id footer in test
  Use Change-Id footer for updating commit msg in RevisionDiffIT
  Avoid magic values in RevisionDiffIT
  Set version to 2.15.22
  Update git submodules
  Update git submodules
  Update git submodules
  Update git submodules
  Move ensureChangeIdIsCorrect from PutMessage to ChangeUtil
  Avoid creating HTTP Sessions for Git-over-HTTP
  Avoid creating HTTP Sessions for Git-over-HTTP
  Avoid creating HTTP Sessions for Git-over-HTTP
  Avoid creating HTTP Sessions for Git-over-HTTP
  Avoid creating HTTP Sessions for Git-over-HTTP
  Avoid creating HTTP Sessions for Git-over-HTTP
  e2e-tests: Init executable README for local setup testing
  Update git submodules
  Disallow editing the Change-Id during inline edits
  Fix badly formatted error message shown in error dialog
  Documentation: enhance attention set / Service Users
  ForRef#check should permit internal users to read all refs

Change-Id: Ie67d03c7830aae38a66f769ea63e9e2ce52f1d2f

Documentation/access-control.txt

@@ -115,8 +115,9 @@ the 'Administrate Server' capability from it, its members are no longer
 Gerrit administrators, despite the group name. The group may also be
 renamed.
 
+anchor:non-interactive_users[]
 
-[[non-interactive_users]]
+[[service_users]]
 === Service Users
 
 This is the Gerrit "batch" identity. The capabilities
@@ -135,6 +136,7 @@ made by the service users from the ones made by the interactive
 users. This ensures that the interactive users can keep working when
 resources are tight.
 
+Before Gerrit 3.3, the 'Service Users' group was named 'Non-Interactive Users'.
 
 == Account Groups
 

Documentation/dev-e2e-tests.txt

@@ -232,6 +232,17 @@ than the default, say `0.8`, will make scenarios wait somewhat less than how the
 Scenario development is often done using locally running Gerrit systems under test, which are
 sometimes dockerized.
 
+==== Number of users
+
+The `number_of_users` property can be used to scale scenario steps to run with the specified number
+of concurrent users. The value of this property remains `1` by default. For example, this sets the
+number of concurrent users to 10:
+
+* `-Dcom.google.gerrit.scenarios.number_of_users=10`
+
+This will make scenarios that support the `number_of_users` property to inject that many users
+concurrently for load testing.
+
 == How to run tests
 
 Run all tests:

Documentation/user-attention-set.txt

@@ -160,10 +160,14 @@ The Attention Set has been available since the 3.3 release (late 2020). It
 is enabled by default, but you can disable it by setting
 link:config-gerrit.html#change.enableAttentionSet[enableAttentionSet] to false.
 
+As part of Gerrit 3.3 upgrade, the user group "Non-Interactive Users" is
+renamed "Service Users". For a new installation, the group is automatically
+created upon initialization.
+
 === Important note for all host owners, project owners, and bot owners
 
 If you are a host/project owner, please make sure all bots that run against your
-host/project are part of the "Service Users" group.
+host/project are part of the link:access-control.html#service_users[Service Users] group.
 
 If you are a bot owner, please make sure your bot is part of the "Service Users"
 group on all hosts it runs on.

e2e-tests/README

@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# Example usage only-
+# 1. Optional: replace test@mail.com below with your own, reachable locally.
+# 2. Use the '>>' operator below instead to not overwrite your known_hosts; keep '>' otherwise.
+# 3. Note that appending as proposed above may potentially repeat the same line multiple times.
+# 4. Init your local Gerrit test site then start it; you may refer to [1] below.
+# 5. Set GIT_HTTP_PASSWORD below to yours, from [2].
+# 6. Change to this directory to execute ./README (this executable file) in its own terminal.
+# 7. Install sbt if missing, based on your operating system; re-run to compile.
+# 8. Optional: add the below generated (displayed) key to your local admin user [3].
+# 9. Otherwise keep the lines below that use your existing user ssh keys for admin testing.
+# 10. This script assumes the google-sourced version of the example json file [4].
+# 11. If running that scenario locally as below reports authentication failures, [4] may be a fork.
+# 12. Uncomment any one of the below sbt commands at will; you may add some locally.
+# 13. See [5] for how to start using JAVA_OPTS below; you may leave it empty for these sbt commands.
+# 14. You can initialize an IDE sbt (Scala) project from/in this root folder; see [6].
+#
+# [1] https://gerrit-review.googlesource.com/Documentation/dev-readme.html#init
+# [2] http://localhost:8080/settings/#HTTPCredentials
+# [3] http://localhost:8080/settings/#SSHKeys
+# [4] ./src/test/resources/data/com/google/gerrit/scenarios/CloneUsingBothProtocols.json
+# [5] https://gerrit-review.googlesource.com/Documentation/dev-e2e-tests.html#_environment_properties
+# [6] https://gerrit-review.googlesource.com/Documentation/dev-e2e-tests.html#_ide_intellij
+
+# DO NOT change this (assumed) directory; force-removed *recursively* below!
+gatlingGitKeys=/tmp/ssh-keys
+
+userSshDir=$HOME/.ssh
+
+# Comment this group of lines out if willing to generate other keys as below.
+rm -f $gatlingGitKeys
+ln -s "$userSshDir" $gatlingGitKeys
+
+# Comment this group of lines out if keys already generated, as either below or above.
+#rm -fr $gatlingGitKeys
+#mkdir $gatlingGitKeys
+#ssh-keygen -m PEM -t rsa -C "test@mail.com" -f $gatlingGitKeys/id_rsa
+
+ssh-keyscan -t rsa -p 29418 localhost > "$userSshDir"/known_hosts
+cat $gatlingGitKeys/id_rsa.pub
+
+export GIT_HTTP_USERNAME="admin"
+export GIT_HTTP_PASSWORD="TODO"
+export JAVA_OPTS="\
+"
+#-Dx=y \
+
+#sbt clean
+#sbt update
+sbt compile
+#sbt "gatling:testOnly com.google.gerrit.scenarios.CloneUsingBothProtocols"
+#sbt "gatling:lastReport"

e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch-body.json

@@ -0,0 +1,3 @@
+{
+  "revision": "master"
+}

e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch.json

@@ -0,0 +1,6 @@
+[
+  {
+    "url": "HTTP_SCHEME://HOSTNAME:HTTP_PORT/a/projects/PROJECT/branches/",
+    "project": "PROJECT"
+  }
+]

e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala

@@ -21,9 +21,9 @@ import io.gatling.core.structure.ScenarioBuilder
 import scala.concurrent.duration._
 
 class CloneUsingBothProtocols extends GitSimulation {
-  private val data: FeederBuilder = jsonFile(resource).convert(keys).queue
+  private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
   private val projectName = className
-  private val duration = 2
+  private val duration = 2 * numberOfUsers
 
   override def replaceOverride(in: String): String = {
     replaceKeyWith("_project", projectName, in)
@@ -43,7 +43,7 @@ class CloneUsingBothProtocols extends GitSimulation {
     ),
     test.inject(
       nothingFor(stepWaitTime(this) seconds),
-      constantUsersPerSec(single) during (duration seconds)
+      constantUsersPerSec(numberOfUsers) during (duration seconds)
     ).protocols(gitProtocol),
     deleteProject.test.inject(
       nothingFor(stepWaitTime(deleteProject) + duration seconds),

e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateBranch.scala

@@ -0,0 +1,56 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.scenarios
+
+import io.gatling.core.Predef.{atOnceUsers, _}
+import io.gatling.core.feeder.FeederBuilder
+import io.gatling.core.structure.ScenarioBuilder
+import io.gatling.http.Predef._
+
+import scala.concurrent.duration._
+
+class CreateBranch extends ProjectSimulation {
+  private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
+  private val branchIdKey = "branchId"
+  private var counter = 0
+
+  private val test: ScenarioBuilder = scenario(uniqueName)
+      .feed(data)
+      .exec(session => {
+        counter += 1
+        session.set(branchIdKey, "branch-" + counter)
+      })
+      .exec(http(uniqueName)
+          .post("${url}${" + branchIdKey + "}")
+          .body(ElFileBody(body)).asJson)
+
+  private val createProject = new CreateProject(projectName)
+  private val deleteProject = new DeleteProject(projectName)
+
+  setUp(
+    createProject.test.inject(
+      nothingFor(stepWaitTime(createProject) seconds),
+      atOnceUsers(single)
+    ),
+    test.inject(
+      nothingFor(stepWaitTime(this) seconds),
+      atOnceUsers(numberOfUsers)
+    ),
+    deleteProject.test.inject(
+      nothingFor(stepWaitTime(deleteProject) seconds),
+      atOnceUsers(single)
+    ),
+  ).protocols(httpProtocol)
+}

e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateChange.scala

@@ -19,12 +19,14 @@ import io.gatling.core.feeder.FeederBuilder
 import io.gatling.core.structure.ScenarioBuilder
 import io.gatling.http.Predef._
 
+import scala.collection.mutable
 import scala.concurrent.duration._
 
 class CreateChange extends ProjectSimulation {
-  private val data: FeederBuilder = jsonFile(resource).convert(keys).queue
+  private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
   private val numberKey = "_number"
   var number = 0
+  var numbers: mutable.Queue[Int] = mutable.Queue[Int]()
 
   override def relativeRuntimeWeight = 2
 
@@ -40,6 +42,7 @@ class CreateChange extends ProjectSimulation {
           .check(regex("\"" + numberKey + "\":(\\d+),").saveAs(numberKey)))
       .exec(session => {
         number = session(numberKey).as[Int]
+        numbers += number
         session
       })
 
@@ -54,11 +57,11 @@ class CreateChange extends ProjectSimulation {
     ),
     test.inject(
       nothingFor(stepWaitTime(this) seconds),
-      atOnceUsers(single)
+      atOnceUsers(numberOfUsers)
     ),
     deleteChange.test.inject(
       nothingFor(stepWaitTime(deleteChange) seconds),
-      atOnceUsers(single)
+      atOnceUsers(numberOfUsers)
     ),
     deleteProject.test.inject(
       nothingFor(stepWaitTime(deleteProject) seconds),

e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteChange.scala

@@ -20,7 +20,7 @@ import io.gatling.core.structure.ScenarioBuilder
 import io.gatling.http.Predef.http
 
 class DeleteChange extends GerritSimulation {
-  private val data: FeederBuilder = jsonFile(resource).convert(keys).queue
+  private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
   private var createChange: Option[CreateChange] = None
 
   override def relativeRuntimeWeight = 2
@@ -34,7 +34,7 @@ class DeleteChange extends GerritSimulation {
       .feed(data)
       .exec(session => {
         if (createChange.nonEmpty) {
-          session.set("number", createChange.get.number)
+          session.set("number", createChange.get.numbers.dequeue())
         } else {
           session
         }

e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala

@@ -34,6 +34,7 @@ class GerritSimulation extends Simulation {
   protected val uniqueName: String = className + "-" + hashCode()
   protected val single = 1
 
+  val numberOfUsers: Int = replaceProperty("number_of_users", single).toInt
   val replicationDelay: Int = replaceProperty("replication_delay", 15).toInt
   private val powerFactor = replaceProperty("power_factor", 1.0).toDouble
   protected val SecondsPerWeightUnit = 2

java/com/google/gerrit/acceptance/GerritServer.java

@@ -590,7 +590,7 @@ public class GerritServer implements AutoCloseable {
     httpAddress = new InetSocketAddress(uri.getHost(), uri.getPort());
   }
 
-  String getUrl() {
+  public String getUrl() {
     return url;
   }
 
@@ -602,6 +602,10 @@ public class GerritServer implements AutoCloseable {
     return testInjector;
   }
 
+  public Injector getHttpdInjector() {
+    return daemon.getHttpdInjector();
+  }
+
   Description getDescription() {
     return desc;
   }

java/com/google/gerrit/httpd/GitOverHttpServlet.java

@@ -360,6 +360,7 @@ public class GitOverHttpServlet extends GitServlet {
     private final Metrics metrics;
     private final PluginSetContext<RequestListener> requestListeners;
     private final UsersSelfAdvertiseRefsHook usersSelfAdvertiseRefsHook;
+    private final Provider<WebSession> sessionProvider;
 
     @Inject
     UploadFilter(
@@ -369,7 +370,8 @@ public class GitOverHttpServlet extends GitServlet {
         GroupAuditService groupAuditService,
         Metrics metrics,
         PluginSetContext<RequestListener> requestListeners,
-        UsersSelfAdvertiseRefsHook usersSelfAdvertiseRefsHook) {
+        UsersSelfAdvertiseRefsHook usersSelfAdvertiseRefsHook,
+        Provider<WebSession> sessionProvider) {
       this.uploadValidatorsFactory = uploadValidatorsFactory;
       this.permissionBackend = permissionBackend;
       this.userProvider = userProvider;
@@ -377,6 +379,7 @@ public class GitOverHttpServlet extends GitServlet {
       this.metrics = metrics;
       this.requestListeners = requestListeners;
       this.usersSelfAdvertiseRefsHook = usersSelfAdvertiseRefsHook;
+      this.sessionProvider = sessionProvider;
     }
 
     @Override
@@ -392,7 +395,7 @@ public class GitOverHttpServlet extends GitServlet {
       HttpServletResponseWithStatusWrapper responseWrapper =
           new HttpServletResponseWithStatusWrapper((HttpServletResponse) response);
       HttpServletRequest httpRequest = (HttpServletRequest) request;
-      String sessionId = httpRequest.getSession().getId();
+      String sessionId = getSessionIdOrNull(sessionProvider);
 
       try (TraceContext traceContext = TraceContext.open()) {
         RequestInfo requestInfo =
@@ -495,6 +498,7 @@ public class GitOverHttpServlet extends GitServlet {
     private final Provider<CurrentUser> userProvider;
     private final GroupAuditService groupAuditService;
     private final Metrics metrics;
+    private final Provider<WebSession> sessionProvider;
 
     @Inject
     ReceiveFilter(
@@ -502,12 +506,14 @@ public class GitOverHttpServlet extends GitServlet {
         PermissionBackend permissionBackend,
         Provider<CurrentUser> userProvider,
         GroupAuditService groupAuditService,
-        Metrics metrics) {
+        Metrics metrics,
+        Provider<WebSession> sessionProvider) {
       this.cache = cache;
       this.permissionBackend = permissionBackend;
       this.userProvider = userProvider;
       this.groupAuditService = groupAuditService;
       this.metrics = metrics;
+      this.sessionProvider = sessionProvider;
     }
 
     @Override
@@ -547,7 +553,7 @@ public class GitOverHttpServlet extends GitServlet {
       } finally {
         groupAuditService.dispatch(
             new HttpAuditEvent(
-                httpRequest.getSession().getId(),
+                getSessionIdOrNull(sessionProvider),
                 userProvider.get(),
                 extractWhat(httpRequest),
                 TimeUtil.nowMs(),
@@ -603,4 +609,12 @@ public class GitOverHttpServlet extends GitServlet {
     @Override
     public void destroy() {}
   }
+
+  private static String getSessionIdOrNull(Provider<WebSession> sessionProvider) {
+    WebSession session = sessionProvider.get();
+    if (session.isSignedIn()) {
+      return session.getSessionId();
+    }
+    return null;
+  }
 }

java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -47,7 +47,6 @@ import static javax.servlet.http.HttpServletResponse.SC_OK;
 import static javax.servlet.http.HttpServletResponse.SC_PRECONDITION_FAILED;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.CharMatcher;
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
@@ -141,7 +140,6 @@ import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonParseException;
-import com.google.gson.JsonPrimitive;
 import com.google.gson.stream.JsonReader;
 import com.google.gson.stream.JsonToken;
 import com.google.gson.stream.JsonWriter;
@@ -1879,10 +1877,6 @@ public class RestApiServlet extends HttpServlet {
   static long replyText(
       @Nullable HttpServletRequest req, HttpServletResponse res, boolean allowTracing, String text)
       throws IOException {
-    if ((req == null || isRead(req)) && isMaybeHTML(text)) {
-      return replyJson(
-          req, res, allowTracing, ImmutableListMultimap.of("pp", "0"), new JsonPrimitive(text));
-    }
     if (!text.endsWith("\n")) {
       text += "\n";
     }
@@ -1892,10 +1886,6 @@ public class RestApiServlet extends HttpServlet {
     return replyBinaryResult(req, res, BinaryResult.create(text).setContentType(PLAIN_TEXT));
   }
 
-  private static boolean isMaybeHTML(String text) {
-    return CharMatcher.anyOf("<&").matchesAnyOf(text);
-  }
-
   private static boolean acceptsGzip(HttpServletRequest req) {
     if (req != null) {
       String accepts = req.getHeader(HttpHeaders.ACCEPT_ENCODING);

java/com/google/gerrit/pgm/Daemon.java

@@ -240,6 +240,11 @@ public class Daemon extends SiteProgram {
     this.replica = replica;
   }
 
+  @VisibleForTesting
+  public Injector getHttpdInjector() {
+    return httpdInjector;
+  }
+
   @Override
   public int run() throws Exception {
     if (stopOnly) {

java/com/google/gerrit/pgm/http/jetty/JettyServer.java

@@ -17,6 +17,7 @@ package com.google.gerrit.pgm.http.jetty;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.gerrit.extensions.client.AuthType;
 import com.google.gerrit.extensions.events.LifecycleListener;
@@ -42,8 +43,11 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 import javax.servlet.DispatcherType;
 import javax.servlet.Filter;
+import javax.servlet.http.HttpSessionEvent;
+import javax.servlet.http.HttpSessionListener;
 import org.eclipse.jetty.http.HttpScheme;
 import org.eclipse.jetty.io.ConnectionStatistics;
 import org.eclipse.jetty.jmx.MBeanContainer;
@@ -200,6 +204,8 @@ public class JettyServer {
   private final Metrics metrics;
   private boolean reverseProxy;
   private ConnectionStatistics connStats;
+  private final SessionHandler sessionHandler;
+  private final AtomicLong sessionsCounter;
 
   @Inject
   JettyServer(
@@ -218,8 +224,27 @@ public class JettyServer {
       connector.addBean(connStats);
     }
     metrics = new Metrics(pool, connStats);
+    sessionHandler = new SessionHandler();
+    sessionsCounter = new AtomicLong();
 
-    Handler app = makeContext(env, cfg);
+    /* Code used for testing purposes for making assertions
+     * on the number of active HTTP sessions.
+     */
+    sessionHandler.addEventListener(
+        new HttpSessionListener() {
+
+          @Override
+          public void sessionDestroyed(HttpSessionEvent se) {
+            sessionsCounter.decrementAndGet();
+          }
+
+          @Override
+          public void sessionCreated(HttpSessionEvent se) {
+            sessionsCounter.incrementAndGet();
+          }
+        });
+
+    Handler app = makeContext(env, cfg, sessionHandler);
     if (cfg.getBoolean("httpd", "requestLog", !reverseProxy)) {
       RequestLogHandler handler = new RequestLogHandler();
       handler.setRequestLog(httpLogFactory.get());
@@ -246,6 +271,11 @@ public class JettyServer {
     httpd.setStopAtShutdown(false);
   }
 
+  @VisibleForTesting
+  public long numActiveSessions() {
+    return sessionsCounter.longValue();
+  }
+
   Metrics getMetrics() {
     return metrics;
   }
@@ -445,7 +475,7 @@ public class JettyServer {
     return pool;
   }
 
-  private Handler makeContext(JettyEnv env, Config cfg) {
+  private Handler makeContext(JettyEnv env, Config cfg, SessionHandler sessionHandler) {
     final Set<String> paths = new HashSet<>();
     for (URI u : listenURLs(cfg)) {
       String p = u.getPath();
@@ -460,7 +490,7 @@ public class JettyServer {
 
     final List<ContextHandler> all = new ArrayList<>();
     for (String path : paths) {
-      all.add(makeContext(path, env, cfg));
+      all.add(makeContext(path, env, cfg, sessionHandler));
     }
 
     if (all.size() == 1) {
@@ -478,13 +508,14 @@ public class JettyServer {
     return r;
   }
 
-  private ContextHandler makeContext(final String contextPath, JettyEnv env, Config cfg) {
+  private ContextHandler makeContext(
+      final String contextPath, JettyEnv env, Config cfg, SessionHandler sessionHandler) {
     final ServletContextHandler app = new ServletContextHandler();
 
     // This enables the use of sessions in Jetty, feature available
     // for Gerrit plug-ins to enable user-level sessions.
     //
-    app.setSessionHandler(new SessionHandler());
+    app.setSessionHandler(sessionHandler);
     app.setErrorHandler(new HiddenErrorHandler());
 
     // This is the path we are accessed by clients within our domain.

java/com/google/gerrit/server/ChangeUtil.java

@@ -22,7 +22,10 @@ import com.google.common.io.BaseEncoding;
 import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.server.config.UrlFormatter;
+import com.google.gerrit.server.util.CommitMessageUtil;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import java.security.SecureRandom;
@@ -34,6 +37,8 @@ import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -109,6 +114,37 @@ public class ChangeUtil {
         id);
   }
 
+  /**
+   * Make sure that the change commit message has a correct footer.
+   *
+   * @param requireChangeId true if Change-Id is a mandatory footer for the project
+   * @param currentChangeId current Change-Id value before the commit message is updated
+   * @param newCommitMessage new commit message for the change
+   * @throws ResourceConflictException if the new commit message has a missing or invalid Change-Id
+   * @throws BadRequestException if the new commit message is null or empty
+   */
+  public static void ensureChangeIdIsCorrect(
+      boolean requireChangeId, String currentChangeId, String newCommitMessage)
+      throws ResourceConflictException, BadRequestException {
+    RevCommit revCommit =
+        RevCommit.parse(
+            Constants.encode("tree " + ObjectId.zeroId().name() + "\n\n" + newCommitMessage));
+
+    // Check that the commit message without footers is not empty
+    CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
+
+    List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
+    if (requireChangeId && changeIdFooters.isEmpty()) {
+      throw new ResourceConflictException("missing Change-Id footer");
+    }
+    if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
+      throw new ResourceConflictException("wrong Change-Id footer");
+    }
+    if (changeIdFooters.size() > 1) {
+      throw new ResourceConflictException("multiple Change-Id footers");
+    }
+  }
+
   public static String status(Change c) {
     return c != null ? c.getStatus().name().toLowerCase() : "deleted";
   }

java/com/google/gerrit/server/edit/ChangeEditModifier.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.edit;
 import static com.google.gerrit.server.project.ProjectCache.illegalState;
 
 import com.google.common.base.Charsets;
+import com.google.gerrit.entities.BooleanProjectConfig;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
@@ -367,8 +368,15 @@ public class ChangeEditModifier {
     PatchSet basePatchset = modificationTarget.getBasePatchset();
     RevCommit basePatchsetCommit = NoteDbEdits.lookupCommit(repository, basePatchset.commitId());
 
+    boolean changeIdRequired =
+        projectCache
+            .get(notes.getChange().getProject())
+            .orElseThrow(illegalState(notes.getChange().getProject()))
+            .is(BooleanProjectConfig.REQUIRE_CHANGE_ID);
+    String currentChangeId = notes.getChange().getKey().get();
     String newCommitMessage =
-        createNewCommitMessage(editBehavior, commitModification, commitToModify);
+        createNewCommitMessage(
+            changeIdRequired, currentChangeId, editBehavior, commitModification, commitToModify);
     newCommitMessage = editBehavior.mergeCommitMessageIfNecessary(newCommitMessage, commitToModify);
 
     Optional<ChangeEdit> unmodifiedEdit =
@@ -464,8 +472,12 @@ public class ChangeEditModifier {
   }
 
   private String createNewCommitMessage(
-      EditBehavior editBehavior, CommitModification commitModification, RevCommit commitToModify)
-      throws InvalidChangeOperationException, BadRequestException {
+      boolean requireChangeId,
+      String currentChangeId,
+      EditBehavior editBehavior,
+      CommitModification commitModification,
+      RevCommit commitToModify)
+      throws InvalidChangeOperationException, BadRequestException, ResourceConflictException {
     if (!commitModification.newCommitMessage().isPresent()) {
       return editBehavior.getUnmodifiedCommitMessage(commitToModify);
     }
@@ -479,6 +491,8 @@ public class ChangeEditModifier {
           "New commit message cannot be same as existing commit message");
     }
 
+    ChangeUtil.ensureChangeIdIsCorrect(requireChangeId, currentChangeId, newCommitMessage);
+
     return newCommitMessage;
   }
 

java/com/google/gerrit/server/restapi/change/PutMessage.java

@@ -16,7 +16,6 @@ package com.google.gerrit.server.restapi.change;
 
 import static com.google.gerrit.server.project.ProjectCache.illegalState;
 
-import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.entities.BooleanProjectConfig;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -51,11 +50,9 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import java.sql.Timestamp;
-import java.util.List;
 import java.util.TimeZone;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.CommitBuilder;
-import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
@@ -116,14 +113,13 @@ public class PutMessage implements RestModifyView<ChangeResource, CommitMessageI
     String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
 
     ensureCanEditCommitMessage(resource.getNotes());
-    sanitizedCommitMessage =
-        ensureChangeIdIsCorrect(
-            projectCache
-                .get(resource.getProject())
-                .orElseThrow(illegalState(resource.getProject()))
-                .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
-            resource.getChange().getKey().get(),
-            sanitizedCommitMessage);
+    ChangeUtil.ensureChangeIdIsCorrect(
+        projectCache
+            .get(resource.getProject())
+            .orElseThrow(illegalState(resource.getProject()))
+            .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+        resource.getChange().getKey().get(),
+        sanitizedCommitMessage);
 
     try (Repository repository = repositoryManager.openRepository(resource.getProject());
         RevWalk revWalk = new RevWalk(repository);
@@ -204,33 +200,4 @@ public class PutMessage implements RestModifyView<ChangeResource, CommitMessageI
       throw new AuthException("modifying commit message not permitted", denied);
     }
   }
-
-  private String ensureChangeIdIsCorrect(
-      boolean requireChangeId, String currentChangeId, String newCommitMessage)
-      throws ResourceConflictException, BadRequestException {
-    RevCommit revCommit =
-        RevCommit.parse(
-            Constants.encode("tree " + ObjectId.zeroId().name() + "\n\n" + newCommitMessage));
-
-    // Check that the commit message without footers is not empty
-    CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
-
-    List<String> changeIdFooters = ChangeUtil.getChangeIdsFromFooter(revCommit, urlFormatter.get());
-    if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
-      throw new ResourceConflictException("wrong Change-Id footer");
-    }
-
-    if (requireChangeId && revCommit.getFooterLines().isEmpty()) {
-      // sanitization always adds '\n' at the end.
-      newCommitMessage += "\n";
-    }
-
-    if (requireChangeId && changeIdFooters.isEmpty()) {
-      newCommitMessage += FooterConstants.CHANGE_ID.getName() + ": " + currentChangeId + "\n";
-    } else if (changeIdFooters.size() > 1) {
-      throw new ResourceConflictException("multiple Change-Id footers");
-    }
-
-    return newCommitMessage;
-  }
 }

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -3815,16 +3815,6 @@ public class ChangeIT extends AbstractDaemonTest {
     assertThat(getCommitMessage(r.getChangeId())).isEqualTo(newMessage);
   }
 
-  @Test
-  public void changeCommitMessageWithNoChangeIdRetainsChangeID() throws Exception {
-    PushOneCommit.Result r = createChange();
-    assertThat(getCommitMessage(r.getChangeId()))
-        .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
-    gApi.changes().id(r.getChangeId()).setMessage("modified commit\n");
-    assertThat(getCommitMessage(r.getChangeId()))
-        .isEqualTo("modified commit\n\nChange-Id: " + r.getChangeId() + "\n");
-  }
-
   @Test
   public void changeCommitMessageNullNotAllowed() throws Exception {
     PushOneCommit.Result r = createChange();

javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java

@@ -537,7 +537,7 @@ public class RevisionDiffIT extends AbstractDaemonTest {
     gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
     gApi.changes().id(changeId).edit().publish();
     String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
-    gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+    gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
     gApi.changes().id(changeId).edit().publish();
 
     DiffInfo diffInfo =
@@ -559,7 +559,7 @@ public class RevisionDiffIT extends AbstractDaemonTest {
     gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
     gApi.changes().id(changeId).edit().publish();
     String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
-    gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+    gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
     gApi.changes().id(changeId).edit().publish();
 
     DiffInfo diffInfo =
@@ -577,7 +577,7 @@ public class RevisionDiffIT extends AbstractDaemonTest {
     gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
     gApi.changes().id(changeId).edit().publish();
     String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
-    gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+    gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
     gApi.changes().id(changeId).edit().publish();
 
     DiffInfo diffInfo =
@@ -2859,6 +2859,10 @@ public class RevisionDiffIT extends AbstractDaemonTest {
     assertThat(e).hasMessageThat().isEqualTo("edit not allowed as base");
   }
 
+  private String updatedCommitMessage() {
+    return "An unchanged patchset\n\nChange-Id: " + changeId;
+  }
+
   private void assertDiffForNewFile(
       PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
     DiffInfo diff =

javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java

@@ -1046,7 +1046,8 @@ public class RobotCommentsIT extends AbstractDaemonTest {
 
   @Test
   public void fixDoesNotModifyCommitMessageOfChangeEdit() throws Exception {
-    String changeEditCommitMessage = "This is the commit message of the change edit.\n";
+    String changeEditCommitMessage =
+        "This is the commit message of the change edit.\n\nChange-Id: " + changeId + "\n";
     gApi.changes().id(changeId).edit().modifyCommitMessage(changeEditCommitMessage);
 
     fixReplacementInfo.path = FILE_NAME;
@@ -1065,7 +1066,8 @@ public class RobotCommentsIT extends AbstractDaemonTest {
   @Test
   public void fixOnCommitMessageCanBeApplied() throws Exception {
     // Set a dedicated commit message.
-    String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n";
+    String footer = "\nChange-Id: " + changeId + "\n";
+    String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n" + footer;
     gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
     gApi.changes().id(changeId).edit().publish();
 
@@ -1080,13 +1082,15 @@ public class RobotCommentsIT extends AbstractDaemonTest {
     gApi.changes().id(changeId).current().applyFix(fixId);
 
     String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
-    assertThat(commitMessage).isEqualTo("Modified line\nLine 2 of commit message\n");
+    assertThat(commitMessage).isEqualTo("Modified line\nLine 2 of commit message\n" + footer);
   }
 
   @Test
   public void fixOnHeaderPartOfCommitMessageCannotBeApplied() throws Exception {
     // Set a dedicated commit message.
-    String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n";
+    String footer = "Change-Id: " + changeId;
+    String originalCommitMessage =
+        "Line 1 of commit message\nLine 2 of commit message\n" + "\n" + footer + "\n";
     gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
     gApi.changes().id(changeId).edit().publish();
 
@@ -1108,8 +1112,9 @@ public class RobotCommentsIT extends AbstractDaemonTest {
   @Test
   public void fixContainingSeveralModificationsOfCommitMessageCanBeApplied() throws Exception {
     // Set a dedicated commit message.
+    String footer = "\nChange-Id: " + changeId + "\n";
     String originalCommitMessage =
-        "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n";
+        "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n" + footer;
     gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
     gApi.changes().id(changeId).edit().publish();
 
@@ -1135,13 +1140,14 @@ public class RobotCommentsIT extends AbstractDaemonTest {
 
     String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
     assertThat(commitMessage)
-        .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n");
+        .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n" + footer);
   }
 
   @Test
   public void fixModifyingTheCommitMessageAndAFileCanBeApplied() throws Exception {
     // Set a dedicated commit message.
-    String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n";
+    String footer = "\nChange-Id: " + changeId + "\n";
+    String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n" + footer;
     gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
     gApi.changes().id(changeId).edit().publish();
 
@@ -1165,7 +1171,7 @@ public class RobotCommentsIT extends AbstractDaemonTest {
     gApi.changes().id(changeId).current().applyFix(fixId);
 
     String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
-    assertThat(commitMessage).isEqualTo("Modified line 1\nLine 2 of commit message\n");
+    assertThat(commitMessage).isEqualTo("Modified line 1\nLine 2 of commit message\n" + footer);
     Optional<BinaryResult> file = gApi.changes().id(changeId).edit().getFile(FILE_NAME2);
     BinaryResultSubject.assertThat(file)
         .value()
@@ -1176,8 +1182,9 @@ public class RobotCommentsIT extends AbstractDaemonTest {
   @Test
   public void twoFixesOnCommitMessageCanBeAppliedOneAfterTheOther() throws Exception {
     // Set a dedicated commit message.
+    String footer = "\nChange-Id: " + changeId + "\n";
     String originalCommitMessage =
-        "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n";
+        "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n" + footer;
     gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
     gApi.changes().id(changeId).edit().publish();
 
@@ -1206,14 +1213,17 @@ public class RobotCommentsIT extends AbstractDaemonTest {
 
     String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
     assertThat(commitMessage)
-        .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n");
+        .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n" + footer);
   }
 
   @Test
   public void twoConflictingFixesOnCommitMessageCanNotBeAppliedOneAfterTheOther() throws Exception {
     // Set a dedicated commit message.
+    String footer = "Change-Id: " + changeId;
     String originalCommitMessage =
-        "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n";
+        "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n\n"
+            + footer
+            + "\n";
     gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
     gApi.changes().id(changeId).edit().publish();
 
@@ -1379,8 +1389,10 @@ public class RobotCommentsIT extends AbstractDaemonTest {
 
   @Test
   public void getFixPreviewForCommitMsg() throws Exception {
+    String footer = "Change-Id: " + changeId;
     updateCommitMessage(
-        changeId, "Commit title\n\nCommit message line 1\nLine 2\nLine 3\nLast line\n");
+        changeId,
+        "Commit title\n\nCommit message line 1\nLine 2\nLine 3\nLast line\n\n" + footer + "\n");
     FixReplacementInfo commitMsgReplacement = new FixReplacementInfo();
     commitMsgReplacement.path = Patch.COMMIT_MSG;
     // The test assumes that the first 5 lines is a header.
@@ -1420,7 +1432,11 @@ public class RobotCommentsIT extends AbstractDaemonTest {
         .isEqualTo("Commit message line 1");
     assertThat(diff).content().element(1).linesOfA().containsExactly("Line 2");
     assertThat(diff).content().element(1).linesOfB().containsExactly("New content");
-    assertThat(diff).content().element(2).commonLines().containsExactly("Line 3", "Last line", "");
+    assertThat(diff)
+        .content()
+        .element(2)
+        .commonLines()
+        .containsExactly("Line 3", "Last line", "", footer, "");
   }
 
   private void updateCommitMessage(String changeId, String newCommitMessage) throws Exception {

javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java

@@ -311,12 +311,14 @@ public class ChangeEditIT extends AbstractDaemonTest {
   @Test
   public void updateCommitMessageByEditingMagicCommitMsgFile() throws Exception {
     createEmptyEditFor(changeId);
+    String updatedCommitMsg = "Foo Bar\n\nChange-Id: " + changeId + "\n";
     gApi.changes()
         .id(changeId)
         .edit()
-        .modifyFile(Patch.COMMIT_MSG, RawInputUtil.create("Foo Bar".getBytes(UTF_8)));
+        .modifyFile(Patch.COMMIT_MSG, RawInputUtil.create(updatedCommitMsg.getBytes(UTF_8)));
     assertThat(getEdit(changeId)).isPresent();
-    ensureSameBytes(getFileContentOfEdit(changeId, Patch.COMMIT_MSG), "Foo Bar\n".getBytes(UTF_8));
+    ensureSameBytes(
+        getFileContentOfEdit(changeId, Patch.COMMIT_MSG), updatedCommitMsg.getBytes(UTF_8));
   }
 
   @Test
@@ -346,6 +348,39 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertThat(commitMessage).isEqualTo(msg);
   }
 
+  @Test
+  public void updateMessageEditChangeIdShouldThrowResourceConflictException() throws Exception {
+    createEmptyEditFor(changeId);
+    String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+    ResourceConflictException thrown =
+        assertThrows(
+            ResourceConflictException.class,
+            () ->
+                gApi.changes()
+                    .id(changeId)
+                    .edit()
+                    .modifyCommitMessage(commitMessage.replaceAll(changeId, changeId2)));
+    assertThat(thrown).hasMessageThat().isEqualTo("wrong Change-Id footer");
+  }
+
+  @Test
+  public void updateMessageEditRemoveChangeIdShouldThrowResourceConflictException()
+      throws Exception {
+    createEmptyEditFor(changeId);
+    String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+    ResourceConflictException thrown =
+        assertThrows(
+            ResourceConflictException.class,
+            () ->
+                gApi.changes()
+                    .id(changeId)
+                    .edit()
+                    .modifyCommitMessage(commitMessage.replaceAll("(Change-Id:).*", "")));
+    assertThat(thrown).hasMessageThat().isEqualTo("missing Change-Id footer");
+  }
+
   @Test
   public void updateMessageNoChange() throws Exception {
     createEmptyEditFor(changeId);

javatests/com/google/gerrit/acceptance/git/AbstractGitOverHttpServlet.java

@@ -19,10 +19,14 @@ import static com.google.common.truth.Truth.assertThat;
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.FakeGroupAuditService;
 import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.pgm.http.jetty.JettyServer;
 import com.google.gerrit.server.audit.HttpAuditEvent;
 import com.google.inject.Inject;
+import java.util.Optional;
 import javax.servlet.http.HttpServletResponse;
 import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.CredentialsProvider;
 import org.eclipse.jgit.transport.RefSpec;
@@ -32,9 +36,11 @@ import org.junit.Test;
 
 public class AbstractGitOverHttpServlet extends AbstractPushForReview {
   @Inject protected FakeGroupAuditService auditService;
+  private JettyServer jettyServer;
 
   @Before
   public void beforeEach() throws Exception {
+    jettyServer = server.getHttpdInjector().getInstance(JettyServer.class);
     CredentialsProvider.setDefault(
         new UsernamePasswordCredentialsProvider(admin.username(), admin.httpPassword()));
     selectProtocol(AbstractPushForReview.Protocol.HTTP);
@@ -70,6 +76,31 @@ public class AbstractGitOverHttpServlet extends AbstractPushForReview {
     assertThat(receivePack.what).endsWith("/git-receive-pack");
     assertThat(receivePack.params).isEmpty();
     assertThat(receivePack.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+    assertThat(jettyServer.numActiveSessions()).isEqualTo(0);
+  }
+
+  @Test
+  public void authenticatedUploadPackAuditEventLog() throws Exception {
+    String remote = "authenticated";
+    Config cfg = testRepo.git().getRepository().getConfig();
+
+    String uri = admin.getHttpUrl(server) + "/a/" + project.get();
+    cfg.setString("remote", remote, "url", uri);
+    cfg.setString("remote", remote, "fetch", "+refs/heads/*:refs/remotes/origin/*");
+
+    uploadPackAuditEventLog(remote, Optional.of(admin.id()));
+  }
+
+  @Test
+  public void anonymousUploadPackAuditEventLog() throws Exception {
+    String remote = "anonymous";
+    Config cfg = testRepo.git().getRepository().getConfig();
+
+    String uri = server.getUrl() + "/" + project.get();
+    cfg.setString("remote", remote, "url", uri);
+    cfg.setString("remote", remote, "fetch", "+refs/heads/*:refs/remotes/origin/*");
+
+    uploadPackAuditEventLog(remote, Optional.empty());
   }
 
   /**
@@ -77,17 +108,15 @@ public class AbstractGitOverHttpServlet extends AbstractPushForReview {
    * See {@code org.eclipse.jgit.transport.BasePackFetchConnection#doFetchV2} for the negotiation
    * details.
    */
-  @Test
-  @Sandboxed
-  public void uploadPackAuditEventLog() throws Exception {
+  private void uploadPackAuditEventLog(String remote, Optional<Account.Id> accountId)
+      throws Exception {
     auditService.drainHttpAuditEvents();
     // testRepo is already a clone. Make a server-side change so we have something to fetch.
     try (Repository repo = repoManager.openRepository(project);
         TestRepository<?> testRepo = new TestRepository<>(repo)) {
       testRepo.branch("master").commit().create();
     }
-
-    testRepo.git().fetch().call();
+    testRepo.git().fetch().setRemote(remote).call();
 
     ImmutableList<HttpAuditEvent> auditEvents = auditService.drainHttpAuditEvents();
     assertThat(auditEvents).hasSize(4);
@@ -96,10 +125,9 @@ public class AbstractGitOverHttpServlet extends AbstractPushForReview {
     // https://git-scm.com/docs/protocol-v2#_capability_advertisement
     HttpAuditEvent infoRef = auditEvents.get(0);
 
-    // JGit supports shared session during fetch and push, see
-    // org.eclipse.jgit.transport.http.HttpConnectionFactory2.GitSession, thus the authentication
-    // details are carried over from the first git clone request.
-    assertThat(infoRef.who.getAccountId()).isEqualTo(admin.id());
+    assertThat(infoRef.who.toString())
+        .isEqualTo(
+            accountId.map(id -> "IdentifiedUser[account " + id.get() + "]").orElse("ANONYMOUS"));
     assertThat(infoRef.what).endsWith("/info/refs?service=git-upload-pack");
     assertThat(infoRef.params).containsExactly("service", "git-upload-pack");
     assertThat(infoRef.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
@@ -124,5 +152,6 @@ public class AbstractGitOverHttpServlet extends AbstractPushForReview {
     assertThat(uploadPackDone.what).endsWith("/git-upload-pack");
     assertThat(uploadPackDone.params).isEmpty();
     assertThat(uploadPackDone.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+    assertThat(jettyServer.numActiveSessions()).isEqualTo(0);
   }
 }