Browse Source

iam: add defensive copies, validation, and XSS fixes for group management

- Memory store: clone groups on store/retrieve to prevent mutation
- Admin dash: deep copy groups before mutation, validate user/policy exists
- HTTP handlers: translate credential errors to proper HTTP status codes,
  use *bool for Enabled field to distinguish missing vs false
- Groups templ: use data attributes + event delegation instead of inline
  onclick for XSS safety, prevent stale async responses
pull/8560/head
Chris Lu 1 day ago
parent
commit
e367600b21
  1. 28
      weed/admin/dash/group_management.go
  2. 39
      weed/admin/handlers/group_handlers.go
  3. 31
      weed/admin/view/app/groups.templ
  4. 26
      weed/credential/memory/memory_group.go

28
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)

39
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"})

31
weed/admin/view/app/groups.templ

@ -110,11 +110,13 @@ templ Groups(data dash.GroupsPageData) {
</td>
<td>
<button class="btn btn-sm btn-outline-primary me-1"
onclick={ templ.ComponentScript{Call: fmt.Sprintf("viewGroup('%s')", group.Name)} }>
data-group-name={group.Name}
data-action="view">
<i class="fas fa-eye"></i>
</button>
<button class="btn btn-sm btn-outline-danger"
onclick={ templ.ComponentScript{Call: fmt.Sprintf("deleteGroup('%s')", group.Name)} }>
data-group-name={group.Name}
data-action="delete">
<i class="fas fa-trash"></i>
</button>
</td>
@ -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);
});
</script>
}

26
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
}
Loading…
Cancel
Save