ReceiveCommits: Record rejections due to missing permissions by permission
This way we can see from the metric due to which missing permissions
pushes are rejected most often.
Bug: Google b/151127672
Release-Notes: skip
Change-Id: I3e472e0286714904059bba8b20c13689392d0acb
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/extensions/api/access/GerritPermission.java b/java/com/google/gerrit/extensions/api/access/GerritPermission.java
index 02afbdc..cd80a09 100644
--- a/java/com/google/gerrit/extensions/api/access/GerritPermission.java
+++ b/java/com/google/gerrit/extensions/api/access/GerritPermission.java
@@ -27,6 +27,8 @@
*/
String describeForException();
+ String permissionName();
+
static String describeEnumValue(Enum<?> value) {
return value.name().toLowerCase(Locale.US).replace('_', ' ');
}
diff --git a/java/com/google/gerrit/extensions/api/access/PluginPermission.java b/java/com/google/gerrit/extensions/api/access/PluginPermission.java
index 1dc5cb6..f536b2c 100644
--- a/java/com/google/gerrit/extensions/api/access/PluginPermission.java
+++ b/java/com/google/gerrit/extensions/api/access/PluginPermission.java
@@ -52,6 +52,11 @@
}
@Override
+ public String permissionName() {
+ return pluginName + "~" + capability;
+ }
+
+ @Override
public int hashCode() {
return Objects.hash(pluginName, capability);
}
diff --git a/java/com/google/gerrit/extensions/api/access/PluginProjectPermission.java b/java/com/google/gerrit/extensions/api/access/PluginProjectPermission.java
index b3680ea..8fa1e55 100644
--- a/java/com/google/gerrit/extensions/api/access/PluginProjectPermission.java
+++ b/java/com/google/gerrit/extensions/api/access/PluginProjectPermission.java
@@ -54,6 +54,11 @@
}
@Override
+ public String permissionName() {
+ return pluginName + "~" + permission;
+ }
+
+ @Override
public int hashCode() {
return Objects.hash(pluginName, permission);
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 7322af3..040e316 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1757,21 +1757,21 @@
private void rejectProhibited(ReceiveCommand cmd, AuthException err) {
err.getAdvice().ifPresent(a -> errors.put(a, cmd.getRefName()));
- reject(cmd, RejectionReason.create(MetricBucket.PROHIBITED, prohibited(err, cmd.getRefName())));
+ reject(cmd, prohibited(err, cmd.getRefName()));
}
- private static String prohibited(AuthException e, String alreadyDisplayedResource) {
- String msg = e.getMessage();
+ private static RejectionReason prohibited(AuthException e, String alreadyDisplayedResource) {
if (e instanceof PermissionDeniedException) {
PermissionDeniedException pde = (PermissionDeniedException) e;
if (pde.getResource().isPresent()
&& pde.getResource().get().equals(alreadyDisplayedResource)) {
// Avoid repeating resource name if exactly the given name was already displayed by the
// generic git push machinery.
- msg = PermissionDeniedException.MESSAGE_PREFIX + pde.describePermission();
+ return RejectionReason.create(pde);
}
}
- return "prohibited by Gerrit: " + msg;
+ return RejectionReason.create(
+ MetricBucket.PROHIBITED, "prohibited by Gerrit: " + e.getMessage());
}
static class MagicBranchInput {
@@ -3912,8 +3912,8 @@
logger.atFine().log("Rejecting command '%s': %s", cmd, reason.why());
metrics.rejectCount.increment(
MagicBranch.isMagicBranch(cmd.getRefName()) ? "magic" : "direct",
- reason.metricBucket().name(),
- reason.metricBucket().statusCode());
+ reason.metricBucket(),
+ reason.statusCode());
cmd.setResult(REJECTED_OTHER_REASON, reason.why());
}
diff --git a/java/com/google/gerrit/server/git/receive/RejectionReason.java b/java/com/google/gerrit/server/git/receive/RejectionReason.java
index bb42ec3..ef36538 100644
--- a/java/com/google/gerrit/server/git/receive/RejectionReason.java
+++ b/java/com/google/gerrit/server/git/receive/RejectionReason.java
@@ -24,6 +24,8 @@
import static javax.servlet.http.HttpServletResponse.SC_REQUEST_TIMEOUT;
import com.google.auto.value.AutoValue;
+import com.google.gerrit.server.permissions.PermissionDeniedException;
+import java.util.Locale;
@AutoValue
public abstract class RejectionReason {
@@ -90,17 +92,29 @@
private MetricBucket(int statusCode) {
this.statusCode = statusCode;
}
-
- public int statusCode() {
- return statusCode;
- }
}
- static RejectionReason create(MetricBucket metricBucket, String why) {
- return new AutoValue_RejectionReason(metricBucket, why);
+ public static RejectionReason create(MetricBucket metricBucket, String why) {
+ return new AutoValue_RejectionReason(metricBucket.statusCode, metricBucket.name(), why);
}
- public abstract MetricBucket metricBucket();
+ public static RejectionReason create(PermissionDeniedException permissionDenied) {
+ return new AutoValue_RejectionReason(
+ SC_FORBIDDEN,
+ "CANNOT_"
+ + permissionDenied
+ .getPermission()
+ .permissionName()
+ .toUpperCase(Locale.US)
+ .replaceAll(" ", "_"),
+ "prohibited by Gerrit: "
+ + PermissionDeniedException.MESSAGE_PREFIX
+ + permissionDenied.describePermission());
+ }
+
+ public abstract int statusCode();
+
+ public abstract String metricBucket();
public abstract String why();
}
diff --git a/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java b/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java
index 622f0cf..ba7caed 100644
--- a/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java
+++ b/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java
@@ -53,7 +53,8 @@
protected abstract String permissionPrefix();
- protected String permissionName() {
+ @Override
+ public String permissionName() {
if (forUser == ON_BEHALF_OF) {
return permissionPrefix() + "As";
}
@@ -119,8 +120,6 @@
return label.value();
}
- public abstract String permissionName();
-
@Override
public final String describeForException() {
if (forUser == ON_BEHALF_OF) {
diff --git a/java/com/google/gerrit/server/permissions/ChangePermission.java b/java/com/google/gerrit/server/permissions/ChangePermission.java
index d9f83c7..b9868dd 100644
--- a/java/com/google/gerrit/server/permissions/ChangePermission.java
+++ b/java/com/google/gerrit/server/permissions/ChangePermission.java
@@ -101,6 +101,11 @@
}
@Override
+ public String permissionName() {
+ return GerritPermission.describeEnumValue(this);
+ }
+
+ @Override
public Optional<String> hintForException() {
return Optional.ofNullable(hint);
}
diff --git a/java/com/google/gerrit/server/permissions/GlobalPermission.java b/java/com/google/gerrit/server/permissions/GlobalPermission.java
index 3429978..d83353c 100644
--- a/java/com/google/gerrit/server/permissions/GlobalPermission.java
+++ b/java/com/google/gerrit/server/permissions/GlobalPermission.java
@@ -160,4 +160,9 @@
public String describeForException() {
return GerritPermission.describeEnumValue(this);
}
+
+ @Override
+ public String permissionName() {
+ return GerritPermission.describeEnumValue(this);
+ }
}
diff --git a/java/com/google/gerrit/server/permissions/PermissionDeniedException.java b/java/com/google/gerrit/server/permissions/PermissionDeniedException.java
index b9e86cd..a007cf7 100644
--- a/java/com/google/gerrit/server/permissions/PermissionDeniedException.java
+++ b/java/com/google/gerrit/server/permissions/PermissionDeniedException.java
@@ -55,4 +55,8 @@
public Optional<String> getResource() {
return resource;
}
+
+ public GerritPermission getPermission() {
+ return permission;
+ }
}
diff --git a/java/com/google/gerrit/server/permissions/ProjectPermission.java b/java/com/google/gerrit/server/permissions/ProjectPermission.java
index a4110ef..c3e9740 100644
--- a/java/com/google/gerrit/server/permissions/ProjectPermission.java
+++ b/java/com/google/gerrit/server/permissions/ProjectPermission.java
@@ -119,4 +119,9 @@
public String describeForException() {
return description != null ? description : GerritPermission.describeEnumValue(this);
}
+
+ @Override
+ public String permissionName() {
+ return GerritPermission.describeEnumValue(this);
+ }
}
diff --git a/java/com/google/gerrit/server/permissions/RefPermission.java b/java/com/google/gerrit/server/permissions/RefPermission.java
index 09eed24..34c46af 100644
--- a/java/com/google/gerrit/server/permissions/RefPermission.java
+++ b/java/com/google/gerrit/server/permissions/RefPermission.java
@@ -88,4 +88,9 @@
public String describeForException() {
return description != null ? description : GerritPermission.describeEnumValue(this);
}
+
+ @Override
+ public String permissionName() {
+ return GerritPermission.describeEnumValue(this);
+ }
}
