diff --git a/weed/admin/view/app/groups.templ b/weed/admin/view/app/groups.templ index 6fb8b0be9..f6d5ad7bc 100644 --- a/weed/admin/view/app/groups.templ +++ b/weed/admin/view/app/groups.templ @@ -267,29 +267,59 @@ templ Groups(data dash.GroupsPageData) { if (!response.ok) throw new Error('Failed to fetch group'); const group = await response.json(); - // Render members - let membersHtml = ''; + // Render members using DOM APIs to prevent XSS + const membersList = document.getElementById('membersList'); + membersList.innerHTML = ''; + const membersTable = document.createElement('table'); + membersTable.className = 'table table-sm'; + const membersTbody = document.createElement('tbody'); if (group.members && group.members.length > 0) { for (const member of group.members) { - membersHtml += ''; + const tr = membersTbody.insertRow(); + const td1 = tr.insertCell(); + td1.textContent = member; + const td2 = tr.insertCell(); + const btn = document.createElement('button'); + btn.className = 'btn btn-sm btn-outline-danger'; + btn.onclick = () => removeMember(member); + btn.innerHTML = ''; + td2.appendChild(btn); } } else { - membersHtml += ''; + const tr = membersTbody.insertRow(); + const td = tr.insertCell(); + td.className = 'text-muted'; + td.textContent = 'No members'; } - membersHtml += '
' + member + '
No members
'; - document.getElementById('membersList').innerHTML = membersHtml; + membersTable.appendChild(membersTbody); + membersList.appendChild(membersTable); - // Render policies - let policiesHtml = ''; + // Render policies using DOM APIs to prevent XSS + const policiesList = document.getElementById('policiesList'); + policiesList.innerHTML = ''; + const policiesTable = document.createElement('table'); + policiesTable.className = 'table table-sm'; + const policiesTbody = document.createElement('tbody'); if (group.policy_names && group.policy_names.length > 0) { for (const policy of group.policy_names) { - policiesHtml += ''; + const tr = policiesTbody.insertRow(); + const td1 = tr.insertCell(); + td1.textContent = policy; + const td2 = tr.insertCell(); + const btn = document.createElement('button'); + btn.className = 'btn btn-sm btn-outline-danger'; + btn.onclick = () => detachPolicy(policy); + btn.innerHTML = ''; + td2.appendChild(btn); } } else { - policiesHtml += ''; + const tr = policiesTbody.insertRow(); + const td = tr.insertCell(); + td.className = 'text-muted'; + td.textContent = 'No policies attached'; } - policiesHtml += '
' + policy + '
No policies attached
'; - document.getElementById('policiesList').innerHTML = policiesHtml; + policiesTable.appendChild(policiesTbody); + policiesList.appendChild(policiesTable); // Update status toggle document.getElementById('groupEnabledSwitch').checked = (group.status === 'enabled'); diff --git a/weed/admin/view/app/groups_templ.go b/weed/admin/view/app/groups_templ.go index 0b417f4bb..ec131521b 100644 --- a/weed/admin/view/app/groups_templ.go +++ b/weed/admin/view/app/groups_templ.go @@ -245,7 +245,7 @@ func Groups(data dash.GroupsPageData) templ.Component { return templ_7745c5c3_Err } } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 26, "
") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 26, "
") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/credential/grpc/grpc_group.go b/weed/credential/grpc/grpc_group.go index 9e2262dd7..394dc13ca 100644 --- a/weed/credential/grpc/grpc_group.go +++ b/weed/credential/grpc/grpc_group.go @@ -7,6 +7,11 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" ) +// NOTE: The gRPC store uses a load-modify-save pattern for all operations, +// which is inherently subject to race conditions under concurrent access. +// This matches the existing pattern used for identities and policies. +// A future improvement would add dedicated gRPC RPCs for atomic group operations. + func (store *IamGrpcStore) CreateGroup(ctx context.Context, group *iam_pb.Group) error { config, err := store.LoadConfiguration(ctx) if err != nil { diff --git a/weed/iamapi/iamapi_group_handlers.go b/weed/iamapi/iamapi_group_handlers.go index 0af42880b..dc4071d94 100644 --- a/weed/iamapi/iamapi_group_handlers.go +++ b/weed/iamapi/iamapi_group_handlers.go @@ -228,21 +228,32 @@ func (iama *IamApiServer) ListGroupsForUser(s3cfg *iam_pb.S3ApiConfiguration, va if !userFound { return resp, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("user %s does not exist", userName)} } - for _, g := range s3cfg.Groups { - for _, m := range g.Members { - if m == userName { - name := g.Name - resp.ListGroupsForUserResult.Groups = append(resp.ListGroupsForUserResult.Groups, &iam.Group{GroupName: &name}) - break - } - } + // Build reverse index for efficient lookup + userGroupsIndex := buildUserGroupsIndex(s3cfg) + for _, gName := range userGroupsIndex[userName] { + name := gName + resp.ListGroupsForUserResult.Groups = append(resp.ListGroupsForUserResult.Groups, &iam.Group{GroupName: &name}) } return resp, nil } // removeUserFromAllGroups removes a user from all groups they belong to. +// Uses a reverse index for efficient lookup of which groups to modify. func removeUserFromAllGroups(s3cfg *iam_pb.S3ApiConfiguration, userName string) { + userGroupsIndex := buildUserGroupsIndex(s3cfg) + groupNames, found := userGroupsIndex[userName] + if !found { + return + } + // Build a set for fast group name lookup + targetGroups := make(map[string]bool, len(groupNames)) + for _, gn := range groupNames { + targetGroups[gn] = true + } for _, g := range s3cfg.Groups { + if !targetGroups[g.Name] { + continue + } for i, m := range g.Members { if m == userName { g.Members = append(g.Members[:i], g.Members[i+1:]...) @@ -276,6 +287,17 @@ func isPolicyAttachedToAnyGroup(s3cfg *iam_pb.S3ApiConfiguration, policyName str return "", false } +// buildUserGroupsIndex builds a reverse index mapping usernames to group names. +func buildUserGroupsIndex(s3cfg *iam_pb.S3ApiConfiguration) map[string][]string { + index := make(map[string][]string) + for _, g := range s3cfg.Groups { + for _, m := range g.Members { + index[m] = append(index[m], g.Name) + } + } + return index +} + // policyNameFromArn extracts policy name from ARN for standalone handlers. func policyNameFromArn(policyArn string) string { return strings.TrimPrefix(policyArn, policyArnPrefix) diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index 320d8e22e..8ea95b666 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -1657,14 +1657,13 @@ func (e *EmbeddedIamApi) ListGroupsForUser(s3cfg *iam_pb.S3ApiConfiguration, val if !userFound { return resp, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("user %s does not exist", userName)} } - for _, g := range s3cfg.Groups { - for _, m := range g.Members { - if m == userName { - name := g.Name - resp.ListGroupsForUserResult.Groups = append(resp.ListGroupsForUserResult.Groups, &iam.Group{GroupName: &name}) - break - } - } + // Use the in-memory reverse index for O(1) lookup + e.iam.m.RLock() + groupNames := e.iam.userGroups[userName] + e.iam.m.RUnlock() + for _, gName := range groupNames { + name := gName + resp.ListGroupsForUserResult.Groups = append(resp.ListGroupsForUserResult.Groups, &iam.Group{GroupName: &name}) } return resp, nil }