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. isOwnerAnyRef() 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: I7e3d11a62ad1e49575e6ef743138158efa831e6a
diff --git a/Documentation/rest-api-access.txt b/Documentation/rest-api-access.txt
index 07a3d78..a90ea1a 100644
--- a/Documentation/rest-api-access.txt
+++ b/Documentation/rest-api-access.txt
@@ -263,6 +263,7 @@
],
"can_upload": true,
"can_add": true,
+ "can_add_tags": true,
"config_visible": true
},
"MyProject": {
@@ -279,6 +280,7 @@
],
"can_upload": true,
"can_add": true,
+ "can_add_tags": true,
"config_visible": true
}
}
@@ -365,6 +367,8 @@
Whether the calling user can upload to any ref.
|`can_add` |not set if `false`|
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`|
Whether the calling user can see the `refs/meta/config` branch of the
project.
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 8e151bc..573337e 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1014,6 +1014,7 @@
],
"can_upload": true,
"can_add": true,
+ "can_add_tags": true,
"config_visible": true
}
----
@@ -1097,6 +1098,7 @@
],
"can_upload": true,
"can_add": true,
+ "can_add_tags": true,
"config_visible": true
}
----
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java
index 0922d95..995c664 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java
@@ -26,5 +26,6 @@
public Set<String> ownerOf;
public Boolean canUpload;
public Boolean canAdd;
+ public Boolean canAddTags;
public Boolean configVisible;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/access/ProjectAccessInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/access/ProjectAccessInfo.java
index b115c7d..88635df 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/access/ProjectAccessInfo.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/access/ProjectAccessInfo.java
@@ -19,6 +19,8 @@
public class ProjectAccessInfo extends JavaScriptObject {
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 configVisible() /*-{ return this.config_visible ? true : false; }-*/;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java
index bf541d3..f66f42b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java
@@ -94,7 +94,7 @@
new GerritCallback<ProjectAccessInfo>() {
@Override
public void onSuccess(ProjectAccessInfo result) {
- addPanel.setVisible(result.canAddRefs());
+ addPanel.setVisible(result.canAddTagRefs());
}
});
query = new Query(match).start(start).run();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java
index b464f68..7989455 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java
@@ -224,6 +224,7 @@
info.canUpload =
toBoolean(pc.isOwner() || (metaConfigControl.isVisible() && metaConfigControl.canUpload()));
info.canAdd = toBoolean(pc.canAddRefs());
+ info.canAddTags = toBoolean(pc.canAddTagRefs());
info.configVisible = pc.isOwner() || metaConfigControl.isVisible();
return info;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index 5684082..fefc84d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.project;
+import static com.google.gerrit.reviewdb.client.RefNames.REFS_TAGS;
+
import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.PageLinks;
@@ -293,6 +295,10 @@
return (canPerformOnAnyRef(Permission.CREATE) || isOwnerAnyRef());
}
+ public boolean canAddTagRefs() {
+ return (canPerformOnTagRef(Permission.CREATE) || isOwnerAnyRef());
+ }
+
public boolean canUpload() {
for (SectionMatcher matcher : access()) {
AccessSection section = matcher.section;
@@ -409,6 +415,26 @@
return new Capable(msg.toString());
}
+ 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) {
for (SectionMatcher matcher : access()) {
AccessSection section = matcher.section;
@@ -417,25 +443,33 @@
continue;
}
- for (PermissionRule rule : permission.getRules()) {
- if (rule.isBlock() || rule.isDeny() || !match(rule)) {
- continue;
- }
-
- // Being in a group that was granted this permission is only an
- // approximation. There might be overrides and doNotInherit
- // that would render this to be false.
- //
- if (controlForRef(section.getName()).canPerform(permissionName)) {
- return true;
- }
- break;
+ 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()) {
+ if (rule.isBlock() || rule.isDeny() || !match(rule)) {
+ continue;
+ }
+
+ // Being in a group that was granted this permission is only an
+ // approximation. There might be overrides and doNotInherit
+ // that would render this to be false.
+ //
+ if (controlForRef(section.getName()).canPerform(permissionName)) {
+ return true;
+ }
+ break;
+ }
+ return null;
+ }
+
private boolean canPerformOnAllRefs(String permission, Set<String> ignore) {
boolean canPerform = false;
Set<String> patterns = allRefPatterns(permission);
