709944 - Expose groups of which a user is part

Status: RESOLVED FIXED

(Bugzilla :: WebService, enhancement)

Description

[Josh Matthews [:jdm]]

For a tool I'm writing which uses the BzAPI, I would to determine if a user is part of the 'default-assignees' group to check if a bug is actually assigned to a user or not. However, that information is not available to the existing API.

Comment 1

[David Lawrence [:dkl]]

Currently User.get does not export group information with User.get which could be added. User.get does have the side effect of allowing you to check if someone is *in* a partcular group if you already know the group name.

User.get({ names => ['[e-mail]'], groups => ['canconfirm'] });

It will return a user object if the user is in canconfirm, or an empty list if that user is not in the group.

I can look into updating User.get to return group information for each user. It should be filtered properly by include_fields, exclude_fields just like other attributes.

dkl

Comment 2

[David Lawrence [:dkl]]

Attachment: Patch to return groups each user is a member of in User.get (v1)

User.get returns new key called groups which is a list of group hashes each user is a member of. The groups are filtered based on whether the calling user is in editusers or can bless the group.

dkl

Comment 3

[:glob ✱]

Comment on attachment 595302 [details] [diff] [review]
Patch to return groups each user is a member of in User.get (v1)

r=glob

Comment 4

[Frédéric Buclin]

Comment on attachment 595302 [details] [diff] [review]
Patch to return groups each user is a member of in User.get (v1)

>+sub _group_to_hash {
>+    my ($self, $group) = @_;
>+    my $item = {
>+        id          => $self->type('int', $group->id), 
>+        name        => $self->type('string', $group->name), 
>+        description => $self->type('string', $group->description), 
>+    };
>+    return $item;
>+}

Is this really the right way to do it? Wouldn't it make more sense to only return group IDs and let the caller call Group.get (which hasn't been implemented yet) to get more info if needed?

Comment 5

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #4)
> Comment on attachment 595302 [details] [diff] [review]
> Patch to return groups each user is a member of in User.get (v1)
> 
> >+sub _group_to_hash {
> >+    my ($self, $group) = @_;
> >+    my $item = {
> >+        id          => $self->type('int', $group->id), 
> >+        name        => $self->type('string', $group->name), 
> >+        description => $self->type('string', $group->description), 
> >+    };
> >+    return $item;
> >+}
> 
> Is this really the right way to do it? Wouldn't it make more sense to only
> return group IDs and let the caller call Group.get (which hasn't been
> implemented yet) to get more info if needed?

Well that would create another separate call for each group returned especially if there are many. I can imagine the client really just wants the group names when they request the information so giving them that at the same time as the id I feel is harmless for now. Especially as Group.get is not yet implemented and people have asked about getting groups for a user sooner rather than later. Also name and description are already loaded even if we just return the id so there is no extra performance hit to do so.

dkl

Comment 6

[David Lawrence [:dkl]]

Apologize that these were two separate checkins. I updated the release notes as well.

trunk:

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 8121.                                                                                           

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified Bugzilla/WebService/User.pm
Committed revision 8122.

dkl

Comment 7

[Aakash Desai [:aakashd]]

Dkl & Glob, thanks for these commits. They're going to be really helpful ya'll.

Comment 8

[Frédéric Buclin]

(In reply to David Lawrence [:dkl] from comment #6)
> Committing to:
> bzr+ssh://[e-mail]/bugzilla/trunk
> modified template/en/default/pages/release-notes.html.tmpl
> Committed revision 8121.                                                    

Please backout this change. We cannot add anything about 4.4 before 4.4rc1 is released. You added notes to the 4.2 release notes!

Comment 9

[Gervase Markham [:gerv]]

Don't we have a thing that we don't reveal group names to people who are not members of that group, for security reasons - or have we dropped that?

If we have that, doesn't this patch break that constraint?

Gerv

Comment 10

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #9)
> Don't we have a thing that we don't reveal group names to people who are not
> members of that group, for security reasons - or have we dropped that?
> 
> If we have that, doesn't this patch break that constraint?

the patch has

> next unless ($user->in_group('editusers') || $user->can_bless($group->id));

which should restrict the group membership visibility.

Comment 11

[Frédéric Buclin]

Added to the relnotes for 4.4.

Comment 12

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/4.4/
modified t/webservice_user_get.t
Committed revision 240.