fix(group): Only org admin/owners and group maintainers can list group members #2269
Conversation
…p members Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
| } | ||
|
|
||
| // userHasPermissionToListGroupMember checks if the user has permission to list group members | ||
| func (g *GroupService) userHasPermissionToListGroupMember(ctx context.Context, orgID string, groupIdentifier *pb.IdentityReference) error { |
There was a problem hiding this comment.
Moved all these methods from servier to GroupService and this is the new one that checks the policy for listing members in a group
|
|
||
| // Validate requester permissions only if RequesterID is provided | ||
| if opts.RequesterID != uuid.Nil { | ||
| // Check if the requester is part of the organization |
There was a problem hiding this comment.
Yes, this code can be found in validateRequesterPermissions that's why I removed it. I must have left it here after some refactoring.
| IdentityReference: &biz.IdentityReference{}, | ||
| Maintainers: req.Maintainers, | ||
| MemberEmail: req.MemberEmail, | ||
| RequesterID: requesterUUID, |
There was a problem hiding this comment.
Is this just for testing? or is it needed for any use case?
There was a problem hiding this comment.
In the same way we have AddMembers and `RemoveMembers, I pass the requester to check the permissions on biz, only if present and to test it, yes. In this way we allow third party code to call this code without the problem of forcing a requester.
| } | ||
|
|
||
| // Validate requester permissions only if RequesterID is provided | ||
| if opts.RequesterID != uuid.Nil { |
There was a problem hiding this comment.
what's this requester part? When is it meant to not to exist?
There was a problem hiding this comment.
When the calling this method from a third party code. For example, we have another service calling AddMembers and RemoveMembers where we don't check the requester because it's a system request.
It's basically here to check permissions a part from the service
@jiparis was that covered in the permission matrix? We originally had this endpoint, |
This patch restricts access to the ListMembers endpoint so that only organization owners, admins, or maintainers of the specific group can view its members.