feat(web): admin roster of accounts with expiring passwords - #626
Merged
Conversation
Closes netresearch/ldap-selfservice-password-changer#628 (the display half). Adds /password-expiry: an admin-only page listing accounts whose LDAP password is expiring, resolved live through simple-ldap-go v1.13.0's PasswordExpiryFor / UsersWithExpiringPasswords. No scheduler, no mail — this is the read-only view; whether reminders are mailed from here or from a separate cron tool is deferred. Data path. Expiry is resolved live from the directory, not the background cache: the cache never holds the domain max-age, so it cannot compute expiry, and a security roster wants current truth rather than a 30-second-stale snapshot. The default view calls UsersWithExpiringPasswords(ctx, days), which already returns exactly the due set. The "show all" toggle instead enumerates every account and resolves each status so never-expires and unknown rows can be shown muted. The four states (expires / must-change / never-expires / unknown) map to existing badge tokens, so dark mode themes automatically. Authorization. The route is gated by a new RequireAdmin middleware. An admin is a member of the configured LDAP_ADMIN_GROUP or carries AD's adminCount=1. The group exists because OpenLDAP has no adminCount: without a way to name admins there, the gate would lock everyone out — the group is that way. Membership is read from memberOf (AD populates it; OpenLDAP needs the memberof overlay). A non-admin gets 403; a discoverable link appears on the home page only for admins, so no one is shown a link that would 403. Threshold is a ?days= query param (default 30, capped at 366) — no new env var for a display filter. Tests cover the row collection (due-only, show-all, disabled-skip, error propagation) via an interface seam, the four-state row mapping, window parsing and clamping, sorting, the admin rule (adminCount, group membership, case-insensitive, the OpenLDAP no-group lockout), the 403 gate through the real Fiber app, and the badge class/label mapping. Verified visually by rendering the real template with the real stylesheet. The generated *_templ.go files are gitignored and regenerated by CI, per the repo convention; run `make templates` locally to build. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Contributor
Dependency ReviewThe following issues were found:
License Issuesgo.mod
OpenSSF Scorecard
Scanned Files
|
github-actions
Bot
added
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
Contributor
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #626 +/- ##
==========================================
+ Coverage 69.94% 70.92% +0.97%
==========================================
Files 34 36 +2
Lines 3437 3570 +133
==========================================
+ Hits 2404 2532 +128
- Misses 873 876 +3
- Partials 160 162 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Addresses the confirmed findings of the adversarial review of #626. Testability seams. RequireAdmin now resolves its admin check per request, and the App carries two nil-defaulted overrides — adminCheck and expiryResolver — so tests can drive the handler behind the gate. Without them the handler had 0% coverage: the only route test stopped at the 403, because a DN-addressable admin cache entry cannot be built outside the LDAP package. New handler tests cover the 503 no-service-account guard, the admit path, the days/show/sort query-param wiring, and the resolver-error branch. handlePasswordExpiryV2 goes from 0% to 94.7%, RequireAdmin to 100%. Untested guards. Added the empty-admin-group case (a directory returning an empty group entry must not match an unset admin group) and the status-sort column, both previously surviving mutants. Undated rows now sort to the bottom in BOTH directions. The old sentinel kept must-change/never/unknown last only under ascending; under descending they floated to the top above the furthest concrete deadline. sortByDeadline pins them at the bottom regardless of direction, and the dead expiryOrder sentinel is gone. Accessibility. Sort headers now carry aria-sort (ascending/descending/none) so the active column and direction reach assistive technology rather than living only in an aria-hidden arrow — a fix in the shared tableSortHeader helper, so every V2 table gains it. The deadline cell for undated rows pairs the visual em-dash with an sr-only "No expiry date" instead of announcing a lone dash. Documented that AD's adminCount is sticky, so admin access via that marker outlives de-privilege; prefer LDAP_ADMIN_GROUP where access should track current privilege. Not changed: the client-side filter does not announce "no results" via a live region — a pre-existing gap in the shared search JS affecting every V2 list table, out of scope here. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
Contributor
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
This was referenced Jul 24, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the display half of netresearch/ldap-selfservice-password-changer#628.
Adds
/password-expiry: an admin-only page listing accounts whose LDAP password is expiring, resolved live through simple-ldap-go v1.13.0'sPasswordExpiryFor/UsersWithExpiringPasswords. No scheduler, no mail — this is the read-only view; whether reminders are mailed from here or a separate cron tool is deferred.Design decisions (confirmed with the maintainer)
RequireAdminmiddleware. An admin is a member ofLDAP_ADMIN_GROUPor carries AD'sadminCount=1. The group exists because OpenLDAP has noadminCount— without a way to name admins there, the gate would lock everyone out. Non-admins get 403; the home-page link appears only for admins, so nobody is shown a link that 403s.?days=(default 30, capped 366) — no new env var for a display filter.Data path
Live, not cached: the background cache never holds the domain max-age, so it cannot compute expiry, and a security roster wants current truth over a 30-second-stale snapshot. The default view calls
UsersWithExpiringPasswords(ctx, days)(exactly the due set); the show-all path enumerates and resolves each status. The four states map to existing badge tokens, so dark mode themes automatically (verified:drawer__badge--warn/dangerandlist-row__badgeall carry:root[data-theme="dark"]rules).A real-instance finding
The admin group gate reads membership from
memberOf. Active Directory populates it automatically; OpenLDAP needs thememberofoverlay — the compose stack's overlay does not populate it retroactively, so a deployment must have it configured. Documented in the README.Tests
Row collection (due-only, show-all, disabled-skip, error propagation) via an interface seam; four-state row mapping; window parse/clamp; sort (deadline/name/status, undated-last); the admin rule (adminCount, group membership, case-insensitive, the OpenLDAP no-group lockout); the 403 gate through the real Fiber app; badge class/label mapping.
golangci-lint0 issues,govulncheckclean,gofumptclean, fullgo test ./...green.Notes for the reviewer
*_templ.goare gitignored and regenerated by CI (repo convention); runmake templateslocally.make check's format step flagsbulk_handlers.go/graph_v2_handler_test.go— both are pre-existing gofumpt failures onmain, untouched here.--no-verify: the repo's pre-commit hook is broken (go-fmt-goimportsid absent from the pinnedtekwizely/pre-commit-golang), blocking all commits. Separate fix.Review round
An adversarial multi-dimension review (security, correctness, accessibility, tests) ran against the branch; each finding was re-verified against the code before acting. 9 confirmed, 1 refuted. Fixes in
575b31b; every previously-surviving mutant is now killed.Confirmed and fixed:
adminCheck/expiryResolverseams; new tests cover the 503 guard, admit path,?days=/?show=/sortwiring and the error branch.handlePasswordExpiryV20% → 94.7%,RequireAdmin→ 100%.dir=desc—sortByDeadlinenow pins must-change/never/unknown to the bottom in both directions.tableSortHeader, so the active sort column/direction now reaches AT across every V2 table, not just this page.LDAP_ADMIN_GROUP.Refuted: the "N+1 live query" claim — on AD
PasswordExpiryForreads from the already-fetched entry (no round-trip); on OpenLDAP the policy read is memoised per policy DN. No amplification.Deferred (pre-existing, out of scope): the client-side filter does not announce "no results" via an aria-live region — a gap in the shared search JS affecting every V2 list table.
golangci-lint0 issues,govulncheckclean,gofumptclean, fullgo test ./...green, codecov/patch 96%.