Return-Path: X-Original-To: apmail-aurora-commits-archive@minotaur.apache.org Delivered-To: apmail-aurora-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 2559318CAF for ; Wed, 30 Sep 2015 23:07:18 +0000 (UTC) Received: (qmail 86778 invoked by uid 500); 30 Sep 2015 23:07:18 -0000 Delivered-To: apmail-aurora-commits-archive@aurora.apache.org Received: (qmail 86743 invoked by uid 500); 30 Sep 2015 23:07:17 -0000 Mailing-List: contact commits-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aurora.apache.org Delivered-To: mailing list commits@aurora.apache.org Received: (qmail 86734 invoked by uid 99); 30 Sep 2015 23:07:17 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 30 Sep 2015 23:07:17 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id C4C57E03BE; Wed, 30 Sep 2015 23:07:17 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: kevints@apache.org To: commits@aurora.apache.org Message-Id: <99e6039d18b0494797b0f7432f137901@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: aurora git commit: Use Shiro to generate audit messages when available. Date: Wed, 30 Sep 2015 23:07:17 +0000 (UTC) Repository: aurora Updated Branches: refs/heads/master 488f29f70 -> 33d7e2170 Use Shiro to generate audit messages when available. This ensures that Shiro is always used to generate audit messages when available. Testing Done: ./gradlew build Reviewed at https://reviews.apache.org/r/38906/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/33d7e217 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/33d7e217 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/33d7e217 Branch: refs/heads/master Commit: 33d7e2170a86f54722a02a2dc9cb1e09fb52df25 Parents: 488f29f Author: Kevin Sweeney Authored: Wed Sep 30 16:06:53 2015 -0700 Committer: Kevin Sweeney Committed: Wed Sep 30 16:06:53 2015 -0700 ---------------------------------------------------------------------- .../aurora/scheduler/thrift/AuditMessages.java | 57 ++++++++++++++++++++ .../thrift/SchedulerThriftInterface.java | 26 +++------ .../scheduler/thrift/AuditMessagesTest.java | 51 ++++++++++++++++++ .../thrift/SchedulerThriftInterfaceTest.java | 18 ++++--- .../aurora/scheduler/thrift/ThriftIT.java | 8 +++ 5 files changed, 134 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java b/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java new file mode 100644 index 0000000..3a29a62 --- /dev/null +++ b/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java @@ -0,0 +1,57 @@ +/** + * 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.thrift; + +import java.util.Objects; +import java.util.Optional; + +import javax.inject.Inject; +import javax.inject.Provider; + +import com.google.common.annotations.VisibleForTesting; + +import org.apache.shiro.subject.Subject; + +/** + * Generates audit messages with usernames from Shiro if available and falls back to + * caller-specified ones if not. + */ +@VisibleForTesting +class AuditMessages { + private final Provider> subjectProvider; + + @Inject + AuditMessages(Provider> subjectProvider) { + this.subjectProvider = Objects.requireNonNull(subjectProvider); + } + + private String getShiroUserNameOr(String defaultUser) { + return subjectProvider.get() + .map(Subject::getPrincipal) + .map(Object::toString) + .orElse(defaultUser); + } + + com.google.common.base.Optional transitionedBy(String user) { + return com.google.common.base.Optional.of("Transition forced by " + getShiroUserNameOr(user)); + } + + com.google.common.base.Optional killedBy(String user) { + return com.google.common.base.Optional.of("Killed by " + getShiroUserNameOr(user)); + } + + com.google.common.base.Optional restartedBy(String user) { + return com.google.common.base.Optional.of("Restarted by " + getShiroUserNameOr(user)); + } +} http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java index d69bc39..14b5a00 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -202,6 +202,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { private final UUIDGenerator uuidGenerator; private final JobUpdateController jobUpdateController; private final ReadOnlyScheduler.Iface readOnlyScheduler; + private final AuditMessages auditMessages; @Inject SchedulerThriftInterface( @@ -217,7 +218,8 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { TaskIdGenerator taskIdGenerator, UUIDGenerator uuidGenerator, JobUpdateController jobUpdateController, - ReadOnlyScheduler.Iface readOnlyScheduler) { + ReadOnlyScheduler.Iface readOnlyScheduler, + AuditMessages auditMessages) { this.storage = requireNonNull(storage); this.lockManager = requireNonNull(lockManager); @@ -232,6 +234,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { this.uuidGenerator = requireNonNull(uuidGenerator); this.jobUpdateController = requireNonNull(jobUpdateController); this.readOnlyScheduler = requireNonNull(readOnlyScheduler); + this.auditMessages = requireNonNull(auditMessages); } @Override @@ -583,7 +586,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { taskId, Optional.absent(), ScheduleStatus.KILLING, - killedByMessage(context.getIdentity())); + auditMessages.killedBy(context.getIdentity())); } return tasksKilled @@ -639,7 +642,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { taskId, Optional.absent(), ScheduleStatus.RESTARTING, - restartedByMessage(context.getIdentity())); + auditMessages.restartedBy(context.getIdentity())); } } }); @@ -736,7 +739,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { taskId, Optional.absent(), status, - transitionMessage(context.getIdentity())); + auditMessages.transitionedBy(context.getIdentity())); } }); @@ -1343,21 +1346,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { } @VisibleForTesting - static Optional transitionMessage(String user) { - return Optional.of("Transition forced by " + user); - } - - @VisibleForTesting - static Optional killedByMessage(String user) { - return Optional.of("Killed by " + user); - } - - @VisibleForTesting - static Optional restartedByMessage(String user) { - return Optional.of("Restarted by " + user); - } - - @VisibleForTesting static String noCronScheduleMessage(IJobKey jobKey) { return String.format("Job %s has no cron schedule", JobKeys.canonicalString(jobKey)); } http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java new file mode 100644 index 0000000..0f8748d --- /dev/null +++ b/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java @@ -0,0 +1,51 @@ +/** + * 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.thrift; + +import java.util.Optional; + +import org.apache.aurora.common.testing.easymock.EasyMockTest; +import org.apache.shiro.subject.Subject; +import org.junit.Test; + +import static org.easymock.EasyMock.expect; +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertThat; + +public class AuditMessagesTest extends EasyMockTest { + @Test + public void testEmptySubject() { + AuditMessages emptyMessages = new AuditMessages(Optional::empty); + + control.replay(); + + assertThat(emptyMessages.killedBy("legacy").get(), containsString("legacy")); + assertThat(emptyMessages.restartedBy("legacy").get(), containsString("legacy")); + assertThat(emptyMessages.transitionedBy("legacy").get(), containsString("legacy")); + } + + @Test + public void testPresentSubject() { + Subject subject = createMock(Subject.class); + AuditMessages presentMessages = new AuditMessages(() -> Optional.of(subject)); + + expect(subject.getPrincipal()).andReturn("shiro").times(3); + + control.replay(); + + assertThat(presentMessages.killedBy("legacy").get(), containsString("shiro")); + assertThat(presentMessages.restartedBy("legacy").get(), containsString("shiro")); + assertThat(presentMessages.transitionedBy("legacy").get(), containsString("shiro")); + } +} http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java index ac89618..83f65a7 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -159,11 +159,8 @@ import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.MAX_TA import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NOOP_JOB_UPDATE_MESSAGE; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NO_CRON; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.jobAlreadyExistsMessage; -import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.killedByMessage; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.noCronScheduleMessage; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.notScheduledCronMessage; -import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.restartedByMessage; -import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.transitionMessage; import static org.easymock.EasyMock.anyInt; import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.eq; @@ -195,6 +192,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { private UUIDGenerator uuidGenerator; private JobUpdateController jobUpdateController; private ReadOnlyScheduler.Iface readOnlyScheduler; + private AuditMessages auditMessages; @Before public void setUp() throws Exception { @@ -214,6 +212,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { uuidGenerator = createMock(UUIDGenerator.class); jobUpdateController = createMock(JobUpdateController.class); readOnlyScheduler = createMock(ReadOnlyScheduler.Iface.class); + auditMessages = createMock(AuditMessages.class); thrift = getResponseProxy( new SchedulerThriftInterface( @@ -229,7 +228,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { taskIdGenerator, uuidGenerator, jobUpdateController, - readOnlyScheduler)); + readOnlyScheduler, + auditMessages)); } private static AuroraAdmin.Iface getResponseProxy(final AuroraAdmin.Iface realThrift) { @@ -644,12 +644,13 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { } private void expectTransitionsToKilling() { + expect(auditMessages.killedBy(USER)).andReturn(Optional.of("test")); expect(stateManager.changeState( storageUtil.mutableStoreProvider, TASK_ID, Optional.absent(), ScheduleStatus.KILLING, - killedByMessage(USER))).andReturn(StateChangeResult.SUCCESS); + Optional.of("test"))).andReturn(StateChangeResult.SUCCESS); } @Test @@ -874,12 +875,13 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { public void testForceTaskState() throws Exception { ScheduleStatus status = ScheduleStatus.FAILED; + expect(auditMessages.transitionedBy(USER)).andReturn(Optional.of("test")); expect(stateManager.changeState( storageUtil.mutableStoreProvider, TASK_ID, Optional.absent(), ScheduleStatus.FAILED, - Optional.of(transitionMessage(USER).get()))).andReturn(StateChangeResult.SUCCESS); + Optional.of("test"))).andReturn(StateChangeResult.SUCCESS); expectAuth(ROOT, true); expectAuth(ROOT, false); @@ -972,12 +974,14 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { Query.instanceScoped(JOB_KEY, shards).active(), buildScheduledTask()); + expect(auditMessages.restartedBy(USER)) + .andReturn(Optional.of("test")); expect(stateManager.changeState( storageUtil.mutableStoreProvider, TASK_ID, Optional.absent(), ScheduleStatus.RESTARTING, - restartedByMessage(USER))).andReturn(StateChangeResult.SUCCESS); + Optional.of("test"))).andReturn(StateChangeResult.SUCCESS); control.replay(); http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/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 4bd8b12..f06481b 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java @@ -15,6 +15,7 @@ package org.apache.aurora.scheduler.thrift; import java.util.Arrays; import java.util.Map; +import java.util.Optional; import java.util.Set; import com.google.common.collect.ImmutableMap; @@ -22,6 +23,7 @@ import com.google.common.collect.ImmutableSet; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.Provides; import org.apache.aurora.auth.CapabilityValidator; import org.apache.aurora.auth.CapabilityValidator.Capability; @@ -50,6 +52,7 @@ 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.apache.shiro.subject.Subject; import org.junit.Before; import org.junit.Test; @@ -173,6 +176,11 @@ public class ThriftIT extends EasyMockTest { bind(IServerInfo.class).toInstance(IServerInfo.build(new ServerInfo())); bindMock(CronPredictor.class); } + + @Provides + Optional provideSubject() { + return Optional.of(createMock(Subject.class)); + } } ); thrift = injector.getInstance(AnnotatedAuroraAdmin.class);