Skip to content

Commit

Permalink
Reduced access for new string constants; added principal ace criteria.
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamcin
adamcin committed Oct 3, 2019
1 parent ccd1881 commit dec823a
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
### Added
- Added `severity` config parameter to ExpectPaths and ExpectAces checks
- Added `principals` config parameter to ExpectAces check as multi-valued alternative to single-value `principal` param.
- Added `principal=` key to allowed ACE criteria syntax in the ExpectAces check to allow per-ACE override of `principal` config parameter.

## [1.5.0] - 2019-10-02

Expand Down
74 changes: 50 additions & 24 deletions core/src/main/java/net/adamcin/oakpal/core/checks/ExpectAces.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,26 +106,29 @@
* <li>path = absolute path (must exist in JCR). If not specified, or the empty string, the ace criteria are evaluated against {@code /rep:repoPolicy}.</li>
* </ul>
* <p>
* OPTIONAL Restrictions - restrictions with comma-separated values are evaluated as multivalued when the restriction
* definition so indicates. Otherwise the comma-separated values are treated as an opaque string.
* OPTIONAL
* <ul>
* <li>principal = overrides the check config {@code principal} parameter for this particular ACE</li>
* <li>rep:glob = rep glob expression</li>
* <li>rep:ntNames = ntNames expression</li>
* <li>rep:nodePath = for system users, aces defined in the user home can reference applicable repo path using this
* restriction</li>
* <li>anyOtherRestriction = any name can be used as a restriction constraint. if the restriction is not allowed for
* use at a particular path, it will be ignored.</li>
* </ul>
* <p>
* Note - restrictions with comma-separated values are evaluated as multivalued when the restriction
* definition so indicates. Otherwise the comma-separated values are treated as an opaque string.
*/
public final class ExpectAces implements ProgressCheckFactory {
private static final Logger LOGGER = LoggerFactory.getLogger(ExpectAces.class);
public static final String CONFIG_PRINCIPAL = "principal";
public static final String CONFIG_PRINCIPALS = "principals";
static final String CONFIG_PRINCIPALS = "principals";
public static final String CONFIG_EXPECTED_ACES = "expectedAces";
public static final String CONFIG_NOT_EXPECTED_ACES = "notExpectedAces";
public static final String CONFIG_AFTER_PACKAGE_ID_RULES = "afterPackageIdRules";
public static final String CONFIG_SEVERITY = "severity";
public static final Violation.Severity DEFAULT_SEVERITY = Violation.Severity.MAJOR;
static final String CONFIG_SEVERITY = "severity";
static final Violation.Severity DEFAULT_SEVERITY = Violation.Severity.MAJOR;
public static final String ACE_PARAM_TYPE = "type";
public static final String ACE_PARAM_PRIVILEGES = "privileges";
public static final String ACE_PARAM_PATH = "path";
Expand All @@ -138,9 +141,6 @@ public ProgressCheck newInstance(final JsonObject config) throws Exception {
final String principal = config.getString(CONFIG_PRINCIPAL, "").trim();
final String[] principals = JavaxJson.mapArrayOfStrings(JavaxJson.arrayOrEmpty(config, CONFIG_PRINCIPALS))
.stream().map(String::trim).filter(inferTest1(String::isEmpty).negate()).toArray(String[]::new);
if (principal.isEmpty() && principals.length == 0) {
throw new Exception("principal or principals must be non-empty");
}

final String[] precedingPrincipals = principal.isEmpty() ? principals : new String[]{principal};

Expand All @@ -152,21 +152,44 @@ public ProgressCheck newInstance(final JsonObject config) throws Exception {
Rule.fromJsonArray(JavaxJson.arrayOrEmpty(config, CONFIG_AFTER_PACKAGE_ID_RULES)), severity);
}

static boolean isPrincipalSpec(final @NotNull String spec) {
return spec.contains(CONFIG_PRINCIPAL + "=");
}

static boolean isGeneralSpec(final @NotNull String spec) {
return !isPrincipalSpec(spec);
}

static List<AceCriteria> parseAceCriteria(final @NotNull JsonObject config,
final @NotNull String[] principals,
final @NotNull String key) throws Exception {
List<AceCriteria> allCriterias = new ArrayList<>();
for (String principal : principals) {
final Result<List<AceCriteria>> expectedAceResults = JavaxJson
.mapArrayOfStrings(JavaxJson.arrayOrEmpty(config, key),
spec -> AceCriteria.parse(principal, spec)).stream()
.collect(Result.tryCollect(Collectors.toList()));
if (expectedAceResults.getError().isPresent()) {
throw new Exception("invalid criteria in " + key + ". " + expectedAceResults.getError()
.map(Throwable::getMessage).orElse(""));
final List<AceCriteria> allCriterias = new ArrayList<>();
final List<String> specs = JavaxJson.mapArrayOfStrings(JavaxJson.arrayOrEmpty(config, key));
final List<String> generalSpecs = specs.stream().filter(ExpectAces::isGeneralSpec).collect(Collectors.toList());
if (!generalSpecs.isEmpty() && principals.length == 0) {
throw new Exception("principal or principals check config param must be non-empty if general ACE criteria are specified");
} else {
for (String principal : principals) {
final Result<List<AceCriteria>> expectedAceResults = generalSpecs.stream()
.map(spec -> AceCriteria.parse(principal, spec))
.collect(Result.tryCollect(Collectors.toList()));
if (expectedAceResults.getError().isPresent()) {
throw new Exception("invalid criteria in " + key + ". " + expectedAceResults.getError()
.map(Throwable::getMessage).orElse(""));
}
expectedAceResults.forEach(allCriterias::addAll);
}
expectedAceResults.forEach(allCriterias::addAll);
}
final Result<List<AceCriteria>> principalAceResults = specs.stream()
.filter(ExpectAces::isPrincipalSpec)
.map(spec -> AceCriteria.parse("", spec))
.collect(Result.tryCollect(Collectors.toList()));
if (principalAceResults.getError().isPresent()) {
throw new Exception("invalid criteria in " + key + ". " + principalAceResults.getError()
.map(Throwable::getMessage).orElse(""));
}
principalAceResults.forEach(allCriterias::addAll);

return allCriterias;
}

Expand Down Expand Up @@ -376,11 +399,7 @@ static Type forName(final String name) {
}

static Result<AceCriteria> parse(final @NotNull String principal, final @NotNull String spec) {
if (principal.isEmpty()) {
return Result.failure("principal must be non-empty: " + spec);
}

final List<Map.Entry<String, Optional<String>>> pairs = Stream.of(spec.trim().split(DELIM_PARAM))
final List<Map.Entry<String, Optional<String>>> pairs = Stream.of(spec.trim().split(DELIM_PARAM))
.map(param -> {
String[] parts = param.split(DELIM_VALUE, 2);
final String trimmedValue = parts.length > 1 ? parts[1].trim() : "";
Expand All @@ -396,6 +415,12 @@ static Result<AceCriteria> parse(final @NotNull String principal, final @NotNull
.map(Fun.mapValue(Optional::get))
.collect(Fun.entriesToMap());

final String effectivePrincipal = valueMap.getOrDefault(CONFIG_PRINCIPAL, principal).trim();
if (effectivePrincipal.isEmpty()) {
return Result.failure("principal must be non-empty: " + spec);
}
keys.remove(CONFIG_PRINCIPAL);

if (!valueMap.containsKey(ACE_PARAM_TYPE)) {
return Result.failure(ACE_PARAM_TYPE + " is a required ace parameter: " + spec);
}
Expand Down Expand Up @@ -431,7 +456,8 @@ static Result<AceCriteria> parse(final @NotNull String principal, final @NotNull
.map(mapEntry(RestrictionCriteria::new))
.toArray(RestrictionCriteria[]::new);

return Result.success(new AceCriteria(principal, type == Type.ALLOW, path, privileges, restrictions, spec));
return Result.success(new AceCriteria(effectivePrincipal,
type == Type.ALLOW, path, privileges, restrictions, spec));
}

String getSpec() {
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/net/adamcin/oakpal/core/checks/ExpectPaths.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ public final class ExpectPaths implements ProgressCheckFactory {
public static final String CONFIG_EXPECTED_PATHS = "expectedPaths";
public static final String CONFIG_NOT_EXPECTED_PATHS = "notExpectedPaths";
public static final String CONFIG_AFTER_PACKAGE_ID_RULES = "afterPackageIdRules";
public static final String CONFIG_SEVERITY = "severity";
public static final Violation.Severity DEFAULT_SEVERITY = Violation.Severity.MAJOR;
static final String CONFIG_SEVERITY = "severity";
static final Violation.Severity DEFAULT_SEVERITY = Violation.Severity.MAJOR;

@Override
public ProgressCheck newInstance(final JsonObject config) {
Expand Down
15 changes: 13 additions & 2 deletions core/src/test/java/net/adamcin/oakpal/core/checks/ExpectAcesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,23 @@ static ExpectAces.Check checkFor(final JsonObject config) throws Exception {

@Test(expected = Exception.class)
public void testNewInstance_missingPrincipal() throws Exception {
checkFor(key("principal", "").get());
checkFor(key("principal", "").key("expectedAces", arr("type=allow;path=/;privileges=jcr:read")).get());
}

@Test(expected = Exception.class)
public void testNewInstance_missingPrincipals() throws Exception {
checkFor(key("principal", "").key("principals", arr("", "")).get());
checkFor(key("principal", "").key("principals", arr("", ""))
.key("expectedAces", arr("type=allow;path=/;privileges=jcr:read")).get());
}

@Test(expected = Exception.class)
public void testNewInstance_principalAceFail() throws Exception {
checkFor(key("expectedAces", arr("principal=nouser;type=foobar;path=/;privileges=jcr:read")).get());
}

@Test
public void testNewInstance_principalAce() throws Exception {
checkFor(key("expectedAces", arr("principal=nouser;type=allow;path=/;privileges=jcr:read")).get());
}

@Test
Expand Down

0 comments on commit dec823a

Please sign in to comment.