Mark sensitive information in Rest Server
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/c627ed72 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/c627ed72 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/c627ed72 Branch: refs/heads/master Commit: c627ed7272a0a5bca1686daafb13f2195021b2b3 Parents: 7b8b1ce Author: Valentin Aitken <bos...@gmail.com> Authored: Tue Jun 7 18:12:55 2016 +0300 Committer: Valentin Aitken <bos...@gmail.com> Committed: Wed Jun 8 14:43:02 2016 +0300 ---------------------------------------------------------------------- .../brooklyn/api/effector/ParameterType.java | 2 + .../core/effector/BasicParameterType.java | 2 +- .../mgmt/internal/TestEntityWithEffectors.java | 9 ++- .../internal/TestEntityWithEffectorsImpl.java | 15 ++-- .../brooklyn/rest/domain/EffectorSummary.java | 9 ++- .../rest/transform/EffectorTransformer.java | 4 +- .../rest/resources/EffectorUtilsRestTest.java | 72 +++++++++++++++++--- 7 files changed, 94 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c627ed72/api/src/main/java/org/apache/brooklyn/api/effector/ParameterType.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/effector/ParameterType.java b/api/src/main/java/org/apache/brooklyn/api/effector/ParameterType.java index 7f0736d..07e86bd 100644 --- a/api/src/main/java/org/apache/brooklyn/api/effector/ParameterType.java +++ b/api/src/main/java/org/apache/brooklyn/api/effector/ParameterType.java @@ -27,6 +27,8 @@ import javax.management.MBeanParameterInfo; * * @see Effector */ +// TODO rename this to ParameterInfo rather than type. A ParameterType object is used for each parameter not for each type of parameter. +// check MethodEffector.AnnotationsOnMethod.toParameterType public interface ParameterType<T> extends Serializable { public String getName(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c627ed72/core/src/main/java/org/apache/brooklyn/core/effector/BasicParameterType.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/effector/BasicParameterType.java b/core/src/main/java/org/apache/brooklyn/core/effector/BasicParameterType.java index eb0417f..5dd6211 100644 --- a/core/src/main/java/org/apache/brooklyn/core/effector/BasicParameterType.java +++ b/core/src/main/java/org/apache/brooklyn/core/effector/BasicParameterType.java @@ -75,7 +75,7 @@ public class BasicParameterType<T> implements ParameterType<T> { @Override public Class<T> getParameterClass() { return type; } - + @Override public String getParameterClassName() { return type.getCanonicalName(); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c627ed72/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectors.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectors.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectors.java index 15a2ac1..f986674 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectors.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectors.java @@ -25,6 +25,11 @@ import org.apache.brooklyn.core.test.entity.TestEntity; @ImplementedBy(TestEntityWithEffectorsImpl.class) public interface TestEntityWithEffectors extends TestEntity { - @Effector(description = "Reset User pass effetor") - Void resetUserPassword(@EffectorParam(name = "newPassword") String param1); + @Effector(description = "Reset password") + Void resetPassword(@EffectorParam(name = "newPassword") String param1, @EffectorParam(name = "secretPin") Integer secretPin); + + @Effector(description = "Reset User and password effector") + Void invokeUserAndPassword(@EffectorParam(name = "user") String user, + @EffectorParam(name = "newPassword", defaultValue = "Test") String newPassword, + @EffectorParam(name = "paramDefault", defaultValue = "DefaultValue") String paramDefault); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c627ed72/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectorsImpl.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectorsImpl.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectorsImpl.java index b090152..8566d59 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectorsImpl.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/TestEntityWithEffectorsImpl.java @@ -18,17 +18,24 @@ */ package org.apache.brooklyn.core.mgmt.internal; -import org.apache.brooklyn.core.annotation.EffectorParam; import org.apache.brooklyn.core.test.entity.TestEntityImpl; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class TestEntityWithEffectorsImpl extends TestEntityImpl implements TestEntityWithEffectors { - private static final Logger log = LoggerFactory.getLogger(EffectorUtils.class); + private static final Logger log = LoggerFactory.getLogger(TestEntityWithEffectorsImpl.class); @Override - public Void resetUserPassword(@EffectorParam(name = "newPassword") String newPassword) { - log.info("Invoking Test effector {}", this); + public Void resetPassword(String newPassword, Integer secretPin) { + log.info("Invoked effector from resetPassword with params {} {}", new Object[] {newPassword, secretPin}); + assert newPassword != null; + return null; + } + + @Override + public Void invokeUserAndPassword(String user,String newPassword, String paramDefault) { + log.info("Invoked effector from invokeUserAndPassword with params {} {} {}", new Object[] {user, newPassword, paramDefault}); + assert user != null; assert newPassword != null; return null; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c627ed72/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EffectorSummary.java ---------------------------------------------------------------------- diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EffectorSummary.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EffectorSummary.java index 8ad7812..daa0e3b 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EffectorSummary.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EffectorSummary.java @@ -40,16 +40,19 @@ public class EffectorSummary implements HasName, Serializable { @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL) private final String description; private final T defaultValue; + private final boolean shouldSanitize; public ParameterSummary ( @JsonProperty("name") String name, @JsonProperty("type") String type, @JsonProperty("description") String description, - @JsonProperty("defaultValue") T defaultValue) { + @JsonProperty("defaultValue") T defaultValue, + @JsonProperty("shouldSanitize") boolean shouldSanitize) { this.name = name; this.type = type; this.description = description; this.defaultValue = defaultValue; + this.shouldSanitize = shouldSanitize; } @Override @@ -61,6 +64,10 @@ public class EffectorSummary implements HasName, Serializable { return type; } + public boolean shouldSanitize() { + return shouldSanitize; + } + public String getDescription() { return description; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c627ed72/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EffectorTransformer.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EffectorTransformer.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EffectorTransformer.java index b374bb3..9304008 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EffectorTransformer.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EffectorTransformer.java @@ -26,6 +26,7 @@ import javax.annotation.Nullable; import org.apache.brooklyn.api.effector.Effector; import org.apache.brooklyn.api.effector.ParameterType; import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.core.config.Sanitizer; import org.apache.brooklyn.rest.domain.EffectorSummary; import org.apache.brooklyn.rest.domain.EffectorSummary.ParameterSummary; import org.apache.brooklyn.rest.util.WebResourceUtils; @@ -83,7 +84,8 @@ public class EffectorTransformer { .context(entity).timeout(ValueResolver.REAL_QUICK_WAIT).getMaybe(); return new ParameterSummary(parameterType.getName(), parameterType.getParameterClassName(), parameterType.getDescription(), - WebResourceUtils.getValueForDisplay(defaultValue.orNull(), true, false)); + WebResourceUtils.getValueForDisplay(defaultValue.orNull(), true, false), + Sanitizer.IS_SECRET_PREDICATE.apply(parameterType.getName())); } catch (Exception e) { throw Exceptions.propagate(e); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c627ed72/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java index 98df14a..5fe729d 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java @@ -18,33 +18,46 @@ */ package org.apache.brooklyn.rest.resources; +import com.google.common.base.Function; +import com.google.common.base.Predicate; +import com.google.common.collect.FluentIterable; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import org.apache.brooklyn.api.effector.Effector; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.core.mgmt.internal.EffectorUtils; import org.apache.brooklyn.core.mgmt.internal.TestEntityWithEffectors; import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; +import org.apache.brooklyn.rest.domain.EffectorSummary; +import org.apache.brooklyn.rest.domain.SummaryComparators; import org.apache.brooklyn.rest.domain.TaskSummary; +import org.apache.brooklyn.rest.transform.EffectorTransformer; import org.apache.brooklyn.rest.transform.TaskTransformer; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.testng.Assert; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import javax.annotation.Nullable; import javax.ws.rs.core.UriBuilder; import java.net.URI; -import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; +import static org.testng.Assert.*; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; + public class EffectorUtilsRestTest extends BrooklynAppUnitTestSupport { private static final Logger log = LoggerFactory.getLogger(EffectorUtils.class); private TestEntityWithEffectors entity; + private UriBuilder uriBuilder = new org.apache.cxf.jaxrs.impl.UriBuilderImpl(URI.create("http://localhost";)); @BeforeMethod(alwaysRun=true) @Override @@ -53,19 +66,18 @@ public class EffectorUtilsRestTest extends BrooklynAppUnitTestSupport { entity = app.createAndManageChild(EntitySpec.create(TestEntityWithEffectors.class)); } - /** * Invoking an effector with sensitive parameters. * Invocation imitates {@link EffectorResource#invoke(String, String, String, String, Map)} */ @Test public void testInvokingEffector() { - Maybe<Effector<?>> effector = EffectorUtils.findEffectorDeclared(entity, "resetUserPassword"); + Maybe<Effector<?>> effector = EffectorUtils.findEffectorDeclared(entity, "resetPassword"); - final String sensitiveField = "newPassword"; - Task<?> t = entity.invoke(effector.get(), ImmutableMap.of(sensitiveField, "#$%'332985$23$#\"sd'")); + final String sensitiveField1 = "newPassword"; + final String sensitiveField2 = "secretPin"; + Task<?> t = entity.invoke(effector.get(), ImmutableMap.of(sensitiveField1, "#$%'332985$23$#\"sd'", "secretPin", 1234)); - UriBuilder uriBuilder = new org.apache.cxf.jaxrs.impl.UriBuilderImpl(URI.create("http://localhost";)); TaskSummary summary = TaskTransformer.taskSummary(t, uriBuilder); try { @@ -73,10 +85,50 @@ public class EffectorUtilsRestTest extends BrooklynAppUnitTestSupport { } catch (InterruptedException|ExecutionException e) { throw Exceptions.propagate(e); } - Assert.assertEquals( + assertEquals( summary.getDescription(), - "Invoking effector resetUserPassword on "+TestEntityWithEffectors.class.getSimpleName()+":"+entity.getId().substring(0,4) - +" with parameters {"+sensitiveField+"=xxxxxxxx}", + "Invoking effector resetPassword on "+TestEntityWithEffectors.class.getSimpleName()+":"+entity.getId().substring(0,4) + +" with parameters {"+sensitiveField1+"=xxxxxxxx, "+sensitiveField2+"=xxxxxxxx}", "Task summary must hide sensitive parameters"); } + + /** + * Invoking an effector with sensitive parameters. + * Invocation imitates {@link EffectorResource#list} + */ + @Test + public void testListingEffectorsParameterSummary() { + List<EffectorSummary> effectorSummaries = FluentIterable + .from(entity.getEntityType().getEffectors()) + .filter(new Predicate<Effector<?>>() { + @Override + public boolean apply(@Nullable Effector<?> input) { + return ImmutableList.of("resetPassword", "invokeUserAndPassword").contains(input.getName()); + } + }) + .transform(new Function<Effector<?>, EffectorSummary>() { + @Override + public EffectorSummary apply(Effector<?> effector) { + return EffectorTransformer.effectorSummary(entity, effector, uriBuilder); + } + }) + .toSortedList(SummaryComparators.nameComparator()); + EffectorSummary.ParameterSummary<?> passwordFieldInSimpleEffector = findEffectorSummary(effectorSummaries, "resetPassword").getParameters().iterator().next(); + assertTrue(passwordFieldInSimpleEffector.shouldSanitize()); + + EffectorSummary longEffector = findEffectorSummary(effectorSummaries, "invokeUserAndPassword"); + EffectorSummary.ParameterSummary<?> resetUsersPassword_user = Iterables.getFirst(longEffector.getParameters(), null); + assertFalse(resetUsersPassword_user.shouldSanitize()); + EffectorSummary.ParameterSummary<?> resetUsersPassword_password = longEffector.getParameters().toArray(new EffectorSummary.ParameterSummary<?>[0])[1]; + assertTrue(resetUsersPassword_password.shouldSanitize()); + } + + private EffectorSummary findEffectorSummary(List<EffectorSummary> effectorSummaries, final String effectorName) { + return Iterables.find(effectorSummaries, new Predicate<EffectorSummary>() { + @Override + public boolean apply(@Nullable EffectorSummary input) { + return input.getName().equals(effectorName); + } + }); + } }
