From aaedb3814bc2a12cfc52d4738cdf6cb612338933 Mon Sep 17 00:00:00 2001 From: Dominik Przybyl Date: Mon, 26 Oct 2026 12:44:32 +0100 Subject: [PATCH] Reveal glob and strict markers in ACL permissions logs The "Applied allow/deny permissions" and "Checked permissions" lines were visually indistinguishable when the same authorizable received multiple calls at the same (or overlapping) paths - the glob restriction, including the STRICT one meant to lock a single node, was not visible, so operators had to cross-check the script source to tell apart a wide-tree grant from a single-node protect. The "Cleared permissions" lines carried the same blind spot for the clear(..., strict) flag that chooses between selective and full removal. Every ACL permissions log line now carries a trailing modifier when it applies: Applied allow permissions for authorizable 'X' at path '/foo' with glob 'STRICT' Skipped setting deny permissions for authorizable 'X' at path '/foo' with glob 'STRICT' (already set) Checked allow permissions for authorizable 'X' at path '/foo' with glob 'STRICT' [true] Cleared permissions for authorizable 'X' at path '/foo' in strict mode Skipped clearing permissions for authorizable 'X' at path '/foo' in strict mode (path not found) Suffixes always sit between the main sentence and any trailing "(reason)" parenthetical, so parsers that match the pre-existing format (no glob, no strict) continue to work unchanged. Covered log sites: - AclAuthorizable.apply - four lines (Applied, Skipped composite node store, Skipped path not found, Skipped already set) - Acl.apply - "authorizable not found" variant - AclAuthorizable.clear - four lines (Cleared, Skipped composite node store, Skipped path not found, Skipped no permissions to clear) - Acl.clear - "authorizable not found" variant - AclChecker.check - the single "Checked permissions" line; its template is reshaped from "...with allow 'true' [...]" to "Checked allow permissions..." so that the glob suffix does not stack a second "with" on top of the allow one, matching how the write-path Applied line is written The glob suffix is produced by a new determineGlobSuffix method on both authorizable.PermissionsOptions and check.PermissionsOptions - the two options classes stay symmetric, the STRICT display string is reconstructed from the internally empty GLOB_STRICT_EFFECTIVE marker in one place, and the three call sites just concatenate the already-formed suffix instead of re-wrapping the raw value. The strict suffix is a local " in strict mode" phrase in AclAuthorizable.clear and Acl.clear driven by ClearOptions.isStrict() / the clear(path, strict) boolean. The regenerated api.md / api.json script catalog therefore lists determineGlobSuffix next to the existing determineAll* methods on both options classes. Closes #307 --- .../java/dev/vml/es/acm/core/acl/Acl.java | 11 +++-- .../dev/vml/es/acm/core/acl/AclChecker.java | 6 ++- .../acl/authorizable/AclAuthorizable.java | 44 ++++++++++++------- .../acl/authorizable/PermissionsOptions.java | 8 ++++ .../core/acl/check/PermissionsOptions.java | 8 ++++ tools/shared/src/catalog/api.json | 10 +++++ .../acm-groovy-script/references/api.md | 2 + 7 files changed, 66 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java b/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java index c3f8563f9..38a390e70 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java @@ -477,11 +477,13 @@ public void clear(ClearOptions options) { context.determineAuthorizable(options.getAuthorizable(), options.getAuthorizableId()); if (authorizable == null) { String authorizableId = context.determineId(options.getAuthorizable(), options.getAuthorizableId()); + String strictSuffix = options.isStrict() ? " in strict mode" : ""; context.getLogger() .info( - "Skipped clearing permissions for authorizable '{}' at path '{}' (authorizable not found)", + "Skipped clearing permissions for authorizable '{}' at path '{}'{} (authorizable not found)", authorizableId, - options.getPath()); + options.getPath(), + strictSuffix); return; } authorizable.clear(options); @@ -548,10 +550,11 @@ private void apply(PermissionsOptions options, boolean allow) { String actionDescription = allow ? "allow permissions" : "deny permissions"; context.getLogger() .info( - "Skipped setting {} for authorizable '{}' at path '{}' (authorizable not found)", + "Skipped setting {} for authorizable '{}' at path '{}'{} (authorizable not found)", actionDescription, authorizableId, - options.getPath()); + options.getPath(), + options.determineGlobSuffix()); return; } if (allow) { diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/AclChecker.java b/core/src/main/java/dev/vml/es/acm/core/acl/AclChecker.java index 99f0952bf..754c232f8 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/AclChecker.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/AclChecker.java @@ -242,12 +242,14 @@ private boolean check( options.determineAllPermissions(), options.determineAllRestrictions(), allow); + String actionDescription = allow ? "allow permissions" : "deny permissions"; context.getLogger() .info( - "Checked permissions for authorizable '{}' at path '{}' with allow '{}' [{}]", + "Checked {} for authorizable '{}' at path '{}'{} [{}]", + actionDescription, id, path, - allow, + options.determineGlobSuffix(), result); return result; } diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/AclAuthorizable.java b/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/AclAuthorizable.java index bac917530..fe4dfd23c 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/AclAuthorizable.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/AclAuthorizable.java @@ -146,28 +146,36 @@ public void clear(ClearOptions options) { } public void clear(String path, boolean strict) { + String strictSuffix = strict ? " in strict mode" : ""; if (context.isCompositeNodeStore() && isAppsOrLibsPath(path)) { context.getLogger() .info( - "Skipped clearing permissions for authorizable '{}' at path '{}' (composite node store)", + "Skipped clearing permissions for authorizable '{}' at path '{}'{} (composite node store)", id, - path); + path, + strictSuffix); return; } if (context.getResourceResolver().getResource(path) == null) { context.getLogger() - .info("Skipped clearing permissions for authorizable '{}' at path '{}' (path not found)", id, path); + .info( + "Skipped clearing permissions for authorizable '{}' at path '{}'{} (path not found)", + id, + path, + strictSuffix); return; } boolean changed = context.getPermissionsManager().clear(authorizable, path, strict); if (changed) { - context.getLogger().info("Cleared permissions for authorizable '{}' at path '{}'", id, path); + context.getLogger() + .info("Cleared permissions for authorizable '{}' at path '{}'{}", id, path, strictSuffix); } else { context.getLogger() .info( - "Skipped clearing permissions for authorizable '{}' at path '{}' (no permissions to clear)", + "Skipped clearing permissions for authorizable '{}' at path '{}'{} (no permissions to clear)", id, - path); + path, + strictSuffix); } } @@ -184,15 +192,17 @@ private void apply(PermissionsOptions options, boolean allow) { List permissions = options.determineAllPermissions(); Map restrictions = options.determineAllRestrictions(); PermissionsOptions.Mode mode = options.getMode(); + String actionDescription = allow ? "allow permissions" : "deny permissions"; + String globSuffix = options.determineGlobSuffix(); if (context.isCompositeNodeStore() && isAppsOrLibsPath(path)) { - String actionDescription = allow ? "allow permissions" : "deny permissions"; context.getLogger() .info( - "Skipped setting {} for authorizable '{}' at path '{}' (composite node store)", + "Skipped setting {} for authorizable '{}' at path '{}'{} (composite node store)", actionDescription, id, - path); + path, + globSuffix); return; } @@ -201,28 +211,28 @@ private void apply(PermissionsOptions options, boolean allow) { throw new AclException(String.format( "Cannot apply permissions for authorizable '%s' at path '%s'! (path not found)", id, path)); } - String actionDescription = allow ? "allow permissions" : "deny permissions"; context.getLogger() .info( - "Skipped setting {} for authorizable '{}' at path '{}' (path not found)", + "Skipped setting {} for authorizable '{}' at path '{}'{} (path not found)", actionDescription, id, - path); + path, + globSuffix); return; } if (context.getPermissionsManager().check(authorizable, path, permissions, restrictions, allow)) { - String actionDescription = allow ? "allow permissions" : "deny permissions"; context.getLogger() .info( - "Skipped setting {} for authorizable '{}' at path '{}' (already set)", + "Skipped setting {} for authorizable '{}' at path '{}'{} (already set)", actionDescription, id, - path); + path, + globSuffix); } else { context.getPermissionsManager().apply(authorizable, path, permissions, restrictions, allow); - String actionDescription = allow ? "allow permissions" : "deny permissions"; - context.getLogger().info("Applied {} for authorizable '{}' at path '{}'", actionDescription, id, path); + context.getLogger() + .info("Applied {} for authorizable '{}' at path '{}'{}", actionDescription, id, path, globSuffix); } } diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/PermissionsOptions.java b/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/PermissionsOptions.java index 15a1457e8..637de6fa5 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/PermissionsOptions.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/authorizable/PermissionsOptions.java @@ -117,6 +117,14 @@ public Map determineAllRestrictions() { return allRestrictions; } + public String determineGlobSuffix() { + if (glob == null) { + return ""; + } + String description = GLOB_STRICT_EFFECTIVE.equals(glob) ? "STRICT" : glob; + return String.format(" with glob '%s'", description); + } + public enum Mode { FAIL, SKIP diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/check/PermissionsOptions.java b/core/src/main/java/dev/vml/es/acm/core/acl/check/PermissionsOptions.java index 6621242ac..af63da668 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/check/PermissionsOptions.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/check/PermissionsOptions.java @@ -106,4 +106,12 @@ public Map determineAllRestrictions() { } return allRestrictions; } + + public String determineGlobSuffix() { + if (glob == null) { + return ""; + } + String description = GLOB_STRICT_EFFECTIVE.equals(glob) ? "STRICT" : glob; + return String.format(" with glob '%s'", description); + } } diff --git a/tools/shared/src/catalog/api.json b/tools/shared/src/catalog/api.json index 8251b4859..e90729ce4 100644 --- a/tools/shared/src/catalog/api.json +++ b/tools/shared/src/catalog/api.json @@ -3041,6 +3041,11 @@ "name": "determineAllRestrictions", "returns": "Map", "params": [] + }, + { + "name": "determineGlobSuffix", + "returns": "String", + "params": [] } ] }, @@ -3434,6 +3439,11 @@ "returns": "Map", "params": [] }, + { + "name": "determineGlobSuffix", + "returns": "String", + "params": [] + }, { "name": "getId", "returns": "String", diff --git a/tools/skills/acm-groovy-script/references/api.md b/tools/skills/acm-groovy-script/references/api.md index 428053105..a8ddb31f6 100644 --- a/tools/skills/acm-groovy-script/references/api.md +++ b/tools/skills/acm-groovy-script/references/api.md @@ -419,6 +419,7 @@ void skipIfPathMissing() void failIfPathMissing() List determineAllPermissions() Map determineAllRestrictions() +String determineGlobSuffix() ``` ### RemovePropertyOptions @@ -521,6 +522,7 @@ void restriction(String name, String value) void restriction(String name, List values) List determineAllPermissions() Map determineAllRestrictions() +String determineGlobSuffix() String getId() void setId(String id) ```