Skip to content

Commit f9701c4

Browse files
committed
feat(ui): bulk + single disable (AD-gated), adminCount-based Privileged
Three wins from the simple-ldap-go v1.12.0 bump: 1. adminCount-based Privileged detection adminUserDNs no longer maintains a hardcoded English CN allowlist ("admins", "domain admins", …). It now reads the AD-native User.AdminCount field set by simple-ldap-go v1.12 from the adminCount=1 attribute AD applies via adminSDHolder to every protected-group member (Domain Admins, Enterprise Admins, Administrators, Account Operators, Backup Operators, Server Operators, Print Operators, Replicator, Schema Admins, Key Admins, Enterprise Key Admins, Read-Only Domain Controllers, Domain Controllers). Benefits: - Catches localised AD directories (Domänen-Admins etc.) the CN list used to miss. - No transitive walk needed — AD already bakes the flag in. - Limitation documented inline on the function: adminCount is sticky (AD doesn't clear it when a user leaves a protected group), so a true value means "is OR was privileged", not a perfect real-time check. 2. Bulk disable for users and computers Replaces the 501 stubs on /users/bulk?action=disable and /computers/bulk?action=disable with real AD UAC writes. Backed by simple-ldap-go v1.12 DisableUserContext / DisableComputerContext which flip the ACCOUNTDISABLE bit (0x2) via read-modify-write, preserving every other UAC flag on the entry. - bulkDisableUsers / bulkDisableComputers dispatch through a shared bulkUACDisable helper (kind, redirectTo, op func) — same shape as bulkDeleteByDN, same flash semantics. - finaliseBulkDisable emits "Disabled N ..." success / "Failed to disable any of N ..." error / "Disabled N / M ..." partial flashes. Cache + template cache refreshed on any success, like bulk delete. 3. AD-gating everywhere Handler side (bulk_handlers.go): each disable dispatch checks a.ldapConfig.IsActiveDirectory. Non-AD deployments still hit the bulkNotImplemented path with the same 501 + message they used to; the contract doesn't regress. Template side (users_v2.templ + computers_v2.templ): the Disable button only renders when vm.IsAD is true. New IsAD field on both UserDrawerVM and ComputerDrawerVM, populated in their build*VM functions from a.ldapConfig.IsActiveDirectory. Tests: - Unit: TestBulkHandler_Users_DisableDispatchesOnAD and the computers equivalent exercise the AD gate by flipping app.ldapConfig.IsActiveDirectory=true and asserting NOT 501, NOT 400. The pre-existing non-AD stub tests renamed to ..._StubbedOnNonAD with docstrings that explain the gate behaviour. - Existing bulk_handlers_test.go + full e2e suite remain green (71.5 s). Dep: github.com/netresearch/simple-ldap-go v1.11.0 → v1.12.0. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent 020d0f0 commit f9701c4

8 files changed

Lines changed: 235 additions & 43 deletions

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ require (
99
github.com/gofiber/storage/bbolt/v2 v2.1.4
1010
github.com/gofiber/storage/memory/v2 v2.1.2
1111
github.com/joho/godotenv v1.5.1
12-
github.com/netresearch/simple-ldap-go v1.11.0
12+
github.com/netresearch/simple-ldap-go v1.12.0
1313
github.com/playwright-community/playwright-go v0.5700.1
1414
github.com/rs/zerolog v1.35.0
1515
github.com/stretchr/testify v1.11.1

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,8 @@ github.com/natefinch/atomic v1.0.1 h1:ZPYKxkqQOx3KZ+RsbnP/YsgvxWQPGxjC0oBt2AhwV0
412412
github.com/natefinch/atomic v1.0.1/go.mod h1:N/D/ELrljoqDyT3rZrsUmtsuzvHkeB/wWjHV22AZRbM=
413413
github.com/netresearch/simple-ldap-go v1.11.0 h1:MoI/3TdYNlVR8R3jUdVHKP2Ez6lFLSB5TRFaX1Ai02M=
414414
github.com/netresearch/simple-ldap-go v1.11.0/go.mod h1:TgUTDRc9SFd0MpBnzdGoM9A/XX5eAhErHdquURlZ0lo=
415+
github.com/netresearch/simple-ldap-go v1.12.0 h1:6OmPlACaM9KgT/PhDQ1d3rF0KA8TPTJyL1GE18RgrpI=
416+
github.com/netresearch/simple-ldap-go v1.12.0/go.mod h1:TgUTDRc9SFd0MpBnzdGoM9A/XX5eAhErHdquURlZ0lo=
415417
github.com/nishanths/exhaustive v0.12.0 h1:vIY9sALmw6T/yxiASewa4TQcFsVYZQQRUQJhKRf3Swg=
416418
github.com/nishanths/exhaustive v0.12.0/go.mod h1:mEZ95wPIZW+x8kC4TgC+9YCUgiST7ecevsVDTgc2obs=
417419
github.com/nishanths/predeclared v0.2.2 h1:V2EPdZPliZymNAn79T8RkNApBjMmVKh5XRpLm/w98Vk=

internal/web/bulk_handlers.go

Lines changed: 104 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@ func (a *App) handleBulkUsers(c *fiber.Ctx) error {
4444
case "delete":
4545
return a.bulkDeleteUsers(c)
4646
case "disable":
47-
// sAMAccountName enable/disable on OpenLDAP (inetOrgPerson) has no
48-
// portable bit to flip — userAccountControl is AD-specific. Rather
49-
// than silently muddle the description attribute we stub here.
50-
return bulkNotImplemented(c, "disable users", "")
47+
if !a.ldapConfig.IsActiveDirectory {
48+
// OpenLDAP inetOrgPerson has no portable enable/disable
49+
// attribute. Return 501 with the same message the template
50+
// used to render so the contract is unchanged for non-AD.
51+
return bulkNotImplemented(c, "disable users", "")
52+
}
53+
54+
return a.bulkDisableUsers(c)
5155
default:
5256
return c.Status(fiber.StatusBadRequest).SendString("unknown bulk action")
5357
}
@@ -83,8 +87,14 @@ func (a *App) handleBulkComputers(c *fiber.Ctx) error {
8387
action := c.Query("action")
8488
switch action {
8589
case "disable":
86-
// Same reasoning as users "disable" — no portable write op.
87-
return bulkNotImplemented(c, "disable computers", "")
90+
if !a.ldapConfig.IsActiveDirectory {
91+
// Non-AD: no userAccountControl to flip. Keep the 501
92+
// contract in place so non-AD deployments see a clear
93+
// message rather than a silent no-op.
94+
return bulkNotImplemented(c, "disable computers", "")
95+
}
96+
97+
return a.bulkDisableComputers(c)
8898
case "delete":
8999
return a.bulkDeleteComputers(c)
90100
default:
@@ -306,6 +316,94 @@ func (a *App) bulkDeleteByDN(c *fiber.Ctx, kind, redirectTo string) error {
306316
return c.Redirect(redirectTo, fiber.StatusSeeOther)
307317
}
308318

319+
// bulkDisableUsers flips the ACCOUNTDISABLE bit (0x2) on each user DN
320+
// in target_dn[] via simple-ldap-go v1.12's DisableUserContext. AD
321+
// only — the caller in handleBulkUsers gates this on
322+
// a.ldapConfig.IsActiveDirectory so non-AD deployments never reach here.
323+
func (a *App) bulkDisableUsers(c *fiber.Ctx) error {
324+
return a.bulkUACDisable(c, "user", "/users", func(client *ldap.LDAP, dn string) error {
325+
return client.DisableUserContext(c.UserContext(), dn)
326+
})
327+
}
328+
329+
// bulkDisableComputers mirrors bulkDisableUsers for computer entries.
330+
// AD-only, same gating in handleBulkComputers.
331+
func (a *App) bulkDisableComputers(c *fiber.Ctx) error {
332+
return a.bulkUACDisable(c, "computer", "/computers", func(client *ldap.LDAP, dn string) error {
333+
return client.DisableComputerContext(c.UserContext(), dn)
334+
})
335+
}
336+
337+
// bulkUACDisable is the shared body for bulkDisableUsers /
338+
// bulkDisableComputers: open a per-user LDAP binding, run the given
339+
// per-DN disable op, count successes, flash "Disabled N / M <kind>s".
340+
// Pattern matches bulkDeleteByDN — different op, same batching.
341+
func (a *App) bulkUACDisable(c *fiber.Ctx, kind, redirectTo string, op func(*ldap.LDAP, string) error) error {
342+
targets := collectTargetDNs(c)
343+
if len(targets) == 0 {
344+
return c.Redirect(redirectTo, fiber.StatusSeeOther)
345+
}
346+
347+
client, err := a.getUserLDAP(c)
348+
if err != nil {
349+
return handle500(c, err)
350+
}
351+
defer func() { _ = client.Close() }()
352+
353+
disabled := 0
354+
var firstErr error
355+
356+
for _, dn := range targets {
357+
if err := op(client, dn); err != nil {
358+
if firstErr == nil {
359+
firstErr = err
360+
}
361+
362+
log.Warn().Err(err).Str("dn", dn).Str("kind", kind).Msg("bulk disable failed")
363+
364+
continue
365+
}
366+
367+
disabled++
368+
}
369+
370+
a.finaliseBulkDisable(c, kind, disabled, len(targets), firstErr)
371+
372+
return c.Redirect(redirectTo, fiber.StatusSeeOther)
373+
}
374+
375+
// finaliseBulkDisable is the disable analogue of finaliseBulkDelete:
376+
// refresh caches on any success, log the summary, flash the result.
377+
func (a *App) finaliseBulkDisable(c *fiber.Ctx, kind string, disabled, total int, firstErr error) {
378+
if disabled > 0 {
379+
a.invalidateTemplateCacheOnModification()
380+
381+
if a.ldapCache != nil {
382+
a.ldapCache.Refresh()
383+
}
384+
}
385+
386+
log.Info().
387+
Int("targeted", total).
388+
Int("disabled", disabled).
389+
Str("kind", kind).
390+
Msg("bulk disable complete")
391+
392+
switch disabled {
393+
case total:
394+
a.setFlash(c, templates.SuccessFlash(
395+
fmt.Sprintf("Disabled %d %s%s.", disabled, kind, pluralSuffix(disabled))))
396+
case 0:
397+
a.setFlash(c, templates.ErrorFlash(
398+
fmt.Sprintf("Failed to disable any of %d %s%s: %s",
399+
total, kind, pluralSuffix(total), humaniseLDAPError(firstErr))))
400+
default:
401+
a.setFlash(c, templates.ErrorFlash(
402+
fmt.Sprintf("Disabled %d / %d %s%s (%s)",
403+
disabled, total, kind, pluralSuffix(total), humaniseLDAPError(firstErr))))
404+
}
405+
}
406+
309407
// finaliseBulkDelete is the shared post-loop cleanup for all three
310408
// bulk-delete handlers: invalidate the template + LDAP caches if any
311409
// delete succeeded, and drop a "Deleted N / M <kind>s" flash on the

internal/web/bulk_handlers_test.go

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,13 @@ func TestBulkHandler_Computers_DeleteDispatches(t *testing.T) {
190190
}
191191
}
192192

193-
// TestBulkHandler_Computers_DisableStubbed verifies the disable action on
194-
// /computers/bulk is explicitly not yet implemented.
195-
func TestBulkHandler_Computers_DisableStubbed(t *testing.T) {
193+
// TestBulkHandler_Computers_DisableStubbedOnNonAD verifies the disable
194+
// action on /computers/bulk returns 501 on non-AD directories (where
195+
// userAccountControl doesn't exist). The default test config has
196+
// IsActiveDirectory=false, so the handler's AD gate falls through to
197+
// bulkNotImplemented. AD deployments dispatch to bulkDisableComputers
198+
// instead — covered by the AD-gated test below.
199+
func TestBulkHandler_Computers_DisableStubbedOnNonAD(t *testing.T) {
196200
app, store := setupFullTestApp(t)
197201

198202
cookies := createAuthSession(t, app, store)
@@ -218,9 +222,10 @@ func TestBulkHandler_Computers_DisableStubbed(t *testing.T) {
218222
}
219223
}
220224

221-
// TestBulkHandler_Users_DisableStubbed verifies the disable action on
222-
// /users/bulk is explicitly not yet implemented for inetOrgPerson.
223-
func TestBulkHandler_Users_DisableStubbed(t *testing.T) {
225+
// TestBulkHandler_Users_DisableStubbedOnNonAD verifies disable on
226+
// /users/bulk still returns 501 when IsActiveDirectory is false
227+
// (the default test harness). The AD path is tested separately.
228+
func TestBulkHandler_Users_DisableStubbedOnNonAD(t *testing.T) {
224229
app, store := setupFullTestApp(t)
225230

226231
cookies := createAuthSession(t, app, store)
@@ -246,6 +251,75 @@ func TestBulkHandler_Users_DisableStubbed(t *testing.T) {
246251
}
247252
}
248253

254+
// TestBulkHandler_Users_DisableDispatchesOnAD verifies the AD gate:
255+
// when IsActiveDirectory is true, /users/bulk?action=disable should
256+
// NOT return 501 — it should reach the bulkDisableUsers path (where
257+
// getUserLDAP fails against the mock server and bubbles a redirect).
258+
// The regression this guards: a future refactor that drops the gate
259+
// would make AD deployments see a spurious 501 even though the code
260+
// is ready for them.
261+
func TestBulkHandler_Users_DisableDispatchesOnAD(t *testing.T) {
262+
app, store := setupFullTestApp(t)
263+
app.ldapConfig.IsActiveDirectory = true
264+
265+
cookies := createAuthSession(t, app, store)
266+
267+
form := url.Values{"target_dn": {"cn=u1,dc=test"}}
268+
req := httptest.NewRequest(http.MethodPost,
269+
"/users/bulk?action=disable",
270+
strings.NewReader(form.Encode()))
271+
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
272+
273+
for _, c := range cookies {
274+
req.AddCookie(c)
275+
}
276+
277+
resp, err := app.fiber.Test(req)
278+
if err != nil {
279+
t.Fatalf("users/bulk POST: %v", err)
280+
}
281+
defer func() { _ = resp.Body.Close() }()
282+
283+
if resp.StatusCode == http.StatusNotImplemented {
284+
t.Fatalf("expected dispatch to bulkDisableUsers on AD (not 501 stub); got 501")
285+
}
286+
if resp.StatusCode == http.StatusBadRequest {
287+
t.Fatalf("unexpected 400 for well-formed disable on AD: %d", resp.StatusCode)
288+
}
289+
}
290+
291+
// TestBulkHandler_Computers_DisableDispatchesOnAD mirrors the users
292+
// AD-gate test for computers.
293+
func TestBulkHandler_Computers_DisableDispatchesOnAD(t *testing.T) {
294+
app, store := setupFullTestApp(t)
295+
app.ldapConfig.IsActiveDirectory = true
296+
297+
cookies := createAuthSession(t, app, store)
298+
299+
form := url.Values{"target_dn": {"cn=pc1,dc=test"}}
300+
req := httptest.NewRequest(http.MethodPost,
301+
"/computers/bulk?action=disable",
302+
strings.NewReader(form.Encode()))
303+
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
304+
305+
for _, c := range cookies {
306+
req.AddCookie(c)
307+
}
308+
309+
resp, err := app.fiber.Test(req)
310+
if err != nil {
311+
t.Fatalf("computers/bulk POST: %v", err)
312+
}
313+
defer func() { _ = resp.Body.Close() }()
314+
315+
if resp.StatusCode == http.StatusNotImplemented {
316+
t.Fatalf("expected dispatch to bulkDisableComputers on AD (not 501); got 501")
317+
}
318+
if resp.StatusCode == http.StatusBadRequest {
319+
t.Fatalf("unexpected 400 for well-formed disable on AD: %d", resp.StatusCode)
320+
}
321+
}
322+
249323
// TestBulkHandler_Groups_AddMembersMissingUser verifies the add-members
250324
// action requires user_dn.
251325
func TestBulkHandler_Groups_AddMembersMissingUser(t *testing.T) {

internal/web/computers_v2_handler.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ func (a *App) buildComputerDrawerVM(computerDN, viewerDN string) (templates.Comp
4848
Pinned: pinned,
4949
OUName: ouName,
5050
OUPivotHref: buildComputerOUPivotHref(ouName),
51+
IsAD: a.ldapConfig.IsActiveDirectory,
5152
}, true
5253
}
5354

internal/web/templates/computers_v2.templ

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ type ComputerDrawerVM struct {
1919
OUName string // plain-text OU name, e.g. "ou=Computers" (for display)
2020
OUPivotHref string // URL-encoded pivot link, e.g. "/computers?ou=ou%3DComputers"
2121
CSRFToken string // current-session CSRF token used by embedded POST forms
22+
IsAD bool // true when backend is Active Directory — gates the Disable button
2223
}
2324

2425
templ ComputersListV2(computers []ldap.Computer, ouFilter string, ous []string, flashes []Flash, palettePinned []PinnedEntry) {
@@ -324,11 +325,13 @@ templ computerDrawerContentsCtx(vm ComputerDrawerVM, inDrawer bool) {
324325
<section class="drawer__section drawer__section--actions">
325326
<h3 class="drawer__section-title">Actions</h3>
326327
<div class="drawer__actions">
327-
<form method="post" action="/computers/bulk?action=disable" data-confirm={ "Disable " + vm.Computer.CN() + "?" }>
328-
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
329-
<input type="hidden" name="target_dn" value={ vm.Computer.DN() }/>
330-
<button type="submit" class="drawer__action drawer__action--warn">Disable</button>
331-
</form>
328+
if vm.IsAD {
329+
<form method="post" action="/computers/bulk?action=disable" data-confirm={ "Disable " + vm.Computer.CN() + "?" }>
330+
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
331+
<input type="hidden" name="target_dn" value={ vm.Computer.DN() }/>
332+
<button type="submit" class="drawer__action drawer__action--warn">Disable</button>
333+
</form>
334+
}
332335
<form method="post" action="/computers/bulk?action=delete" data-confirm={ "Delete " + vm.Computer.CN() + "? This cannot be undone." }>
333336
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
334337
<input type="hidden" name="target_dn" value={ vm.Computer.DN() }/>

internal/web/templates/users_v2.templ

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ type UserDrawerVM struct {
2626
CSRFToken string // current-session CSRF token used by embedded POST forms
2727
UnassignedGroups []ldap.Group // groups the user is NOT a member of (datalist for add-to-group)
2828
FlashError string // set by modify handlers when an LDAP op fails; shown inline
29+
IsAD bool // true when the backend is Active Directory — gates the Disable button since OpenLDAP has no portable disable mechanism
2930
}
3031

3132
templ UsersListV2(users []ldap.User, showDisabled bool, ouFilter string, lastLogon string, memberOfDN, memberOfCN string, ous []string, flashes []Flash, palettePinned []PinnedEntry, adminDNs map[string]struct{}) {
@@ -423,11 +424,13 @@ templ userDrawerContentsCtx(vm UserDrawerVM, inDrawer bool) {
423424
<section class="drawer__section drawer__section--actions">
424425
<h3 class="drawer__section-title">Actions</h3>
425426
<div class="drawer__actions">
426-
<form method="post" action="/users/bulk?action=disable" data-confirm={ "Disable " + vm.User.CN() + "? The account can be re-enabled later." }>
427-
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
428-
<input type="hidden" name="target_dn" value={ vm.User.DN() }/>
429-
<button type="submit" class="drawer__action drawer__action--warn">Disable</button>
430-
</form>
427+
if vm.IsAD {
428+
<form method="post" action="/users/bulk?action=disable" data-confirm={ "Disable " + vm.User.CN() + "? The account can be re-enabled later." }>
429+
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
430+
<input type="hidden" name="target_dn" value={ vm.User.DN() }/>
431+
<button type="submit" class="drawer__action drawer__action--warn">Disable</button>
432+
</form>
433+
}
431434
<form method="post" action="/users/bulk?action=delete" data-confirm={ "Delete " + vm.User.CN() + "? This cannot be undone." }>
432435
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
433436
<input type="hidden" name="target_dn" value={ vm.User.DN() }/>

internal/web/users_v2_handler.go

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func (a *App) buildUserDrawerVM(userDN, viewerDN string) (templates.UserDrawerVM
4949
OUName: ouFilter,
5050
OUPivotHref: buildOUPivotHref(ouFilter),
5151
UnassignedGroups: unassigned,
52+
IsAD: a.ldapConfig.IsActiveDirectory,
5253
}, true
5354
}
5455

@@ -116,33 +117,43 @@ func (a *App) handleUsersV2(c *fiber.Ctx) error {
116117
return page.Render(c.UserContext(), c.Response().BodyWriter())
117118
}
118119

119-
// adminUserDNs collects the DNs of users that are members of a group
120-
// whose CN matches a short "privileged" name list (admins, domain
121-
// admins, enterprise admins — case-insensitive). Returns a set so the
122-
// list template's check stays O(1) per row. A nil cache yields nil.
120+
// adminUserDNs collects the DNs of users flagged as privileged by AD's
121+
// `adminCount=1` attribute (surfaced as `User.AdminCount` in
122+
// simple-ldap-go v1.12+).
123+
//
124+
// Why adminCount and not a CN allowlist:
125+
//
126+
// AD sets adminCount=1 via adminSDHolder on every member of its
127+
// protected groups (Domain Admins, Enterprise Admins,
128+
// Administrators, Account Operators, Backup Operators, Server
129+
// Operators, Print Operators, Replicator, Schema Admins, Key
130+
// Admins, Enterprise Key Admins, Read-Only Domain Controllers,
131+
// Domain Controllers). This catches every AD-recognised privileged
132+
// user without us hard-coding English CNs (which would miss
133+
// localised directories like "Domänen-Admins") and without walking
134+
// nested group membership.
135+
//
136+
// Limitations:
137+
//
138+
// The attribute is STICKY: AD does not clear adminCount when a
139+
// user leaves a protected group. A true value therefore means
140+
// "is or was privileged" — still a strong UI signal but not a
141+
// perfect real-time membership check. For non-AD directories
142+
// (OpenLDAP) the attribute is never set, so the shield never
143+
// renders. That matches the read-only nature of those deployments.
144+
//
145+
// Returns a set so the list template's per-row check stays O(1).
146+
// A nil cache yields nil.
123147
func adminUserDNs(cache *ldap_cache.Manager) map[string]struct{} {
124148
if cache == nil {
125149
return nil
126150
}
127151

128-
privilegedNames := map[string]struct{}{
129-
"admins": {},
130-
"domain admins": {},
131-
"enterprise admins": {},
132-
"administrators": {},
133-
"schema admins": {},
134-
}
135-
136152
out := make(map[string]struct{})
137153

138-
for _, g := range cache.FindGroups() {
139-
cn := strings.ToLower(g.CN())
140-
if _, ok := privilegedNames[cn]; !ok {
141-
continue
142-
}
143-
144-
for _, memberDN := range g.Members {
145-
out[memberDN] = struct{}{}
154+
for _, u := range cache.FindUsers(true /* include disabled — privileged flag applies regardless */) {
155+
if u.AdminCount {
156+
out[u.DN()] = struct{}{}
146157
}
147158
}
148159

0 commit comments

Comments
 (0)