Commit ba8c9a8e authored by Cyril Dangerville's avatar Cyril Dangerville

- Fix #34: NPE with indeterminate <AllOf> and no <AllOf> matched in same

<AnyOf>
- New Generic Non-regression test class (NonRegression): more info: (
#32 in Gitlab) src/test/resources/NonRegression/README.md 
- Non-Regression test to validate fix for #19 (request w/o subject-id)
and #16 (fix stackoverflow error with Audit log aspect) in
src/test/resources/NonRegression/16_19
- Added TestAttributeFinder ( #31 ) to be used for unit tests requiring
dumb attribute finders for any attribute id/category (configurable as
attribute finder parameter), currently used in new NonRegression test
aforementioned
- New validated test case for #29, i.e. BasicMultipleRequestsV3 using
Multiple Decision Profile with repeated categories but no
<MultipleRequests>, section 2.3 of XACML MDP spec
- Fix #30: test matchResult() not working on Response with multiple
results (Multi Decision Profile)
- Improved error message for illegal parameters to function ( #33 ) to
say which function, which type or number of args, etc.
- Moved debug logs showing evaluation results of rule/policy to the
AuditAspect.aj
- Improved debug logs for AuditAspect to print joinpoing kind and target
triggering the aspect (class and method signature)
- Added debug logs for troubleshooting AnyOf/AllOf matches
- Made package name in src/test/java start like the one in
src/main/java: com.thalesgroup.authzforce.core.test...
parent 24d37da0
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>com.thalesgroup.authzforce</groupId>
......@@ -14,12 +15,12 @@
<description>XACML-compliant Authorization PDP Engine Core</description>
<inceptionYear>2011</inceptionYear>
<scm>
<!-- Used by Jenkins - Maven prepare-release -->
<!-- Used by Jenkins - Maven release plugin -->
<connection>scm:git:https://gitlab.dev.theresis.org/authzforce/core.git</connection>
<developerConnection>scm:git:https://gitlab.dev.theresis.org/authzforce/core.git</developerConnection>
<tag>HEAD</tag>
<!-- Publicly browsable repository URL. For example, via Gitlab web UI. -->
<!-- Publicly browsable repository URL. For example, via Gitlab web UI. -->
<url>https://gitlab.dev.theresis.org/authzforce/core</url>
</scm>
<properties>
......@@ -210,6 +211,63 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.jvnet.jaxb2.maven2</groupId>
<artifactId>maven-jaxb2-plugin</artifactId>
<configuration>
<!-- debug=true will generate JAXBDebug class. More info: https://github.com/highsource/maven-jaxb2-plugin/wiki/Miscellaneous -->
<debug>false</debug>
<verbose>true</verbose>
<extension>true</extension>
<strict>false</strict>
<!-- Episodes: Only episodes for schemas referenced (imported/included) by schema(s) in schemaDirectory
can be listed here. If not possible, just create an empty schema in schemaDirectory which imports all
the episode elements but does nothing with them. -->
<!-- <episodes> -->
<!-- </episodes> -->
<plugins>
<plugin>
<groupId>com.thalesgroup.ktd.scis</groupId>
<artifactId>oasis-xacml-model</artifactId>
</plugin>
<plugin>
<groupId>com.thalesgroup.authzforce</groupId>
<artifactId>authzforce-core-model</artifactId>
</plugin>
</plugins>
<useDependenciesAsEpisodes>true</useDependenciesAsEpisodes>
<bindingDirectory>src/main/resources</bindingDirectory>
<catalog>src/main/resources/catalog.xml</catalog>
<catalogResolver>org.jvnet.jaxb2.maven2.resolver.tools.ClasspathCatalogResolver</catalogResolver>
<removeOldOutput>true</removeOldOutput>
</configuration>
<executions>
<execution>
<id>jaxb-generate-compile-sources</id>
<goals>
<goal>generate</goal>
</goals>
<configuration>
<schemaDirectory>src/main/resources</schemaDirectory>
<generateDirectory>${project.build.directory}/generated-sources/xjc</generateDirectory>
<addCompileSourceRoot>true</addCompileSourceRoot>
<addTestCompileSourceRoot>false</addTestCompileSourceRoot>
</configuration>
</execution>
<execution>
<id>jaxb-generate-test-sources</id>
<goals>
<goal>generate</goal>
</goals>
<configuration>
<schemaDirectory>src/test/resources</schemaDirectory>
<generateDirectory>${project.build.directory}/generated-test-sources/xjc</generateDirectory>
<addCompileSourceRoot>false</addCompileSourceRoot>
<addTestCompileSourceRoot>true</addTestCompileSourceRoot>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>aspectj-maven-plugin</artifactId>
......@@ -298,63 +356,6 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.jvnet.jaxb2.maven2</groupId>
<artifactId>maven-jaxb2-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>generate</goal>
</goals>
</execution>
</executions>
<configuration>
<debug>true</debug>
<verbose>true</verbose>
<extension>true</extension>
<args>
<arg>-Xannotate</arg>
<args>-Xequals</args>
<args>-XhashCode</args>
</args>
<!-- Episodes: Only episodes for schemas referenced (imported/included) by schema(s) in schemaDirectory
can be listed here. If not possible, just create an empty schema in schemaDirectory which imports all
the episode elements but does nothing with them. -->
<episodes>
<episode>
<groupId>com.thalesgroup.ktd.scis</groupId>
<artifactId>oasis-xacml-model</artifactId>
</episode>
<episode>
<groupId>com.thalesgroup.authzforce</groupId>
<artifactId>authzforce-core-model</artifactId>
</episode>
</episodes>
<plugins>
<plugin>
<groupId>org.jvnet.jaxb2_commons</groupId>
<artifactId>jaxb2-basics-annotate</artifactId>
<version>0.6.4</version>
</plugin>
<plugin>
<groupId>org.jvnet.jaxb2_commons</groupId>
<artifactId>jaxb2-basics</artifactId>
<version>0.6.4</version>
</plugin>
<plugin>
<groupId>com.thalesgroup.ktd.scis</groupId>
<artifactId>oasis-xacml-model</artifactId>
</plugin>
<plugin>
<groupId>com.thalesgroup.authzforce</groupId>
<artifactId>authzforce-core-model</artifactId>
</plugin>
</plugins>
<catalog>src/main/resources/catalog.xml</catalog>
<catalogResolver>org.jvnet.jaxb2.maven2.resolver.tools.ClasspathCatalogResolver</catalogResolver>
<schemaDirectory>src/main/resources</schemaDirectory>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
......
......@@ -48,6 +48,7 @@ import oasis.names.tc.xacml._3_0.core.schema.wd_17.AttributeAssignment;
import oasis.names.tc.xacml._3_0.core.schema.wd_17.AttributeAssignmentExpression;
import oasis.names.tc.xacml._3_0.core.schema.wd_17.CombinerParametersType;
import oasis.names.tc.xacml._3_0.core.schema.wd_17.DecisionType;
import oasis.names.tc.xacml._3_0.core.schema.wd_17.EffectType;
import oasis.names.tc.xacml._3_0.core.schema.wd_17.ObligationExpression;
import org.slf4j.Logger;
......@@ -297,7 +298,6 @@ public abstract class AbstractPolicySet extends oasis.names.tc.xacml._3_0.core.s
*/
adviceExpressions = null;
_children = root.getChildNodes();
// List myPolicies = new ArrayList<>();
for (int i = 0; i < _children.getLength(); i++)
{
......@@ -421,7 +421,7 @@ public abstract class AbstractPolicySet extends oasis.names.tc.xacml._3_0.core.s
{
return URI.create(policySetId);
}
return null;
}
......@@ -519,7 +519,8 @@ public abstract class AbstractPolicySet extends oasis.names.tc.xacml._3_0.core.s
*
* @return the result of evaluation
*
* FIXME: the match logic should be done by evaluate, doing two different function calls (match + evaluate) every time is useless
* FIXME: the match logic should be done by evaluate, doing two different function calls
* (match + evaluate) every time is useless
*/
@Override
public Result evaluate(EvaluationCtx context)
......@@ -545,79 +546,71 @@ public abstract class AbstractPolicySet extends oasis.names.tc.xacml._3_0.core.s
// evaluate
result = this.combiningAlg.combine(context, combParams, policies);
try
if (obligationExpressions != null && !obligationExpressions.getObligationExpressions().isEmpty())
{
if (obligationExpressions != null && !obligationExpressions.getObligationExpressions().isEmpty())
{
// now, see if we should add any obligations to the set
int effect = result.getDecision().ordinal();
// now, see if we should add any obligations to the set
DecisionType decision = result.getDecision();
if ((effect == DecisionType.INDETERMINATE.ordinal()) || (effect == DecisionType.NOT_APPLICABLE.ordinal()))
{
// we didn't permit/deny, so we never return obligations
return result;
}
if ((decision == DecisionType.INDETERMINATE) || (decision == DecisionType.NOT_APPLICABLE))
{
// we didn't permit/deny, so we never return obligations
return result;
}
for (ObligationExpression myObligation : obligationExpressions.getObligationExpressions())
for (ObligationExpression myObligation : obligationExpressions.getObligationExpressions())
{
final DecisionType obligEffect = myObligation.getFulfillOn() == EffectType.PERMIT? DecisionType.PERMIT: DecisionType.DENY;
if (obligEffect == decision)
{
if (myObligation.getFulfillOn().ordinal() == effect)
{
result.addObligation(myObligation, context);
}
result.addObligation(myObligation, context);
}
}
/* If we have advice, it's definitely a 3.0 policy */
if (adviceExpressions != null && !adviceExpressions.getAdviceExpressions().isEmpty())
{
int effect = result.getDecision().ordinal();
}
/* If we have advice, it's definitely a 3.0 policy */
if (adviceExpressions != null && !adviceExpressions.getAdviceExpressions().isEmpty())
{
DecisionType decision = result.getDecision();
if ((effect == DecisionType.INDETERMINATE.ordinal()) || (effect == DecisionType.NOT_APPLICABLE.ordinal()))
{
// we didn't permit/deny, so we never return advices
return result;
}
if ((decision == DecisionType.INDETERMINATE) || (decision == DecisionType.NOT_APPLICABLE))
{
// we didn't permit/deny, so we never return advices
return result;
}
final AssociatedAdvice returnAssociatedAdvice = result.getAssociatedAdvice();
final AssociatedAdvice newAssociatedAdvice;
if (returnAssociatedAdvice == null)
{
newAssociatedAdvice = new AssociatedAdvice();
result.setAssociatedAdvice(newAssociatedAdvice);
} else
{
newAssociatedAdvice = returnAssociatedAdvice;
}
final AssociatedAdvice returnAssociatedAdvice = result.getAssociatedAdvice();
final AssociatedAdvice newAssociatedAdvice;
if (returnAssociatedAdvice == null)
{
newAssociatedAdvice = new AssociatedAdvice();
result.setAssociatedAdvice(newAssociatedAdvice);
} else
{
newAssociatedAdvice = returnAssociatedAdvice;
}
for (AdviceExpression adviceExpr : adviceExpressions.getAdviceExpressions())
for (AdviceExpression adviceExpr : adviceExpressions.getAdviceExpressions())
{
final DecisionType adviceAppliesTo = adviceExpr.getAppliesTo() == EffectType.PERMIT? DecisionType.PERMIT: DecisionType.DENY;
if (adviceAppliesTo == decision)
{
if (adviceExpr.getAppliesTo().ordinal() == effect)
Advice advice = new Advice();
advice.setAdviceId(adviceExpr.getAdviceId());
for (AttributeAssignmentExpression attrExpr : adviceExpr.getAttributeAssignmentExpressions())
{
Advice advice = new Advice();
advice.setAdviceId(adviceExpr.getAdviceId());
for (AttributeAssignmentExpression attrExpr : adviceExpr.getAttributeAssignmentExpressions())
{
AttributeAssignment myAttrAssType = new AttributeAssignment();
myAttrAssType.setAttributeId(attrExpr.getAttributeId());
myAttrAssType.setCategory(attrExpr.getCategory());
myAttrAssType.getContent().add(attrExpr.getExpression());
myAttrAssType.setIssuer(attrExpr.getIssuer());
advice.getAttributeAssignments().add(myAttrAssType);
}
newAssociatedAdvice.getAdvices().add(advice);
AttributeAssignment myAttrAssType = new AttributeAssignment();
myAttrAssType.setAttributeId(attrExpr.getAttributeId());
myAttrAssType.setCategory(attrExpr.getCategory());
myAttrAssType.getContent().add(attrExpr.getExpression());
myAttrAssType.setIssuer(attrExpr.getIssuer());
advice.getAttributeAssignments().add(myAttrAssType);
}
newAssociatedAdvice.getAdvices().add(advice);
}
}
if (context.getIncludeInResults().size() > 0)
{
result.getAttributes().addAll(context.getIncludeInResults());
}
return result;
} finally
{
LOGGER.debug("{} returned: {}", this, result);
}
return result;
}
@Override
......
......@@ -35,114 +35,127 @@ package com.sun.xacml;
import com.sun.xacml.ctx.Status;
/**
* This is used as the return value for the various target matching functions.
* It communicates that either the target matches the input request, the
* target doesn't match the input request, or the result is Indeterminate.
*
* This is used as the return value for the various target matching functions. It communicates that
* either the target matches the input request, the target doesn't match the input request, or the
* result is Indeterminate.
*
* @since 1.0
* @author Seth Proctor
*/
public class MatchResult
{
/**
* An integer value indicating the the target matches the request
*/
public static final int MATCH = 0;
/**
* An integer value indicating that the target doesn't match the request
*/
public static final int NO_MATCH = 1;
/**
* An integer value indicating the the result is Indeterminate
*/
public static final int INDETERMINATE = 2;
//
private int result;
private Status status;
/**
* Constructor that creates a <code>MatchResult</code> with no Status
*
* @param result the applicable result
*/
public MatchResult(int result) {
this(result, null);
}
/**
* Constructor that creates a <code>MatchResult</code>, including Status
* data
*
* @param result the applicable result
* @param status the error information
*
* @throws IllegalArgumentException if the input result isn't a valid value
*/
public MatchResult(int result, Status status)
throws IllegalArgumentException
{
// check if input result is a valid value
if ((result != MATCH) &&
(result != NO_MATCH) &&
(result != INDETERMINATE))
throw new IllegalArgumentException("Input result is not a valid" +
"value");
this.result = result;
this.status = status;
}
/**
* Returns the applicable result
*
* @return the applicable result
*/
public int getResult() {
return result;
}
/**
* Returns the status if there was an error, or null if no error occurred
*
* @return the error status data or null
*/
public Status getStatus() {
return status;
}
/**
* @param status the status to set
/**
* TODO: use enum constants for values below
*/
/**
* An integer value indicating the the target matches the request
*/
public static final int MATCH = 0;
/**
* An integer value indicating that the target doesn't match the request
*/
public static final int NO_MATCH = 1;
/**
* An integer value indicating the the result is Indeterminate
*/
public static final int INDETERMINATE = 2;
//
private int result;
private Status status;
/**
* Constructor that creates a <code>MatchResult</code> with no Status
*
* @param result
* the applicable result
*/
public MatchResult(int result)
{
this(result, null);
}
/**
* Constructor that creates a <code>MatchResult</code>, including Status data
*
* @param result
* the applicable result
* @param status
* the error information
*
* @throws IllegalArgumentException
* if the input result isn't a valid value
*/
protected void setStatus(Status status) {
public MatchResult(int result, Status status) throws IllegalArgumentException
{
// check if input result is a valid value
if ((result != MATCH) && (result != NO_MATCH) && (result != INDETERMINATE)) {
throw new IllegalArgumentException("Input result is not a valid" + "value");
}
this.result = result;
this.status = status;
}
/**
* Returns the applicable result
*
* @return the applicable result
*/
public int getResult()
{
return result;
}
/**
* Returns the status if there was an error, or null if no error occurred
*
* @return the error status data or null
*/
public Status getStatus()
{
return status;
}
/**
* @param status
* the status to set
*/
protected void setStatus(Status status)
{
this.status = status;
}
@Override
public String toString() {
String matchResult = null;
if (result == 0) {
matchResult = "MATCH";
} else if (result == 1) {
matchResult = "NO_MATCH";
} else if (result == 2) {
matchResult = "INDETERMINATE";
}
String msg = "MatchResult: " + matchResult;
if (status != null) {
msg += " " + status.toString();
}
return msg;
}
public String toString()
{
String matchResult = null;
if (result == 0)
{
matchResult = "MATCH";
} else if (result == 1)
{
matchResult = "NO_MATCH";
} else if (result == 2)
{
matchResult = "INDETERMINATE";
}
String msg = "MatchResult: " + matchResult;
if (status != null)
{
msg += " " + status;
}
return msg;
}
}
This diff is collapsed.
This diff is collapsed.
......@@ -106,6 +106,8 @@ public abstract class FunctionBase extends Function
private String[] paramTypes;
private boolean[] paramsAreBags;
private final String INVALID_ARGUMENT_MESSAGE_FORMAT = "Invalid argument #%s to function '%s': type='%s', isBag='%s'. Required: type='%s', isBag='%s'.";
/**
* Constructor that sets up the function as having some number of parameters all of the same
* given type. If <code>numParams</code> is -1, then the length is variable
......@@ -405,14 +407,15 @@ public abstract class FunctionBase extends Function
if (numParams != -1)
{
if (inputs.size() != numParams)
throw new IllegalArgumentException("wrong number of args" + " to " + functionName);
throw new IllegalArgumentException("wrong number of args to " + functionName + ". Required: " + numParams);
} else
{
if (inputs.size() < minParams)
throw new IllegalArgumentException("not enough args" + " to " + functionName);
throw new IllegalArgumentException("not enough args to " + functionName + ". Required: >=" + minParams);
}
// now, make sure everything is of the same, correct type
int argIndex = 0;
for (final ExpressionType eval : inputs)
{
/*
......@@ -423,24 +426,28 @@ public abstract class FunctionBase extends Function
AttributeDesignator evalTmp = (AttributeDesignator) eval;
if ((!evalTmp.getDataType().toString().equals(paramType)) || (evalTmp.returnsBag() != paramIsBag))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, argIndex, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramType, paramIsBag));
}
} else if (eval instanceof AttributeSelectorType)
} else if (eval instanceof AttributeSelector)
{
AttributeSelector evalTmp = (AttributeSelector) eval;
if ((!evalTmp.getDataType().toString().equals(paramType)) || (evalTmp.returnsBag() != paramIsBag))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, argIndex, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramType, paramIsBag));
}
} else if (eval instanceof AttributeValueType)
} else if (eval instanceof AttributeValue)
{
AttributeValue evalTmp = (AttributeValue) eval;
if ((!evalTmp.getDataType().toString().equals(paramType)) || (evalTmp.returnsBag() != paramIsBag))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, argIndex, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramType, paramIsBag));
}
}
argIndex += 1;
}
} else
{
......@@ -460,21 +467,24 @@ public abstract class FunctionBase extends Function
AttributeDesignator evalTmp = (AttributeDesignator) eval;
if ((!evalTmp.getType().toString().equals(paramTypes[i])) || (evalTmp.returnsBag() != paramsAreBags[i]))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, i, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramTypes[i], paramsAreBags[i]));
}
} else if (eval instanceof AttributeSelector)
{
AttributeSelector evalTmp = (AttributeSelector) eval;
if ((!evalTmp.getType().toString().equals(paramTypes[i])) || (evalTmp.returnsBag() != paramsAreBags[i]))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, i, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramTypes[i], paramsAreBags[i]));
}
} else if (eval instanceof AttributeValue)
{
AttributeValue evalTmp = (AttributeValue) eval;
if ((!evalTmp.getType().toString().equals(paramTypes[i])) || (evalTmp.returnsBag() != paramsAreBags[i]))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, i, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramTypes[i], paramsAreBags[i]));
}
}
......@@ -502,48 +512,53 @@ public abstract class FunctionBase extends Function
{
// first check to see if we need bags
if (paramIsBag)
throw new IllegalArgumentException(functionName + "needs" + "bags on input");
throw new IllegalArgumentException(functionName + " needs bags on input");
// now check on the length
if (numParams != -1)
{
if (inputs.size() != numParams)
throw new IllegalArgumentException("wrong number of args" + " to " + functionName);
throw new IllegalArgumentException("wrong number of args to " + functionName + ". Required: " + numParams);
} else
{
if (inputs.size() < minParams)
throw new IllegalArgumentException("not enough args" + " to " + functionName);
throw new IllegalArgumentException("not enough args to " + functionName + ". Required: >=" + minParams);
}
// finally check param list
int argIndex = 0;
for (final ExpressionType eval : inputs)
{
/*
* FIXME: Need to be rethink. Too much duplication and introspection
*/
if (eval instanceof AttributeDesignatorType)
if (eval instanceof AttributeDesignator)
{
AttributeDesignatorType evalTmp = (AttributeDesignatorType) eval;
AttributeDesignator evalTmp = (AttributeDesignator) eval;
if (!evalTmp.getDataType().toString().equals(paramType))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, argIndex, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramType, paramIsBag));
}
} else if (eval instanceof AttributeSelectorType)
} else if (eval instanceof AttributeSelector)
{
AttributeSelectorType evalTmp = (AttributeSelectorType) eval;
AttributeSelector evalTmp = (AttributeSelector) eval;
if (!evalTmp.getDataType().toString().equals(paramType))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, argIndex, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramType, paramIsBag));
}
} else if (eval instanceof AttributeValueType)
} else if (eval instanceof AttributeValue)
{
AttributeValueType evalTmp = (AttributeValueType) eval;
AttributeValue evalTmp = (AttributeValue) eval;
if (!evalTmp.getDataType().toString().equals(paramType))
{
throw new IllegalArgumentException("illegal parameter");
throw new IllegalArgumentException(String.format(INVALID_ARGUMENT_MESSAGE_FORMAT, argIndex, functionName,
evalTmp.getDataType(), evalTmp.returnsBag(), paramType, paramIsBag));
}
}