diff --git a/CHANGELOG.md b/CHANGELOG.md index 40dec57..bcbb1d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to this project are documented here. Format: [Keep a Changel ## [Unreleased] +- Fix: native **Create Group** now obeys the two-person gate. With `twoPersonApproval=true`, creating a group whose permissions expand to any `authentik.*` permission queues an `authentik_approval_request` for a second person instead of persisting the grant immediately; the same identity-permission expansion and second-person condition as Edit Group apply, nothing is written before approval, self-approval is refused and a decided request cannot be approved again. Previously a new group with `authentik.users.read` was created without approval. - **Breaking (security):** `AuthentikMaintenance.applyPolicy` runs as the host's maintenance service principal (QQQ service identities, qqq `docs/SERVICE-IDENTITY.md`; config `maintenancePrincipal`, default `maintenance`). The self-made `MaintenanceSession`, which granted itself `policies.manage` and the document's grants, is gone; under OAuth2 it was rejected anyway (its audit insert failed). The principal's Authentik service account must hold `policies.manage` and every grant it applies, and the subset rule still holds. The audit operator is `service:{principal}`; the job name (`run-once:`/`maintenance:`) prefixes the reason. Hosts declare the principal: `ServiceIdentitiesMetaData.declare(qInstance, new QServicePrincipalMetaData().withName("maintenance"))`. Needs the QQQ release with service identities (qqq#1255). - 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: `, `malformed ` 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"). diff --git a/src/main/java/com/kingsrook/qbits/authentik/admin/AuthentikAdmin.java b/src/main/java/com/kingsrook/qbits/authentik/admin/AuthentikAdmin.java index fe8c7d0..abed392 100644 --- a/src/main/java/com/kingsrook/qbits/authentik/admin/AuthentikAdmin.java +++ b/src/main/java/com/kingsrook/qbits/authentik/admin/AuthentikAdmin.java @@ -1407,6 +1407,10 @@ public class AuthentikAdmin } plan.withChange(null, name + (parentPks.isEmpty() ? "" : ", under " + request.str("parents")) + (permissions.isEmpty() ? "" : ", permissions " + String.join(", ", permissions))); + if(config.getTwoPersonApproval() && !request.bool("approved") && addsIdentityPermission(permissions, List.of())) + { + plan.withState(AdminPlan.State.NEEDS_APPROVAL).withMessage("granting authentik.* needs a second person"); + } return (plan); } diff --git a/src/test/java/com/kingsrook/qbits/authentik/admin/AuthentikAdminTest.java b/src/test/java/com/kingsrook/qbits/authentik/admin/AuthentikAdminTest.java index a086965..31bdeb6 100644 --- a/src/test/java/com/kingsrook/qbits/authentik/admin/AuthentikAdminTest.java +++ b/src/test/java/com/kingsrook/qbits/authentik/admin/AuthentikAdminTest.java @@ -541,6 +541,34 @@ class AuthentikAdminTest + /******************************************************************************* + ** Creation-time identity grants obey the same second-person gate as edits. + *******************************************************************************/ + @Test + void creatingGroupWithIdentityPermissionsNeedsASecondPerson() throws QException + { + AdminRequest request = AdminRequest.of(AdminAction.CREATE_GROUP, "", "synthetic approval check") + .with("name", "synthetic-identity-readers").with("permissions", "authentik.users.read"); + AuthentikAdmin.AdminResult queued = run(request); + assertThat(queued.ok()).as(queued.message()).isTrue(); + assertThat(queued.pending()).isTrue(); + assertThat(t.writes()).isEmpty(); + List approvals = rows(AuthentikLocalTables.APPROVAL_REQUEST); + assertThat(approvals).hasSize(1); + java.io.Serializable id = approvals.get(0).getValue("id"); + assertThat(t.admin().approve(id).ok()).isFalse(); + assertThat(t.writes()).isEmpty(); + t.loginAs("second@example.test", IdentityPermission.APPROVE, IdentityPermission.GROUPS_MANAGE); + AuthentikAdmin.AdminResult approved = t.admin().approve(id); + assertThat(approved.ok()).as(approved.message()).isTrue(); + assertThat(t.authentik.groupByName("synthetic-identity-readers").getJSONObject("attributes") + .getJSONArray("qqq.io/permissions").toList()).containsExactly("authentik.users.read"); + assertThat(rows(AuthentikLocalTables.APPROVAL_REQUEST).get(0).getValueString("status")).isEqualTo("APPROVED"); + assertThat(t.admin().approve(id).ok()).isFalse(); + } + + + /******************************************************************************* ** Re-parenting: loops and superuser parents are refused. *******************************************************************************/