[java-identity-provider] branch master updated: IDP-1168 - Collapse duplicated scripted implementations
Scott Cantor
cantor.2 at osu.edu
Wed May 3 20:24:49 EDT 2017
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch master
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=9269e7d30b741c3d0bd22e7efc338d5546a1897b
The following commit(s) were added to refs/heads/master by this push:
new 9269e7d IDP-1168 - Collapse duplicated scripted implementations
9269e7d is described below
commit 9269e7d30b741c3d0bd22e7efc338d5546a1897b
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed May 3 20:24:47 2017 -0400
IDP-1168 - Collapse duplicated scripted implementations
https://issues.shibboleth.net/jira/browse/IDP-1168
Re-base scripted Matcher and PolicyRule.
---
.../filter/matcher/impl/ScriptedMatcher.java | 194 ++++++++++++---------
.../filter/policyrule/impl/ScriptedPolicyRule.java | 173 ++++++++++--------
.../filter/matcher/impl/ScriptedMatcherTest.java | 2 +-
3 files changed, 207 insertions(+), 162 deletions(-)
diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java
index 03246a1..02a8aa5 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java
@@ -26,8 +26,6 @@ import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
import javax.script.ScriptContext;
-import javax.script.ScriptException;
-import javax.script.SimpleScriptContext;
import javax.security.auth.Subject;
import net.shibboleth.idp.attribute.IdPAttribute;
@@ -44,6 +42,7 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.component.UnmodifiableComponent;
import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.scripting.AbstractScriptEvaluator;
import net.shibboleth.utilities.java.support.scripting.EvaluableScript;
import org.opensaml.messaging.context.navigate.ChildContextLookup;
@@ -68,20 +67,20 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(ScriptedMatcher.class);
- /** Script to be evaluated. */
+ /** The script to evaluate. */
@NonnullAfterInit private EvaluableScript script;
+ /** Evaluator. */
+ @NonnullAfterInit private MatcherScriptEvaluator scriptEvaluator;
+
+ /** Custom object for script. */
+ @Nullable private Object customObject;
+
/** Strategy used to locate the {@link ProfileRequestContext} to use. */
- @Nonnull private Function<AttributeFilterContext, ProfileRequestContext> prcLookupStrategy;
+ @Nonnull private Function<AttributeFilterContext,ProfileRequestContext> prcLookupStrategy;
/** Strategy used to locate the {@link SubjectContext} to use. */
- @Nonnull private Function<ProfileRequestContext, SubjectContext> scLookupStrategy;
-
- /** Log prefix. */
- private String logPrefix;
-
- /** The custom object we inject into all scripts. */
- @Nullable private Object customObject;
+ @Nonnull private Function<ProfileRequestContext,SubjectContext> scLookupStrategy;
/** Constructor. */
public ScriptedMatcher() {
@@ -106,7 +105,9 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
*
* @param object the custom object
*/
- @Nullable public void setCustomObject(final Object object) {
+ public void setCustomObject(@Nullable final Object object) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
customObject = object;
}
@@ -115,7 +116,7 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
*
* @return the script to be evaluated
*/
- @Nonnull public EvaluableScript getScript() {
+ @NonnullAfterInit public EvaluableScript getScript() {
return script;
}
@@ -127,7 +128,7 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
public void setScript(@Nonnull final EvaluableScript matcherScript) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
- script = Constraint.isNotNull(matcherScript, "Attribute value matching script can not be null");
+ script = Constraint.isNotNull(matcherScript, "Attribute value matching script cannot be null");
}
/**
@@ -158,7 +159,21 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
scLookupStrategy = Constraint.isNotNull(strategy, "SubjectContext lookup strategy cannot be null");
}
-
+ /** {@inheritDoc} */
+ @Override protected void doInitialize() throws ComponentInitializationException {
+ super.doInitialize();
+
+ if (null == script) {
+ throw new ComponentInitializationException("No script has been provided");
+ }
+
+ scriptEvaluator = new MatcherScriptEvaluator(script);
+ scriptEvaluator.setCustomObject(customObject);
+
+ final StringBuilder builder = new StringBuilder("Scripted Attribute Filter '").append(getId()).append("':");
+ scriptEvaluator.setLogPrefix(builder.toString());
+ }
+
/**
* Perform the AttributeValueMatching.
* <p>
@@ -174,69 +189,13 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
*/
@Override @Nullable @NonnullElements @Unmodifiable public Set<IdPAttributeValue<?>> getMatchingValues(
@Nonnull final IdPAttribute attribute, @Nonnull final AttributeFilterContext filterContext) {
- Constraint.isNotNull(attribute, "Attribute to be filtered can not be null");
- Constraint.isNotNull(filterContext, "Attribute filter context can not be null");
+ Constraint.isNotNull(attribute, "Attribute to be filtered cannot be null");
+ Constraint.isNotNull(filterContext, "AttributeFilterContext cannot be null");
+
ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
-
- final EvaluableScript currentScript = script;
- final SimpleScriptContext scriptContext = new SimpleScriptContext();
- scriptContext.setAttribute("filterContext", filterContext, ScriptContext.ENGINE_SCOPE);
- scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
- final ProfileRequestContext prc = prcLookupStrategy.apply(filterContext);
- scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
- scriptContext.setAttribute("attribute", attribute, ScriptContext.ENGINE_SCOPE);
-
- final SubjectContext sc;
- if (null == prc) {
- log.error("{} Could not locate ProfileRequestContext", getLogPrefix());
- sc = null;
- } else {
- sc = scLookupStrategy.apply(prc);
- }
- if (null == sc) {
- log.warn("{} Could not locate SubjectContext", getLogPrefix());
- } else {
- final List<Subject> subjects = sc.getSubjects();
- if (null == subjects) {
- log.warn("{} Could not locate Subjects", getLogPrefix());
- } else {
- scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
- ScriptContext.ENGINE_SCOPE);
- }
- }
-
- try {
- final Object result = currentScript.eval(scriptContext);
- if (null == result) {
- log.error("{} Matcher script did not return a result.", getLogPrefix());
- return null;
- }
-
- if (result instanceof Set) {
- final HashSet<IdPAttributeValue<?>> returnValues = new HashSet<>(attribute.getValues());
- returnValues.retainAll((Set) result);
- return Collections.unmodifiableSet(returnValues);
- } else {
- log.error("{} Matcher script did not return a Set.", getLogPrefix());
- return null;
- }
- } catch (final ScriptException e) {
- log.error("{} Error while executing value matching script", getLogPrefix(), e);
- return null;
- }
- }
-
- /** {@inheritDoc} */
- @Override protected void doInitialize() throws ComponentInitializationException {
- super.doInitialize();
- // Clear name cache now that the name is definitive
- logPrefix = null;
-
- if (null == script) {
- throw new ComponentInitializationException("No script has been provided");
- }
+ return scriptEvaluator.execute(attribute, filterContext);
}
/** {@inheritDoc} */
@@ -269,20 +228,81 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
}
/**
- * return a string which is to be prepended to all log messages.
- *
- * @return "Scripted Attribute Filter '<filterID>' :"
+ * Evaluator bound to the Matcher semantic.
*/
- protected String getLogPrefix() {
- // local cache of cached entry to allow unsynchronised clearing.
- String prefix = logPrefix;
- if (null == prefix) {
- final StringBuilder builder = new StringBuilder("Scripted Attribute Filter '").append(getId()).append("':");
- prefix = builder.toString();
- if (null == logPrefix) {
- logPrefix = prefix;
+ private class MatcherScriptEvaluator extends AbstractScriptEvaluator {
+
+ /**
+ * Constructor.
+ *
+ * @param theScript the script we will evaluate.
+ */
+ public MatcherScriptEvaluator(@Nonnull final EvaluableScript theScript) {
+ super(theScript);
+ // Guarantee result is a Set or null.
+ setOutputType(Set.class);
+ // Turn ScriptException result into default "error" result, which is left at null.
+ setHideExceptions(true);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ @Nullable public Object getCustomObject() {
+ return super.getCustomObject();
+ }
+
+ /**
+ * Execution hook.
+ *
+ * @param attribute attribute to be filtered
+ * @param filterContext filter context
+ *
+ * @return script result
+ */
+ @Nullable @NonnullElements @Unmodifiable public Set<IdPAttributeValue<?>> execute(
+ @Nonnull final IdPAttribute attribute, @Nonnull final AttributeFilterContext filterContext) {
+ final Object result = evaluate(attribute, filterContext);
+ if (null == result) {
+ log.error("{} Matcher script did not return a result", getLogPrefix());
+ return null;
+ }
+
+ final HashSet<IdPAttributeValue<?>> returnValues = new HashSet<>(attribute.getValues());
+ returnValues.retainAll((Set) result);
+ return Collections.unmodifiableSet(returnValues);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void prepareContext(@Nonnull final ScriptContext scriptContext, @Nullable final Object... input) {
+
+ scriptContext.setAttribute("attribute", input[0], ScriptContext.ENGINE_SCOPE);
+ scriptContext.setAttribute("filterContext", input[1], ScriptContext.ENGINE_SCOPE);
+ scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
+
+ final ProfileRequestContext prc = prcLookupStrategy.apply((AttributeFilterContext) input[1]);
+ scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
+
+ final SubjectContext sc;
+ if (null == prc) {
+ log.error("{} Could not locate ProfileRequestContext", getLogPrefix());
+ sc = null;
+ } else {
+ sc = scLookupStrategy.apply(prc);
+ }
+
+ if (null == sc) {
+ log.warn("{} Could not locate SubjectContext", getLogPrefix());
+ } else {
+ final List<Subject> subjects = sc.getSubjects();
+ if (null == subjects) {
+ log.warn("{} Could not locate Subjects", getLogPrefix());
+ } else {
+ scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
+ ScriptContext.ENGINE_SCOPE);
+ }
}
}
- return prefix;
}
+
}
\ No newline at end of file
diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java
index e4e421f..d383d50 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java
@@ -23,19 +23,19 @@ import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
import javax.script.ScriptContext;
-import javax.script.ScriptException;
-import javax.script.SimpleScriptContext;
import javax.security.auth.Subject;
import net.shibboleth.idp.attribute.filter.PolicyRequirementRule;
import net.shibboleth.idp.attribute.filter.context.AttributeFilterContext;
import net.shibboleth.idp.authn.context.SubjectContext;
import net.shibboleth.idp.profile.context.RelyingPartyContext;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
import net.shibboleth.utilities.java.support.component.AbstractIdentifiableInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.component.UnmodifiableComponent;
import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.scripting.AbstractScriptEvaluator;
import net.shibboleth.utilities.java.support.scripting.EvaluableScript;
import org.opensaml.messaging.context.navigate.ChildContextLookup;
@@ -59,19 +59,19 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
UnmodifiableComponent {
/** Class logger. */
- private final Logger log = LoggerFactory.getLogger(ScriptedPolicyRule.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(ScriptedPolicyRule.class);
/** Script to be evaluated. */
- @Nonnull private EvaluableScript script;
+ @NonnullAfterInit private EvaluableScript script;
- /** Log prefix. */
- private String logPrefix;
+ /** Evaluator. */
+ @NonnullAfterInit private PolicyRuleScriptEvaluator scriptEvaluator;
/** Strategy used to locate the {@link ProfileRequestContext} to use. */
- @Nonnull private Function<AttributeFilterContext, ProfileRequestContext> prcLookupStrategy;
+ @Nonnull private Function<AttributeFilterContext,ProfileRequestContext> prcLookupStrategy;
/** Strategy used to locate the {@link SubjectContext} to use. */
- @Nonnull private Function<ProfileRequestContext, SubjectContext> scLookupStrategy;
+ @Nonnull private Function<ProfileRequestContext,SubjectContext> scLookupStrategy;
/** The custom object we inject into all scripts. */
@Nullable private Object customObject;
@@ -99,7 +99,9 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
*
* @param object the custom object
*/
- @Nullable public void setCustomObject(final Object object) {
+ public void setCustomObject(@Nullable final Object object) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
customObject = object;
}
@@ -108,7 +110,7 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
*
* @return the script to be evaluated
*/
- @Nonnull public EvaluableScript getScript() {
+ @NonnullAfterInit public EvaluableScript getScript() {
return script;
}
@@ -162,71 +164,30 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
* </p>
* {@inheritDoc}
*/
- @Override public Tristate matches(@Nonnull final AttributeFilterContext filterContext) {
- Constraint.isNotNull(filterContext, "Attribute filter context can not be null");
+ @Override
+ @Nonnull public Tristate matches(@Nonnull final AttributeFilterContext filterContext) {
+ Constraint.isNotNull(filterContext, "Attribute filter context cannot be null");
- final EvaluableScript currentScript = script;
ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
-
- final SimpleScriptContext scriptContext = new SimpleScriptContext();
- scriptContext.setAttribute("filterContext", filterContext, ScriptContext.ENGINE_SCOPE);
- scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
- final ProfileRequestContext prc = prcLookupStrategy.apply(filterContext);
- final SubjectContext sc;
- if (null == prc) {
- log.error("{} Could not locate ProfileRequestContext", getLogPrefix());
- sc = null;
- } else {
- sc = scLookupStrategy.apply(prc);
- }
-
- scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
- if (null == sc) {
- log.warn("{} Could not locate SubjectContext", getLogPrefix());
- } else {
- final List<Subject> subjects = sc.getSubjects();
- if (null == subjects) {
- log.warn("{} Could not locate Subjects", getLogPrefix());
- } else {
- scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
- ScriptContext.ENGINE_SCOPE);
- }
- }
-
- try {
- final Object result = currentScript.eval(scriptContext);
- if (null == result) {
- log.error("{} Matcher script did not return a result", getLogPrefix());
- return Tristate.FAIL;
- }
-
- if (result instanceof Boolean) {
- if (((Boolean) result).booleanValue()) {
- return Tristate.TRUE;
- }
- return Tristate.FALSE;
- } else {
- log.error("{} Matcher script returned a {}, not a java.lang.Boolean", getLogPrefix(), result.getClass()
- .toString());
- return Tristate.FAIL;
- }
- } catch (final ScriptException e) {
- log.error("{} Error while executing value matching script", getLogPrefix(), e);
- return Tristate.FAIL;
- }
+
+ return scriptEvaluator.execute(filterContext);
}
/** {@inheritDoc} */
@Override protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
- // clear cached name
- logPrefix = null;
if (null == script) {
// never met so long as we have the assert in the constructor
throw new ComponentInitializationException("No script has been provided");
}
+
+ scriptEvaluator = new PolicyRuleScriptEvaluator(script);
+ scriptEvaluator.setCustomObject(customObject);
+
+ final StringBuilder builder = new StringBuilder("Scripted Attribute Filter '").append(getId()).append("':");
+ scriptEvaluator.setLogPrefix(builder.toString());
}
/** {@inheritDoc} */
@@ -259,20 +220,84 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
}
/**
- * return a string which is to be prepended to all log messages.
- *
- * @return "Scripted Attribute Filter '<filterID>' :"
+ * Evaluator bound to the Matcher semantic.
*/
- protected String getLogPrefix() {
- // local cache of cached entry to allow unsynchronised clearing.
- String prefix = logPrefix;
- if (null == prefix) {
- final StringBuilder builder = new StringBuilder("Scripted Attribute Filter '").append(getId()).append("':");
- prefix = builder.toString();
- if (null == logPrefix) {
- logPrefix = prefix;
+ private class PolicyRuleScriptEvaluator extends AbstractScriptEvaluator {
+
+ /**
+ * Constructor.
+ *
+ * @param theScript the script we will evaluate.
+ */
+ public PolicyRuleScriptEvaluator(@Nonnull final EvaluableScript theScript) {
+ super(theScript);
+ // Guarantee result is a Boolean.
+ setOutputType(Boolean.class);
+ // Return a FAIL result on errors.
+ setReturnOnError(Tristate.FAIL);
+ // Turn ScriptException into default error result (FAIL).
+ setHideExceptions(true);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ @Nullable public Object getCustomObject() {
+ return super.getCustomObject();
+ }
+
+ /**
+ * Execution hook.
+ *
+ * @param filterContext filter context
+ *
+ * @return script result
+ */
+ @Nonnull public Tristate execute(@Nonnull final AttributeFilterContext filterContext) {
+
+ final Object result = evaluate(filterContext);
+ if (null == result) {
+ log.error("{} Matcher script did not return a result", getLogPrefix());
+ return Tristate.FAIL;
+ }
+
+ // Non-null result can only be a Boolean or Tristate.
+ if (result instanceof Boolean) {
+ return ((Boolean) result).booleanValue() ? Tristate.TRUE : Tristate.FALSE;
+ } else {
+ return (Tristate) result;
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void prepareContext(@Nonnull final ScriptContext scriptContext, @Nullable final Object... input) {
+
+ scriptContext.setAttribute("filterContext", input[0], ScriptContext.ENGINE_SCOPE);
+ scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
+
+ final ProfileRequestContext prc = prcLookupStrategy.apply((AttributeFilterContext) input[0]);
+ scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
+
+ final SubjectContext sc;
+ if (null == prc) {
+ log.error("{} Could not locate ProfileRequestContext", getLogPrefix());
+ sc = null;
+ } else {
+ sc = scLookupStrategy.apply(prc);
+ }
+
+ if (null == sc) {
+ log.warn("{} Could not locate SubjectContext", getLogPrefix());
+ } else {
+ final List<Subject> subjects = sc.getSubjects();
+ if (null == subjects) {
+ log.warn("{} Could not locate Subjects", getLogPrefix());
+ } else {
+ scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
+ ScriptContext.ENGINE_SCOPE);
+ }
}
}
- return prefix;
}
+
}
\ No newline at end of file
diff --git a/idp-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcherTest.java b/idp-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcherTest.java
index 91a6ef1..08d6af6 100644
--- a/idp-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcherTest.java
+++ b/idp-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcherTest.java
@@ -193,8 +193,8 @@ public class ScriptedMatcherTest extends AbstractMatcherPolicyRuleTest {
final ScriptedMatcher matcher = newScriptedMatcher(new EvaluableScript("custom;"));
final Set<IdPAttributeValue> custom = Collections.singleton((IdPAttributeValue)attribute.getValues().get(0));
matcher.setId("Test");
- matcher.initialize();
matcher.setCustomObject(custom);
+ matcher.initialize();
final Set<IdPAttributeValue<?>> result = matcher.getMatchingValues(attribute, filterContext);
Assert.assertNotNull(result);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list