opendev
/
gerrit
Code Issues Proposed changes

Rename 'Push Annotated/Signed Tag' permission to 'Create Annotated/Signed Tag' 

Each tag type requires a special permission for the tag creation:
- Lightweight tags require 'Create Reference'
- Annontated tags require 'Push Annotated Tag'
- Signed tags require 'Push Signed Tag'

This naming is inconsistent and may be confusing. E.g. whether tags
can be updated is controlled by the 'Push' permission on 'refs/tags/*'
and not by the 'Push Annotated/Signed Tag' permission, as some users
might expect.

This change includes a schema migration that renames the permissions
for creating annotated/signed tags.

Permission rules in project.config that use the old names are still
respected. They are automatically converted when the project config is
saved the next time. This is needed so that multi-master sites can do
a multi-step-migration:

1. First upgrade all hosts to the new binary:
   Projects may still contain permissions with the old names,
   new permissions are saved with the new names.
2. Run a background job on all hosts that migrates the permissions for
   all projects to the new names:
   Projects do not contain permissions with the old names,
   new permissions are saved with the new names.
3. Upgrade all hosts to a binary that doesn't respect the old names
   anymore.

The migration for schema 130 is rewritten because ProjectConfig no
longer allows to change the force flag for 'pushTag' without
converting it to 'createTag'.

Change-Id: I839be24f82a908b5184f15e746f3588a0d397b7e
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-09-05 14:32:38 +02:00 committed by David Pursehouse
parent 5554c61ca9
commit 62c156857e
17 changed files with 241 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
Documentation/access-control.txt
View File

@ -660,7 +660,8 @@ of merge commits.
[[category_push_annotated]]
=== Push Annotated Tag
[[category_create_annotated]]
=== Create Annotated Tag
This category permits users to push an annotated tag object into the
project's repository. Typically this would be done with a command line
@ -687,7 +688,7 @@ tagger email address must be verified for the current user.
To push tags created by users other than the current user (such
as tags mirrored from an upstream project), `Forge Committer Identity`
must be also granted in addition to `Push Annotated Tag`.
must be also granted in addition to `Create Annotated Tag`.
To push lightweight (non annotated) tags, grant
<<category_create,`Create Reference`>> for reference name
@ -706,7 +707,8 @@ is only allowed by granting `Push` with `force` option on `+refs/tags/*+`.
[[category_push_signed]]
=== Push Signed Tag
[[category_create_signed]]
=== Create Signed Tag
This category permits users to push a PGP signed tag object into the
project's repository. Typically this would be done with a command
@ -1019,7 +1021,7 @@ Suggested access rights to grant:
* <<category_push_merge,`Push merge commit`>> to 'refs/heads/*'
* <<category_forge_committer,`Forge Committer Identity`>> to 'refs/for/refs/heads/*'
* <<category_create,`Create Reference`>> to 'refs/heads/*'
* <<category_push_annotated,`Push Annotated Tag`>> to 'refs/tags/*'
* <<category_create_annotated,`Create Annotated Tag`>> to 'refs/tags/*'
[[examples_project-owner]]

4
Documentation/error-prohibited-by-gerrit.txt
View File

@ -17,10 +17,10 @@ In particular this error occurs:
link:access-control.html#category_create['Create Reference'] access
right on `+refs/heads/*+`
4. if you push an annotated tag without
link:access-control.html#category_push_annotated['Push Annotated Tag']
link:access-control.html#category_create_annotated['Create Annotated Tag']
access right on `+refs/tags/*+`
5. if you push a signed tag without
link:access-control.html#category_push_signed['Push Signed Tag']
link:access-control.html#category_create_signed['Create Signed Tag']
access right on `+refs/tags/*+`
6. if you push a lightweight tag without the access right link:access-control.html#category_create['Create
Reference'] for the reference name `+refs/tags/*+`

4
Documentation/rest-api-access.txt
View File

@ -132,7 +132,7 @@ The entries in the map are sorted by project name.
},
"refs/tags/*": {
"permissions": {
"pushSignedTag": {
"createSignedTag": {
"rules": {
"53a4f647a89ea57992571187d8025f830625192a": {
"action": "ALLOW"
@ -142,7 +142,7 @@ The entries in the map are sorted by project name.
}
}
},
"pushTag": {
"createTag": {
"rules": {
"53a4f647a89ea57992571187d8025f830625192a": {
"action": "ALLOW"

4
Documentation/user-upload.txt
View File

@ -403,11 +403,11 @@ To grant this access, configure
link:access-control.html#category_push_direct['Push'] with the
'Force' option ticked.
To push annotated tags, the `Push Annotated Tag` project right must
To push annotated tags, the `Create Annotated Tag` project right must
be granted to one (or more) of the user's groups. There is only
one level of access in this category.
Project owners may wish to grant themselves `Push Annotated Tag`
Project owners may wish to grant themselves `Create Annotated Tag`
only at times when a new release is being prepared, and otherwise
grant nothing at all. This ensures that accidental pushes don't
make undesired changes to the public repository.

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushTagIT.java
View File

@ -42,7 +42,7 @@ import org.junit.Test;
public class PushTagIT extends AbstractDaemonTest {
enum TagType {
LIGHTWEIGHT(Permission.CREATE),
ANNOTATED(Permission.PUSH_TAG);
ANNOTATED(Permission.CREATE_TAG);
final String createPermission;

6
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/TagsIT.java
View File

@ -248,7 +248,7 @@ public class TagsIT extends AbstractDaemonTest {
@Test
public void createAnnotatedTagNotAllowed() throws Exception {
block(Permission.PUSH_TAG, REGISTERED_USERS, R_TAGS + "*");
block(Permission.CREATE_TAG, REGISTERED_USERS, R_TAGS + "*");
TagInput input = new TagInput();
input.ref = "test";
input.message = "annotation";
@ -339,8 +339,8 @@ public class TagsIT extends AbstractDaemonTest {
private void grantTagPermissions() throws Exception {
grant(Permission.CREATE, project, R_TAGS + "*");
grant(Permission.PUSH_TAG, project, R_TAGS + "*");
grant(Permission.PUSH_SIGNED_TAG, project, R_TAGS + "*");
grant(Permission.CREATE_TAG, project, R_TAGS + "*");
grant(Permission.CREATE_SIGNED_TAG, project, R_TAGS + "*");
}
private ListRefsRequest<TagInfo> getTags() throws Exception {

8
gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
View File

@ -25,6 +25,8 @@ public class Permission implements Comparable<Permission> {
public static final String ADD_PATCH_SET = "addPatchSet";
public static final String CREATE = "create";
public static final String DELETE = "delete";
public static final String CREATE_TAG = "createTag";
public static final String CREATE_SIGNED_TAG = "createSignedTag";
public static final String DELETE_DRAFTS = "deleteDrafts";
public static final String EDIT_HASHTAGS = "editHashtags";
public static final String EDIT_TOPIC_NAME = "editTopicName";
@ -37,8 +39,6 @@ public class Permission implements Comparable<Permission> {
public static final String PUBLISH_DRAFTS = "publishDrafts";
public static final String PUSH = "push";
public static final String PUSH_MERGE = "pushMerge";
public static final String PUSH_TAG = "pushTag";
public static final String PUSH_SIGNED_TAG = "pushSignedTag";
public static final String READ = "read";
public static final String REBASE = "rebase";
public static final String REMOVE_REVIEWER = "removeReviewer";
@ -57,14 +57,14 @@ public class Permission implements Comparable<Permission> {
NAMES_LC.add(ABANDON.toLowerCase());
NAMES_LC.add(ADD_PATCH_SET.toLowerCase());
NAMES_LC.add(CREATE.toLowerCase());
NAMES_LC.add(CREATE_TAG.toLowerCase());
NAMES_LC.add(CREATE_SIGNED_TAG.toLowerCase());
NAMES_LC.add(DELETE.toLowerCase());
NAMES_LC.add(FORGE_AUTHOR.toLowerCase());
NAMES_LC.add(FORGE_COMMITTER.toLowerCase());
NAMES_LC.add(FORGE_SERVER.toLowerCase());
NAMES_LC.add(PUSH.toLowerCase());
NAMES_LC.add(PUSH_MERGE.toLowerCase());
NAMES_LC.add(PUSH_TAG.toLowerCase());
NAMES_LC.add(PUSH_SIGNED_TAG.toLowerCase());
NAMES_LC.add(LABEL.toLowerCase());
NAMES_LC.add(LABEL_AS.toLowerCase());
NAMES_LC.add(REBASE.toLowerCase());

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
View File

@ -123,6 +123,8 @@ permissionNames = \
abandon, \
addPatchSet, \
create, \
createTag, \
createSignedTag, \
delete, \
deleteDrafts, \
editHashtags, \
@ -134,8 +136,6 @@ permissionNames = \
publishDrafts, \
push, \
pushMerge, \
pushTag, \
pushSignedTag, \
read, \
rebase, \
removeReviewer, \
@ -146,6 +146,8 @@ permissionNames = \
abandon = Abandon
addPatchSet = Add Patch Set
create = Create Reference
createTag = Create Annotated Tag
createSignedTag = Create Signed Tag
delete = Delete Reference
deleteDrafts = Delete Drafts
editHashtags = Edit Hashtags
@ -157,8 +159,6 @@ owner = Owner
publishDrafts = Publish Drafts
push = Push
pushMerge = Push Merge Commit
pushTag = Push Annotated Tag
pushSignedTag = Push Signed Tag
read = Read
rebase = Rebase
removeReviewer = Remove Reviewer

34
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@ -91,7 +91,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private static final String PROJECT = "project";
private static final String KEY_DESCRIPTION = "description";
private static final String ACCESS = "access";
public static final String ACCESS = "access";
private static final String KEY_INHERIT_FROM = "inheritFrom";
private static final String KEY_GROUP_PERMISSIONS = "exclusiveGroupPermissions";
@ -155,6 +155,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private static final Set<String> LABEL_FUNCTIONS = ImmutableSet.of(
"MaxWithBlock", "AnyWithBlock", "MaxNoBlock", "NoBlock", "NoOp", "PatchSetLock");
private static final String LEGACY_PERMISSION_PUSH_TAG = "pushTag";
private static final String LEGACY_PERMISSION_PUSH_SIGNED_TAG = "pushSignedTag";
private static final String PLUGIN = "plugin";
private static final SubmitType defaultSubmitAction =
@ -180,6 +183,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private Map<String, Config> pluginConfigs;
private boolean checkReceivedObjects;
private Set<String> sectionsWithUnknownPermissions;
private boolean hasLegacyPermissions;
public static ProjectConfig read(MetaDataUpdate update) throws IOException,
ConfigInvalidException {
@ -627,6 +631,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
for (String varName : rc.getStringList(ACCESS, refName, KEY_GROUP_PERMISSIONS)) {
for (String n : varName.split("[, \t]{1,}")) {
n = convertLegacyPermission(n);
if (isPermission(n)) {
as.getPermission(n, true).setExclusiveGroup(true);
}
@ -634,10 +639,11 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
}
for (String varName : rc.getNames(ACCESS, refName)) {
if (isPermission(varName)) {
Permission perm = as.getPermission(varName, true);
String convertedName = convertLegacyPermission(varName);
if (isPermission(convertedName)) {
Permission perm = as.getPermission(convertedName, true);
loadPermissionRules(rc, ACCESS, refName, varName, groupsByName,
perm, Permission.hasRange(varName));
perm, Permission.hasRange(convertedName));
} else {
sectionsWithUnknownPermissions.add(as.getName());
}
@ -1147,7 +1153,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
}
for (String varName : rc.getNames(ACCESS, refName)) {
if (isPermission(varName) && !have.contains(varName.toLowerCase())) {
if (isPermission(convertLegacyPermission(varName))
&& !have.contains(varName.toLowerCase())) {
rc.unset(ACCESS, refName, varName);
}
}
@ -1282,4 +1289,21 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
Collections.sort(r);
return r;
}
public boolean hasLegacyPermissions() {
return hasLegacyPermissions;
}
private String convertLegacyPermission(String permissionName) {
switch(permissionName) {
case LEGACY_PERMISSION_PUSH_TAG:
hasLegacyPermissions = true;
return Permission.CREATE_TAG;
case LEGACY_PERMISSION_PUSH_SIGNED_TAG:
hasLegacyPermissions = true;
return Permission.CREATE_SIGNED_TAG;
default:
return permissionName;
}
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/project/CreateTag.java
View File

@ -116,7 +116,7 @@ public class CreateTag implements RestModifyView<ProjectResource, TagInput> {
if (isSigned) {
throw new MethodNotAllowedException(
"Cannot create signed tag \"" + ref + "\"");
} else if (isAnnotated && !refControl.canPerform(Permission.PUSH_TAG)) {
} else if (isAnnotated && !refControl.canPerform(Permission.CREATE_TAG)) {
throw new AuthException("Cannot create annotated tag \"" + ref + "\"");
} else if (!refControl.canPerform(Permission.CREATE)) {
throw new AuthException("Cannot create tag \"" + ref + "\"");

4
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
View File

@ -328,8 +328,8 @@ public class ProjectControl {
/** @return true if the user can upload to at least one reference */
public Capable canPushToAtLeastOneRef() {
if (! canPerformOnAnyRef(Permission.PUSH) &&
! canPerformOnAnyRef(Permission.PUSH_TAG)) {
if (!canPerformOnAnyRef(Permission.PUSH) &&
!canPerformOnAnyRef(Permission.CREATE_TAG)) {
String pName = state.getProject().getName();
return new Capable("Upload denied for project '" + pName + "'");
}

4
gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
View File

@ -317,9 +317,9 @@ public class RefControl {
// than if it doesn't have a PGP signature.
//
if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
return canPerform(Permission.PUSH_SIGNED_TAG);
return canPerform(Permission.CREATE_SIGNED_TAG);
}
return canPerform(Permission.PUSH_TAG);
return canPerform(Permission.CREATE_TAG);
} else {
return false;
}

4
gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java
View File

@ -166,8 +166,8 @@ public class AllProjectsCreator {
grant(config, heads, Permission.EDIT_TOPIC_NAME, true, admin, owners);
grant(config, tags, Permission.CREATE, admin, owners);
grant(config, tags, Permission.PUSH_TAG, admin, owners);
grant(config, tags, Permission.PUSH_SIGNED_TAG, admin, owners);
grant(config, tags, Permission.CREATE_TAG, admin, owners);
grant(config, tags, Permission.CREATE_SIGNED_TAG, admin, owners);
grant(config, magic, Permission.PUSH, registered);
grant(config, magic, Permission.PUSH_MERGE, registered);

109
gerrit-server/src/main/java/com/google/gerrit/server/schema/ProjectConfigSchemaUpdate.java Normal file
View File

@ -0,0 +1,109 @@
// Copyright (C) 2016 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.server.schema;
import static com.google.gerrit.server.git.ProjectConfig.ACCESS;
import com.google.common.base.Function;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.VersionedMetaData;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
public class ProjectConfigSchemaUpdate extends VersionedMetaData {
private final MetaDataUpdate update;
private Config config;
private boolean updated;
public static ProjectConfigSchemaUpdate read(MetaDataUpdate update)
throws IOException, ConfigInvalidException {
ProjectConfigSchemaUpdate r = new ProjectConfigSchemaUpdate(update);
r.load(update);
return r;
}
private ProjectConfigSchemaUpdate(MetaDataUpdate update) {
this.update = update;
}
@Override
protected String getRefName() {
return RefNames.REFS_CONFIG;
}
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
config = readConfig(ProjectConfig.PROJECT_CONFIG);
}
public void removeForceFromPermission(String name) {
for (String subsection : config.getSubsections(ACCESS)) {
Set<String> names = config.getNames(ACCESS, subsection);
if (names.contains(name)) {
List<String> values =
Arrays.asList(config.getStringList(ACCESS, subsection, name));
values = Lists.transform(values, new Function<String, String>() {
@Override
public String apply(String ruleString) {
PermissionRule rule = PermissionRule.fromString(ruleString, false);
if (rule.getForce()) {
rule.setForce(false);
updated = true;
}
return rule.asString(false);
}
});
config.setStringList(ACCESS, subsection, name, values);
}
}
}
@Override
protected boolean onSave(CommitBuilder commit)
throws IOException, ConfigInvalidException {
saveConfig(ProjectConfig.PROJECT_CONFIG, config);
return true;
}
public void save(PersonIdent personIdent, String commitMessage)
throws OrmException {
if (!updated) {
return;
}
update.getCommitBuilder().setAuthor(personIdent);
update.getCommitBuilder().setCommitter(personIdent);
update.setMessage(commitMessage);
try {
commit(update);
} catch (IOException e) {
throw new OrmException(e);
}
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

@ -33,7 +33,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
public static final Class<Schema_130> C = Schema_130.class;
public static final Class<Schema_131> C = Schema_131.class;
public static int getBinaryVersion() {
return guessVersion(C);

31
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_130.java
View File

@ -14,16 +14,12 @@
package com.google.gerrit.server.schema;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -59,30 +55,9 @@ public class Schema_130 extends SchemaVersion {
try (Repository git = repoManager.openRepository(projectName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED,
projectName, git)) {
ProjectConfig config = ProjectConfig.read(md);
boolean update = false;
for (AccessSection accessSection : config.getAccessSections()) {
Permission pushTagPermission =
accessSection.getPermission(Permission.PUSH_TAG);
if (pushTagPermission == null) {
continue;
}
for (PermissionRule rule : pushTagPermission.getRules()) {
if (rule.getForce()) {
rule.setForce(false);
update = true;
}
}
}
if (!update) {
continue;
}
md.getCommitBuilder().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser);
md.setMessage(COMMIT_MSG);
config.commit(md);
ProjectConfigSchemaUpdate cfg = ProjectConfigSchemaUpdate.read(md);
cfg.removeForceFromPermission("pushTag");
cfg.save(serverUser, COMMIT_MSG);
} catch (ConfigInvalidException | IOException ex) {
throw new OrmException(ex);
}

68
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_131.java Normal file
View File

@ -0,0 +1,68 @@
// Copyright (C) 2016 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.server.schema;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
public class Schema_131 extends SchemaVersion {
private static final String COMMIT_MSG =
"Rename 'Push Annotated/Signed Tag' permission to 'Create Annotated/Signed Tag'";
private final GitRepositoryManager repoManager;
private final PersonIdent serverUser;
@Inject
Schema_131(Provider<Schema_130> prior,
GitRepositoryManager repoManager,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.serverUser = serverUser;
}
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
for (Project.NameKey projectName : repoManager.list()) {
try (Repository git = repoManager.openRepository(projectName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED,
projectName, git)) {
ProjectConfig config = ProjectConfig.read(md);
if (config.hasLegacyPermissions()) {
md.getCommitBuilder().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser);
md.setMessage(COMMIT_MSG);
config.commit(md);
}
} catch (ConfigInvalidException | IOException ex) {
throw new OrmException(ex);
}
}
}
}