CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to this project are documented here. Format: [Keep a Changel
 
 ## [Unreleased]
 
+- Fix: a degraded `authentik.mapper` health check now says what to fix. The message and details (`flaggedGroups`, `summary`) name each flagged group with a safe reason: `grants nothing: <permission>`, `malformed <attribute>` or `superuser group carries qqq.io/permissions`. It used to say only "Mapper drift detected". Typed attribute values and service-account activity subjects (people) stay out of the outcome. A group with no `qqq.io/*` attributes is not drift and never was.
 - Fix: the `token_broker` health check probes with the grant the broker uses (token exchange) and sends no subject token, so nothing is issued. `invalid_request` (credentials passed) is UP, `invalid_client` is DOWN (bad secret), and `invalid_grant` is DOWN (provider does not allow token exchange). It used to probe `client_credentials`, which the broker provider does not allow, so Authentik logged a 400 every 5 minutes ("Invalid grant_type for provider").
 - Replace expired token-delivery and credential-scope reflection bridges with the RC.7 APIs; retain conservative permission narrowing and explicit unavailable-delivery tests.
 - Ship the expanded, host-neutral QBIT help bundle using QQQ 4.1.0-RC.7 registration and validation, with coverage for optional features and live references. Preserve inline app/table help for hosts without help storage; document HOST bundle overrides.

src/main/java/com/kingsrook/qbits/authentik/drift/AuthentikDriftReport.java

@@ -22,6 +22,7 @@ import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.regex.Pattern;
 import com.kingsrook.qbits.authentik.AuthentikQBitConfig;
 import com.kingsrook.qbits.authentik.client.AuthentikActionUtil;
 import com.kingsrook.qbits.authentik.client.AuthentikJson;
@@ -61,6 +62,9 @@ public class AuthentikDriftReport
 {
    private static final QLogger LOG = QLogger.getLogger(AuthentikDriftReport.class);
 
+   private static final Pattern SAFE_PERMISSION = Pattern.compile("[A-Za-z0-9_.*-]{1,120}");
+   private static final Pattern SAFE_ATTRIBUTE  = Pattern.compile("[A-Za-z0-9_./-]{1,60}");
+
    private final AuthentikActionUtil client;
    private final AuthentikQBitConfig config;
    private final AuthentikGuard      guard;
@@ -373,6 +377,19 @@ public class AuthentikDriftReport
 
 
 
+   /*******************************************************************************
+    ** The attribute name of a GroupGraph malformed entry ("key: detail"), never
+    ** the detail, which can echo whatever a person typed.
+    *******************************************************************************/
+   static String attributeOf(String malformedEntry)
+   {
+      int    colon = malformedEntry == null ? -1 : malformedEntry.indexOf(": ");
+      String key   = colon < 0 ? "" : malformedEntry.substring(0, colon);
+      return (SAFE_ATTRIBUTE.matcher(key).matches() ? key : "attribute");
+   }
+
+
+
    /*******************************************************************************
     ** 6
     *******************************************************************************/
@@ -388,14 +405,16 @@ public class AuthentikDriftReport
          for(String bad : node.malformed())
          {
             issues++;
-            findings.add(DriftFinding.warning("group attributes", node.name(), "Malformed entry ignored (" + bad + "); qqq.io/permissions and qqq.io/app-access must be lists of strings."));
+            findings.add(DriftFinding.warning("group attributes", node.name(), "Malformed entry ignored (" + bad + "); qqq.io/permissions and qqq.io/app-access must be lists of strings.")
+               .withReason("malformed " + attributeOf(bad)));
          }
          for(String permission : node.permissions())
          {
             if(resolver.expand(permission).isEmpty())
             {
                issues++;
-               findings.add(DriftFinding.warning("group attributes", node.name(), "qqq.io/permissions entry \"" + permission + "\" grants nothing in this app (unknown permission or empty wildcard)."));
+               findings.add(DriftFinding.warning("group attributes", node.name(), "qqq.io/permissions entry \"" + permission + "\" grants nothing in this app (unknown permission or empty wildcard; a name the app checks only in code, not in its metadata, is not in the catalog).")
+                  .withReason("grants nothing: " + (SAFE_PERMISSION.matcher(permission).matches() ? permission : "(unprintable name)")));
             }
          }
          for(String app : node.appAccess())
@@ -409,7 +428,8 @@ public class AuthentikDriftReport
          if(graph.isSuperuserInherited(node.pk()) && !node.permissions().isEmpty())
          {
             issues++;
-            findings.add(DriftFinding.warning("group attributes", node.name(), "Superuser group (or under one) carries qqq.io/permissions. QQQ cannot manage it; keep QQQ grants on ordinary groups."));
+            findings.add(DriftFinding.warning("group attributes", node.name(), "Superuser group (or under one) carries qqq.io/permissions. QQQ cannot manage it; keep QQQ grants on ordinary groups.")
+               .withReason("superuser group carries qqq.io/permissions"));
          }
       }
       if(issues == 0)

src/main/java/com/kingsrook/qbits/authentik/drift/DriftFinding.java

@@ -25,10 +25,33 @@ package com.kingsrook.qbits.authentik.drift;
  ** @param check    the check's short name, e.g. "blueprints"
  ** @param subject  what the finding is about, e.g. a group name
  ** @param message  plain-language explanation and what to do
+ ** @param reason   a short, safe summary for health outcomes (no attribute
+ **                 values a person typed beyond validated permission names);
+ **                 null when the finding has none
  *******************************************************************************/
-public record DriftFinding(Severity severity, String check, String subject, String message)
+public record DriftFinding(Severity severity, String check, String subject, String message, String reason)
 {
 
+   /*******************************************************************************
+    ** A finding without a health reason.
+    *******************************************************************************/
+   public DriftFinding(Severity severity, String check, String subject, String message)
+   {
+      this(severity, check, subject, message, null);
+   }
+
+
+
+   /*******************************************************************************
+    ** This finding with a short, safe reason for health outcomes.
+    *******************************************************************************/
+   public DriftFinding withReason(String reason)
+   {
+      return (new DriftFinding(severity, check, subject, message, reason));
+   }
+
+
+
    /*******************************************************************************
     ** Severity, most severe first.
     *******************************************************************************/

src/main/java/com/kingsrook/qbits/authentik/health/AuthentikHealthChecks.java

@@ -19,7 +19,10 @@ package com.kingsrook.qbits.authentik.health;
 
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
+import java.util.TreeMap;
 import java.util.TreeSet;
 import com.kingsrook.qbits.authentik.AuthentikQBitConfig;
 import com.kingsrook.qbits.authentik.client.AuthentikActionUtil;
@@ -45,6 +48,8 @@ import org.json.JSONObject;
  *******************************************************************************/
 public final class AuthentikHealthChecks
 {
+   private static final String GROUP_ATTRIBUTES = "group attributes";
+
    /*******************************************************************************
     ** Utility class.
     *******************************************************************************/
@@ -151,30 +156,91 @@ public final class AuthentikHealthChecks
 
 
    /*******************************************************************************
-    ** Findings carry fixed codes only, never subjects or diagnostic messages.
+    ** Findings carry fixed codes, plus the flagged group names and each group
+    ** finding's safe reason (DriftFinding.reason), so a degraded mapper says
+    ** what to fix. Service-account activity subjects (people) stay private.
     *******************************************************************************/
-   private static HealthCheckOutcome mapper(AuthentikActionUtil util, AuthentikQBitConfig config)
+   static HealthCheckOutcome mapper(AuthentikActionUtil util, AuthentikQBitConfig config)
    {
-      HealthStatus status = HealthStatus.UP;
-      TreeSet<String> codes = new TreeSet<>();
-      for(DriftFinding finding : new AuthentikDriftReport(util, config).runMapperChecks())
+      return (mapperOutcome(new AuthentikDriftReport(util, config).runMapperChecks()));
+   }
+
+
+
+   /*******************************************************************************
+    ** The mapper outcome for a set of drift findings.
+    *******************************************************************************/
+   static HealthCheckOutcome mapperOutcome(List<DriftFinding> findings)
+   {
+      HealthStatus                                      status  = HealthStatus.UP;
+      TreeSet<String>                                   codes   = new TreeSet<>();
+      TreeMap<String, TreeMap<String, TreeSet<String>>> byGroup = new TreeMap<>();
+      TreeSet<String>                                   other   = new TreeSet<>();
+      for(DriftFinding finding : findings)
       {
-         if(finding.severity() == DriftFinding.Severity.ERROR || finding.severity() == DriftFinding.Severity.WARNING)
+         if(finding.severity() != DriftFinding.Severity.ERROR && finding.severity() != DriftFinding.Severity.WARNING)
          {
-            String code = "group attributes".equals(finding.check()) ? "group_attributes" : "service_account_activity";
-            codes.add(code + "." + finding.severity().name().toLowerCase(java.util.Locale.ROOT));
-            if(finding.severity() == DriftFinding.Severity.ERROR)
+            continue;
+         }
+         boolean groupCheck = GROUP_ATTRIBUTES.equals(finding.check());
+         codes.add((groupCheck ? "group_attributes" : "service_account_activity") + "." + finding.severity().name().toLowerCase(Locale.ROOT));
+         status = finding.severity() == DriftFinding.Severity.ERROR || status == HealthStatus.DOWN ? HealthStatus.DOWN : HealthStatus.DEGRADED;
+         if(groupCheck && finding.reason() != null)
+         {
+            String reason = finding.reason();
+            int    colon  = reason.indexOf(": ");
+            TreeSet<String> values = byGroup.computeIfAbsent(safeName(finding.subject()), k -> new TreeMap<>())
+               .computeIfAbsent(colon < 0 ? reason : reason.substring(0, colon), k -> new TreeSet<>());
+            if(colon >= 0)
             {
-               status = HealthStatus.DOWN;
-            }
-            else if(status != HealthStatus.DOWN)
-            {
-               status = HealthStatus.DEGRADED;
+               values.add(reason.substring(colon + 2));
             }
          }
+         else
+         {
+            other.add(groupCheck ? "group attributes check could not run" : "service-account activity (see the identity drift report)");
+         }
       }
-      return (new HealthCheckOutcome().withStatus(status).withMessage(status == HealthStatus.UP ? "Mapper checks passed" : "Mapper drift detected")
-         .withDetails(new HealthDetails().withValue("findingCodes", String.join(",", codes))));
+      if(status == HealthStatus.UP)
+      {
+         return (HealthCheckOutcome.up("Mapper checks passed").withDetails(new HealthDetails().withValue("findingCodes", "")));
+      }
+
+      List<String> parts = new ArrayList<>();
+      byGroup.forEach((group, reasons) ->
+      {
+         List<String> texts = new ArrayList<>();
+         reasons.forEach((label, values) -> texts.add(values.isEmpty() ? label : label + ": " + String.join(", ", values)));
+         parts.add(group + " (" + String.join("; ", texts) + ")");
+      });
+      parts.addAll(other);
+      String summary = String.join("; ", parts);
+      return (new HealthCheckOutcome().withStatus(status).withMessage(truncate("Mapper drift: " + summary, 500))
+         .withDetails(new HealthDetails()
+            .withValue("findingCodes", String.join(",", codes))
+            .withValue("flaggedGroups", truncate(String.join(", ", byGroup.keySet()), 500))
+            .withValue("summary", truncate(summary, 500))));
+   }
+
+
+
+   /*******************************************************************************
+    ** A group name fit for a health message: printable and bounded.
+    *******************************************************************************/
+   private static String safeName(String name)
+   {
+      String printable = name == null ? "" : name.replaceAll("\\p{Cntrl}", "").trim();
+      return (printable.isEmpty() ? "(unnamed group)" : truncate(printable, 100));
+   }
+
+
+
+   /*******************************************************************************
+    ** Cut a text to a length, marking the cut.
+    *******************************************************************************/
+   private static String truncate(String text, int max)
+   {
+      return (text.length() <= max ? text : text.substring(0, max - 3) + "...");
    }
 
 

src/test/java/com/kingsrook/qbits/authentik/drift/AuthentikDriftReportTest.java

@@ -186,4 +186,20 @@ class AuthentikDriftReportTest
       assertThatThrownBy(() -> new AuthentikDriftReportWidgetRenderer().render(input)).isInstanceOf(QPermissionDeniedException.class);
    }
 
+
+
+
+   /*******************************************************************************
+    ** A malformed entry's health reason keeps the attribute name, never the
+    ** value a person typed.
+    *******************************************************************************/
+   @Test
+   void malformedReasonKeepsOnlyTheAttribute()
+   {
+      assertThat(AuthentikDriftReport.attributeOf("qqq.io/permissions: {\"x\":\"typed value\"}")).isEqualTo("qqq.io/permissions");
+      assertThat(AuthentikDriftReport.attributeOf("qqq.io/policy: line 1: bad")).isEqualTo("qqq.io/policy");
+      assertThat(AuthentikDriftReport.attributeOf("no separator")).isEqualTo("attribute");
+      assertThat(AuthentikDriftReport.attributeOf("bad key with spaces: x")).isEqualTo("attribute");
+      assertThat(AuthentikDriftReport.attributeOf(null)).isEqualTo("attribute");
+   }
 }

src/test/java/com/kingsrook/qbits/authentik/health/AuthentikHealthChecksTest.java

@@ -26,6 +26,7 @@ import com.kingsrook.qbits.authentik.AuthentikTestSupport;
 import com.kingsrook.qbits.authentik.client.AuthentikActionUtil;
 import com.kingsrook.qbits.authentik.client.FixtureActionUtil;
 import com.kingsrook.qbits.authentik.client.TokenBucketThrottle;
+import com.kingsrook.qbits.authentik.drift.DriftFinding;
 import com.kingsrook.qqq.backend.core.context.QContext;
 import com.kingsrook.qqq.backend.core.exceptions.QException;
 import com.kingsrook.qqq.backend.core.health.HealthCategory;
@@ -176,22 +177,49 @@ class AuthentikHealthChecksTest
 
 
    /*******************************************************************************
-    ** Only the two relevant drift checks run; their subjects stay private.
+    ** Only the two relevant drift checks run. Group findings name the group and
+    ** a safe reason; typed attribute values and people (service-account
+    ** activity subjects) stay private.
     *******************************************************************************/
    @Test
    void mapperStates() throws QException
    {
       assertThat(run("mapper").getStatus()).isEqualTo(HealthStatus.UP);
-      JSONObject group = new JSONObject().put("pk", "g1").put("name", SECRET)
-         .put("attributes", new JSONObject().put("qqq.io/permissions", new JSONArray().put(SECRET)));
-      FixtureActionUtil.groupGraphBody = new JSONObject(FixtureActionUtil.EMPTY_PAGE).put("results", new JSONArray().put(group)).toString();
+
+      //////////////////////////////////////////////////////////////////////////////
+      // a group with no qqq.io/* attributes (kof22-admins on its static floor) is //
+      // not drift                                                                 //
+      //////////////////////////////////////////////////////////////////////////////
+      JSONObject floorOnly = new JSONObject().put("pk", "g0").put("name", "kof22-admins").put("attributes", new JSONObject());
+      groups(floorOnly);
       var outcome = run("mapper");
+      assertThat(outcome.getStatus()).isEqualTo(HealthStatus.UP);
+      assertThat(outcome.getMessage()).isEqualTo("Mapper checks passed");
+
+      JSONObject unknown = new JSONObject().put("pk", "g1").put("name", "file-gateway-admins")
+         .put("attributes", new JSONObject().put("qqq.io/permissions", new JSONArray().put("file_gateway.view").put("file_gateway.admin")));
+      JSONObject malformed = new JSONObject().put("pk", "g2").put("name", "editors")
+         .put("attributes", new JSONObject().put("qqq.io/permissions", new JSONArray().put(new JSONObject().put("x", SECRET))));
+      JSONObject superuser = new JSONObject().put("pk", "g3").put("name", "authentik Admins").put("is_superuser", true)
+         .put("attributes", new JSONObject().put("qqq.io/permissions", new JSONArray().put("authentik.users.read")));
+      groups(floorOnly, unknown, malformed, superuser);
+      outcome = run("mapper");
       assertThat(outcome.getStatus()).isEqualTo(HealthStatus.DEGRADED);
-      assertThat(outcome.getDetails().getValues()).containsEntry("findingCodes", "group_attributes.warning");
+      assertThat(outcome.getMessage()).isEqualTo("Mapper drift: authentik Admins (superuser group carries qqq.io/permissions); "
+         + "editors (malformed qqq.io/permissions); file-gateway-admins (grants nothing: file_gateway.admin, file_gateway.view)");
+      assertThat(outcome.getDetails().getValues())
+         .containsEntry("findingCodes", "group_attributes.warning")
+         .containsEntry("flaggedGroups", "authentik Admins, editors, file-gateway-admins")
+         .containsEntry("summary", outcome.getMessage().substring("Mapper drift: ".length()));
+      assertClean(outcome);
+
+      groups(unknown);
       page(new JSONObject().put("action", "model_deleted").put("context", new JSONObject()
          .put("model", new JSONObject().put("model_name", "user").put("pk", "1").put("name", SECRET))));
       outcome = run("mapper");
       assertThat(outcome.getStatus()).isEqualTo(HealthStatus.DOWN);
+      assertThat(outcome.getMessage()).isEqualTo("Mapper drift: file-gateway-admins (grants nothing: file_gateway.admin, file_gateway.view); "
+         + "service-account activity (see the identity drift report)");
       assertClean(outcome);
       assertThat(FixtureActionUtil.REQUESTS).allSatisfy(request -> assertThat(request.getURI().getPath())
          .doesNotContain("/admin/version/", "/core/tokens/", "/managed/blueprints/"));
@@ -199,6 +227,52 @@ class AuthentikHealthChecksTest
 
 
 
+   /*******************************************************************************
+    ** A group check that cannot run, unprintable names and long summaries stay
+    ** readable and within the health limits.
+    *******************************************************************************/
+   @Test
+   void mapperOutcomeEdges()
+   {
+      var outcome = AuthentikHealthChecks.mapperOutcome(List.of(
+         DriftFinding.error("group attributes", "-", "Check could not run: " + SECRET),
+         DriftFinding.info("group attributes", "x", "info only").withReason("ignored"),
+         DriftFinding.warning("group attributes", "bad\u0007name", "m").withReason("grants nothing: (unprintable name)"),
+         DriftFinding.warning("group attributes", "", "m").withReason("malformed attribute")));
+      assertThat(outcome.getStatus()).isEqualTo(HealthStatus.DOWN);
+      assertThat(outcome.getMessage()).isEqualTo("Mapper drift: (unnamed group) (malformed attribute); badname (grants nothing: (unprintable name)); "
+         + "group attributes check could not run");
+      assertThat(outcome.getDetails().getValues()).containsEntry("findingCodes", "group_attributes.error,group_attributes.warning");
+      assertClean(outcome);
+
+      List<DriftFinding> many = new java.util.ArrayList<>();
+      for(int i = 0; i < 60; i++)
+      {
+         many.add(DriftFinding.warning("group attributes", "group-number-" + i, "m").withReason("grants nothing: some.long.permission.name." + i));
+      }
+      outcome = AuthentikHealthChecks.mapperOutcome(many);
+      assertThat(outcome.getMessage()).hasSize(500).endsWith("...");
+      assertThat(outcome.getDetails().toJson().getBytes(java.nio.charset.StandardCharsets.UTF_8).length).isLessThanOrEqualTo(4096);
+      assertThat(outcome.getDetails().getValues()).containsKeys("findingCodes", "flaggedGroups", "summary");
+   }
+
+
+
+   /*******************************************************************************
+    ** Serve these groups as the group graph.
+    *******************************************************************************/
+   private static void groups(JSONObject... groups)
+   {
+      JSONArray results = new JSONArray();
+      for(JSONObject group : groups)
+      {
+         results.put(group);
+      }
+      FixtureActionUtil.groupGraphBody = new JSONObject(FixtureActionUtil.EMPTY_PAGE).put("results", results).toString();
+   }
+
+
+
    /*******************************************************************************
     ** The broker check uses the grant the broker uses (token exchange) with no
     ** subject: invalid_request proves the credentials; nothing is ever issued.