Skip to content

Commit bf8926f

Browse files
committed
fix(ui): gate Disable action on User/Computer.Enabled
The drawer Actions section in both users_v2.templ and computers_v2.templ previously rendered the Disable form whenever the backend was Active Directory, regardless of the entity's current enabled state. An already-disabled user kept seeing a "Disable" button that, when clicked, asked the directory to re-disable an already-disabled account. Wrap the form in the combined guard (IsAD && Enabled) so the action only surfaces when it can do something. Comment on the IsAD field updated to call out the Enabled requirement. Coverage: new drawer_disable_gating_test.go asserts the IsAD × Enabled matrix for both User and Computer drawer fragments. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent f20888f commit bf8926f

3 files changed

Lines changed: 105 additions & 4 deletions

File tree

internal/web/templates/computers_v2.templ

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +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
22+
IsAD bool // true when backend is Active Directory — gates the Disable button (which also requires Computer.Enabled)
2323
}
2424

2525
templ ComputersListV2(computers []ldap.Computer, ouFilter string, ous []string, flashes []Flash, palettePinned []PinnedEntry) {
@@ -325,7 +325,7 @@ templ computerDrawerContentsCtx(vm ComputerDrawerVM, inDrawer bool) {
325325
<section class="drawer__section drawer__section--actions">
326326
<h3 class="drawer__section-title">Actions</h3>
327327
<div class="drawer__actions">
328-
if vm.IsAD {
328+
if vm.IsAD && vm.Computer.Enabled {
329329
<form method="post" action="/computers/bulk?action=disable" data-confirm={ "Disable " + vm.Computer.CN() + "?" }>
330330
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
331331
<input type="hidden" name="target_dn" value={ vm.Computer.DN() }/>
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// internal/web/templates/drawer_disable_gating_test.go
2+
package templates
3+
4+
import (
5+
"bytes"
6+
"context"
7+
"strings"
8+
"testing"
9+
10+
ldap "github.com/netresearch/simple-ldap-go"
11+
12+
"github.com/netresearch/ldap-manager/internal/ldap_cache"
13+
)
14+
15+
// TestUserDrawerDisableGating asserts that the Disable action form is only
16+
// rendered when both (a) the backend is Active Directory and (b) the user
17+
// is currently enabled. Bug: the drawer previously emitted the Disable
18+
// button for an already-disabled user, so the account was told to disable
19+
// itself again — visually contradictory and operationally useless.
20+
func TestUserDrawerDisableGating(t *testing.T) {
21+
const disableMarker = `action="/users/bulk?action=disable"`
22+
23+
cases := []struct {
24+
name string
25+
isAD bool
26+
enabled bool
27+
wantDisable bool
28+
wantStatusOK string
29+
}{
30+
{"AD + enabled shows Disable", true, true, true, "Enabled"},
31+
{"AD + already disabled hides Disable", true, false, false, "Disabled"},
32+
{"OpenLDAP + enabled hides Disable", false, true, false, "Enabled"},
33+
{"OpenLDAP + disabled hides Disable", false, false, false, "Disabled"},
34+
}
35+
36+
for _, tc := range cases {
37+
t.Run(tc.name, func(t *testing.T) {
38+
vm := UserDrawerVM{
39+
User: &ldap_cache.FullLDAPUser{
40+
User: ldap.User{Enabled: tc.enabled, SAMAccountName: "bob"},
41+
},
42+
IsAD: tc.isAD,
43+
}
44+
45+
var buf bytes.Buffer
46+
if err := UserDrawerFragment(vm).Render(context.Background(), &buf); err != nil {
47+
t.Fatalf("render drawer fragment: %v", err)
48+
}
49+
50+
html := buf.String()
51+
hasDisable := strings.Contains(html, disableMarker)
52+
53+
if hasDisable != tc.wantDisable {
54+
t.Errorf("disable form present=%v, want=%v (IsAD=%v, Enabled=%v)",
55+
hasDisable, tc.wantDisable, tc.isAD, tc.enabled)
56+
}
57+
if !strings.Contains(html, tc.wantStatusOK) {
58+
t.Errorf("expected drawer to render status %q, missing in output", tc.wantStatusOK)
59+
}
60+
})
61+
}
62+
}
63+
64+
// TestComputerDrawerDisableGating mirrors TestUserDrawerDisableGating for
65+
// the computer detail drawer. Same bug symptom (Disable shown on an
66+
// already-disabled machine account), same fix shape.
67+
func TestComputerDrawerDisableGating(t *testing.T) {
68+
const disableMarker = `action="/computers/bulk?action=disable"`
69+
70+
cases := []struct {
71+
name string
72+
isAD bool
73+
enabled bool
74+
wantDisable bool
75+
}{
76+
{"AD + enabled shows Disable", true, true, true},
77+
{"AD + already disabled hides Disable", true, false, false},
78+
{"OpenLDAP + enabled hides Disable", false, true, false},
79+
{"OpenLDAP + disabled hides Disable", false, false, false},
80+
}
81+
82+
for _, tc := range cases {
83+
t.Run(tc.name, func(t *testing.T) {
84+
vm := ComputerDrawerVM{
85+
Computer: ldap.Computer{Enabled: tc.enabled, SAMAccountName: "pc01$"},
86+
IsAD: tc.isAD,
87+
}
88+
89+
var buf bytes.Buffer
90+
if err := ComputerDrawerFragment(vm).Render(context.Background(), &buf); err != nil {
91+
t.Fatalf("render drawer fragment: %v", err)
92+
}
93+
94+
hasDisable := strings.Contains(buf.String(), disableMarker)
95+
if hasDisable != tc.wantDisable {
96+
t.Errorf("disable form present=%v, want=%v (IsAD=%v, Enabled=%v)",
97+
hasDisable, tc.wantDisable, tc.isAD, tc.enabled)
98+
}
99+
})
100+
}
101+
}

internal/web/templates/users_v2.templ

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +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
29+
IsAD bool // true when the backend is Active Directory — gates the Disable button (which also requires User.Enabled) since OpenLDAP has no portable disable mechanism
3030
}
3131

3232
templ UsersListV2(users []ldap.User, showDisabled bool, ouFilter string, lastLogon string, memberOfDN, memberOfCN string, ous []string, flashes []Flash, palettePinned []PinnedEntry, adminDNs map[string]struct{}) {
@@ -424,7 +424,7 @@ templ userDrawerContentsCtx(vm UserDrawerVM, inDrawer bool) {
424424
<section class="drawer__section drawer__section--actions">
425425
<h3 class="drawer__section-title">Actions</h3>
426426
<div class="drawer__actions">
427-
if vm.IsAD {
427+
if vm.IsAD && vm.User.Enabled {
428428
<form method="post" action="/users/bulk?action=disable" data-confirm={ "Disable " + vm.User.CN() + "? The account can be re-enabled later." }>
429429
<input type="hidden" name="csrf_token" value={ vm.CSRFToken }/>
430430
<input type="hidden" name="target_dn" value={ vm.User.DN() }/>

0 commit comments

Comments
 (0)