Repository: aurora Updated Branches: refs/heads/master cab465e03 -> e27163440
Explicitly bind SessionContext. Discussion on bugs like this and a potential solution (enabling `requireExplicitBindings`) are here: https://github.com/google/guice/issues/740 Testing Done: Ran kerberos e2e test. Bugs closed: AURORA-1352 Reviewed at https://reviews.apache.org/r/35627/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/e2716344 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/e2716344 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/e2716344 Branch: refs/heads/master Commit: e271634405822883d139f7d105582ba9b14f8736 Parents: cab465e Author: Kevin Sweeney <[email protected]> Authored: Thu Jun 18 15:01:45 2015 -0700 Committer: Kevin Sweeney <[email protected]> Committed: Thu Jun 18 15:01:45 2015 -0700 ---------------------------------------------------------------------- .../apache/aurora/auth/UnsecureAuthModule.java | 12 ++++++-- .../aurora/auth/UnsecureSessionContext.java | 9 ++++++ .../aurora/auth/UnsecureSessionContextTest.java | 30 +++++++++++++++++--- .../aurora/e2e/test_kerberos_end_to_end.sh | 5 ++++ 4 files changed, 50 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/e2716344/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java b/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java index c89ff0f..010e714 100644 --- a/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java +++ b/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java @@ -19,6 +19,7 @@ import java.util.logging.Logger; import com.google.inject.AbstractModule; import com.google.inject.Inject; +import org.apache.aurora.auth.SessionValidator.SessionContext; import org.apache.aurora.gen.SessionKey; import static java.util.Objects.requireNonNull; @@ -36,13 +37,20 @@ public class UnsecureAuthModule extends AbstractModule { LOG.info("Using default (UNSECURE!!!) authentication module."); bind(SessionValidator.class).to(UnsecureSessionValidator.class); bind(CapabilityValidator.class).to(UnsecureCapabilityValidator.class); + // NOTE: This binding is very important, as UnsecureSessionContext has an optional injection, so its provider must + // be created in the same injector as the one that *might* have its optional dependency. Omitting this binding will + // cause a Just-In-Time binding to be created in the parent injector, where it will not have access to the optional + // dependency in the child injector (so its optional dependency will never be used). This was the cause of + // https://issues.apache.org/jira/browse/AURORA-1352. This can be mitigated slightly by + // https://issues.apache.org/jira/browse/AURORA-1357 + bind(SessionContext.class).to(UnsecureSessionContext.class); } static class UnsecureSessionValidator implements SessionValidator { private final SessionContext sessionContext; @Inject - UnsecureSessionValidator(UnsecureSessionContext sessionContext) { + UnsecureSessionValidator(SessionContext sessionContext) { this.sessionContext = requireNonNull(sessionContext); } @@ -66,7 +74,7 @@ public class UnsecureAuthModule extends AbstractModule { @Inject UnsecureCapabilityValidator( SessionValidator sessionValidator, - UnsecureSessionContext sessionContext) { + SessionContext sessionContext) { this.sessionValidator = requireNonNull(sessionValidator); this.sessionContext = requireNonNull(sessionContext); http://git-wip-us.apache.org/repos/asf/aurora/blob/e2716344/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 57132ac..0226144 100644 --- a/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java +++ b/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java @@ -21,6 +21,7 @@ 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; /** @@ -29,8 +30,16 @@ 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; http://git-wip-us.apache.org/repos/asf/aurora/blob/e2716344/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 0a842cb..a8badd4 100644 --- a/src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java +++ b/src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java @@ -15,29 +15,49 @@ package org.apache.aurora.auth; import javax.inject.Provider; +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.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.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; + @Before public void setUp() { + statsProvider = createMock(StatsProvider.class); subject = createMock(Subject.class); subjectProvider = Providers.of(subject); - sessionContext = new UnsecureSessionContext(); + } + + private void constructAndReplay() { + Capture<Supplier<Integer>> gaugeCapture = createCapture(); + expect(statsProvider.makeGauge(eq(UnsecureSessionContext.SHIRO_AUDIT_LOGGING_ENABLED), capture(gaugeCapture))) + .andReturn(null); + + control.replay(); + + sessionContext = new UnsecureSessionContext(statsProvider); + gauge = gaugeCapture.getValue(); + assertEquals(0, (int) gauge.get()); } private void assertIdentityEquals(String identity) { @@ -46,7 +66,7 @@ public class UnsecureSessionContextTest extends EasyMockTest { @Test public void testNoSubjectProvider() { - control.replay(); + constructAndReplay(); assertIdentityEquals(UnsecureSessionContext.UNSECURE); } @@ -55,10 +75,11 @@ public class UnsecureSessionContextTest extends EasyMockTest { public void testSubjectProviderReturnsNull() { expect(subject.getPrincipals()).andReturn(new SimplePrincipalCollection()); - control.replay(); + constructAndReplay(); sessionContext.setSubjectProvider(subjectProvider); assertIdentityEquals(UnsecureSessionContext.UNSECURE); + assertEquals(1, (int) gauge.get()); } @Test @@ -67,9 +88,10 @@ public class UnsecureSessionContextTest extends EasyMockTest { expect(subject.getPrincipals()).andReturn(new SimplePrincipalCollection(userName, "realm")); - control.replay(); + constructAndReplay(); sessionContext.setSubjectProvider(subjectProvider); assertIdentityEquals(userName); + assertEquals(1, (int) gauge.get()); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/e2716344/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 4d6043a..79377c3 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,6 +131,10 @@ function test_clients { kdestroy } +function test_audit_logging_enabled { + curl -s localhost:8081/vars | grep -q 'shiro_audit_logging_enabled 1' +} + function tear_down { sudo cp /vagrant/examples/vagrant/clusters.json /etc/aurora/clusters.json sudo stop aurora-scheduler-kerberos || true @@ -146,6 +150,7 @@ function main { else trap tear_down EXIT setup + test_audit_logging_enabled test_snapshot test_clients test_h2console
