opendev/gerrit
Code Issues Proposed changes

Disallow creating changes on internal Gerrit refs

Browse Source 
This is a follow up to change If5fb50e1f7b which allowed creating
changes using the REST API to particular refs, namely: refs/heads,
refs/meta/dashboards, HEAD or refs/meta/config.

We found some cases with changes created on other ref patterns, e.g.
refs/experimental. Although these cases were not frequent, but the
previous change could break existing workflows.

With this change, we modify the behaviour to disallow creating changes
on refs/tags/* and Gerrit internal refs instead, such as refs/users/*,
refs/groups/*, refs/meta/external-ids/*, etc...

Change-Id: I35ca35be9e8294acf2f4f906c380331a45e0234a
This commit is contained in:
Youssef Elghareeb
2021-01-07 11:30:57 +01:00
parent 9418a1effe
commit ae7b747dfa
4 changed files with 63 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@@ -182,13 +182,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
}
@Test
public void cannotCreateChangeOnNoteDbRefs() throws Exception {
String[] disallowedBranches = {
"refs/users/82/1000002",
"refs/tags/v2.1",
"refs/cache-automerge/ec/00000000000000000000000000000000000000"
};
public void cannotCreateChangeOnGerritInternalRefs() throws Exception {
requestScopeOperations.setApiUser(admin.id());
projectOperations
.project(project)
@@ -196,21 +190,41 @@ public class CreateChangeIT extends AbstractDaemonTest {
.add(allow(CREATE).ref("refs/*").group(REGISTERED_USERS))
.update();
for (String branchName : disallowedBranches) {
requestScopeOperations.setApiUser(admin.id());
BranchNameKey branchNameKey = BranchNameKey.create(project, branchName);
createBranch(branchNameKey);
String disallowedRef = "refs/changes/00/1000"; // All Gerrit internal refs behave the same way
requestScopeOperations.setApiUser(admin.id());
BranchNameKey branchNameKey = BranchNameKey.create(project, disallowedRef);
createBranch(branchNameKey);
requestScopeOperations.setApiUser(user.id());
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "Subject";
ci.branch = branchName;
requestScopeOperations.setApiUser(user.id());
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "Subject";
ci.branch = disallowedRef;
Throwable thrown = assertThrows(RestApiException.class, () -> gApi.changes().create(ci));
assertThat(thrown)
.hasMessageThat()
.contains("Cannot create a change on ref " + ci.branch + ". ");
}
Throwable thrown = assertThrows(RestApiException.class, () -> gApi.changes().create(ci));
assertThat(thrown).hasMessageThat().contains("Cannot create a change on ref " + ci.branch);
}
@Test
public void cannotCreateChangeOnTagRefs() throws Exception {
requestScopeOperations.setApiUser(admin.id());
projectOperations
.project(project)
.forUpdate()
.add(allow(CREATE).ref("refs/*").group(REGISTERED_USERS))
.update();
String branchName = "refs/tags/v1.0";
requestScopeOperations.setApiUser(admin.id());
BranchNameKey branchNameKey = BranchNameKey.create(project, branchName);
createBranch(branchNameKey);
requestScopeOperations.setApiUser(user.id());
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "Subject";
ci.branch = branchName;
Throwable thrown = assertThrows(RestApiException.class, () -> gApi.changes().create(ci));
assertThat(thrown).hasMessageThat().contains("Cannot create a change on ref " + ci.branch);
}
@Test