diff --git a/weed/admin/dash/group_management.go b/weed/admin/dash/group_management.go index 26e519e84..43f70615e 100644 --- a/weed/admin/dash/group_management.go +++ b/weed/admin/dash/group_management.go @@ -8,6 +8,23 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" ) +// cloneGroup creates a deep copy of an iam_pb.Group to avoid mutating stored state. +func cloneGroup(g *iam_pb.Group) *iam_pb.Group { + clone := &iam_pb.Group{ + Name: g.Name, + Disabled: g.Disabled, + } + if g.Members != nil { + clone.Members = make([]string, len(g.Members)) + copy(clone.Members, g.Members) + } + if g.PolicyNames != nil { + clone.PolicyNames = make([]string, len(g.PolicyNames)) + copy(clone.PolicyNames, g.PolicyNames) + } + return clone +} + func (s *AdminServer) GetGroups(ctx context.Context) ([]GroupData, error) { if s.credentialManager == nil { return nil, fmt.Errorf("credential manager not available") @@ -99,6 +116,10 @@ func (s *AdminServer) AddGroupMember(ctx context.Context, groupName, username st if err != nil { return fmt.Errorf("failed to get group: %w", err) } + g = cloneGroup(g) + if _, err := s.credentialManager.GetUser(ctx, username); err != nil { + return fmt.Errorf("user %s not found: %w", username, err) + } for _, m := range g.Members { if m == username { return nil // already a member @@ -120,6 +141,7 @@ func (s *AdminServer) RemoveGroupMember(ctx context.Context, groupName, username if err != nil { return fmt.Errorf("failed to get group: %w", err) } + g = cloneGroup(g) found := false var newMembers []string for _, m := range g.Members { @@ -148,6 +170,10 @@ func (s *AdminServer) AttachGroupPolicy(ctx context.Context, groupName, policyNa if err != nil { return fmt.Errorf("failed to get group: %w", err) } + g = cloneGroup(g) + if _, err := s.credentialManager.GetPolicy(ctx, policyName); err != nil { + return fmt.Errorf("policy %s not found: %w", policyName, err) + } for _, p := range g.PolicyNames { if p == policyName { return nil // already attached @@ -169,6 +195,7 @@ func (s *AdminServer) DetachGroupPolicy(ctx context.Context, groupName, policyNa if err != nil { return fmt.Errorf("failed to get group: %w", err) } + g = cloneGroup(g) found := false var newPolicies []string for _, p := range g.PolicyNames { @@ -197,6 +224,7 @@ func (s *AdminServer) SetGroupStatus(ctx context.Context, groupName string, enab if err != nil { return fmt.Errorf("failed to get group: %w", err) } + g = cloneGroup(g) g.Disabled = !enabled if err := s.credentialManager.UpdateGroup(ctx, g); err != nil { return fmt.Errorf("failed to update group: %w", err) diff --git a/weed/admin/handlers/group_handlers.go b/weed/admin/handlers/group_handlers.go index 721072c53..ee625decc 100644 --- a/weed/admin/handlers/group_handlers.go +++ b/weed/admin/handlers/group_handlers.go @@ -2,6 +2,7 @@ package handlers import ( "bytes" + "errors" "net/http" "time" @@ -9,9 +10,23 @@ import ( "github.com/seaweedfs/seaweedfs/weed/admin/dash" "github.com/seaweedfs/seaweedfs/weed/admin/view/app" "github.com/seaweedfs/seaweedfs/weed/admin/view/layout" + "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/glog" ) +func groupErrorToHTTPStatus(err error) int { + if errors.Is(err, credential.ErrGroupNotFound) { + return http.StatusNotFound + } + if errors.Is(err, credential.ErrGroupAlreadyExists) { + return http.StatusConflict + } + if errors.Is(err, credential.ErrUserNotInGroup) { + return http.StatusBadRequest + } + return http.StatusInternalServerError +} + type GroupHandlers struct { adminServer *dash.AdminServer } @@ -59,7 +74,7 @@ func (h *GroupHandlers) CreateGroup(w http.ResponseWriter, r *http.Request) { group, err := h.adminServer.CreateGroup(r.Context(), req.Name) if err != nil { glog.Errorf("Failed to create group: %v", err) - writeJSONError(w, http.StatusInternalServerError, "Failed to create group: "+err.Error()) + writeJSONError(w, groupErrorToHTTPStatus(err), "Failed to create group: "+err.Error()) return } writeJSON(w, http.StatusOK, group) @@ -70,7 +85,7 @@ func (h *GroupHandlers) GetGroupDetails(w http.ResponseWriter, r *http.Request) group, err := h.adminServer.GetGroupDetails(r.Context(), name) if err != nil { glog.Errorf("Failed to get group details: %v", err) - writeJSONError(w, http.StatusNotFound, "Group not found") + writeJSONError(w, groupErrorToHTTPStatus(err), "Group not found") return } writeJSON(w, http.StatusOK, group) @@ -80,7 +95,7 @@ func (h *GroupHandlers) DeleteGroup(w http.ResponseWriter, r *http.Request) { name := mux.Vars(r)["name"] if err := h.adminServer.DeleteGroup(r.Context(), name); err != nil { glog.Errorf("Failed to delete group: %v", err) - writeJSONError(w, http.StatusInternalServerError, "Failed to delete group: "+err.Error()) + writeJSONError(w, groupErrorToHTTPStatus(err), "Failed to delete group: "+err.Error()) return } writeJSON(w, http.StatusOK, map[string]string{"message": "Group deleted successfully"}) @@ -110,7 +125,7 @@ func (h *GroupHandlers) AddGroupMember(w http.ResponseWriter, r *http.Request) { return } if err := h.adminServer.AddGroupMember(r.Context(), name, req.Username); err != nil { - writeJSONError(w, http.StatusInternalServerError, "Failed to add member: "+err.Error()) + writeJSONError(w, groupErrorToHTTPStatus(err), "Failed to add member: "+err.Error()) return } writeJSON(w, http.StatusOK, map[string]string{"message": "Member added successfully"}) @@ -120,7 +135,7 @@ func (h *GroupHandlers) RemoveGroupMember(w http.ResponseWriter, r *http.Request name := mux.Vars(r)["name"] username := mux.Vars(r)["username"] if err := h.adminServer.RemoveGroupMember(r.Context(), name, username); err != nil { - writeJSONError(w, http.StatusInternalServerError, "Failed to remove member: "+err.Error()) + writeJSONError(w, groupErrorToHTTPStatus(err), "Failed to remove member: "+err.Error()) return } writeJSON(w, http.StatusOK, map[string]string{"message": "Member removed successfully"}) @@ -150,7 +165,7 @@ func (h *GroupHandlers) AttachGroupPolicy(w http.ResponseWriter, r *http.Request return } if err := h.adminServer.AttachGroupPolicy(r.Context(), name, req.PolicyName); err != nil { - writeJSONError(w, http.StatusInternalServerError, "Failed to attach policy: "+err.Error()) + writeJSONError(w, groupErrorToHTTPStatus(err), "Failed to attach policy: "+err.Error()) return } writeJSON(w, http.StatusOK, map[string]string{"message": "Policy attached successfully"}) @@ -160,7 +175,7 @@ func (h *GroupHandlers) DetachGroupPolicy(w http.ResponseWriter, r *http.Request name := mux.Vars(r)["name"] policyName := mux.Vars(r)["policyName"] if err := h.adminServer.DetachGroupPolicy(r.Context(), name, policyName); err != nil { - writeJSONError(w, http.StatusInternalServerError, "Failed to detach policy: "+err.Error()) + writeJSONError(w, groupErrorToHTTPStatus(err), "Failed to detach policy: "+err.Error()) return } writeJSON(w, http.StatusOK, map[string]string{"message": "Policy detached successfully"}) @@ -169,14 +184,18 @@ func (h *GroupHandlers) DetachGroupPolicy(w http.ResponseWriter, r *http.Request func (h *GroupHandlers) SetGroupStatus(w http.ResponseWriter, r *http.Request) { name := mux.Vars(r)["name"] var req struct { - Enabled bool `json:"enabled"` + Enabled *bool `json:"enabled"` } if err := decodeJSONBody(newJSONMaxReader(w, r), &req); err != nil { writeJSONError(w, http.StatusBadRequest, "Invalid request: "+err.Error()) return } - if err := h.adminServer.SetGroupStatus(r.Context(), name, req.Enabled); err != nil { - writeJSONError(w, http.StatusInternalServerError, "Failed to update group status: "+err.Error()) + if req.Enabled == nil { + writeJSONError(w, http.StatusBadRequest, "enabled field is required") + return + } + if err := h.adminServer.SetGroupStatus(r.Context(), name, *req.Enabled); err != nil { + writeJSONError(w, groupErrorToHTTPStatus(err), "Failed to update group status: "+err.Error()) return } writeJSON(w, http.StatusOK, map[string]string{"message": "Group status updated"}) diff --git a/weed/admin/view/app/groups.templ b/weed/admin/view/app/groups.templ index f6d5ad7bc..fd729106a 100644 --- a/weed/admin/view/app/groups.templ +++ b/weed/admin/view/app/groups.templ @@ -110,11 +110,13 @@ templ Groups(data dash.GroupsPageData) { @@ -257,14 +259,15 @@ templ Groups(data dash.GroupsPageData) { async function viewGroup(name) { currentGroupName = name; document.getElementById('viewGroupTitle').textContent = 'Group: ' + name; - await refreshGroupDetails(); + await refreshGroupDetails(name); new bootstrap.Modal(document.getElementById('viewGroupModal')).show(); } - async function refreshGroupDetails() { + async function refreshGroupDetails(requestedName) { try { - const response = await fetch('/api/groups/' + encodeURIComponent(currentGroupName)); + const response = await fetch('/api/groups/' + encodeURIComponent(requestedName)); if (!response.ok) throw new Error('Failed to fetch group'); + if (requestedName !== currentGroupName) return; // stale response const group = await response.json(); // Render members using DOM APIs to prevent XSS @@ -338,7 +341,7 @@ templ Groups(data dash.GroupsPageData) { body: JSON.stringify({ username: username }) }); if (response.ok) { - await refreshGroupDetails(); + await refreshGroupDetails(currentGroupName); showAlert('Member added', 'success'); } else { const error = await response.json().catch(() => ({})); @@ -355,7 +358,7 @@ templ Groups(data dash.GroupsPageData) { method: 'DELETE' }); if (response.ok) { - await refreshGroupDetails(); + await refreshGroupDetails(currentGroupName); showAlert('Member removed', 'success'); } else { const error = await response.json().catch(() => ({})); @@ -376,7 +379,7 @@ templ Groups(data dash.GroupsPageData) { body: JSON.stringify({ policy_name: policyName }) }); if (response.ok) { - await refreshGroupDetails(); + await refreshGroupDetails(currentGroupName); showAlert('Policy attached', 'success'); } else { const error = await response.json().catch(() => ({})); @@ -393,7 +396,7 @@ templ Groups(data dash.GroupsPageData) { method: 'DELETE' }); if (response.ok) { - await refreshGroupDetails(); + await refreshGroupDetails(currentGroupName); showAlert('Policy detached', 'success'); } else { const error = await response.json().catch(() => ({})); @@ -422,5 +425,15 @@ templ Groups(data dash.GroupsPageData) { showAlert('Failed to update status: ' + error.message, 'error'); } } + + // Event delegation for group action buttons + document.addEventListener('click', function(e) { + const btn = e.target.closest('[data-action]'); + if (!btn) return; + const name = btn.dataset.groupName; + if (!name) return; + if (btn.dataset.action === 'view') viewGroup(name); + else if (btn.dataset.action === 'delete') deleteGroup(name); + }); } diff --git a/weed/credential/memory/memory_group.go b/weed/credential/memory/memory_group.go index 6c5db9789..180ce51c5 100644 --- a/weed/credential/memory/memory_group.go +++ b/weed/credential/memory/memory_group.go @@ -7,6 +7,26 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" ) +// cloneGroup creates a deep copy of an iam_pb.Group. +func cloneGroup(g *iam_pb.Group) *iam_pb.Group { + if g == nil { + return nil + } + clone := &iam_pb.Group{ + Name: g.Name, + Disabled: g.Disabled, + } + if g.Members != nil { + clone.Members = make([]string, len(g.Members)) + copy(clone.Members, g.Members) + } + if g.PolicyNames != nil { + clone.PolicyNames = make([]string, len(g.PolicyNames)) + copy(clone.PolicyNames, g.PolicyNames) + } + return clone +} + func (store *MemoryStore) CreateGroup(ctx context.Context, group *iam_pb.Group) error { store.mu.Lock() defer store.mu.Unlock() @@ -14,7 +34,7 @@ func (store *MemoryStore) CreateGroup(ctx context.Context, group *iam_pb.Group) if _, exists := store.groups[group.Name]; exists { return credential.ErrGroupAlreadyExists } - store.groups[group.Name] = group + store.groups[group.Name] = cloneGroup(group) return nil } @@ -23,7 +43,7 @@ func (store *MemoryStore) GetGroup(ctx context.Context, groupName string) (*iam_ defer store.mu.RUnlock() if g, exists := store.groups[groupName]; exists { - return g, nil + return cloneGroup(g), nil } return nil, credential.ErrGroupNotFound } @@ -57,6 +77,6 @@ func (store *MemoryStore) UpdateGroup(ctx context.Context, group *iam_pb.Group) if _, exists := store.groups[group.Name]; !exists { return credential.ErrGroupNotFound } - store.groups[group.Name] = group + store.groups[group.Name] = cloneGroup(group) return nil }