Repository: aurora Updated Branches: refs/heads/master e63c518e7 -> 545e8396d
Use Provider<Optional<Subject>> instead of optional injection for Shiro audit logging. Use injection of an optional value instead of optional injection. Testing Done: ./gradlew build Bugs closed: AURORA-1352 Reviewed at https://reviews.apache.org/r/36490/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/545e8396 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/545e8396 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/545e8396 Branch: refs/heads/master Commit: 545e8396d46d787ce21859ee4b5501abf6ef6c58 Parents: e63c518 Author: Kevin Sweeney <kevi...@apache.org> Authored: Wed Jul 15 11:55:28 2015 -0700 Committer: Kevin Sweeney <kevi...@apache.org> Committed: Wed Jul 15 11:55:28 2015 -0700 ---------------------------------------------------------------------- .../aurora/auth/UnsecureSessionContext.java | 25 ++----- .../http/api/security/HttpSecurityModule.java | 27 ++++++-- .../aurora/auth/UnsecureSessionContextTest.java | 69 +++++--------------- .../aurora/e2e/test_kerberos_end_to_end.sh | 5 -- 4 files changed, 45 insertions(+), 81 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/545e8396/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java b/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java index f547c44..47f81aa 100644 --- a/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java +++ b/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java @@ -13,14 +13,13 @@ */ package org.apache.aurora.auth; +import java.util.Objects; import java.util.Optional; -import javax.annotation.Nullable; import javax.inject.Provider; import com.google.common.annotations.VisibleForTesting; import com.google.inject.Inject; -import com.twitter.common.stats.StatsProvider; import org.apache.shiro.subject.Subject; @@ -30,30 +29,20 @@ import org.apache.shiro.subject.Subject; */ class UnsecureSessionContext implements SessionValidator.SessionContext { @VisibleForTesting - static final String SHIRO_AUDIT_LOGGING_ENABLED = "shiro_audit_logging_enabled"; - - @VisibleForTesting static final String UNSECURE = "UNSECURE"; - @Inject - UnsecureSessionContext(StatsProvider statsProvider) { - statsProvider.makeGauge(SHIRO_AUDIT_LOGGING_ENABLED, () -> subjectProvider == null ? 0 : 1); - } - - @Nullable - private Provider<Subject> subjectProvider; + private final Provider<Optional<Subject>> subjectProvider; - @Inject(optional = true) - void setSubjectProvider(Provider<Subject> subjectProvider) { - this.subjectProvider = subjectProvider; + @Inject + UnsecureSessionContext(Provider<Optional<Subject>> subjectProvider) { + this.subjectProvider = Objects.requireNonNull(subjectProvider); } @Override public String getIdentity() { - return Optional.ofNullable(subjectProvider) - .map(Provider::get) + return subjectProvider.get() .map(Subject::getPrincipals) - .map((principalCollection) -> principalCollection.oneByType(String.class)) + .map(principalCollection -> principalCollection.oneByType(String.class)) .orElse(UNSECURE); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/545e8396/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java index a3b0a6b..2f82dcd 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java +++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java @@ -14,15 +14,18 @@ package org.apache.aurora.scheduler.http.api.security; import java.lang.reflect.Method; +import java.util.Optional; import java.util.Set; import javax.servlet.Filter; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; +import com.google.inject.AbstractModule; import com.google.inject.Key; import com.google.inject.Module; import com.google.inject.Provides; +import com.google.inject.TypeLiteral; import com.google.inject.matcher.Matcher; import com.google.inject.matcher.Matchers; import com.google.inject.name.Names; @@ -124,12 +127,28 @@ public class HttpSecurityModule extends ServletModule { @Override protected void configureServlets() { - if (mechanism != HttpAuthenticationMechanism.NONE) { + if (mechanism == HttpAuthenticationMechanism.NONE) { + // TODO(ksweeney): Use an OptionalBinder here once we're on Guice 4.0. + bind(new TypeLiteral<Optional<Subject>>() { }).toInstance(Optional.empty()); + } else { doConfigureServlets(); } } private void doConfigureServlets() { + bind(Subject.class).toProvider(SecurityUtils::getSubject).in(RequestScoped.class); + install(new AbstractModule() { + @Override + protected void configure() { + // Provides-only module to provide Optional<Subject>. + // TODO(ksweeney): Use an OptionalBinder here once we're on Guice 4.0. + } + + @Provides + Optional<Subject> provideOptionalSubject(Subject subject) { + return Optional.of(subject); + } + }); install(guiceFilterModule(API_PATH)); install(guiceFilterModule(H2_PATH)); install(guiceFilterModule(H2_PATH + "/*")); @@ -194,10 +213,4 @@ public class HttpSecurityModule extends ServletModule { AURORA_ADMIN_SERVICE, adminInterceptor); } - - @Provides - @RequestScoped - Subject provideSubject() { - return SecurityUtils.getSubject(); - } } http://git-wip-us.apache.org/repos/asf/aurora/blob/545e8396/src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java b/src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java index 6391736..91335c2 100644 --- a/src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java +++ b/src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java @@ -13,86 +13,53 @@ */ package org.apache.aurora.auth; -import javax.inject.Provider; +import java.util.Optional; -import com.google.common.base.Supplier; -import com.google.inject.util.Providers; -import com.twitter.common.stats.StatsProvider; import com.twitter.common.testing.easymock.EasyMockTest; -import org.apache.shiro.subject.SimplePrincipalCollection; +import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.Subject; -import org.easymock.Capture; import org.junit.Before; import org.junit.Test; -import static org.easymock.EasyMock.capture; -import static org.easymock.EasyMock.eq; +import static org.apache.aurora.auth.UnsecureSessionContext.UNSECURE; import static org.easymock.EasyMock.expect; import static org.junit.Assert.assertEquals; public class UnsecureSessionContextTest extends EasyMockTest { - private StatsProvider statsProvider; private Subject subject; - private Provider<Subject> subjectProvider; - - private UnsecureSessionContext sessionContext; - - private Supplier<Integer> gauge; + private PrincipalCollection principalCollection; @Before public void setUp() { - statsProvider = createMock(StatsProvider.class); subject = createMock(Subject.class); - subjectProvider = Providers.of(subject); - + principalCollection = createMock(PrincipalCollection.class); } - private void constructAndReplay() { - Capture<Supplier<Integer>> gaugeCapture = createCapture(); - expect(statsProvider.makeGauge( - eq(UnsecureSessionContext.SHIRO_AUDIT_LOGGING_ENABLED), - capture(gaugeCapture))).andReturn(null); + @Test + public void testNonStringPrincipal() { + expect(subject.getPrincipals()).andReturn(principalCollection); + expect(principalCollection.oneByType(String.class)).andReturn(null); control.replay(); - sessionContext = new UnsecureSessionContext(statsProvider); - gauge = gaugeCapture.getValue(); - assertEquals(0, (int) gauge.get()); - } - - private void assertIdentityEquals(String identity) { - assertEquals(identity, sessionContext.getIdentity()); - } - - @Test - public void testNoSubjectProvider() { - constructAndReplay(); - - assertIdentityEquals(UnsecureSessionContext.UNSECURE); + assertEquals(UNSECURE, new UnsecureSessionContext(() -> Optional.of(subject)).getIdentity()); } @Test - public void testSubjectProviderReturnsNull() { - expect(subject.getPrincipals()).andReturn(new SimplePrincipalCollection()); - - constructAndReplay(); + public void testEmptySubject() { + control.replay(); - sessionContext.setSubjectProvider(subjectProvider); - assertIdentityEquals(UnsecureSessionContext.UNSECURE); - assertEquals(1, (int) gauge.get()); + assertEquals(UNSECURE, new UnsecureSessionContext(Optional::empty).getIdentity()); } @Test - public void testSubjectProviderReturnsValue() { - String userName = "jsmith"; - - expect(subject.getPrincipals()).andReturn(new SimplePrincipalCollection(userName, "realm")); + public void testStringSubject() { + expect(subject.getPrincipals()).andReturn(principalCollection); + expect(principalCollection.oneByType(String.class)).andReturn("jsmith"); - constructAndReplay(); + control.replay(); - sessionContext.setSubjectProvider(subjectProvider); - assertIdentityEquals(userName); - assertEquals(1, (int) gauge.get()); + assertEquals("jsmith", new UnsecureSessionContext(() -> Optional.of(subject)).getIdentity()); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/545e8396/src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh ---------------------------------------------------------------------- diff --git a/src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh b/src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh index 6e4a294..760997c 100755 --- a/src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh +++ b/src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh @@ -131,10 +131,6 @@ function test_clients { kdestroy } -function test_audit_logging_enabled { - curl -s localhost:8081/vars | grep -q 'shiro_audit_logging_enabled 1' -} - function tear_down { local retcode=$1 sudo cp /vagrant/examples/vagrant/clusters.json /etc/aurora/clusters.json @@ -157,7 +153,6 @@ function main { else trap 'tear_down 1' EXIT setup - test_audit_logging_enabled test_snapshot test_clients test_h2console
