aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kevi...@apache.org
Subject aurora git commit: Extract job key from RPC parameters
Date Fri, 03 Apr 2015 21:20:12 GMT
Repository: aurora
Updated Branches:
  refs/heads/master 9e46dd8b1 -> 05d75e5dc


Extract job key from RPC parameters

Testing Done:
./gradlew -Pq build

Bugs closed: AURORA-1187

Reviewed at https://reviews.apache.org/r/32329/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/05d75e5d
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/05d75e5d
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/05d75e5d

Branch: refs/heads/master
Commit: 05d75e5dc104f0f2adce011639acf085042cee99
Parents: 9e46dd8
Author: Kevin Sweeney <kevints@apache.org>
Authored: Fri Apr 3 17:19:55 2015 -0400
Committer: Kevin Sweeney <kevints@apache.org>
Committed: Fri Apr 3 17:19:55 2015 -0400

----------------------------------------------------------------------
 config/pmd/custom.xml                           |  34 +-
 .../aurora/scheduler/http/api/ApiBeta.java      |   4 +-
 .../aurora/scheduler/http/api/ApiModule.java    |   3 +-
 .../http/api/security/ApiSecurityModule.java    |  42 ++-
 .../http/api/security/AuthorizingParam.java     |  10 +-
 .../http/api/security/FieldGetter.java          |  73 ++++
 .../http/api/security/FieldGetters.java         |  50 +++
 .../ShiroAuthenticatingThriftInterceptor.java   |  63 ++++
 .../security/ShiroAuthorizingInterceptor.java   | 100 +++++
 .../ShiroAuthorizingParamInterceptor.java       | 372 +++++++++++++++++++
 .../api/security/ShiroThriftInterceptor.java    | 101 -----
 .../http/api/security/ThriftFieldGetter.java    |  66 ++++
 .../aurora/scheduler/thrift/aop/AopModule.java  |   2 +-
 .../aurora/scheduler/http/api/ApiBetaTest.java  |   8 +-
 .../apache/aurora/scheduler/http/api/ApiIT.java |   8 +-
 .../http/api/security/ApiSecurityIT.java        |  64 +++-
 ...hiroAuthenticatingThriftInterceptorTest.java |  66 ++++
 .../ShiroAuthorizingInterceptorTest.java        |  94 +++++
 .../ShiroAuthorizingParamInterceptorTest.java   | 189 ++++++++++
 .../security/ShiroThriftInterceptorTest.java    | 106 ------
 .../api/security/ThriftFieldGetterTest.java     |  46 +++
 .../aurora/scheduler/thrift/ThriftIT.java       |   3 +-
 .../scheduler/thrift/aop/AopModuleTest.java     |   2 +-
 23 files changed, 1264 insertions(+), 242 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/config/pmd/custom.xml
----------------------------------------------------------------------
diff --git a/config/pmd/custom.xml b/config/pmd/custom.xml
index 521fd50..8ad6228 100644
--- a/config/pmd/custom.xml
+++ b/config/pmd/custom.xml
@@ -22,7 +22,21 @@ limitations under the License.
     Aurora PMD ruleset.
   </description>
 
-  <rule ref="rulesets/java/basic.xml"/>
+  <rule ref="rulesets/java/basic.xml">
+    <!-- Duplicate definitions - defined in empty.xml below and marked deprecated.
+         See http://sourceforge.net/p/pmd/discussion/188193/thread/6e9c6017/ -->
+    <exclude name="EmptyCatchBlock"/>
+    <exclude name="EmptyIfStmt"/>
+    <exclude name="EmptyWhileStmt"/>
+    <exclude name="EmptyTryBlock"/>
+    <exclude name="EmptyFinallyBlock"/>
+    <exclude name="EmptySwitchStatements"/>
+    <exclude name="EmptySynchronizedBlock"/>
+    <exclude name="EmptyStatementNotInLoop"/>
+    <exclude name="EmptyInitializer"/>
+    <exclude name="EmptyStatementBlock"/>
+    <exclude name="EmptyStaticInitializer"/>
+  </rule>
   <rule ref="rulesets/java/design.xml">
     <!-- We're not currently focusing on localization. -->
     <exclude name="SimpleDateFormatNeedsLocale"/>
@@ -38,10 +52,20 @@ limitations under the License.
          TODO(wfarner): Break apart god classes. -->
     <exclude name="GodClass"/>
   </rule>
-  <rule ref="rulesets/java/empty.xml"/>
-  <rule ref="rulesets/java/imports.xml">
-    <!-- We frequently use static imports for enum fields (making other code more concise), but
-         those trip this rule. -->
+  <rule ref="rulesets/java/empty.xml">
+    <!-- Configured below -->
+    <exclude name="EmptyCatchBlock"/>
+  </rule>
+  <rule ref="rulesets/java/empty.xml/EmptyCatchBlock">
+    <properties>
+          <!-- Some APIs, like the Java Reflection API, use exceptions to indicate the absence of
+               a value and we legitimately want to ignore them. -->
+          <property name="allowCommentedBlocks" value="true"/>
+        </properties>
+      </rule>
+      <rule ref="rulesets/java/imports.xml">
+        <!-- We frequently use static imports for enum fields (making other code more concise), but
+             those trip this rule. -->
     <exclude name="TooManyStaticImports"/>
   </rule>
   <rule ref="rulesets/java/logging-java.xml">

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java b/src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java
index 827e85b..690e82e 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java
@@ -46,10 +46,10 @@ import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
 import com.google.gson.JsonSyntaxException;
 
-import org.apache.aurora.gen.AuroraAdmin;
 import org.apache.aurora.gen.AuroraAdmin.Iface;
 import org.apache.aurora.scheduler.storage.entities.AuroraAdminMetadata;
 import org.apache.aurora.scheduler.thrift.Responses;
+import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 
 import static org.apache.aurora.scheduler.http.api.GsonMessageBodyHandler.GSON;
 
@@ -66,7 +66,7 @@ public class ApiBeta {
   private final Iface api;
 
   @Inject
-  ApiBeta(AuroraAdmin.Iface api) {
+  ApiBeta(AnnotatedAuroraAdmin api) {
     this.api = Objects.requireNonNull(api);
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java b/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
index 2408cd1..63c31ee 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
@@ -28,6 +28,7 @@ import org.apache.aurora.gen.AuroraAdmin;
 import org.apache.aurora.scheduler.http.CorsFilter;
 import org.apache.aurora.scheduler.http.JettyServerModule;
 import org.apache.aurora.scheduler.http.LeaderRedirectFilter;
+import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.apache.thrift.protocol.TJSONProtocol;
 import org.apache.thrift.server.TServlet;
 import org.eclipse.jetty.servlet.DefaultServlet;
@@ -80,7 +81,7 @@ public class ApiModule extends ServletModule {
 
   @Provides
   @Singleton
-  TServlet provideApiThriftServlet(AuroraAdmin.Iface schedulerThriftInterface) {
+  TServlet provideApiThriftServlet(AnnotatedAuroraAdmin schedulerThriftInterface) {
     return new TServlet(
         new AuroraAdmin.Processor<>(schedulerThriftInterface), new TJSONProtocol.Factory());
   }

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
index ec6a02c..1f773ca 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
@@ -13,6 +13,7 @@
  */
 package org.apache.aurora.scheduler.http.api.security;
 
+import java.lang.reflect.Method;
 import java.util.Set;
 
 import javax.inject.Singleton;
@@ -21,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.Module;
 import com.google.inject.Provides;
+import com.google.inject.matcher.Matcher;
 import com.google.inject.matcher.Matchers;
 import com.google.inject.name.Names;
 import com.google.inject.servlet.RequestScoped;
@@ -34,6 +36,7 @@ import org.apache.aurora.gen.AuroraAdmin;
 import org.apache.aurora.gen.AuroraSchedulerManager;
 import org.apache.aurora.scheduler.app.Modules;
 import org.apache.aurora.scheduler.http.api.ApiModule;
+import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.guice.aop.ShiroAopModule;
 import org.apache.shiro.guice.web.ShiroWebModule;
@@ -50,6 +53,16 @@ import static java.util.Objects.requireNonNull;
  * this package.
  */
 public class ApiSecurityModule extends ServletModule {
+  /**
+   * Prefix for the permission protecting all AuroraSchedulerManager RPCs.
+   */
+  public static final String AURORA_SCHEDULER_MANAGER_PERMISSION = "thrift.AuroraSchedulerManager";
+
+  /**
+   * Prefix for the permission protecting all AuroraAdmin RPCs.
+   */
+  public static final String AURORA_ADMIN_PERMISSION = "thrift.AuroraAdmin";
+
   public static final String HTTP_REALM_NAME = "Apache Aurora Scheduler";
 
   @CmdLine(name = "enable_api_security",
@@ -61,6 +74,14 @@ public class ApiSecurityModule extends ServletModule {
   private static final Arg<Set<Module>> SHIRO_REALM_MODULE = Arg.<Set<Module>>create(
       ImmutableSet.of(Modules.lazilyInstantiated(IniShiroRealmModule.class)));
 
+  @VisibleForTesting
+  static final Matcher<Method> AURORA_SCHEDULER_MANAGER_SERVICE =
+      GuiceUtils.interfaceMatcher(AuroraSchedulerManager.Iface.class, true);
+
+  @VisibleForTesting
+  static final Matcher<Method> AURORA_ADMIN_SERVICE =
+      GuiceUtils.interfaceMatcher(AuroraAdmin.Iface.class, true);
+
   private final boolean enableApiSecurity;
   private final Set<Module> shiroConfigurationModules;
 
@@ -110,18 +131,29 @@ public class ApiSecurityModule extends ServletModule {
     // TODO(ksweeney): Disable RememberMe cookie.
 
     install(new ShiroAopModule());
-    MethodInterceptor apiInterceptor = new ShiroThriftInterceptor("thrift.AuroraSchedulerManager");
+
+    // It is important that authentication happen before authorization is attempted, otherwise
+    // the authorizing interceptor will always fail.
+    MethodInterceptor authenticatingInterceptor = new ShiroAuthenticatingThriftInterceptor();
+    requestInjection(authenticatingInterceptor);
+    bindInterceptor(
+        Matchers.subclassesOf(AuroraSchedulerManager.Iface.class),
+        AURORA_SCHEDULER_MANAGER_SERVICE.or(AURORA_ADMIN_SERVICE),
+        authenticatingInterceptor);
+
+    MethodInterceptor apiInterceptor =
+        new ShiroAuthorizingParamInterceptor(AURORA_SCHEDULER_MANAGER_PERMISSION);
     requestInjection(apiInterceptor);
     bindInterceptor(
         Matchers.subclassesOf(AuroraSchedulerManager.Iface.class),
-        GuiceUtils.interfaceMatcher(AuroraSchedulerManager.Iface.class, true),
+        AURORA_SCHEDULER_MANAGER_SERVICE,
         apiInterceptor);
 
-    MethodInterceptor adminInterceptor = new ShiroThriftInterceptor("thrift.AuroraAdmin");
+    MethodInterceptor adminInterceptor = new ShiroAuthorizingInterceptor(AURORA_ADMIN_PERMISSION);
     requestInjection(adminInterceptor);
     bindInterceptor(
-        Matchers.subclassesOf(AuroraAdmin.Iface.class),
-        GuiceUtils.interfaceMatcher(AuroraAdmin.Iface.class, true),
+        Matchers.subclassesOf(AnnotatedAuroraAdmin.class),
+        AURORA_ADMIN_SERVICE,
         adminInterceptor);
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
index 8089879..11d7e46 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
@@ -13,16 +13,17 @@
  */
 package org.apache.aurora.scheduler.http.api.security;
 
+import java.lang.annotation.Documented;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * Signals to {@link org.apache.aurora.scheduler.http.api.security.ShiroThriftInterceptor} that this
- * parameter should be used to determine the instance the calling subject is attempting to operate
- * on. Methods using this parameter should ensure that the RPC cannot operate on an instance
- * outside the scope of this parameter, otherwise a privilege escalation vulnerability exists.
+ * Signals to {@link ShiroAuthorizingParamInterceptor} that this  parameter should be used to
+ * determine the instance the calling subject is attempting to operate on. Methods using this
+ * parameter should ensure that the RPC cannot operate on an instance outside the scope of this
+ * parameter, otherwise a privilege escalation vulnerability exists.
  *
  * <p>
  * A method intercepted by this interceptor that does not contain an AuthorizingParam or with
@@ -69,4 +70,5 @@ import java.lang.annotation.Target;
  */
 @Target(ElementType.PARAMETER)
 @Retention(RetentionPolicy.RUNTIME)
+@Documented
 public @interface AuthorizingParam { }

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetter.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetter.java
new file mode 100644
index 0000000..b2ca012
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetter.java
@@ -0,0 +1,73 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Function for retrieving an optional field from a thrift struct with runtime type-information.
+ *
+ * @param <T> the container struct
+ * @param <V> a field that can be contained within T
+ */
+interface FieldGetter<T, V> extends Function<T, Optional<V>> {
+  /**
+   * The type of the container struct.
+   */
+  Class<T> getStructClass();
+
+  /**
+   * The type of the optionally-contained struct.
+   */
+  Class<V> getValueClass();
+
+  abstract class AbstractFieldGetter<T, V> implements FieldGetter<T, V> {
+    private final Class<T> structClass;
+    private final Class<V> valueClass;
+
+    protected AbstractFieldGetter(Class<T> structClass, Class<V> valueClass) {
+      this.structClass = requireNonNull(structClass);
+      this.valueClass = requireNonNull(valueClass);
+    }
+
+    @Override
+    public final Class<T> getStructClass() {
+      return structClass;
+    }
+
+    @Override
+    public final Class<V> getValueClass() {
+      return valueClass;
+    }
+  }
+
+  /**
+   * Special case of field getter that can get itself.
+   *
+   * @param <T> The input and ouput type.
+   */
+  class IdentityFieldGetter<T> extends AbstractFieldGetter<T, T> {
+    IdentityFieldGetter(Class<T> structClass) {
+      super(structClass, structClass);
+    }
+
+    @Override
+    public Optional<T> apply(T input) {
+      return Optional.fromNullable(input);
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetters.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetters.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetters.java
new file mode 100644
index 0000000..a833672
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetters.java
@@ -0,0 +1,50 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import com.google.common.base.Optional;
+
+import org.apache.thrift.TBase;
+
+final class FieldGetters {
+  private FieldGetters() {
+    // Utility class.
+  }
+
+  public static <P extends TBase<P, ?>, C extends TBase<C, ?>, G extends TBase<G, ?>>
+      FieldGetter<P, G> compose(final FieldGetter<P, C> parent, final FieldGetter<C, G> child) {
+
+    return new FieldGetter<P, G>() {
+      @Override
+      public Class<P> getStructClass() {
+        return parent.getStructClass();
+      }
+
+      @Override
+      public Class<G> getValueClass() {
+        return child.getValueClass();
+      }
+
+      @Override
+      public Optional<G> apply(P input) {
+        Optional<C> parentValue = parent.apply(input);
+        if (parentValue.isPresent()) {
+          return child.apply(parentValue.get());
+        } else {
+          return Optional.absent();
+        }
+      }
+    };
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
new file mode 100644
index 0000000..bf7828b
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
@@ -0,0 +1,63 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import javax.inject.Provider;
+
+import com.google.inject.Inject;
+
+import org.aopalliance.intercept.MethodInterceptor;
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.shiro.authz.UnauthenticatedException;
+import org.apache.shiro.subject.Subject;
+
+import static java.util.Objects.requireNonNull;
+
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Prevents invocation of intercepted methods if the current {@link Subject} is not authenticated.
+ */
+class ShiroAuthenticatingThriftInterceptor implements MethodInterceptor {
+  private volatile boolean initialized;
+
+  private Provider<Subject> subjectProvider;
+
+  ShiroAuthenticatingThriftInterceptor() {
+    // Guice constructor.
+  }
+
+  @Inject
+  void initialize(Provider<Subject> newSubjectProvider) {
+    checkState(!initialized);
+
+    subjectProvider = requireNonNull(newSubjectProvider);
+
+    initialized = true;
+  }
+
+  @Override
+  public Object invoke(MethodInvocation invocation) throws Throwable {
+    checkState(initialized);
+    Subject subject = subjectProvider.get();
+    if (subject.isAuthenticated()) {
+      return invocation.proceed();
+    } else {
+      // This is a special exception that will signal the BasicHttpAuthenticationFilter to send
+      // a 401 with a challenge. This is necessary at this layer since we only apply this
+      // interceptor to methods that require authentication.
+      throw new UnauthenticatedException();
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptor.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptor.java
new file mode 100644
index 0000000..7a124cc
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptor.java
@@ -0,0 +1,100 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import java.lang.reflect.Method;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.logging.Logger;
+
+import javax.inject.Inject;
+import javax.inject.Provider;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
+import com.twitter.common.base.MorePreconditions;
+import com.twitter.common.stats.StatsProvider;
+
+import org.aopalliance.intercept.MethodInterceptor;
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.aurora.gen.Response;
+import org.apache.aurora.gen.ResponseCode;
+import org.apache.aurora.scheduler.thrift.Responses;
+import org.apache.shiro.authz.Permission;
+import org.apache.shiro.authz.permission.WildcardPermission;
+import org.apache.shiro.subject.Subject;
+
+import static java.util.Objects.requireNonNull;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Does not allow the intercepted method call to proceed if the caller does not have permission to
+ * call it.
+ * The {@link org.apache.shiro.authz.Permission} checked is a
+ * {@link org.apache.shiro.authz.permission.WildcardPermission} constructed from a prefix and
+ * the name of the method being invoked. For example if the prefix is {@code api} and the method
+ * is {@code snapshot} the current {@link org.apache.shiro.subject.Subject} must have the
+ * {@code api:snapshot} permission.
+ */
+class ShiroAuthorizingInterceptor implements MethodInterceptor {
+  private static final Logger LOG = Logger.getLogger(ShiroAuthorizingInterceptor.class.getName());
+
+  @VisibleForTesting
+  static final String SHIRO_AUTHORIZATION_FAILURES = "shiro_authorization_failures";
+
+  private static final Joiner PERMISSION_JOINER = Joiner.on(":");
+
+  private final String permissionPrefix;
+
+  private volatile boolean initialized;
+
+  private Provider<Subject> subjectProvider;
+  private AtomicLong shiroAdminAuthorizationFailures;
+
+  ShiroAuthorizingInterceptor(String permissionPrefix) {
+    this.permissionPrefix = MorePreconditions.checkNotBlank(permissionPrefix);
+  }
+
+  @Inject
+  void initialize(Provider<Subject> newSubjectProvider, StatsProvider statsProvider) {
+    checkState(!initialized);
+
+    subjectProvider = requireNonNull(newSubjectProvider);
+    shiroAdminAuthorizationFailures = statsProvider.makeCounter(SHIRO_AUTHORIZATION_FAILURES);
+
+    initialized = true;
+  }
+
+  @Override
+  public Object invoke(MethodInvocation invocation) throws Throwable {
+    checkState(initialized);
+    Method method = invocation.getMethod();
+    checkArgument(Response.class.isAssignableFrom(method.getReturnType()));
+
+    Subject subject = subjectProvider.get();
+    Permission checkedPermission = new WildcardPermission(
+        PERMISSION_JOINER.join(permissionPrefix, method.getName()));
+    if (subject.isPermitted(checkedPermission)) {
+      return invocation.proceed();
+    } else {
+      shiroAdminAuthorizationFailures.incrementAndGet();
+      String responseMessage =
+          "Subject " + subject.getPrincipal() + " lacks permission " + checkedPermission;
+      LOG.warning(responseMessage);
+      // TODO(ksweeney): 403 FORBIDDEN would be a more accurate translation of this response code.
+      return Responses.addMessage(Responses.empty(), ResponseCode.AUTH_FAILED, responseMessage);
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
new file mode 100644
index 0000000..fde6c84
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
@@ -0,0 +1,372 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import java.lang.reflect.Method;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+import javax.inject.Provider;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.base.Optional;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.AbstractSequentialIterator;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.reflect.Invokable;
+import com.google.common.reflect.Parameter;
+import com.twitter.common.base.MorePreconditions;
+import com.twitter.common.stats.StatsProvider;
+
+import org.aopalliance.intercept.MethodInterceptor;
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.aurora.gen.AddInstancesConfig;
+import org.apache.aurora.gen.JobConfiguration;
+import org.apache.aurora.gen.JobKey;
+import org.apache.aurora.gen.JobUpdateKey;
+import org.apache.aurora.gen.JobUpdateRequest;
+import org.apache.aurora.gen.Lock;
+import org.apache.aurora.gen.LockKey;
+import org.apache.aurora.gen.Response;
+import org.apache.aurora.gen.ResponseCode;
+import org.apache.aurora.gen.TaskConfig;
+import org.apache.aurora.gen.TaskQuery;
+import org.apache.aurora.scheduler.base.JobKeys;
+import org.apache.aurora.scheduler.base.Query;
+import org.apache.aurora.scheduler.http.api.security.FieldGetter.AbstractFieldGetter;
+import org.apache.aurora.scheduler.http.api.security.FieldGetter.IdentityFieldGetter;
+import org.apache.aurora.scheduler.storage.entities.IJobKey;
+import org.apache.aurora.scheduler.thrift.Responses;
+import org.apache.shiro.authz.Permission;
+import org.apache.shiro.authz.permission.WildcardPermission;
+import org.apache.shiro.subject.Subject;
+import org.apache.thrift.TBase;
+
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Interceptor that extracts and validates job keys from parameters annotated with
+ * {@link org.apache.aurora.scheduler.http.api.security.AuthorizingParam} and performs permission
+ * checks scoped to it.
+ *
+ * <p>
+ * For example, if intercepting a class that implements {@code A}:
+ *
+ * <pre>
+ * public interface A {
+ *   Response setInstances(@AuthorizingParam JobKey jobKey, int instances);
+ * }
+ * </pre>
+ *
+ * This interceptor will check that the current {@link org.apache.shiro.subject.Subject} has the
+ * permission (prefix + ":setInstances:role:env:name").
+ *
+ * <p>
+ * It is important that this interceptor only be applied to methods returning
+ * {@link org.apache.aurora.gen.Response} and that authentication is called before this interceptor
+ * is invoked, otherwise this interceptor will not allow the invocation to proceed.
+ */
+class ShiroAuthorizingParamInterceptor implements MethodInterceptor {
+  @VisibleForTesting
+  static final FieldGetter<TaskQuery, JobKey> QUERY_TO_JOB_KEY =
+      new AbstractFieldGetter<TaskQuery, JobKey>(TaskQuery.class, JobKey.class) {
+        @Override
+        public Optional<JobKey> apply(TaskQuery input) {
+          Optional<Set<IJobKey>> targetJobs = JobKeys.from(Query.arbitrary(input));
+          if (targetJobs.isPresent() && targetJobs.get().size() == 1) {
+            return Optional.of(Iterables.getOnlyElement(targetJobs.get()))
+                .transform(IJobKey.TO_BUILDER);
+          } else {
+            return Optional.absent();
+          }
+        }
+      };
+
+  private static final FieldGetter<JobUpdateRequest, TaskConfig> UPDATE_REQUEST_GETTER =
+      new ThriftFieldGetter<>(
+          JobUpdateRequest.class,
+          JobUpdateRequest._Fields.TASK_CONFIG,
+          TaskConfig.class);
+
+  private static final FieldGetter<TaskConfig, JobKey> TASK_CONFIG_GETTER =
+      new ThriftFieldGetter<>(TaskConfig.class, TaskConfig._Fields.JOB, JobKey.class);
+
+  private static final FieldGetter<JobConfiguration, JobKey> JOB_CONFIGURATION_GETTER =
+      new ThriftFieldGetter<>(JobConfiguration.class, JobConfiguration._Fields.KEY, JobKey.class);
+
+  private static final FieldGetter<Lock, LockKey> LOCK_GETTER =
+      new ThriftFieldGetter<>(Lock.class, Lock._Fields.KEY, LockKey.class);
+
+  private static final FieldGetter<LockKey, JobKey> LOCK_KEY_GETTER =
+      new ThriftFieldGetter<>(LockKey.class, LockKey._Fields.JOB, JobKey.class);
+
+  private static final FieldGetter<JobUpdateKey, JobKey> JOB_UPDATE_KEY_GETTER =
+      new ThriftFieldGetter<>(JobUpdateKey.class, JobUpdateKey._Fields.JOB, JobKey.class);
+
+  private static final FieldGetter<AddInstancesConfig, JobKey> ADD_INSTANCES_CONFIG_GETTER =
+      new ThriftFieldGetter<>(
+          AddInstancesConfig.class,
+          AddInstancesConfig._Fields.KEY,
+          JobKey.class);
+
+  @SuppressWarnings("unchecked")
+  private static final Set<FieldGetter<?, JobKey>> FIELD_GETTERS =
+      ImmutableSet.<FieldGetter<?, JobKey>>of(
+          FieldGetters.compose(UPDATE_REQUEST_GETTER, TASK_CONFIG_GETTER),
+          TASK_CONFIG_GETTER,
+          JOB_CONFIGURATION_GETTER,
+          FieldGetters.compose(LOCK_GETTER, LOCK_KEY_GETTER),
+          LOCK_KEY_GETTER,
+          JOB_UPDATE_KEY_GETTER,
+          ADD_INSTANCES_CONFIG_GETTER,
+          QUERY_TO_JOB_KEY,
+          new IdentityFieldGetter<>(JobKey.class));
+
+  private static final Map<Class<?>, Function<?, Optional<JobKey>>> FIELD_GETTERS_BY_TYPE =
+      ImmutableMap.<Class<?>, Function<?, Optional<JobKey>>>builder()
+          .putAll(Maps.uniqueIndex(
+              FIELD_GETTERS,
+              new Function<FieldGetter<?, JobKey>, Class<?>>() {
+                @Override
+                public Class<?> apply(FieldGetter<?, JobKey> input) {
+                  return input.getStructClass();
+                }
+              }))
+          .build();
+
+  @VisibleForTesting
+  static final String SHIRO_AUTHORIZATION_FAILURES = "shiro_authorization_failures";
+
+  @VisibleForTesting
+  static final String SHIRO_BAD_REQUESTS = "shiro_bad_requests";
+
+  /**
+   * Return each method in the inheritance hierarchy of method in the order described by
+   * {@link AuthorizingParam}.
+   *
+   * @see org.apache.aurora.scheduler.http.api.security.AuthorizingParam
+   */
+  private static Iterable<Method> getCandidateMethods(final Method method) {
+    return new Iterable<Method>() {
+      @Override
+      public Iterator<Method> iterator() {
+        return new AbstractSequentialIterator<Method>(method) {
+          @Override
+          protected Method computeNext(Method previous) {
+            String name = previous.getName();
+            Class<?>[] parameterTypes = previous.getParameterTypes();
+            Class<?> declaringClass = previous.getDeclaringClass();
+
+            if (declaringClass.isInterface()) {
+              return null;
+            }
+
+            Iterable<Class<?>> searchOrder = ImmutableList.<Class<?>>builder()
+                .addAll(Optional.fromNullable(declaringClass.getSuperclass()).asSet())
+                .addAll(ImmutableList.copyOf(declaringClass.getInterfaces()))
+                .build();
+
+            for (Class<?> klazz : searchOrder) {
+              try {
+                return klazz.getMethod(name, parameterTypes);
+              } catch (NoSuchMethodException ignored) {
+                // Expected.
+              }
+            }
+
+            return null;
+          }
+        };
+      }
+    };
+  }
+
+  private static int annotatedParameterIndex(Method method) {
+    for (Method candidateMethod : getCandidateMethods(method)) {
+      List<Parameter> parameters = Invokable.from(candidateMethod).getParameters();
+      List<Integer> parameterIndicies = Lists.newArrayList();
+      for (int i = 0; i < parameters.size(); i++) {
+        if (parameters.get(i).isAnnotationPresent(AuthorizingParam.class)) {
+          parameterIndicies.add(i);
+        }
+      }
+
+      if (parameterIndicies.size() == 1) {
+        return Iterables.getOnlyElement(parameterIndicies);
+      } else if (parameterIndicies.size() > 1) {
+        throw new UnsupportedOperationException(
+            "Too many parameters annotated with "
+                + AuthorizingParam.class.getName()
+                + " found on method "
+                + method.getName()
+                + " of class "
+                + method.getDeclaringClass().getName());
+      }
+    }
+
+    throw new UnsupportedOperationException(
+        "No parameter annotated with "
+            + AuthorizingParam.class.getName()
+            + " found on method "
+            + method.getName()
+            + " of "
+            + method.getDeclaringClass().getName()
+            + " or any of its superclasses.");
+  }
+
+  private static final CacheLoader<Method, Function<Object[], Optional<JobKey>>> LOADER =
+      new CacheLoader<Method, Function<Object[], Optional<JobKey>>>() {
+        @Override
+        public Function<Object[], Optional<JobKey>> load(Method method) {
+          if (!Response.class.isAssignableFrom(method.getReturnType())) {
+            throw new UnsupportedOperationException(
+                "Method "
+                    + method.getName()
+                    + " of class "
+                    + method.getDeclaringClass().getName()
+                    + " does not return "
+                    + Response.class.getName());
+          }
+
+          final int index = annotatedParameterIndex(method);
+          ImmutableList<Parameter> parameters = Invokable.from(method).getParameters();
+          Class<?> parameterType = parameters.get(index).getType().getRawType();
+          if (!TBase.class.isAssignableFrom(parameterType)) {
+            throw new UnsupportedOperationException(
+                "Annotated parameter must be a thrift struct.");
+          }
+          @SuppressWarnings("unchecked")
+          final Optional<Function<Object, Optional<JobKey>>> jobKeyGetter =
+              Optional.fromNullable(
+                  (Function<Object, Optional<JobKey>>) FIELD_GETTERS_BY_TYPE.get(parameterType));
+          if (!jobKeyGetter.isPresent()) {
+            throw new UnsupportedOperationException(
+                "No "
+                    + JobKey.class.getName()
+                    + " field getter was supplied for "
+                    + parameterType.getName());
+          }
+          return new Function<Object[], Optional<JobKey>>() {
+            @Override
+            public Optional<JobKey> apply(Object[] arguments) {
+              Optional<Object> argument = Optional.fromNullable(arguments[index]);
+              if (argument.isPresent()) {
+                return jobKeyGetter.get().apply(argument.get());
+              } else {
+                return Optional.absent();
+              }
+            }
+          };
+        }
+      };
+
+  private static final Joiner COLON_JOINER = Joiner.on(":");
+
+  private final LoadingCache<Method, Function<Object[], Optional<JobKey>>> authorizingParamGetters =
+      CacheBuilder.<Method, Function<Object[], Optional<JobKey>>>newBuilder().build(LOADER);
+
+  private final String permissionPrefix;
+  private volatile boolean initialized;
+
+  private Provider<Subject> subjectProvider;
+  private AtomicLong authorizationFailures;
+  private AtomicLong badRequests;
+
+  ShiroAuthorizingParamInterceptor(String permissionPrefix) {
+    this.permissionPrefix = MorePreconditions.checkNotBlank(permissionPrefix);
+  }
+
+  @Inject
+  void initialize(Provider<Subject> newSubjectProvider, StatsProvider statsProvider) {
+    checkState(!initialized);
+
+    this.subjectProvider = Objects.requireNonNull(newSubjectProvider);
+    authorizationFailures = statsProvider.makeCounter(SHIRO_AUTHORIZATION_FAILURES);
+    badRequests = statsProvider.makeCounter(SHIRO_BAD_REQUESTS);
+
+    initialized = true;
+  }
+
+  @VisibleForTesting
+  Permission makeWildcardPermission(String methodName) {
+    return new WildcardPermission(
+        COLON_JOINER.join(permissionPrefix, methodName));
+  }
+
+  @VisibleForTesting
+  Permission makeTargetPermission(String methodName, IJobKey jobKey) {
+    return new WildcardPermission(
+        COLON_JOINER.join(
+            permissionPrefix,
+            methodName,
+            jobKey.getRole(),
+            jobKey.getEnvironment(),
+            jobKey.getName()));
+  }
+
+  @Override
+  public Object invoke(MethodInvocation invocation) throws Throwable {
+    checkState(initialized);
+
+    Method method = invocation.getMethod();
+    // Short-circuit request processing restrictions if the caller would be allowed to
+    // operate on every possible job key. This allows a broadly-scoped TaskQuery.
+    Subject subject = subjectProvider.get();
+    if (subject.isPermitted(makeWildcardPermission(method.getName()))) {
+      return invocation.proceed();
+    }
+
+    Optional<IJobKey> jobKey = authorizingParamGetters
+        .getUnchecked(invocation.getMethod())
+        .apply(invocation.getArguments())
+        .transform(IJobKey.FROM_BUILDER);
+    if (jobKey.isPresent() && JobKeys.isValid(jobKey.get())) {
+      Permission targetPermission = makeTargetPermission(method.getName(), jobKey.get());
+      if (subject.isPermitted(targetPermission)) {
+        return invocation.proceed();
+      } else {
+        authorizationFailures.incrementAndGet();
+        return Responses.addMessage(
+            Responses.empty(),
+            ResponseCode.AUTH_FAILED,
+            "Subject " + subject + " is not permitted to " + targetPermission + ".");
+      }
+    } else {
+      badRequests.incrementAndGet();
+      return Responses.addMessage(
+          Responses.empty(),
+          ResponseCode.INVALID_REQUEST,
+          "Missing or invalid job key from request.");
+    }
+  }
+
+  @VisibleForTesting
+  LoadingCache<Method, Function<Object[], Optional<JobKey>>> getAuthorizingParamGetters() {
+    return authorizingParamGetters;
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
deleted file mode 100644
index 4e341e0..0000000
--- a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
+++ /dev/null
@@ -1,101 +0,0 @@
-/**
- * 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 org.apache.aurora.scheduler.http.api.security;
-
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.logging.Logger;
-
-import javax.inject.Provider;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Joiner;
-import com.google.inject.Inject;
-import com.twitter.common.base.MorePreconditions;
-import com.twitter.common.stats.StatsProvider;
-
-import org.aopalliance.intercept.MethodInterceptor;
-import org.aopalliance.intercept.MethodInvocation;
-import org.apache.aurora.gen.ResponseCode;
-import org.apache.aurora.scheduler.thrift.Responses;
-import org.apache.shiro.authz.Permission;
-import org.apache.shiro.authz.UnauthenticatedException;
-import org.apache.shiro.authz.permission.WildcardPermission;
-import org.apache.shiro.subject.Subject;
-
-import static java.util.Objects.requireNonNull;
-
-import static com.google.common.base.Preconditions.checkState;
-
-/**
- * Prevents invocation of intercepted methods if the current {@link Subject} is not authenticated
- * or the current subject does not have the permission defined a prefix the name of the method
- * attempting to be invoked. The {@link org.apache.shiro.authz.Permission} checked is a
- * {@link org.apache.shiro.authz.permission.WildcardPermission} constructed from the prefix and
- * the name of the method being invoked, for example if the prefix is {@code api} and the method
- * is {@code snapshot} the current subject must have the permission {@code api:snapshot} permission.
- */
-class ShiroThriftInterceptor implements MethodInterceptor {
-  private static final Logger LOG = Logger.getLogger(ShiroThriftInterceptor.class.getName());
-  private static final Joiner PERMISSION_JOINER = Joiner.on(":");
-
-  @VisibleForTesting
-  static final String SHIRO_AUTHORIZATION_FAILURES = "shiro_authorization_failures";
-
-  private final String permissionPrefix;
-
-  private volatile boolean initialized;
-
-  private Provider<Subject> subjectProvider;
-  private AtomicLong shiroAuthorizationFailures;
-
-  @Inject
-  void initialize(Provider<Subject> newSubjectProvider, StatsProvider statsProvider) {
-    checkState(!initialized);
-
-    this.subjectProvider = requireNonNull(newSubjectProvider);
-    shiroAuthorizationFailures = statsProvider.makeCounter(SHIRO_AUTHORIZATION_FAILURES);
-
-    initialized = true;
-  }
-
-  public ShiroThriftInterceptor(String permissionPrefix) {
-    this.permissionPrefix = MorePreconditions.checkNotBlank(permissionPrefix);
-  }
-
-  @Override
-  public Object invoke(MethodInvocation invocation) throws Throwable {
-    checkState(initialized);
-
-    Subject subject = subjectProvider.get();
-    if (!subject.isAuthenticated()) {
-      // This is a special exception that will signal the BasicHttpAuthenticationFilter to send
-      // a 401 with a challenge. This is necessary at this layer since we only apply this
-      // interceptor to methods that require authentication.
-      throw new UnauthenticatedException();
-    }
-
-    Permission checkedPermission = new WildcardPermission(
-        PERMISSION_JOINER.join(permissionPrefix, invocation.getMethod().getName()));
-    if (subject.isPermitted(checkedPermission)) {
-      return invocation.proceed();
-    } else {
-      shiroAuthorizationFailures.incrementAndGet();
-      String responseMessage =
-          "Subject " + subject.getPrincipal() + " lacks permission " + checkedPermission;
-      LOG.warning(responseMessage);
-      // TODO(ksweeney): 403 FORBIDDEN would be a more accurate translation of this response code.
-      return Responses.addMessage(Responses.empty(), ResponseCode.AUTH_FAILED, responseMessage);
-    }
-  }
-}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetter.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetter.java
new file mode 100644
index 0000000..2044b79
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetter.java
@@ -0,0 +1,66 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import com.google.common.base.Optional;
+
+import org.apache.aurora.scheduler.http.api.security.FieldGetter.AbstractFieldGetter;
+import org.apache.thrift.TBase;
+import org.apache.thrift.TFieldIdEnum;
+import org.apache.thrift.meta_data.FieldMetaData;
+import org.apache.thrift.meta_data.FieldValueMetaData;
+import org.apache.thrift.meta_data.StructMetaData;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Retrieves an optional struct-type field from a struct.
+ */
+class ThriftFieldGetter<T extends TBase<T, F>, F extends TFieldIdEnum, V extends TBase<V, ?>>
+    extends AbstractFieldGetter<T, V> {
+
+  private final F fieldId;
+
+  ThriftFieldGetter(Class<T> structClass, F fieldId, Class<V> valueClass) {
+    super(structClass, valueClass);
+
+    FieldValueMetaData fieldValueMetaData = FieldMetaData
+        .getStructMetaDataMap(structClass)
+        .get(fieldId)
+        .valueMetaData;
+
+    checkArgument(fieldValueMetaData instanceof StructMetaData);
+    StructMetaData structMetaData = (StructMetaData) fieldValueMetaData;
+    checkArgument(
+        valueClass.equals(structMetaData.structClass),
+        "Value class "
+            + valueClass.getName()
+            + " does not match field metadata for "
+            + fieldId
+            + " (expected " + structMetaData.structClass
+            + ").");
+
+    this.fieldId = fieldId;
+  }
+
+  @Override
+  @SuppressWarnings("unchecked")
+  public Optional<V> apply(T input) {
+    if (input.isSet(fieldId)) {
+      return Optional.of((V) input.getFieldValue(fieldId));
+    } else {
+      return Optional.absent();
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
index bdd2185..3490394 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
@@ -59,7 +59,7 @@ public class AopModule extends AbstractModule {
   private static final Arg<Boolean> ENABLE_JOB_CREATION = Arg.create(true);
 
   private static final Matcher<? super Class<?>> THRIFT_IFACE_MATCHER =
-      Matchers.subclassesOf(AuroraAdmin.Iface.class)
+      Matchers.subclassesOf(AnnotatedAuroraAdmin.class)
           .and(Matchers.annotatedWith(DecoratedThrift.class));
 
   private final Map<String, Boolean> toggledMethods;

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java b/src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java
index cafd10f..08e1284 100644
--- a/src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java
@@ -30,7 +30,6 @@ import com.sun.jersey.api.client.config.ClientConfig;
 import com.sun.jersey.api.client.config.DefaultClientConfig;
 
 import org.apache.aurora.gen.AssignedTask;
-import org.apache.aurora.gen.AuroraAdmin;
 import org.apache.aurora.gen.Constraint;
 import org.apache.aurora.gen.CronCollisionPolicy;
 import org.apache.aurora.gen.ExecutorConfig;
@@ -55,6 +54,7 @@ import org.apache.aurora.gen.TaskQuery;
 import org.apache.aurora.scheduler.http.JettyServerModuleTest;
 import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
+import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -64,11 +64,11 @@ import static org.easymock.EasyMock.expect;
 import static org.junit.Assert.assertEquals;
 
 public class ApiBetaTest extends JettyServerModuleTest {
-  private AuroraAdmin.Iface thrift;
+  private AnnotatedAuroraAdmin thrift;
 
   @Before
   public void setUp() {
-    thrift = createMock(AuroraAdmin.Iface.class);
+    thrift = createMock(AnnotatedAuroraAdmin.class);
   }
 
   @Override
@@ -78,7 +78,7 @@ public class ApiBetaTest extends JettyServerModuleTest {
         new AbstractModule() {
           @Override
           protected void configure() {
-            bind(AuroraAdmin.Iface.class).toInstance(thrift);
+            bind(AnnotatedAuroraAdmin.class).toInstance(thrift);
           }
         }
     );

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java b/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
index ed284f4..aa3a85a 100644
--- a/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
@@ -20,9 +20,9 @@ import com.google.inject.Module;
 import com.google.inject.util.Modules;
 import com.sun.jersey.api.client.ClientResponse;
 
-import org.apache.aurora.gen.AuroraAdmin;
 import org.apache.aurora.gen.Response;
 import org.apache.aurora.scheduler.http.JettyServerModuleTest;
+import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -30,11 +30,11 @@ import static org.easymock.EasyMock.expect;
 import static org.junit.Assert.assertEquals;
 
 public class ApiIT extends JettyServerModuleTest {
-  private AuroraAdmin.Iface thrift;
+  private AnnotatedAuroraAdmin thrift;
 
   @Before
   public void setUp() {
-    thrift = createMock(AuroraAdmin.Iface.class);
+    thrift = createMock(AnnotatedAuroraAdmin.class);
   }
 
   @Override
@@ -44,7 +44,7 @@ public class ApiIT extends JettyServerModuleTest {
         new AbstractModule() {
           @Override
           protected void configure() {
-            bind(AuroraAdmin.Iface.class).toInstance(thrift);
+            bind(AnnotatedAuroraAdmin.class).toInstance(thrift);
           }
         });
   }

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
index 76cb691..45a23fd 100644
--- a/src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
@@ -17,6 +17,7 @@ import java.io.IOException;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.testing.TearDown;
@@ -29,8 +30,12 @@ import org.apache.aurora.gen.AuroraAdmin;
 import org.apache.aurora.gen.Lock;
 import org.apache.aurora.gen.Response;
 import org.apache.aurora.gen.ResponseCode;
+import org.apache.aurora.gen.TaskQuery;
+import org.apache.aurora.scheduler.base.JobKeys;
+import org.apache.aurora.scheduler.base.Query;
 import org.apache.aurora.scheduler.http.JettyServerModuleTest;
 import org.apache.aurora.scheduler.http.api.ApiModule;
+import org.apache.aurora.scheduler.storage.entities.IJobKey;
 import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.apache.aurora.scheduler.thrift.aop.MockDecoratedThrift;
 import org.apache.http.auth.AuthScope;
@@ -67,6 +72,8 @@ public class ApiSecurityIT extends JettyServerModuleTest {
       new UsernamePasswordCredentials("ksweeney", "12345");
   private static final UsernamePasswordCredentials BACKUP_SERVICE =
       new UsernamePasswordCredentials("backupsvc", "s3cret!!1");
+  private static final UsernamePasswordCredentials DEPLOY_SERVICE =
+      new UsernamePasswordCredentials("deploysvc", "0_0-x_0");
 
   private static final UsernamePasswordCredentials INCORRECT =
       new UsernamePasswordCredentials("root", "wrong");
@@ -79,24 +86,45 @@ public class ApiSecurityIT extends JettyServerModuleTest {
   private static final Set<Credentials> VALID_CREDENTIALS =
       ImmutableSet.<Credentials>of(ROOT, WFARNER, UNPRIVILEGED, BACKUP_SERVICE);
 
+  private static final IJobKey ADS_STAGING_JOB = JobKeys.from("ads", "staging", "job");
+
   private Ini ini;
   private AnnotatedAuroraAdmin auroraAdmin;
   private StatsProvider statsProvider;
 
+  private static final Joiner COMMA_JOINER = Joiner.on(", ");
+  private static final String ADMIN_ROLE = "admin";
+  private static final String ENG_ROLE = "eng";
+  private static final String BACKUP_ROLE = "backup";
+  private static final String DEPLOY_ROLE = "deploy";
+
   @Before
   public void setUp() {
     ini = new Ini();
 
     Ini.Section users = ini.addSection(IniRealm.USERS_SECTION_NAME);
-    users.put(ROOT.getUserName(), ROOT.getPassword() + ", admin");
-    users.put(WFARNER.getUserName(), WFARNER.getPassword() + ", eng");
+    users.put(ROOT.getUserName(), COMMA_JOINER.join(ROOT.getPassword(), ADMIN_ROLE));
+    users.put(WFARNER.getUserName(), COMMA_JOINER.join(WFARNER.getPassword(), ENG_ROLE));
     users.put(UNPRIVILEGED.getUserName(), UNPRIVILEGED.getPassword());
-    users.put(BACKUP_SERVICE.getUserName(), BACKUP_SERVICE.getPassword() + ", backupsvc");
+    users.put(
+        BACKUP_SERVICE.getUserName(),
+        COMMA_JOINER.join(BACKUP_SERVICE.getPassword(), BACKUP_ROLE));
+    users.put(
+        DEPLOY_SERVICE.getUserName(),
+        COMMA_JOINER.join(DEPLOY_SERVICE.getPassword(), DEPLOY_ROLE));
 
     Ini.Section roles = ini.addSection(IniRealm.ROLES_SECTION_NAME);
-    roles.put("admin", "*");
-    roles.put("eng", "thrift.AuroraSchedulerManager:*");
-    roles.put("backupsvc", "thrift.AuroraAdmin:listBackups");
+    roles.put(ADMIN_ROLE, "*");
+    roles.put(ENG_ROLE, "thrift.AuroraSchedulerManager:*");
+    roles.put(BACKUP_ROLE, "thrift.AuroraAdmin:listBackups");
+    roles.put(
+        DEPLOY_ROLE,
+        "thrift.AuroraSchedulerManager:killTasks:"
+            + ADS_STAGING_JOB.getRole()
+            + ":"
+            + ADS_STAGING_JOB.getEnvironment()
+            + ":"
+            + ADS_STAGING_JOB.getName());
 
     auroraAdmin = createMock(AnnotatedAuroraAdmin.class);
     statsProvider = createMock(StatsProvider.class);
@@ -173,6 +201,10 @@ public class ApiSecurityIT extends JettyServerModuleTest {
     expect(auroraAdmin.killTasks(null, new Lock().setMessage("1"), null)).andReturn(OK);
     expect(auroraAdmin.killTasks(null, new Lock().setMessage("2"), null)).andReturn(OK);
 
+    TaskQuery jobScopedQuery = Query.jobScoped(JobKeys.from("role", "env", "name")).get();
+    TaskQuery adsScopedQuery = Query.jobScoped(ADS_STAGING_JOB).get();
+    expect(auroraAdmin.killTasks(adsScopedQuery, null, null)).andReturn(OK);
+
     replayAndStart();
 
     assertEquals(OK,
@@ -180,11 +212,29 @@ public class ApiSecurityIT extends JettyServerModuleTest {
     assertEquals(OK,
         getAuthenticatedClient(ROOT).killTasks(null, new Lock().setMessage("2"), null));
     assertEquals(
-        ResponseCode.AUTH_FAILED,
+        ResponseCode.INVALID_REQUEST,
         getAuthenticatedClient(UNPRIVILEGED).killTasks(null, null, null).getResponseCode());
     assertEquals(
         ResponseCode.AUTH_FAILED,
+        getAuthenticatedClient(UNPRIVILEGED)
+            .killTasks(jobScopedQuery, null, null)
+            .getResponseCode());
+    assertEquals(
+        ResponseCode.INVALID_REQUEST,
         getAuthenticatedClient(BACKUP_SERVICE).killTasks(null, null, null).getResponseCode());
+    assertEquals(
+        ResponseCode.AUTH_FAILED,
+        getAuthenticatedClient(BACKUP_SERVICE)
+            .killTasks(jobScopedQuery, null, null)
+            .getResponseCode());
+    assertEquals(
+        ResponseCode.AUTH_FAILED,
+        getAuthenticatedClient(DEPLOY_SERVICE)
+            .killTasks(jobScopedQuery, null, null)
+            .getResponseCode());
+    assertEquals(
+        OK,
+        getAuthenticatedClient(DEPLOY_SERVICE).killTasks(adsScopedQuery, null, null));
 
     assertKillTasksFails(getUnauthenticatedClient());
     assertKillTasksFails(getAuthenticatedClient(INCORRECT));

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
new file mode 100644
index 0000000..568cd8f
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
@@ -0,0 +1,66 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import com.google.inject.util.Providers;
+import com.twitter.common.testing.easymock.EasyMockTest;
+
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.aurora.gen.Response;
+import org.apache.aurora.scheduler.thrift.Responses;
+import org.apache.shiro.authz.UnauthenticatedException;
+import org.apache.shiro.subject.Subject;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.easymock.EasyMock.expect;
+import static org.junit.Assert.assertSame;
+
+public class ShiroAuthenticatingThriftInterceptorTest extends EasyMockTest {
+  private ShiroAuthenticatingThriftInterceptor interceptor;
+  private Subject subject;
+  private MethodInvocation methodInvocation;
+
+  @Before
+  public void setUp() throws Exception {
+    interceptor = new ShiroAuthenticatingThriftInterceptor();
+    subject = createMock(Subject.class);
+    methodInvocation = createMock(MethodInvocation.class);
+  }
+
+  private void replayAndInitialize() {
+    control.replay();
+    interceptor.initialize(Providers.of(subject));
+  }
+
+  @Test(expected = UnauthenticatedException.class)
+  public void testInvokeNotAuthenticated() throws Throwable {
+    expect(subject.isAuthenticated()).andReturn(false);
+
+    replayAndInitialize();
+
+    interceptor.invoke(methodInvocation);
+  }
+
+  @Test
+  public void testInvokeAuthenticated() throws Throwable {
+    Response response = Responses.ok();
+    expect(subject.isAuthenticated()).andReturn(true);
+    expect(methodInvocation.proceed()).andReturn(response);
+
+    replayAndInitialize();
+
+    assertSame(response, interceptor.invoke(methodInvocation));
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptorTest.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptorTest.java
new file mode 100644
index 0000000..16f2da5
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptorTest.java
@@ -0,0 +1,94 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import java.lang.reflect.Method;
+import java.util.concurrent.atomic.AtomicLong;
+
+import com.google.inject.util.Providers;
+import com.twitter.common.stats.StatsProvider;
+import com.twitter.common.testing.easymock.EasyMockTest;
+
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.aurora.gen.AuroraAdmin;
+import org.apache.aurora.gen.Response;
+import org.apache.aurora.gen.ResponseCode;
+import org.apache.aurora.gen.SessionKey;
+import org.apache.aurora.scheduler.thrift.Responses;
+import org.apache.shiro.authz.permission.WildcardPermission;
+import org.apache.shiro.subject.Subject;
+import org.easymock.IExpectationSetters;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.apache.aurora.scheduler.http.api.security.ShiroAuthorizingInterceptor.SHIRO_AUTHORIZATION_FAILURES;
+import static org.easymock.EasyMock.expect;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+
+public class ShiroAuthorizingInterceptorTest extends EasyMockTest {
+  private static final String PERMISSION_PREFIX = "adminRPC";
+
+  private Subject subject;
+  private StatsProvider statsProvider;
+  private MethodInvocation methodInvocation;
+  private Method interceptedMethod;
+
+  private ShiroAuthorizingInterceptor interceptor;
+
+  @Before
+  public void setUp() throws NoSuchMethodException {
+    interceptor = new ShiroAuthorizingInterceptor(PERMISSION_PREFIX);
+    subject = createMock(Subject.class);
+    statsProvider = createMock(StatsProvider.class);
+    methodInvocation = createMock(MethodInvocation.class);
+    interceptedMethod = AuroraAdmin.Iface.class.getMethod("snapshot", SessionKey.class);
+    expect(statsProvider.makeCounter(SHIRO_AUTHORIZATION_FAILURES)).andReturn(new AtomicLong());
+  }
+
+  private void replayAndInitialize() {
+    control.replay();
+    interceptor.initialize(Providers.of(subject), statsProvider);
+  }
+
+  private IExpectationSetters<Boolean> expectSubjectPermitted() {
+    return expect(subject.isPermitted(
+        new WildcardPermission(PERMISSION_PREFIX + ":" + interceptedMethod.getName())));
+  }
+
+  @Test
+  public void testAuthorized() throws Throwable {
+    Response response = Responses.ok();
+    expect(methodInvocation.getMethod()).andReturn(interceptedMethod);
+    expectSubjectPermitted().andReturn(true);
+    expect(methodInvocation.proceed()).andReturn(response);
+
+    replayAndInitialize();
+
+    assertSame(response, interceptor.invoke(methodInvocation));
+  }
+
+  @Test
+  public void testNotAuthorized() throws Throwable {
+    expect(methodInvocation.getMethod()).andReturn(interceptedMethod);
+    expectSubjectPermitted().andReturn(false);
+    expect(subject.getPrincipal()).andReturn("ksweeney");
+
+    replayAndInitialize();
+
+    assertEquals(
+        ResponseCode.AUTH_FAILED,
+        ((Response) interceptor.invoke(methodInvocation)).getResponseCode());
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
new file mode 100644
index 0000000..781cf5a
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
@@ -0,0 +1,189 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import java.lang.reflect.Method;
+import java.util.concurrent.atomic.AtomicLong;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.matcher.Matchers;
+import com.twitter.common.stats.StatsProvider;
+import com.twitter.common.testing.easymock.EasyMockTest;
+
+import org.apache.aurora.gen.JobConfiguration;
+import org.apache.aurora.gen.Response;
+import org.apache.aurora.gen.ResponseCode;
+import org.apache.aurora.gen.TaskQuery;
+import org.apache.aurora.scheduler.base.JobKeys;
+import org.apache.aurora.scheduler.base.Query;
+import org.apache.aurora.scheduler.storage.entities.IJobKey;
+import org.apache.aurora.scheduler.thrift.Responses;
+import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
+import org.apache.aurora.scheduler.thrift.aop.MockDecoratedThrift;
+import org.apache.shiro.subject.Subject;
+import org.apache.thrift.TException;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.apache.aurora.scheduler.http.api.security.ShiroAuthorizingParamInterceptor.QUERY_TO_JOB_KEY;
+import static org.apache.aurora.scheduler.http.api.security.ShiroAuthorizingParamInterceptor.SHIRO_AUTHORIZATION_FAILURES;
+import static org.apache.aurora.scheduler.http.api.security.ShiroAuthorizingParamInterceptor.SHIRO_BAD_REQUESTS;
+import static org.easymock.EasyMock.expect;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+
+public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest {
+  private static final String PERMISSION_PREFIX = "testperm";
+
+  private ShiroAuthorizingParamInterceptor interceptor;
+
+  private Subject subject;
+  private AnnotatedAuroraAdmin thrift;
+  private StatsProvider statsProvider;
+
+  private AnnotatedAuroraAdmin decoratedThrift;
+
+  private static final IJobKey JOB_KEY = JobKeys.from("role", "env", "name");
+
+  @Before
+  public void setUp() {
+    interceptor = new ShiroAuthorizingParamInterceptor(PERMISSION_PREFIX);
+    subject = createMock(Subject.class);
+    statsProvider = createMock(StatsProvider.class);
+    thrift = createMock(AnnotatedAuroraAdmin.class);
+  };
+
+  private void replayAndInitialize() {
+    expect(statsProvider.makeCounter(SHIRO_AUTHORIZATION_FAILURES))
+        .andReturn(new AtomicLong());
+    expect(statsProvider.makeCounter(SHIRO_BAD_REQUESTS))
+        .andReturn(new AtomicLong());
+    control.replay();
+    decoratedThrift = Guice
+        .createInjector(new AbstractModule() {
+          @Override
+          protected void configure() {
+            bind(Subject.class).toInstance(subject);
+            MockDecoratedThrift.bindForwardedMock(binder(), thrift);
+            bindInterceptor(
+                Matchers.subclassesOf(AnnotatedAuroraAdmin.class),
+                ApiSecurityModule.AURORA_SCHEDULER_MANAGER_SERVICE,
+                interceptor);
+            bind(StatsProvider.class).toInstance(statsProvider);
+            requestInjection(interceptor);
+          }
+        }).getInstance(AnnotatedAuroraAdmin.class);
+  }
+
+  @Test
+  public void testHandlesAllDecoratedParamTypes() {
+    control.replay();
+
+    for (Method method : AnnotatedAuroraAdmin.class.getMethods()) {
+      if (ApiSecurityModule.AURORA_SCHEDULER_MANAGER_SERVICE.matches(method)) {
+        interceptor.getAuthorizingParamGetters().getUnchecked(method);
+      }
+    }
+  }
+
+  @Test
+  public void testCreateJobWithScopedPermission() throws TException {
+    JobConfiguration jobConfiguration = new JobConfiguration().setKey(JOB_KEY.newBuilder());
+    Response response = Responses.ok();
+
+    expect(subject.isPermitted(interceptor.makeWildcardPermission("createJob")))
+        .andReturn(false);
+    expect(subject
+        .isPermitted(interceptor.makeTargetPermission("createJob", JOB_KEY)))
+        .andReturn(true);
+    expect(thrift.createJob(jobConfiguration, null, null))
+        .andReturn(response);
+
+    replayAndInitialize();
+
+    assertSame(response, decoratedThrift.createJob(jobConfiguration, null, null));
+  }
+
+  @Test
+  public void testKillTasksWithWildcardPermission() throws TException {
+    TaskQuery taskQuery = Query.unscoped().get();
+    Response response = Responses.ok();
+
+    expect(subject.isPermitted(interceptor.makeWildcardPermission("killTasks")))
+        .andReturn(true);
+    expect(thrift.killTasks(taskQuery, null, null))
+        .andReturn(response);
+
+    replayAndInitialize();
+
+    assertSame(response, decoratedThrift.killTasks(taskQuery, null, null));
+  }
+
+  @Test
+  public void testKillTasksWithoutWildcardPermission() throws TException {
+    TaskQuery taskQuery = Query.unscoped().get();
+
+    expect(subject.isPermitted(interceptor.makeWildcardPermission("killTasks")))
+        .andReturn(false);
+
+    replayAndInitialize();
+
+    assertEquals(
+        ResponseCode.INVALID_REQUEST,
+        decoratedThrift.killTasks(taskQuery, null, null).getResponseCode());
+  }
+
+  @Test
+  public void testExtractTaskQuerySingleJobKey() {
+    replayAndInitialize();
+
+    assertEquals(
+        JOB_KEY.newBuilder(),
+        QUERY_TO_JOB_KEY
+            .apply(new TaskQuery()
+                .setRole(JOB_KEY.getRole())
+                .setEnvironment(JOB_KEY.getEnvironment())
+                .setJobName(JOB_KEY.getName()))
+            .orNull());
+
+    assertEquals(
+        JOB_KEY.newBuilder(),
+        QUERY_TO_JOB_KEY.apply(new TaskQuery().setJobKeys(ImmutableSet.of(JOB_KEY.newBuilder())))
+            .orNull());
+  }
+
+  @Test
+  public void testExtractTaskQueryBroadlyScoped() {
+    control.replay();
+
+    assertNull(QUERY_TO_JOB_KEY.apply(new TaskQuery().setRole("role")).orNull());
+  }
+
+  @Test
+  public void testExtractTaskQueryMultiScoped() {
+    // TODO(ksweeney): Reconsider behavior here, this is possibly too restrictive as it
+    // will mean that only admins are authorized to operate on multiple jobs at once regardless
+    // of whether they share a common role.
+    control.replay();
+
+    assertNull(QUERY_TO_JOB_KEY
+        .apply(
+            new TaskQuery().setJobKeys(
+                ImmutableSet.of(JOB_KEY.newBuilder(), JOB_KEY.newBuilder().setName("other"))))
+        .orNull());
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
deleted file mode 100644
index d2ba273..0000000
--- a/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
+++ /dev/null
@@ -1,106 +0,0 @@
-/**
- * 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 org.apache.aurora.scheduler.http.api.security;
-
-import java.lang.reflect.Method;
-import java.util.concurrent.atomic.AtomicLong;
-
-import com.google.inject.util.Providers;
-import com.twitter.common.stats.StatsProvider;
-import com.twitter.common.testing.easymock.EasyMockTest;
-
-import org.aopalliance.intercept.MethodInvocation;
-import org.apache.aurora.gen.AuroraAdmin;
-import org.apache.aurora.gen.Response;
-import org.apache.aurora.gen.ResponseCode;
-import org.apache.aurora.gen.SessionKey;
-import org.apache.aurora.scheduler.thrift.Responses;
-import org.apache.shiro.authz.UnauthenticatedException;
-import org.apache.shiro.authz.permission.WildcardPermission;
-import org.apache.shiro.subject.Subject;
-import org.easymock.IExpectationSetters;
-import org.junit.Before;
-import org.junit.Test;
-
-import static org.easymock.EasyMock.expect;
-import static org.junit.Assert.assertEquals;
-
-public class ShiroThriftInterceptorTest extends EasyMockTest {
-  private static final String PERMISSION = "test";
-  private static final String PRINCIPAL = "test-user";
-
-  private ShiroThriftInterceptor interceptor;
-  private Subject subject;
-  private StatsProvider statsProvider;
-  private AtomicLong shiroAuthorizationFailures;
-  private MethodInvocation methodInvocation;
-  private Method interceptedMethod;
-
-  @Before
-  public void setUp() throws Exception {
-    interceptor = new ShiroThriftInterceptor(PERMISSION);
-    subject = createMock(Subject.class);
-    statsProvider = createMock(StatsProvider.class);
-    shiroAuthorizationFailures = new AtomicLong();
-    expect(statsProvider.makeCounter(ShiroThriftInterceptor.SHIRO_AUTHORIZATION_FAILURES))
-        .andReturn(shiroAuthorizationFailures);
-    methodInvocation = createMock(MethodInvocation.class);
-    interceptedMethod = AuroraAdmin.Iface.class.getMethod("snapshot", SessionKey.class);
-  }
-
-  private void replayAndInitialize() {
-    control.replay();
-    interceptor.initialize(Providers.of(subject), statsProvider);
-  }
-
-  @Test(expected = UnauthenticatedException.class)
-  public void testInvokeNotAuthenticated() throws Throwable {
-    expect(subject.isAuthenticated()).andReturn(false);
-
-    replayAndInitialize();
-
-    interceptor.invoke(methodInvocation);
-  }
-
-  private IExpectationSetters<Boolean> expectSubjectPermitted() {
-    return expect(subject.isPermitted(
-        new WildcardPermission(PERMISSION + ":" + interceptedMethod.getName())));
-  }
-
-  @Test
-  public void testInvokeNotAuthorized() throws Throwable {
-    expect(subject.isAuthenticated()).andReturn(true);
-    expect(methodInvocation.getMethod()).andReturn(interceptedMethod);
-    expectSubjectPermitted().andReturn(false);
-    expect(subject.getPrincipal()).andReturn(PRINCIPAL);
-
-    replayAndInitialize();
-
-    assertEquals(
-        ResponseCode.AUTH_FAILED,
-        ((Response) interceptor.invoke(methodInvocation)).getResponseCode());
-  }
-
-  @Test
-  public void testInvokeAuthorized() throws Throwable {
-    expect(subject.isAuthenticated()).andReturn(true);
-    expect(methodInvocation.getMethod()).andReturn(interceptedMethod);
-    expectSubjectPermitted().andReturn(true);
-    expect(methodInvocation.proceed()).andReturn(Responses.ok());
-
-    replayAndInitialize();
-
-    assertEquals(Responses.ok(), interceptor.invoke(methodInvocation));
-  }
-}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetterTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetterTest.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetterTest.java
new file mode 100644
index 0000000..b0a8d75
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetterTest.java
@@ -0,0 +1,46 @@
+/**
+ * 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 org.apache.aurora.scheduler.http.api.security;
+
+import org.apache.aurora.gen.JobConfiguration;
+import org.apache.aurora.gen.JobConfiguration._Fields;
+import org.apache.aurora.gen.JobKey;
+import org.apache.aurora.gen.TaskConfig;
+import org.junit.Test;
+
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+
+public class ThriftFieldGetterTest {
+  @Test
+  public void testStructFieldGetter() {
+    JobKey jobKey = new JobKey();
+    FieldGetter<JobConfiguration, JobKey> fieldGetter =
+        new ThriftFieldGetter<>(JobConfiguration.class, _Fields.KEY, JobKey.class);
+
+    JobConfiguration jobConfiguration = new JobConfiguration().setKey(jobKey);
+
+    assertSame(jobKey, fieldGetter.apply(jobConfiguration).orNull());
+  }
+
+  @Test
+  public void testStructFieldGetterUnsetField() {
+    FieldGetter<JobConfiguration, TaskConfig> fieldGetter =
+        new ThriftFieldGetter<>(JobConfiguration.class, _Fields.TASK_CONFIG, TaskConfig.class);
+
+    JobConfiguration jobConfiguration = new JobConfiguration().setInstanceCount(5);
+
+    assertNull(fieldGetter.apply(jobConfiguration).orNull());
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
index 1f24e7d..2a2b499 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
@@ -47,6 +47,7 @@ import org.apache.aurora.scheduler.storage.backup.StorageBackup;
 import org.apache.aurora.scheduler.storage.entities.IResourceAggregate;
 import org.apache.aurora.scheduler.storage.entities.IServerInfo;
 import org.apache.aurora.scheduler.storage.testing.StorageTestUtil;
+import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.apache.aurora.scheduler.thrift.auth.ThriftAuthModule;
 import org.apache.aurora.scheduler.updater.JobUpdateController;
 import org.junit.Before;
@@ -174,7 +175,7 @@ public class ThriftIT extends EasyMockTest {
           }
         }
     );
-    thrift = injector.getInstance(AuroraAdmin.Iface.class);
+    thrift = injector.getInstance(AnnotatedAuroraAdmin.class);
   }
 
   private void setQuota(String user, boolean allowed) throws Exception {

http://git-wip-us.apache.org/repos/asf/aurora/blob/05d75e5d/src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java
index d20c9da..5c85300 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java
@@ -65,7 +65,7 @@ public class AopModuleTest extends EasyMockTest {
           }
         },
         new AopModule(toggledMethods));
-    return injector.getInstance(Iface.class);
+    return injector.getInstance(AnnotatedAuroraAdmin.class);
   }
 
   @Test


Mime
View raw message