Merge branch 'stable-2.5'
* stable-2.5: Improve wording in Gerrit 2.5.1 release notes Add release notes for Gerrit 2.5.1 Allow assigning Push for refs/meta/config on All-Projects Fix auto closing of changes on direct push Match all git fetch/clone/push commands to the command executor Fix RequestCleanup bug with Git over HTTP Make sure only Gerrit admins can change the parent of a project Correctly identify Git-over-HTTP operations Conflicts: gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java The modification done by Correctly identify Git-over-HTTP operations were discarded since the issue fixed by this commit is already fixed in master. Change-Id: If16c8f8e99a481917661049da6a0e853b9febc84 Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
commit
9d726c8803
94
ReleaseNotes/ReleaseNotes-2.5.1.txt
Normal file
94
ReleaseNotes/ReleaseNotes-2.5.1.txt
Normal file
@ -0,0 +1,94 @@
|
||||
Release notes for Gerrit 2.5.1
|
||||
==============================
|
||||
|
||||
Gerrit 2.5.1 is now available:
|
||||
|
||||
link:http://code.google.com/p/gerrit/downloads/detail?name=gerrit-full-2.5.1.war[http://code.google.com/p/gerrit/downloads/detail?name=gerrit-full-2.5.1.war]
|
||||
|
||||
There are no schema changes from 2.5, or 2.5.1.
|
||||
|
||||
However, if upgrading from anything earlier version, follow the upgrade
|
||||
procedure in the 2.5 link:ReleaseNotes-2.5.html[Release Notes].
|
||||
|
||||
Security Fixes
|
||||
--------------
|
||||
* Correctly identify Git-over-HTTP operations
|
||||
+
|
||||
Git operations over HTTP should be classified as using AccessPath.GIT
|
||||
and not WEB_UI. This ensures RefControl will correctly test for Create,
|
||||
Push or Delete access on a reference instead of Owner.
|
||||
+
|
||||
E.g. without this fix project owners are able to force push commits
|
||||
via HTTP that are already in the history of the target branch, even
|
||||
without having any Push access right assigned.
|
||||
|
||||
* Make sure only Gerrit admins can change the parent of a project
|
||||
+
|
||||
Only Gerrit administrators should be able to change the parent of a
|
||||
project because by changing the parent project access rights and BLOCK
|
||||
rules which are configured on a parent project can be avoided.
|
||||
+
|
||||
The `set-project-parent` SSH command already verifies that the caller
|
||||
is a Gerrit administrator, however project owners can change the parent
|
||||
project by modifying the `project.config` file and pushing to the
|
||||
`refs/meta/config` branch.
|
||||
+
|
||||
This fix ensures that changes to the `project.config` file that change
|
||||
the parent project can only be pushed/submitted by Gerrit
|
||||
administrators.
|
||||
+
|
||||
In addition it is now no longer possible to
|
||||
- set a non-existing project as parent (as this would make the project
|
||||
be orphaned)
|
||||
- set a parent project for the `All-Projects` root project (the root
|
||||
project by definition has no parent)
|
||||
by pushing changes of the `project.config` file to `refs/meta/config`.
|
||||
|
||||
Bug Fixes
|
||||
---------
|
||||
* Fix RequestCleanup bug with Git over HTTP
|
||||
+
|
||||
Decide if a continuation is going to be used early, before the filter
|
||||
that will attempt to cleanup a RequestCleanup. If so don't allow
|
||||
entering the RequestCleanup part of the system until the request is
|
||||
actually going to be processed.
|
||||
+
|
||||
This fixes the IllegalStateException `Request has already been cleaned
|
||||
up` that occurred when running on Jetty and pushing over HTTP for URLs
|
||||
where the path starts with `/p/`.
|
||||
|
||||
* Match all git fetch/clone/push commands to the command executor
|
||||
+
|
||||
Route not just `/p/` but any Git access to the same thread pool as the
|
||||
SSH server is using, allowing all requests to compete fairly for
|
||||
resources.
|
||||
|
||||
* Fix auto closing of changes on direct push
|
||||
+
|
||||
When a commit is directly pushed into a repository (bypassing code
|
||||
review) and this commit has a Change-Id in its commit message then the
|
||||
corresponding change is automatically closed if it is open.
|
||||
|
||||
* Allow assigning `Push` for `refs/meta/config` on `All-Projects`
|
||||
+
|
||||
The `refs/meta/config` branch of the `All-Projects project` should only
|
||||
be modified by Gerrit administrators because being able to do
|
||||
modifications on this branch means that the user could assign himself
|
||||
administrator permissions.
|
||||
+
|
||||
In addition to being administrator we already require that the
|
||||
administrator has the `Push` access right for `refs/meta/config` in
|
||||
order to be able to modify it (just as with all other branches
|
||||
administrators do not have edit permissions by default).
|
||||
+
|
||||
The problem was that assigning the `Push` access right for
|
||||
`refs/meta/config` on the `All-Projects` project was not allowed.
|
||||
+
|
||||
Having the `Push` access right for `refs/meta/config` on the
|
||||
`All-Projects` project without being administrator already has no
|
||||
effect.
|
||||
+
|
||||
Prohibiting to assign the Push access right for `refs/meta/config` on
|
||||
the `All-Project` project was anyway pointless since it was e.g.
|
||||
possible to assign the `Push` access right on `refs/meta/*`.
|
||||
|
@ -4,6 +4,7 @@ Gerrit Code Review - Release Notes
|
||||
[[2_5]]
|
||||
Version 2.5.x
|
||||
-------------
|
||||
* link:ReleaseNotes-2.5.1.html[2.5.1]
|
||||
* link:ReleaseNotes-2.5.html[2.5]
|
||||
|
||||
[[2_4]]
|
||||
|
@ -62,7 +62,26 @@ enum CommitMergeStatus {
|
||||
/** */
|
||||
NOT_FAST_FORWARD("Project policy requires all submissions to be a fast-forward.\n"
|
||||
+ "\n"
|
||||
+ "Please rebase the change locally and upload again for review.");
|
||||
+ "Please rebase the change locally and upload again for review."),
|
||||
|
||||
/** */
|
||||
INVALID_PROJECT_CONFIGURATION("Change contains an invalid project configuration."),
|
||||
|
||||
/** */
|
||||
INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND(
|
||||
"Change contains an invalid project configuration:\n"
|
||||
+ "Parent project does not exist."),
|
||||
|
||||
/** */
|
||||
INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT(
|
||||
"Change contains an invalid project configuration:\n"
|
||||
+ "The root project cannot have a parent."),
|
||||
|
||||
/** */
|
||||
SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN(
|
||||
"Change contains a project configuration that changes the parent project.\n"
|
||||
+ "The change must be submitted by a Gerrit administrator.");
|
||||
|
||||
|
||||
private String message;
|
||||
|
||||
|
@ -42,6 +42,7 @@ import com.google.gerrit.server.ChangeUtil;
|
||||
import com.google.gerrit.server.GerritPersonIdent;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.account.AccountCache;
|
||||
import com.google.gerrit.server.config.AllProjectsName;
|
||||
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
||||
import com.google.gerrit.server.mail.MergeFailSender;
|
||||
import com.google.gerrit.server.mail.MergedSender;
|
||||
@ -153,6 +154,7 @@ public class MergeOp {
|
||||
private final SubmoduleOp.Factory subOpFactory;
|
||||
private final WorkQueue workQueue;
|
||||
private final RequestScopePropagator requestScopePropagator;
|
||||
private final AllProjectsName allProjectsName;
|
||||
|
||||
@Inject
|
||||
MergeOp(final GitRepositoryManager grm, final SchemaFactory<ReviewDb> sf,
|
||||
@ -169,7 +171,8 @@ public class MergeOp {
|
||||
final SubmitStrategyFactory submitStrategyFactory,
|
||||
final SubmoduleOp.Factory subOpFactory,
|
||||
final WorkQueue workQueue,
|
||||
final RequestScopePropagator requestScopePropagator) {
|
||||
final RequestScopePropagator requestScopePropagator,
|
||||
final AllProjectsName allProjectsName) {
|
||||
repoManager = grm;
|
||||
schemaFactory = sf;
|
||||
functionState = fs;
|
||||
@ -190,6 +193,7 @@ public class MergeOp {
|
||||
this.subOpFactory = subOpFactory;
|
||||
this.workQueue = workQueue;
|
||||
this.requestScopePropagator = requestScopePropagator;
|
||||
this.allProjectsName = allProjectsName;
|
||||
this.myIdent = myIdent;
|
||||
destBranch = branch;
|
||||
toMerge = ArrayListMultimap.create();
|
||||
@ -542,6 +546,52 @@ public class MergeOp {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (GitRepositoryManager.REF_CONFIG.equals(destBranch.get())) {
|
||||
final Project.NameKey newParent;
|
||||
try {
|
||||
ProjectConfig cfg =
|
||||
new ProjectConfig(destProject.getProject().getNameKey());
|
||||
cfg.load(repo, commit);
|
||||
newParent = cfg.getProject().getParent(allProjectsName);
|
||||
} catch (Exception e) {
|
||||
commits.put(changeId, CodeReviewCommit
|
||||
.error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION));
|
||||
continue;
|
||||
}
|
||||
final Project.NameKey oldParent =
|
||||
destProject.getProject().getParent(allProjectsName);
|
||||
if (oldParent == null) {
|
||||
// update of the 'All-Projects' project
|
||||
if (newParent != null) {
|
||||
commits.put(changeId, CodeReviewCommit
|
||||
.error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT));
|
||||
continue;
|
||||
}
|
||||
} else {
|
||||
if (!oldParent.equals(newParent)) {
|
||||
final PatchSetApproval psa = getSubmitter(db, ps.getId());
|
||||
if (psa == null) {
|
||||
commits.put(changeId, CodeReviewCommit
|
||||
.error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
|
||||
continue;
|
||||
}
|
||||
final IdentifiedUser submitter =
|
||||
identifiedUserFactory.create(psa.getAccountId());
|
||||
if (!submitter.getCapabilities().canAdministrateServer()) {
|
||||
commits.put(changeId, CodeReviewCommit
|
||||
.error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
|
||||
continue;
|
||||
}
|
||||
|
||||
if (projectCache.get(newParent) == null) {
|
||||
commits.put(changeId, CodeReviewCommit
|
||||
.error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND));
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
commit.change = chg;
|
||||
commit.patchsetId = ps.getId();
|
||||
commit.originalOrder = commitOrder++;
|
||||
@ -702,7 +752,11 @@ public class MergeOp {
|
||||
case PATH_CONFLICT:
|
||||
case CRISS_CROSS_MERGE:
|
||||
case CANNOT_CHERRY_PICK_ROOT:
|
||||
case NOT_FAST_FORWARD: {
|
||||
case NOT_FAST_FORWARD:
|
||||
case INVALID_PROJECT_CONFIGURATION:
|
||||
case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND:
|
||||
case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT:
|
||||
case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN: {
|
||||
setNew(c, message(c, txt));
|
||||
break;
|
||||
}
|
||||
|
@ -58,6 +58,7 @@ import com.google.gerrit.server.ChangeUtil;
|
||||
import com.google.gerrit.server.GerritPersonIdent;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.account.AccountResolver;
|
||||
import com.google.gerrit.server.config.AllProjectsName;
|
||||
import com.google.gerrit.server.config.CanonicalWebUrl;
|
||||
import com.google.gerrit.server.config.TrackingFooters;
|
||||
import com.google.gerrit.server.events.CommitReceivedEvent;
|
||||
@ -256,6 +257,7 @@ public class ReceiveCommits {
|
||||
private final ListeningExecutorService changeUpdateExector;
|
||||
private final RequestScopePropagator requestScopePropagator;
|
||||
private final SshInfo sshInfo;
|
||||
private final AllProjectsName allProjectsName;
|
||||
|
||||
private final ProjectControl projectControl;
|
||||
private final Project project;
|
||||
@ -313,6 +315,7 @@ public class ReceiveCommits {
|
||||
final RequestScopePropagator requestScopePropagator,
|
||||
final SshInfo sshInfo,
|
||||
final DynamicSet<CommitValidationListener> commitValidationListeners,
|
||||
final AllProjectsName allProjectsName,
|
||||
@Assisted final ProjectControl projectControl,
|
||||
@Assisted final Repository repo,
|
||||
final SubmoduleOp.Factory subOpFactory) throws IOException {
|
||||
@ -337,6 +340,7 @@ public class ReceiveCommits {
|
||||
this.changeUpdateExector = changeUpdateExector;
|
||||
this.requestScopePropagator = requestScopePropagator;
|
||||
this.sshInfo = sshInfo;
|
||||
this.allProjectsName = allProjectsName;
|
||||
|
||||
this.projectControl = projectControl;
|
||||
this.project = projectControl.getProject();
|
||||
@ -838,6 +842,26 @@ public class ReceiveCommits {
|
||||
+ cmd.getNewId().name() + " for " + project.getName());
|
||||
continue;
|
||||
}
|
||||
Project.NameKey newParent = cfg.getProject().getParent(allProjectsName);
|
||||
Project.NameKey oldParent = project.getParent(allProjectsName);
|
||||
if (oldParent == null) {
|
||||
// update of the 'All-Projects' project
|
||||
if (newParent != null) {
|
||||
reject(cmd, "invalid project configuration: root project cannot have parent");
|
||||
continue;
|
||||
}
|
||||
} else {
|
||||
if (!oldParent.equals(newParent)
|
||||
&& !currentUser.getCapabilities().canAdministrateServer()) {
|
||||
reject(cmd, "invalid project configuration: only Gerrit admin can set parent");
|
||||
continue;
|
||||
}
|
||||
|
||||
if (projectCache.get(newParent) == null) {
|
||||
reject(cmd, "invalid project configuration: parent does not exist");
|
||||
continue;
|
||||
}
|
||||
}
|
||||
} catch (Exception e) {
|
||||
reject(cmd, "invalid project configuration");
|
||||
log.error("User " + currentUser.getUserName()
|
||||
|
Loading…
x
Reference in New Issue
Block a user