From c54435161718fc294fa76260fce75f7aa18f1fbe Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 31 Aug 2013 14:13:20 +0200 Subject: [PATCH] Refactor capability check out from REST and SSH code Move the code to one place to simplfy the maintenance. Change-Id: Ief075899e7d7f3d383ffbbe03ec75c7d20768cc0 --- .../gerrit/httpd/restapi/RestApiServlet.java | 58 ++----------- .../server/account/CapabilityUtils.java | 85 +++++++++++++++++++ .../google/gerrit/sshd/DispatchCommand.java | 61 +++---------- 3 files changed, 104 insertions(+), 100 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityUtils.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 5afadf6aaf..d8a264821f 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -46,8 +46,6 @@ import com.google.common.math.IntMath; import com.google.common.net.HttpHeaders; import com.google.gerrit.audit.AuditService; import com.google.gerrit.audit.HttpAuditEvent; -import com.google.gerrit.extensions.annotations.CapabilityScope; -import com.google.gerrit.extensions.annotations.RequiresCapability; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AcceptsCreate; import com.google.gerrit.extensions.restapi.AcceptsPost; @@ -76,7 +74,7 @@ import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.OptionUtil; import com.google.gerrit.server.OutputFormat; -import com.google.gerrit.server.account.CapabilityControl; +import com.google.gerrit.server.account.CapabilityUtils; import com.google.gson.ExclusionStrategy; import com.google.gson.FieldAttributes; import com.google.gson.FieldNamingPolicy; @@ -200,7 +198,8 @@ public class RestApiServlet extends HttpServlet { List path = splitPath(req); RestCollection rc = members.get(); - checkAccessAnnotations(null, rc.getClass()); + CapabilityUtils.checkRequiresCapability(globals.currentUser, + null, rc.getClass()); RestResource rsrc = TopLevelResource.INSTANCE; ViewData viewData = new ViewData(null, null); @@ -238,7 +237,7 @@ public class RestApiServlet extends HttpServlet { viewData = view(rc, req.getMethod(), path); } } - checkAccessAnnotations(viewData); + checkRequiresCapability(viewData); while (viewData.view instanceof RestCollection) { @SuppressWarnings("unchecked") @@ -279,7 +278,7 @@ public class RestApiServlet extends HttpServlet { viewData = view(c, req.getMethod(), path); } } - checkAccessAnnotations(viewData); + checkRequiresCapability(viewData); } if (notModified(req, rsrc)) { @@ -870,50 +869,9 @@ public class RestApiServlet extends HttpServlet { return !("GET".equals(method) || "HEAD".equals(method)); } - private void checkAccessAnnotations(ViewData viewData) throws AuthException { - checkAccessAnnotations(viewData.pluginName, viewData.view.getClass()); - } - - private void checkAccessAnnotations(String pluginName, Class clazz) - throws AuthException { - RequiresCapability rc = getRequiresCapability(clazz); - if (rc != null) { - CurrentUser user = globals.currentUser.get(); - CapabilityControl ctl = user.getCapabilities(); - String capability = rc.value(); - - if (pluginName != null && !"gerrit".equals(pluginName) - && (rc.scope() == CapabilityScope.PLUGIN - || rc.scope() == CapabilityScope.CONTEXT)) { - capability = String.format("%s-%s", pluginName, rc.value()); - } else if (rc.scope() == CapabilityScope.PLUGIN) { - log.error(String.format( - "Class %s uses @%s(scope=%s), but is not within a plugin", - clazz.getName(), - RequiresCapability.class.getSimpleName(), - CapabilityScope.PLUGIN.name())); - throw new AuthException("cannot check capability"); - } - - if (!ctl.canPerform(capability) && !ctl.canAdministrateServer()) { - throw new AuthException(String.format( - "Capability %s is required to access this resource", - capability)); - } - } - } - - private static RequiresCapability getRequiresCapability(Class clazz) { - RequiresCapability rc = clazz.getAnnotation(RequiresCapability.class); - if (rc != null) { - return rc; - } - - if (clazz.getSuperclass() != null) { - return getRequiresCapability(clazz.getSuperclass()); - } - - return null; + private void checkRequiresCapability(ViewData viewData) throws AuthException { + CapabilityUtils.checkRequiresCapability(globals.currentUser, + viewData.pluginName, viewData.view.getClass()); } private static void handleException(Throwable err, HttpServletRequest req, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityUtils.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityUtils.java new file mode 100644 index 0000000000..6b680325fa --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityUtils.java @@ -0,0 +1,85 @@ +// Copyright (C) 2013 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.account; + +import com.google.gerrit.extensions.annotations.CapabilityScope; +import com.google.gerrit.extensions.annotations.RequiresCapability; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.account.CapabilityControl; +import com.google.inject.Provider; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.annotation.Annotation; + +public class CapabilityUtils { + private static final Logger log = LoggerFactory + .getLogger(CapabilityUtils.class); + + public static void checkRequiresCapability(Provider userProvider, + String pluginName, Class clazz) + throws AuthException { + RequiresCapability rc = getClassAnnotation(clazz, RequiresCapability.class); + if (rc != null) { + CurrentUser user = userProvider.get(); + CapabilityControl ctl = user.getCapabilities(); + if (ctl.canAdministrateServer()) { + return; + } + + String capability = rc.value(); + if (pluginName != null && !"gerrit".equals(pluginName) + && (rc.scope() == CapabilityScope.PLUGIN + || rc.scope() == CapabilityScope.CONTEXT)) { + capability = String.format("%s-%s", pluginName, rc.value()); + } else if (rc.scope() == CapabilityScope.PLUGIN) { + log.error(String.format( + "Class %s uses @%s(scope=%s), but is not within a plugin", + clazz.getName(), + RequiresCapability.class.getSimpleName(), + CapabilityScope.PLUGIN.name())); + throw new AuthException("cannot check capability"); + } + + if (!ctl.canPerform(capability)) { + throw new AuthException(String.format( + "Capability %s is required to access this resource", + capability)); + } + } + } + + /** + * Find an instance of the specified annotation, walking up the inheritance + * tree if necessary. + * + * @param Annotation type to search for + * @param clazz root class to search, may be null + * @param annotationClass class object of Annotation subclass to search for + * @return the requested annotation or null if none + */ + private static T getClassAnnotation(Class clazz, + Class annotationClass) { + for (; clazz != null; clazz = clazz.getSuperclass()) { + T t = clazz.getAnnotation(annotationClass); + if (t != null) { + return t; + } + } + return null; + } +} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java index 0ed8ee945d..fa5ab53ea7 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java @@ -17,10 +17,9 @@ package com.google.gerrit.sshd; import com.google.common.base.Strings; import com.google.common.collect.Sets; import com.google.common.util.concurrent.Atomics; -import com.google.gerrit.extensions.annotations.CapabilityScope; -import com.google.gerrit.extensions.annotations.RequiresCapability; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.account.CapabilityControl; +import com.google.gerrit.server.account.CapabilityUtils; import com.google.gerrit.server.args4j.SubcommandHandler; import com.google.inject.Inject; import com.google.inject.Provider; @@ -29,8 +28,6 @@ import com.google.inject.assistedinject.Assisted; import org.apache.sshd.server.Command; import org.apache.sshd.server.Environment; import org.kohsuke.args4j.Argument; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.StringWriter; @@ -43,9 +40,6 @@ import java.util.concurrent.atomic.AtomicReference; * Command that dispatches to a subcommand from its command table. */ final class DispatchCommand extends BaseCommand { - private static final Logger log = LoggerFactory - .getLogger(DispatchCommand.class); - interface Factory { DispatchCommand create(Map map); } @@ -121,50 +115,17 @@ final class DispatchCommand extends BaseCommand { private void checkRequiresCapability(Command cmd) throws UnloggedFailure { - RequiresCapability rc = getRequiresCapability(cmd.getClass()); - if (rc != null) { - CurrentUser user = currentUser.get(); - CapabilityControl ctl = user.getCapabilities(); - String capability = rc.value(); - - if (cmd instanceof BaseCommand) { - String pluginName = ((BaseCommand) cmd).getPluginName(); - if (pluginName != null && !"gerrit".equals(pluginName) - && (rc.scope() == CapabilityScope.PLUGIN - || rc.scope() == CapabilityScope.CONTEXT)) { - capability = String.format("%s-%s", pluginName, rc.value()); - } else if (rc.scope() == CapabilityScope.PLUGIN) { - log.error(String.format( - "Class %s uses @%s(scope=%s), but is not within a plugin", - cmd.getClass().getName(), - RequiresCapability.class.getSimpleName(), - CapabilityScope.PLUGIN.name())); - throw new UnloggedFailure( - BaseCommand.STATUS_NOT_ADMIN, - "fatal: cannot check capability"); - } - } - - if (!ctl.canPerform(capability) && !ctl.canAdministrateServer()) { - String msg = String.format( - "fatal: %s does not have \"%s\" capability.", - user.getUserName(), capability); - throw new UnloggedFailure(BaseCommand.STATUS_NOT_ADMIN, msg); - } + String pluginName = null; + if (cmd instanceof BaseCommand) { + pluginName = ((BaseCommand) cmd).getPluginName(); } - } - - private static RequiresCapability getRequiresCapability(Class clazz) { - RequiresCapability rc = clazz.getAnnotation(RequiresCapability.class); - if (rc != null) { - return rc; + try { + CapabilityUtils.checkRequiresCapability(currentUser, + pluginName, cmd.getClass()); + } catch (AuthException e) { + throw new UnloggedFailure(BaseCommand.STATUS_NOT_ADMIN, + e.getMessage()); } - - if (clazz.getSuperclass() != null) { - return getRequiresCapability(clazz.getSuperclass()); - } - - return null; } @Override