ProjectTagsScreen: Base visibility on the create refs/tags/* permission

Before this change, the Tags creation form fields were visible also if
either refs/* or refs/head/* was allowed for Create Reference. This fix
limits that visibility to a create refs/tags/* permission solely, as per
current documentation anyway. isAdmin() still also makes the panel
visible, overriding potentially missing ref creation permissions.

Create Annotated Tag is still also required for the user to be able to
use the optional Annotation field. In this case, the created tag is no
longer lightweight but becomes annotated. Both kinds of tags are still
supported through such a single Tags creation panel or form, thus the
need to allow both permissions even if aiming for annotated tags only.
(Command line does not have that design limitation indeed.)

Bug: Issue 9689
Change-Id: Ib7c3f1a0fdebaee30da371cc64850fa6d6d8dd05
This commit is contained in:
Marco Miller 2018-09-10 16:52:54 -04:00
parent 8664030bde
commit c2114ac086
8 changed files with 75 additions and 14 deletions

View File

@ -263,6 +263,7 @@ The entries in the map are sorted by project name.
], ],
"can_upload": true, "can_upload": true,
"can_add": true, "can_add": true,
"can_add_tags": true,
"config_visible": true, "config_visible": true,
"groups": { "groups": {
"53a4f647a89ea57992571187d8025f830625192a": { "53a4f647a89ea57992571187d8025f830625192a": {
@ -313,6 +314,7 @@ The entries in the map are sorted by project name.
], ],
"can_upload": true, "can_upload": true,
"can_add": true, "can_add": true,
"can_add_tags": true,
"config_visible": true "config_visible": true
} }
} }
@ -399,6 +401,8 @@ Whether the calling user owns this project.
Whether the calling user can upload to any ref. Whether the calling user can upload to any ref.
|`can_add` |not set if `false`| |`can_add` |not set if `false`|
Whether the calling user can add any ref. Whether the calling user can add any ref.
|`can_add_tags` |not set if `false`|
Whether the calling user can add any tag ref.
|`config_visible` |not set if `false`| |`config_visible` |not set if `false`|
Whether the calling user can see the `refs/meta/config` branch of the Whether the calling user can see the `refs/meta/config` branch of the
project. project.

View File

@ -1034,6 +1034,7 @@ entity is returned.
], ],
"can_upload": true, "can_upload": true,
"can_add": true, "can_add": true,
"can_add_tags": true,
"config_visible": true, "config_visible": true,
"groups": { "groups": {
"c2ce4749a32ceb82cd6adcce65b8216e12afb41c": { "c2ce4749a32ceb82cd6adcce65b8216e12afb41c": {
@ -1135,6 +1136,7 @@ entity is returned.
], ],
"can_upload": true, "can_upload": true,
"can_add": true, "can_add": true,
"can_add_tags": true,
"config_visible": true, "config_visible": true,
"groups": { "groups": {
"global:Anonymous-Users": { "global:Anonymous-Users": {

View File

@ -27,6 +27,7 @@ public class ProjectAccessInfo {
public Set<String> ownerOf; public Set<String> ownerOf;
public Boolean canUpload; public Boolean canUpload;
public Boolean canAdd; public Boolean canAdd;
public Boolean canAddTags;
public Boolean configVisible; public Boolean configVisible;
public Map<String, GroupInfo> groups; public Map<String, GroupInfo> groups;
} }

View File

@ -19,6 +19,8 @@ import com.google.gwt.core.client.JavaScriptObject;
public class ProjectAccessInfo extends JavaScriptObject { public class ProjectAccessInfo extends JavaScriptObject {
public final native boolean canAddRefs() /*-{ return this.can_add ? true : false; }-*/; public final native boolean canAddRefs() /*-{ return this.can_add ? true : false; }-*/;
public final native boolean canAddTagRefs() /*-{ return this.can_add_tags ? true : false; }-*/;
public final native boolean isOwner() /*-{ return this.is_owner ? true : false; }-*/; public final native boolean isOwner() /*-{ return this.is_owner ? true : false; }-*/;
public final native boolean configVisible() /*-{ return this.config_visible ? true : false; }-*/; public final native boolean configVisible() /*-{ return this.config_visible ? true : false; }-*/;

View File

@ -94,7 +94,7 @@ public class ProjectTagsScreen extends PaginatedProjectScreen {
new GerritCallback<ProjectAccessInfo>() { new GerritCallback<ProjectAccessInfo>() {
@Override @Override
public void onSuccess(ProjectAccessInfo result) { public void onSuccess(ProjectAccessInfo result) {
addPanel.setVisible(result.canAddRefs()); addPanel.setVisible(result.canAddTagRefs());
} }
}); });
query = new Query(match).start(start).run(); query = new Query(match).start(start).run();

View File

@ -57,6 +57,21 @@ public enum ProjectPermission {
*/ */
CREATE_REF, CREATE_REF,
/**
* Can create at least one tag reference in the project.
*
* <p>This project level permission only validates the user may create some tag reference within
* the project. The exact reference name must be checked at creation:
*
* <pre>permissionBackend
* .user(user)
* .project(proj)
* .ref(ref)
* .check(RefPermission.CREATE);
* </pre>
*/
CREATE_TAG_REF,
/** /**
* Can create at least one change in the project. * Can create at least one change in the project.
* *

View File

@ -16,6 +16,7 @@ package com.google.gerrit.server.project;
import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE_SERVER; import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE_SERVER;
import static com.google.gerrit.server.permissions.ProjectPermission.CREATE_REF; import static com.google.gerrit.server.permissions.ProjectPermission.CREATE_REF;
import static com.google.gerrit.server.permissions.ProjectPermission.CREATE_TAG_REF;
import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE; import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE;
import static com.google.gerrit.server.permissions.RefPermission.READ; import static com.google.gerrit.server.permissions.RefPermission.READ;
import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toMap;
@ -248,6 +249,7 @@ public class GetAccess implements RestReadView<ProjectResource> {
pc.isOwner() pc.isOwner()
|| (checkReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE))); || (checkReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE)));
info.canAdd = toBoolean(perm.testOrFalse(CREATE_REF)); info.canAdd = toBoolean(perm.testOrFalse(CREATE_REF));
info.canAddTags = toBoolean(perm.testOrFalse(CREATE_TAG_REF));
info.configVisible = checkReadConfig || pc.isOwner(); info.configVisible = checkReadConfig || pc.isOwner();
info.groups = info.groups =

View File

@ -15,6 +15,7 @@
package com.google.gerrit.server.project; package com.google.gerrit.server.project;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_TAGS;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
@ -251,6 +252,10 @@ public class ProjectControl {
return (canPerformOnAnyRef(Permission.CREATE) || isAdmin()); return (canPerformOnAnyRef(Permission.CREATE) || isAdmin());
} }
private boolean canAddTagRefs() {
return (canPerformOnTagRef(Permission.CREATE) || isAdmin());
}
private boolean canCreateChanges() { private boolean canCreateChanges() {
for (SectionMatcher matcher : access()) { for (SectionMatcher matcher : access()) {
AccessSection section = matcher.section; AccessSection section = matcher.section;
@ -281,6 +286,26 @@ public class ProjectControl {
return declaredOwner; return declaredOwner;
} }
private boolean canPerformOnTagRef(String permissionName) {
for (SectionMatcher matcher : access()) {
AccessSection section = matcher.section;
if (section.getName().startsWith(REFS_TAGS)) {
Permission permission = section.getPermission(permissionName);
if (permission == null) {
continue;
}
Boolean can = canPerform(permissionName, section, permission);
if (can != null) {
return can;
}
}
}
return false;
}
private boolean canPerformOnAnyRef(String permissionName) { private boolean canPerformOnAnyRef(String permissionName) {
for (SectionMatcher matcher : access()) { for (SectionMatcher matcher : access()) {
AccessSection section = matcher.section; AccessSection section = matcher.section;
@ -289,6 +314,16 @@ public class ProjectControl {
continue; continue;
} }
Boolean can = canPerform(permissionName, section, permission);
if (can != null) {
return can;
}
}
return false;
}
private Boolean canPerform(String permissionName, AccessSection section, Permission permission) {
for (PermissionRule rule : permission.getRules()) { for (PermissionRule rule : permission.getRules()) {
if (rule.isBlock() || rule.isDeny() || !match(rule)) { if (rule.isBlock() || rule.isDeny() || !match(rule)) {
continue; continue;
@ -303,9 +338,7 @@ public class ProjectControl {
} }
break; break;
} }
} return null;
return false;
} }
private boolean canPerformOnAllRefs(String permission, Set<String> ignore) { private boolean canPerformOnAllRefs(String permission, Set<String> ignore) {
@ -463,6 +496,8 @@ public class ProjectControl {
case CREATE_REF: case CREATE_REF:
return canAddRefs(); return canAddRefs();
case CREATE_TAG_REF:
return canAddTagRefs();
case CREATE_CHANGE: case CREATE_CHANGE:
return canCreateChanges(); return canCreateChanges();