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 <[email protected]> Authored: Fri Apr 3 17:19:55 2015 -0400 Committer: Kevin Sweeney <[email protected]> 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
