From 060d6a8414cfc0a77227497df112d09f46d83fa2 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Tue, 15 Sep 2026 17:05:45 +0200 Subject: [PATCH 01/14] Bug 2065173 - Migrate Group REST resource to native Mojo API --- Bugzilla/API/V1/Group.pm | 311 +++++++++ Bugzilla/WebService.pm | 2 - Bugzilla/WebService/Constants.pm | 1 - Bugzilla/WebService/Group.pm | 628 ------------------ Bugzilla/WebService/Server/REST.pm | 1 - .../WebService/Server/REST/Resources/Group.pm | 64 -- 6 files changed, 311 insertions(+), 696 deletions(-) create mode 100644 Bugzilla/API/V1/Group.pm delete mode 100644 Bugzilla/WebService/Group.pm delete mode 100644 Bugzilla/WebService/Server/REST/Resources/Group.pm diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm new file mode 100644 index 0000000000..f07f0e7bc6 --- /dev/null +++ b/Bugzilla/API/V1/Group.pm @@ -0,0 +1,311 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::API::V1::Group; + +use 5.10.1; +use Mojo::Base qw( Mojolicious::Controller ); + +use Mojo::JSON qw(decode_json true false); +use Try::Tiny; + +use Bugzilla::Constants; +use Bugzilla::Error; +use Bugzilla::Group; +use Bugzilla::User; +use Bugzilla::WebService::Util qw(params_to_objects translate validate); + +use constant MAPPED_RETURNS => + {userregexp => 'user_regexp', isactive => 'is_active'}; + +sub setup_routes { + my ($class, $r) = @_; + my $routes = $r->under( + '/group' => sub { Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); }); + $routes->get('/')->to('V1::Group#get'); + $routes->get('/:id')->to('V1::Group#get'); + $routes->post('/')->to('V1::Group#create'); + $routes->put('/:id')->to('V1::Group#update'); + + foreach my $path ('/', '/:id') { + $routes->options($path)->to('V1::Group#options'); + } +} + +sub options { + my ($self) = @_; + + $self->res->headers->header('Allow' => 'GET, POST, PUT'); + $self->res->headers->header('Access-Control-Allow-Methods' => 'GET, POST, PUT'); + + return $self->rendered(200); +} + +sub create { + my ($self) = @_; + + my $user = $self->bugzilla->login; + $user->id || return $self->user_error('login_required'); + $user->in_group('creategroups') + || return $self->user_error('auth_failure', + {group => 'creategroups', action => 'add', object => 'group'}); + + my $params = $self->_request_params; + + my $group = Bugzilla::Group->create({ + name => $params->{name}, + description => $params->{description}, + userregexp => $params->{user_regexp}, + isactive => $params->{is_active}, + isbuggroup => 1, + icon_url => $params->{icon_url}, + }); + + return $self->render(json => {id => 0 + $group->id}, status => 201); +} + +sub update { + my ($self) = @_; + + my $user = $self->bugzilla->login; + $user->id || return $self->user_error('login_required'); + $user->in_group('creategroups') + || return $self->user_error('auth_failure', + {group => 'creategroups', action => 'edit', object => 'group'}); + + my $params = $self->_request_params; + if (defined(my $id_or_name = $self->param('id'))) { + $params + = $id_or_name =~ /^\d+$/ + ? {%$params, ids => [$id_or_name], names => undef} + : {%$params, names => [$id_or_name], ids => undef}; + } + + defined($params->{names}) || defined($params->{ids}) + || return $self->code_error('params_required', + {function => 'Group.update', params => ['ids', 'names']}); + + my $group_objects = params_to_objects($params, 'Bugzilla::Group'); + + # Some groups are protected from being edited by non-admins. + foreach my $group (@$group_objects) { + $group->check_can_be_edited(); + } + + my %values = %$params; + delete $values{names}; + delete $values{ids}; + + my $dbh = Bugzilla->dbh; + $dbh->bz_start_transaction(); + foreach my $group (@$group_objects) { + $group->set_all(\%values); + } + + my %changes; + foreach my $group (@$group_objects) { + my $returned_changes = $group->update(); + $changes{$group->id} = translate($returned_changes, MAPPED_RETURNS); + } + $dbh->bz_commit_transaction(); + + my @result; + foreach my $group (@$group_objects) { + my %hash = (id => 0 + $group->id, changes => {}); + foreach my $field (keys %{$changes{$group->id}}) { + my $change = $changes{$group->id}->{$field}; + $hash{changes}{$field} + = {removed => "$change->[0]", added => "$change->[1]"}; + } + push(@result, \%hash); + } + + return $self->render(json => {groups => \@result}); +} + +sub get { + my ($self) = @_; + + my $user = $self->bugzilla->login; + $user->id || return $self->user_error('login_required'); + + my $params = $self->_request_params; + if (defined(my $id_or_name = $self->param('id'))) { + $params + = $id_or_name =~ /^\d+$/ + ? {%$params, ids => [$id_or_name]} + : {%$params, names => [$id_or_name]}; + } + (undef, $params) = validate($self, $params, qw(ids names type)); + + my $can_see_groups = $user->in_group('can_see_groups'); + return $self->user_error('group_cannot_view') + if !$can_see_groups && !$user->can_bless; + + Bugzilla->switch_to_shadow_db(); + + my $groups = []; + + if (defined $params->{ids}) { + + # Get the groups by id + $groups = Bugzilla::Group->new_from_list($params->{ids}); + } + + if (defined $params->{names}) { + + # Get the groups by name. check() will throw an error if a bad name is + # given. + foreach my $name (@{$params->{names}}) { + + # Skip if we got this from params->{ids} + next if grep { $_->name eq $name } @$groups; + + push @$groups, Bugzilla::Group->check({name => $name}); + } + } + + if (!defined $params->{ids} && !defined $params->{names}) { + if ($can_see_groups) { + @$groups = Bugzilla::Group->get_all; + } + else { + # Get only groups the user has bless privileges for. + $groups = $user->bless_groups; + } + } + + # Filter groups by blessability if user is not allowed to see all groups. + # NOTE: this mirrors a pre-existing quirk in the legacy WebService + # implementation: $user->can_bless() expects a group id, not a Group + # object, so this filter is a no-op that leaves $groups untouched in + # practice rather than actually filtering by blessability. + if (!$can_see_groups) { + $groups = [map { $user->can_bless($_) } @{$groups}]; + } + + my @result = map { $self->_group_to_hash($params, $_) } @$groups; + + return $self->render(json => {groups => \@result}); +} + +sub _group_to_hash { + my ($self, $params, $group) = @_; + my $user = Bugzilla->user; + + my $field_data + = {id => 0 + $group->id, name => $group->name, description => $group->description}; + + if ($user->in_group('creategroups')) { + $field_data->{is_active} = $group->is_active ? true : false; + $field_data->{is_bug_group} = $group->is_bug_group ? true : false; + $field_data->{user_regexp} = $group->user_regexp; + } + + if ($params->{membership}) { + $field_data->{membership} = $self->_get_group_membership($group); + } + + return $field_data; +} + +sub _get_group_membership { + my ($self, $group) = @_; + my $user = Bugzilla->user; + + my $dbh = Bugzilla->dbh; + my $editusers = $user->in_group('editusers'); + + my $query = 'SELECT userid FROM profiles'; + my $visible_groups; + + if (!$editusers && Bugzilla->params->{usevisibilitygroups}) { + + # Show only users in visible groups. + $visible_groups = $user->visible_groups_inherited; + + if (scalar @$visible_groups) { + $query .= qq{, user_group_map AS ugm + WHERE ugm.user_id = profiles.userid + AND ugm.isbless = 0 + AND } . $dbh->sql_in('ugm.group_id', $visible_groups); + } + } + elsif ($editusers + || $user->can_bless($group->id) + || $user->in_group('creategroups')) + { + $visible_groups = 1; + $query .= qq{, user_group_map AS ugm + WHERE ugm.user_id = profiles.userid + AND ugm.isbless = 0 + }; + } + + # Use ThrowUserError (not $self->user_error) so an invisible group aborts + # the request cleanly even though this runs nested inside the map() in + # get() -- returning a rendered response from here would leave get()'s + # own render() call to fire a second time. + ThrowUserError('group_not_visible', {group => $group}) unless $visible_groups; + + my $grouplist = Bugzilla::Group->flatten_group_membership($group->id); + $query .= ' AND ' . $dbh->sql_in('ugm.group_id', $grouplist); + + my $userids = $dbh->selectcol_arrayref($query); + my $user_objects = Bugzilla::User->new_from_list($userids); + + return [ + map { + { + id => 0 + $_->id, + real_name => $_->name, + nick => $_->nick, + name => $_->login, + email => $_->email, + can_login => $_->is_enabled ? true : false, + email_enabled => $_->email_enabled ? true : false, + login_denied_text => $_->disabledtext, + } + } @$user_objects + ]; +} + +sub _request_params { + my ($self) = @_; + + # $self->req->params already covers the query string plus, for POST/PUT, + # an application/x-www-form-urlencoded or multipart body. Layer a JSON + # body underneath that (silently ignored if absent or not valid JSON) so + # params work from either the query string or a JSON request body. + # Query-string values win on a key collision, matching the legacy REST + # layer and the documented behavior in docs/en/rst/api/core/v1/general.rst. + my $params = $self->req->params->to_hash; + + if (length $self->req->body) { + my $body_params; + try { $body_params = decode_json($self->req->body); } + catch { $body_params = undef; }; + $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; + } + + return $params; +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::API::V1::Group - The API for creating, changing, and getting +information about Groups. + +=head1 DESCRIPTION + +This part of the Bugzilla API allows you to create Groups and get +information about them. See L for the +public REST documentation. diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm index 6df5c9f6a3..b908f01340 100644 --- a/Bugzilla/WebService.pm +++ b/Bugzilla/WebService.pm @@ -413,8 +413,6 @@ objects. =item L -=item L - =item L =item L diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 7b53afe215..2ee842e2f8 100644 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -316,7 +316,6 @@ sub WS_DISPATCH { 'Bug' => 'Bugzilla::WebService::Bug', 'User' => 'Bugzilla::WebService::User', 'Product' => 'Bugzilla::WebService::Product', - 'Group' => 'Bugzilla::WebService::Group', 'BugUserLastVisit' => 'Bugzilla::WebService::BugUserLastVisit', %hook_dispatch }; diff --git a/Bugzilla/WebService/Group.pm b/Bugzilla/WebService/Group.pm deleted file mode 100644 index abda27f8b6..0000000000 --- a/Bugzilla/WebService/Group.pm +++ /dev/null @@ -1,628 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -# This Source Code Form is "Incompatible With Secondary Licenses", as -# defined by the Mozilla Public License, v. 2.0. - -package Bugzilla::WebService::Group; - -use 5.10.1; -use strict; -use warnings; - -use base qw(Bugzilla::WebService); -use Bugzilla::Constants; -use Bugzilla::Error; -use Bugzilla::WebService::Util qw(validate translate params_to_objects); - -use constant PUBLIC_METHODS => qw( - create - get - update -); - -use constant MAPPED_RETURNS => - {userregexp => 'user_regexp', isactive => 'is_active'}; - -sub create { - my ($self, $params) = @_; - - Bugzilla->login(LOGIN_REQUIRED); - Bugzilla->user->in_group('creategroups') - || ThrowUserError("auth_failure", - {group => "creategroups", action => "add", object => "group"}); - - # Create group - my $group = Bugzilla::Group->create({ - name => $params->{name}, - description => $params->{description}, - userregexp => $params->{user_regexp}, - isactive => $params->{is_active}, - isbuggroup => 1, - icon_url => $params->{icon_url} - }); - return {id => $self->type('int', $group->id)}; -} - -sub update { - my ($self, $params) = @_; - - my $dbh = Bugzilla->dbh; - - Bugzilla->login(LOGIN_REQUIRED); - Bugzilla->user->in_group('creategroups') - || ThrowUserError("auth_failure", - {group => "creategroups", action => "edit", object => "group"}); - - defined($params->{names}) - || defined($params->{ids}) - || ThrowCodeError('params_required', - {function => 'Group.update', params => ['ids', 'names']}); - - my $group_objects = params_to_objects($params, 'Bugzilla::Group'); - - # Some groups are protected from being edited by non-admins. - foreach my $group (@$group_objects) { - $group->check_can_be_edited(); - } - - my %values = %$params; - - # We delete names and ids to keep only new values to set. - delete $values{names}; - delete $values{ids}; - - $dbh->bz_start_transaction(); - foreach my $group (@$group_objects) { - $group->set_all(\%values); - } - - my %changes; - foreach my $group (@$group_objects) { - my $returned_changes = $group->update(); - $changes{$group->id} = translate($returned_changes, MAPPED_RETURNS); - } - $dbh->bz_commit_transaction(); - - my @result; - foreach my $group (@$group_objects) { - my %hash = (id => $group->id, changes => {},); - foreach my $field (keys %{$changes{$group->id}}) { - my $change = $changes{$group->id}->{$field}; - $hash{changes}{$field} = { - removed => $self->type('string', $change->[0]), - added => $self->type('string', $change->[1]) - }; - } - push(@result, \%hash); - } - - return {groups => \@result}; -} - -sub get { - my ($self, $params) = validate(@_, 'ids', 'names', 'type'); - - Bugzilla->login(LOGIN_REQUIRED); - - # Reject access if there is no sense in continuing. - my $user = Bugzilla->user; - my $can_see_groups = $user->in_group('can_see_groups'); - if (!$can_see_groups && !$user->can_bless) { - ThrowUserError('group_cannot_view'); - } - - Bugzilla->switch_to_shadow_db(); - - my $groups = []; - - if (defined $params->{ids}) { - - # Get the groups by id - $groups = Bugzilla::Group->new_from_list($params->{ids}); - } - - if (defined $params->{names}) { - - # Get the groups by name. Check will throw an error if a bad name is given - foreach my $name (@{$params->{names}}) { - - # Skip if we got this from params->{id} - next if grep { $_->name eq $name } @$groups; - - push @$groups, Bugzilla::Group->check({name => $name}); - } - } - - if (!defined $params->{ids} && !defined $params->{names}) { - if ($can_see_groups) { - @$groups = Bugzilla::Group->get_all; - } - else { - # Get only groups the user has bless groups too - $groups = $user->bless_groups; - } - } - - # Filter groups by blessability if user is not allowed to see all groups - if (!$can_see_groups) { - $groups = [map { $user->can_bless($_) } @{$groups}]; - } - - # Now create a result entry for each. - my @groups = map { $self->_group_to_hash($params, $_) } @$groups; - return {groups => \@groups}; -} - -sub _group_to_hash { - my ($self, $params, $group) = @_; - my $user = Bugzilla->user; - - my $field_data = { - id => $self->type('int', $group->id), - name => $self->type('string', $group->name), - description => $self->type('string', $group->description), - }; - - if ($user->in_group('creategroups')) { - $field_data->{is_active} = $self->type('boolean', $group->is_active); - $field_data->{is_bug_group} = $self->type('boolean', $group->is_bug_group); - $field_data->{user_regexp} = $self->type('string', $group->user_regexp); - } - - if ($params->{membership}) { - $field_data->{membership} = $self->_get_group_membership($group, $params); - } - return $field_data; -} - -sub _get_group_membership { - my ($self, $group, $params) = @_; - my $user = Bugzilla->user; - - my %users_only; - my $dbh = Bugzilla->dbh; - my $editusers = $user->in_group('editusers'); - - my $query = 'SELECT userid FROM profiles'; - my $visibleGroups; - - if (!$editusers && Bugzilla->params->{'usevisibilitygroups'}) { - - # Show only users in visible groups. - $visibleGroups = $user->visible_groups_inherited; - - if (scalar @$visibleGroups) { - $query .= qq{, user_group_map AS ugm - WHERE ugm.user_id = profiles.userid - AND ugm.isbless = 0 - AND } . $dbh->sql_in('ugm.group_id', $visibleGroups); - } - } - elsif ($editusers - || $user->can_bless($group->id) - || $user->in_group('creategroups')) - { - $visibleGroups = 1; - $query .= qq{, user_group_map AS ugm - WHERE ugm.user_id = profiles.userid - AND ugm.isbless = 0 - }; - } - if (!$visibleGroups) { - ThrowUserError('group_not_visible', {group => $group}); - } - - my $grouplist = Bugzilla::Group->flatten_group_membership($group->id); - $query .= ' AND ' . $dbh->sql_in('ugm.group_id', $grouplist); - - my $userids = $dbh->selectcol_arrayref($query); - my $user_objects = Bugzilla::User->new_from_list($userids); - my @users = map { { - id => $self->type('int', $_->id), - real_name => $self->type('string', $_->name), - nick => $self->type('string', $_->nick), - name => $self->type('string', $_->login), - email => $self->type('string', $_->email), - can_login => $self->type('boolean', $_->is_enabled), - email_enabled => $self->type('boolean', $_->email_enabled), - login_denied_text => $self->type('string', $_->disabledtext), - } } @$user_objects; - - return \@users; -} - -1; - -__END__ - -=head1 NAME - -Bugzilla::Webservice::Group - The API for creating, changing, and getting -information about Groups. - -=head1 DESCRIPTION - -This part of the Bugzilla API allows you to create Groups and -get information about them. - -=head1 METHODS - -See L for a description of how parameters are passed, -and what B, B, and B mean. - -Although the data input and output is the same for JSON-RPC and REST, -the directions for how to access the data via REST is noted in each method -where applicable. - -=head1 Group Creation and Modification - -=head2 create - -B - -=over - -=item B - -This allows you to create a new group in Bugzilla. - -=item B - -POST /rest/group - -The params to include in the POST body as well as the returned data format, -are the same as below. - -=item B - -Some params must be set, or an error will be thrown. These params are -marked B. - -=over - -=item C - -B C A short name for this group. Must be unique. This -is not usually displayed in the user interface, except in a few places. - -=item C - -B C A human-readable name for this group. Should be -relatively short. This is what will normally appear in the UI as the -name of the group. - -=item C - -C A regular expression. Any user whose Bugzilla username matches -this regular expression will automatically be granted membership in this group. - -=item C - -C C if new group can be used for bugs, C if this -is a group that will only contain users and no bugs will be restricted -to it. - -=item C - -C A URL pointing to a small icon used to identify the group. -This icon will show up next to users' names in various parts of Bugzilla -if they are in this group. - -=back - -=item B - -A hash with one element, C. This is the id of the newly-created group. - -=item B - -=over - -=item 800 (Empty Group Name) - -You must specify a value for the C field. - -=item 801 (Group Exists) - -There is already another group with the same C. - -=item 802 (Group Missing Description) - -You must specify a value for the C field. - -=item 803 (Group Regexp Invalid) - -You specified an invalid regular expression in the C field. - -=back - -=item B - -=over - -=item REST API call added in Bugzilla B<5.0>. - -=back - -=back - -=head2 update - -B - -=over - -=item B - -This allows you to update a group in Bugzilla. - -You must be in the C group. Additionally, the C group can -only be updated by members of the C group, and the insider group can -only be updated by members of the C or insider groups. - -=item B - -PUT /rest/group/ - -The params to include in the PUT body as well as the returned data format, -are the same as below. The C param will be overridden as it is pulled -from the URL path. - -=item B - -At least C or C must be set, or an error will be thrown. - -=over - -=item C - -B C Contain ids of groups to update. - -=item C - -B C Contain names of groups to update. - -=item C - -C A new name for group. - -=item C - -C A new description for groups. This is what will appear in the UI -as the name of the groups. - -=item C - -C A new regular expression for email. Will automatically grant -membership to these groups to anyone with an email address that matches -this Perl regular expression. - -=item C - -C Set if groups are active and eligible to be used for bugs. -True if bugs can be restricted to this group, false otherwise. - -=item C - -C A URL pointing to an icon that will appear next to the name of -users who are in this group. - -=back - -=item B - -A C with a single field "groups". This points to an array of hashes -with the following fields: - -=over - -=item C - -C The id of the group that was updated. - -=item C - -C The changes that were actually done on this group. The keys are -the names of the fields that were changed, and the values are a hash -with two keys: - -=over - -=item C - -C The values that were added to this field, -possibly a comma-and-space-separated list if multiple values were added. - -=item C - -C The values that were removed from this field, possibly a -comma-and-space-separated list if multiple values were removed. - -=back - -=back - -=item B - -The same as L. - -=item B - -=over - -=item REST API call added in Bugzilla B<5.0>. - -=back - -=back - -=head1 Group Information - -=head2 get - -B - -=over - -=item B - -Returns information about L. - -=item B - -To return information about a specific group by C or C: - -GET /rest/group/ - -You can also return information about more than one specific group -by using the following in your query string: - -GET /rest/group?ids=1&ids=2&ids=3 or GET /group?names=ProductOne&names=Product2 - -the returned data format is same as below. - -=item B - -If neither ids or names is passed, and you are in the creategroups or -editusers group, then all groups will be retrieved. Otherwise, only groups -that you have bless privileges for will be returned. - -=over - -=item C - -C Contain ids of groups to update. - -=item C - -C Contain names of groups to update. - -=item C - -C Set to 1 then a list of members of the passed groups' names and -ids will be returned. - -=back - -=item B - -If the user is a member of the "creategroups" group they will receive -information about all groups or groups matching the criteria that they passed. -You have to be in the creategroups group unless you're requesting membership -information. - -If the user is not a member of the "creategroups" group, but they are in the -"editusers" group or have bless privileges to the groups they require -membership information for, the is_active, is_bug_group and user_regexp values -are not supplied. - -The return value will be a hash containing group names as the keys, each group -name will point to a hash that describes the group and has the following items: - -=over - -=item id - -C The unique integer ID that Bugzilla uses to identify this group. -Even if the name of the group changes, this ID will stay the same. - -=item name - -C The name of the group. - -=item description - -C The description of the group. - -=item is_bug_group - -C Whether this groups is to be used for bug reports or is only administrative specific. - -=item user_regexp - -C A regular expression that allows users to be added to this group if their login matches. - -=item is_active - -C Whether this group is currently active or not. - -=item users - -C An array of hashes, each hash contains a user object for one of the -members of this group, only returned if the user sets the C -parameter to 1, the user hash has the following items: - -=over - -=item id - -C The id of the user. - -=item real_name - -C The actual name of the user. - -=item nick - -C The user's nickname. Currently this is extracted from the real_name, -name or email field. - -=item email - -C The email address of the user. - -=item name - -C The login name of the user. Note that in some situations this is -different than their email. - -=item can_login - -C A boolean value to indicate if the user can login into Bugzilla. - -=item email_enabled - -C A boolean value to indicate if bug-related mail will be sent -to the user or not. - -=item disabled_text - -C A text field that holds the reason for disabling a user from logging -into Bugzilla, if empty then the user account is enabled otherwise it is -disabled/closed. - -=back - -=back - -=item B - -=over - -=item 51 (Invalid Object) - -A non existing group name was passed to the function, as a result no -group object existed for that invalid name. - -=item 805 (Cannot view groups) - -Logged-in users are not authorized to edit Bugzilla groups as they are not -members of the creategroups group in Bugzilla, or they are not authorized to -access group member's information as they are not members of the "editusers" -group or can bless the group. - -=back - -=item B - -=over - -=item This function was added in Bugzilla B<5.0>. - -=back - -=back - -=cut diff --git a/Bugzilla/WebService/Server/REST.pm b/Bugzilla/WebService/Server/REST.pm index 862a8a35b0..72047ca41a 100644 --- a/Bugzilla/WebService/Server/REST.pm +++ b/Bugzilla/WebService/Server/REST.pm @@ -23,7 +23,6 @@ use Bugzilla::WebService::Util qw(fix_credentials set_rest_cors_headers taint_da # Load resource modules use Bugzilla::WebService::Server::REST::Resources::Bug; -use Bugzilla::WebService::Server::REST::Resources::Group; use Bugzilla::WebService::Server::REST::Resources::Product; use Bugzilla::WebService::Server::REST::Resources::User; use Bugzilla::WebService::Server::REST::Resources::BugUserLastVisit; diff --git a/Bugzilla/WebService/Server/REST/Resources/Group.pm b/Bugzilla/WebService/Server/REST/Resources/Group.pm deleted file mode 100644 index b6a1b9b34d..0000000000 --- a/Bugzilla/WebService/Server/REST/Resources/Group.pm +++ /dev/null @@ -1,64 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -# This Source Code Form is "Incompatible With Secondary Licenses", as -# defined by the Mozilla Public License, v. 2.0. - -package Bugzilla::WebService::Server::REST::Resources::Group; - -use 5.10.1; -use strict; -use warnings; - -use Bugzilla::WebService::Constants; -use Bugzilla::WebService::Group; - -BEGIN { - *Bugzilla::WebService::Group::rest_resources = \&_rest_resources; -} - -sub _rest_resources { - my $rest_resources = [ - qr{^/group$}, - { - GET => {method => 'get'}, - POST => {method => 'create', success_code => STATUS_CREATED} - }, - qr{^/group/([^/]+)$}, - { - GET => { - method => 'get', - params => sub { - my $param = $_[0] =~ /^\d+$/ ? 'ids' : 'names'; - return {$param => [$_[0]]}; - } - }, - PUT => { - method => 'update', - params => sub { - my $param = $_[0] =~ /^\d+$/ ? 'ids' : 'names'; - return {$param => [$_[0]]}; - } - } - } - ]; - return $rest_resources; -} - -1; - -__END__ - -=head1 NAME - -Bugzilla::Webservice::Server::REST::Resources::Group - The REST API for -creating, changing, and getting information about Groups. - -=head1 DESCRIPTION - -This part of the Bugzilla REST API allows you to create Groups and -get information about them. - -See L for more details on how to use this part -of the REST API. From 740a663b846e3df5b61f8e85a8cdeedfcaa87d9d Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Tue, 15 Sep 2026 19:43:32 +0200 Subject: [PATCH 02/14] Bug 2065173 - Fix i_am_webservice() to recognize USAGE_MODE_MOJO_REST Error messages from native-Mojo REST controllers were being word-wrapped at 72 columns, introducing literal newlines that broke message-content assertions like rest_group_create.t. --- Bugzilla/Util.pm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 7c3e4f8928..d9570aba05 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -298,7 +298,9 @@ sub i_am_cgi { sub i_am_webservice { my $usage_mode = Bugzilla->usage_mode; - return $usage_mode == USAGE_MODE_JSON || $usage_mode == USAGE_MODE_REST; + return $usage_mode == USAGE_MODE_JSON + || $usage_mode == USAGE_MODE_REST + || $usage_mode == USAGE_MODE_MOJO_REST; } sub is_webserver_group { From 5aa903cbc70b6ed36cf50b9c573198a79fff47e8 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Tue, 15 Sep 2026 20:18:23 +0200 Subject: [PATCH 03/14] Bug 2065173 - Fix object => 'group' typo in Group.create/update auth_failure The 'auth_failure' error template only has a branch for 'groups' (plural), 'group' fell through to the empty default, so the unauthorized-user message read "...not authorized to add new ." with the object word missing. --- Bugzilla/API/V1/Group.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index f07f0e7bc6..bc242e6de2 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -52,7 +52,7 @@ sub create { $user->id || return $self->user_error('login_required'); $user->in_group('creategroups') || return $self->user_error('auth_failure', - {group => 'creategroups', action => 'add', object => 'group'}); + {group => 'creategroups', action => 'add', object => 'groups'}); my $params = $self->_request_params; @@ -75,7 +75,7 @@ sub update { $user->id || return $self->user_error('login_required'); $user->in_group('creategroups') || return $self->user_error('auth_failure', - {group => 'creategroups', action => 'edit', object => 'group'}); + {group => 'creategroups', action => 'edit', object => 'groups'}); my $params = $self->_request_params; if (defined(my $id_or_name = $self->param('id'))) { From bedeacc8bfc57d2d7af5686a22b71dcd36eb88cc Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 14:52:04 +0200 Subject: [PATCH 04/14] Bug 2065173 - Use shared merge_request_params helper _request_params duplicated the same query-string/JSON-body merge logic already written for BugUserLastVisit.pm (bug 2065171). Now call a single shared merge_request_params helper, so it's a one-place change to drop later if query-string-on-POST support is ever removed. Please note that BugUserLastVisit.pm (bug 2065171) is being updated separately to call the same helper instead of its own copy. --- Bugzilla/API/V1/Group.pm | 32 +++++--------------------------- Bugzilla/WebService/Util.pm | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index bc242e6de2..16059970ba 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -10,14 +10,13 @@ package Bugzilla::API::V1::Group; use 5.10.1; use Mojo::Base qw( Mojolicious::Controller ); -use Mojo::JSON qw(decode_json true false); -use Try::Tiny; +use Mojo::JSON qw(true false); use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Group; use Bugzilla::User; -use Bugzilla::WebService::Util qw(params_to_objects translate validate); +use Bugzilla::WebService::Util qw(merge_request_params params_to_objects translate validate); use constant MAPPED_RETURNS => {userregexp => 'user_regexp', isactive => 'is_active'}; @@ -54,7 +53,7 @@ sub create { || return $self->user_error('auth_failure', {group => 'creategroups', action => 'add', object => 'groups'}); - my $params = $self->_request_params; + my $params = merge_request_params($self); my $group = Bugzilla::Group->create({ name => $params->{name}, @@ -77,7 +76,7 @@ sub update { || return $self->user_error('auth_failure', {group => 'creategroups', action => 'edit', object => 'groups'}); - my $params = $self->_request_params; + my $params = merge_request_params($self); if (defined(my $id_or_name = $self->param('id'))) { $params = $id_or_name =~ /^\d+$/ @@ -133,7 +132,7 @@ sub get { my $user = $self->bugzilla->login; $user->id || return $self->user_error('login_required'); - my $params = $self->_request_params; + my $params = merge_request_params($self); if (defined(my $id_or_name = $self->param('id'))) { $params = $id_or_name =~ /^\d+$/ @@ -274,27 +273,6 @@ sub _get_group_membership { ]; } -sub _request_params { - my ($self) = @_; - - # $self->req->params already covers the query string plus, for POST/PUT, - # an application/x-www-form-urlencoded or multipart body. Layer a JSON - # body underneath that (silently ignored if absent or not valid JSON) so - # params work from either the query string or a JSON request body. - # Query-string values win on a key collision, matching the legacy REST - # layer and the documented behavior in docs/en/rst/api/core/v1/general.rst. - my $params = $self->req->params->to_hash; - - if (length $self->req->body) { - my $body_params; - try { $body_params = decode_json($self->req->body); } - catch { $body_params = undef; }; - $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; - } - - return $params; -} - 1; __END__ diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 32b34a4bb0..f7ade2aecb 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -21,6 +21,8 @@ use Storable qw(dclone); use URI::Escape qw(uri_unescape); use Type::Params qw( compile ); use Types::Standard -all; +use Mojo::JSON qw(decode_json); +use Try::Tiny; use base qw(Exporter); @@ -38,6 +40,7 @@ our @EXPORT_OK = qw( params_to_objects fix_credentials set_rest_cors_headers + merge_request_params ); sub set_rest_cors_headers { @@ -297,6 +300,29 @@ sub params_to_objects { return \@objects; } +sub merge_request_params { + my ($c) = @_; + + # $c->req->params already covers the query string plus, for POST/PUT, an + # application/x-www-form-urlencoded or multipart body. Layer a JSON body + # underneath that (silently ignored if absent or not valid JSON), so + # params work from either the query string or a JSON request body. + # Query-string values win on a key collision, matching the legacy REST + # layer (see fix_credentials/_retrieve_json_params in + # Bugzilla::WebService::Server::REST) and the documented behavior in + # docs/en/rst/api/core/v1/general.rst. + my $params = $c->req->params->to_hash; + + if (length $c->req->body) { + my $body_params; + try { $body_params = decode_json($c->req->body); } + catch { $body_params = undef; }; + $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; + } + + return $params; +} + sub fix_credentials { my ($params, $cgi) = @_; @@ -399,6 +425,15 @@ Helps make life simpler for WebService methods that internally create objects via both "ids" and "names" fields. Also de-duplicates objects that were loaded by both "ids" and "names". Returns an arrayref of objects. +=head2 merge_request_params + +Takes a Mojolicious controller and returns a hashref merging its query +string/form-body params (C<< $c->req->params->to_hash >>) with a decoded +JSON request body, if any. Query-string/form-body values win on a key +collision. For use by native Mojo REST controllers that need to accept +parameters from either the query string or a JSON body on non-GET +requests. + =head2 fix_credentials Allows for certain parameters related to authentication such as Bugzilla_login, From 9bf9dd8b8be6049a93aa82de6c88427a69d85f8e Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 16:54:32 +0200 Subject: [PATCH 05/14] Bug 2065173 - Fix crash filtering groups by blessability can_bless() takes a group id, not a Group object. Passing the object made every entry falsy, and the next line's ->id call on that died. Reachable by bless-privileged users without can_see_groups. Was ported faithfully from legacy as a described no-op, although it's actually a genuine crash, so fixing it here. --- Bugzilla/API/V1/Group.pm | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index 16059970ba..e21b84e4aa 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -179,12 +179,12 @@ sub get { } # Filter groups by blessability if user is not allowed to see all groups. - # NOTE: this mirrors a pre-existing quirk in the legacy WebService - # implementation: $user->can_bless() expects a group id, not a Group - # object, so this filter is a no-op that leaves $groups untouched in - # practice rather than actually filtering by blessability. + # can_bless() takes a group id, not a Group object -- the legacy + # WebService code passed the object itself here, which is always false, + # so it wasn't actually filtering anything. Passing the id instead makes + # the filter do what the surrounding comment always claimed it did. if (!$can_see_groups) { - $groups = [map { $user->can_bless($_) } @{$groups}]; + $groups = [grep { $user->can_bless($_->id) } @{$groups}]; } my @result = map { $self->_group_to_hash($params, $_) } @$groups; From 6712d877cbe02993a95e87d64cbe77f9b576d194 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 17:10:31 +0200 Subject: [PATCH 06/14] Bug 2065173 - Whitelist update() fields instead of blacklisting names/ids set_all() throws unknown_method for any stray key without a matching set_ method. Cookie-authenticated PUT hit this via Bugzilla_api_token (legacy deleted it before the method ran, the Mojo cookie-auth path doesn't), include_fields/exclude_fields hit it too. Whitelist the update fields instead. --- Bugzilla/API/V1/Group.pm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index e21b84e4aa..98fe8d15cb 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -95,9 +95,11 @@ sub update { $group->check_can_be_edited(); } - my %values = %$params; - delete $values{names}; - delete $values{ids}; + # Whitelist the documented update fields; set_all() throws unknown_method + # for any stray key (e.g. Bugzilla_api_token, include_fields). + my %values = map { $_ => $params->{$_} } + grep { exists $params->{$_} } + qw(name description user_regexp is_active icon_url); my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction(); From 26c885eb95fd7ccf1a33d0c8947511f0b90e7e01 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 17:22:50 +0200 Subject: [PATCH 07/14] Bug 2065173 - Use relaxed #id placeholder to allow dots in group names --- Bugzilla/API/V1/Group.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index 98fe8d15cb..55e8df3014 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -26,11 +26,11 @@ sub setup_routes { my $routes = $r->under( '/group' => sub { Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); }); $routes->get('/')->to('V1::Group#get'); - $routes->get('/:id')->to('V1::Group#get'); + $routes->get('/#id')->to('V1::Group#get'); $routes->post('/')->to('V1::Group#create'); - $routes->put('/:id')->to('V1::Group#update'); + $routes->put('/#id')->to('V1::Group#update'); - foreach my $path ('/', '/:id') { + foreach my $path ('/', '/#id') { $routes->options($path)->to('V1::Group#options'); } } From 44d1ec9d1b0bb25bd5637141f92571cf282907ef Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 18:02:07 +0200 Subject: [PATCH 08/14] Bug 2065173 - Preserve null in changes when a field goes to/from undef --- Bugzilla/API/V1/Group.pm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index 55e8df3014..fb2a5c0a1c 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -119,8 +119,10 @@ sub update { my %hash = (id => 0 + $group->id, changes => {}); foreach my $field (keys %{$changes{$group->id}}) { my $change = $changes{$group->id}->{$field}; - $hash{changes}{$field} - = {removed => "$change->[0]", added => "$change->[1]"}; + $hash{changes}{$field} = { + removed => defined $change->[0] ? "$change->[0]" : undef, + added => defined $change->[1] ? "$change->[1]" : undef, + }; } push(@result, \%hash); } From f3766b7a30e35dc8e9d8124bbb5fdebc4bf38cc4 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 21:24:23 +0200 Subject: [PATCH 09/14] Bug 2065173 - Adapt to merge_request_params returning ($params, $error) The shared helper landed in master with a two-element return, so calling it in scalar context assigned the error rather than the params. create/update/get now unpack both and report a malformed JSON body via user_error. --- Bugzilla/API/V1/Group.pm | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index fb2a5c0a1c..ecd030e5c1 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -53,7 +53,8 @@ sub create { || return $self->user_error('auth_failure', {group => 'creategroups', action => 'add', object => 'groups'}); - my $params = merge_request_params($self); + my ($params, $error) = merge_request_params($self); + return $self->user_error($error) if $error; my $group = Bugzilla::Group->create({ name => $params->{name}, @@ -76,7 +77,9 @@ sub update { || return $self->user_error('auth_failure', {group => 'creategroups', action => 'edit', object => 'groups'}); - my $params = merge_request_params($self); + my ($params, $error) = merge_request_params($self); + return $self->user_error($error) if $error; + if (defined(my $id_or_name = $self->param('id'))) { $params = $id_or_name =~ /^\d+$/ @@ -136,7 +139,9 @@ sub get { my $user = $self->bugzilla->login; $user->id || return $self->user_error('login_required'); - my $params = merge_request_params($self); + my ($params, $error) = merge_request_params($self); + return $self->user_error($error) if $error; + if (defined(my $id_or_name = $self->param('id'))) { $params = $id_or_name =~ /^\d+$/ From b8bbbfad223ccccb5045fd6e3721ae9bbee0d54f Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 21:47:25 +0200 Subject: [PATCH 10/14] Bug 2065173 - Let unknown update fields error instead of dropping them Whitelisting the documented fields made a typo or an unsupported field a silent no-op returning 200 with empty changes. Delete the request-level keys and pass the rest to set_all(), which still raises unknown_method. --- Bugzilla/API/V1/Group.pm | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index ecd030e5c1..5bfa63a304 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -98,11 +98,14 @@ sub update { $group->check_can_be_edited(); } - # Whitelist the documented update fields; set_all() throws unknown_method - # for any stray key (e.g. Bugzilla_api_token, include_fields). - my %values = map { $_ => $params->{$_} } - grep { exists $params->{$_} } - qw(name description user_regexp is_active icon_url); + # Drop the request-level keys that are not group fields and pass everything + # else through, so set_all() still raises unknown_method on an unrecognized + # field rather than silently ignoring it. + my %values = %$params; + delete @values{ + qw(ids names include_fields exclude_fields + Bugzilla_api_key Bugzilla_api_token Bugzilla_login Bugzilla_password) + }; my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction(); From 581c9c57075d4bbd042d402b3ddfd33b9c779c94 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 21:49:10 +0200 Subject: [PATCH 11/14] Bug 2065173 - Advertise the methods each group route actually serves Both OPTIONS routes shared one Allow value, so /rest/group advertised PUT and /rest/group/ advertised POST, neither of which exists. The allowed methods now come from the route, which also fixes Access-Control-Allow-Methods. --- Bugzilla/API/V1/Group.pm | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index 5bfa63a304..86560adfa0 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -30,16 +30,16 @@ sub setup_routes { $routes->post('/')->to('V1::Group#create'); $routes->put('/#id')->to('V1::Group#update'); - foreach my $path ('/', '/#id') { - $routes->options($path)->to('V1::Group#options'); - } + $routes->options('/')->to('V1::Group#options', allow => 'GET, POST'); + $routes->options('/#id')->to('V1::Group#options', allow => 'GET, PUT'); } sub options { my ($self) = @_; - $self->res->headers->header('Allow' => 'GET, POST, PUT'); - $self->res->headers->header('Access-Control-Allow-Methods' => 'GET, POST, PUT'); + my $allow = $self->stash('allow'); + $self->res->headers->header('Allow' => $allow); + $self->res->headers->header('Access-Control-Allow-Methods' => $allow); return $self->rendered(200); } From c0b0db2a806da6443587479dc881ab5b105154ec Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 21:51:07 +0200 Subject: [PATCH 12/14] Bug 2065173 - Correct the blessability comment and cover the fixed case The legacy code mapped rather than grepped can_bless($group_object), so every element became 0 and _group_to_hash called ->id on it: a 500 for a blesser without can_see_groups, not the no-op the comment claimed. --- Bugzilla/API/V1/Group.pm | 11 ++++++----- qa/t/rest_group_get.t | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index 86560adfa0..dd307e183d 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -190,11 +190,12 @@ sub get { } } - # Filter groups by blessability if user is not allowed to see all groups. - # can_bless() takes a group id, not a Group object -- the legacy - # WebService code passed the object itself here, which is always false, - # so it wasn't actually filtering anything. Passing the id instead makes - # the filter do what the surrounding comment always claimed it did. + # Filter groups by blessability if the user is not allowed to see all + # groups. can_bless() takes a group id, not a Group object. The legacy + # WebService code mapped (not grepped) can_bless($group_object) over the + # list; that numifies the ref and always returns 0, so every element became + # 0 and _group_to_hash then called ->id on it, i.e. any blesser without + # can_see_groups got a 500. Passing the id filters the list instead. if (!$can_see_groups) { $groups = [grep { $user->can_bless($_->id) } @{$groups}]; } diff --git a/qa/t/rest_group_get.t b/qa/t/rest_group_get.t index 42ca30a8d0..39365df2a7 100644 --- a/qa/t/rest_group_get.t +++ b/qa/t/rest_group_get.t @@ -11,6 +11,7 @@ use 5.10.1; use lib qw(lib ../../lib ../../local/lib/perl5); use Bugzilla; +use Bugzilla::Constants; use QA::Util qw(get_config); use Mojo::JSON qw(true); @@ -95,4 +96,39 @@ $t->put_ok($url => json => {groups => {remove => ['can_see_groups']}})->status_is(200) ->json_has('/users'); +# A user who can bless one group, but is not in can_see_groups, asking for a +# different group: the blessability filter must leave it out rather than blow +# up. The legacy code mapped can_bless($group_object) over the list, which +# always returned 0, and _group_to_hash then called ->id on that 0. + +my $bless_group = { + name => 'bless-only-group', + description => 'Blessable, but not visible', + is_active => true +}; +$t->post_ok($url + . 'rest/group' => {'X-Bugzilla-API-Key' => $admin_api_key} => json => + $bless_group)->status_is(201)->json_has('/id'); + +my $bless_group_id = $t->tx->res->json->{id}; + +# HACK: bless privileges cannot be granted over the API. +Bugzilla->dbh->do( + 'INSERT INTO user_group_map (user_id, group_id, isbless, grant_type) + SELECT userid, ?, 1, ? FROM profiles WHERE login_name = ?', + undef, $bless_group_id, GRANT_DIRECT, $unprivileged_login +); + +$t->get_ok($url + . "rest/group/$group_id" => {'X-Bugzilla-API-Key' => $unprivileged_api_key}) + ->status_is(200)->json_is('/groups' => []); + +# Clean up: leave this user belonging to nothing, as later tests expect. +Bugzilla->dbh->do( + 'DELETE FROM user_group_map + WHERE group_id = ? AND isbless = 1 + AND user_id = (SELECT userid FROM profiles WHERE login_name = ?)', + undef, $bless_group_id, $unprivileged_login +); + done_testing(); From 9eaf66e244e25755ecb6ce66c62201b73ed99e32 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 21:52:12 +0200 Subject: [PATCH 13/14] Bug 2065173 - Treat an empty visible group list as no visible groups An empty arrayref is truthy, so the group_not_visible check passed and the membership query became a bare SELECT with an ' AND ...' appended, which is a SQL error. Carried over from the legacy code. --- Bugzilla/API/V1/Group.pm | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index dd307e183d..2562725cfd 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -237,10 +237,13 @@ sub _get_group_membership { if (!$editusers && Bugzilla->params->{usevisibilitygroups}) { - # Show only users in visible groups. + # Show only users in visible groups. An empty arrayref is still truthy, so + # normalise it to undef: otherwise the group_not_visible check below passes + # and the query is left as a bare SELECT with an ' AND ...' appended to it. $visible_groups = $user->visible_groups_inherited; + $visible_groups = undef unless @$visible_groups; - if (scalar @$visible_groups) { + if ($visible_groups) { $query .= qq{, user_group_map AS ugm WHERE ugm.user_id = profiles.userid AND ugm.isbless = 0 From 6cba9cf5c373b53df25b9aa6976c15c0504de37f Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 22:56:12 +0200 Subject: [PATCH 14/14] Bug 2065173 - Declare ids and names as list parameters merge_request_params collapses parameters to scalars unless the caller says otherwise, so ?ids=1&ids=2 returned only the last group. update() and get() now declare both list parameters; create() takes only scalar fields and is left alone. Covers the repeated form, which no test exercised. --- Bugzilla/API/V1/Group.pm | 4 ++-- qa/t/rest_group_get.t | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Bugzilla/API/V1/Group.pm b/Bugzilla/API/V1/Group.pm index 2562725cfd..b5f9a7dfeb 100644 --- a/Bugzilla/API/V1/Group.pm +++ b/Bugzilla/API/V1/Group.pm @@ -77,7 +77,7 @@ sub update { || return $self->user_error('auth_failure', {group => 'creategroups', action => 'edit', object => 'groups'}); - my ($params, $error) = merge_request_params($self); + my ($params, $error) = merge_request_params($self, ['ids', 'names']); return $self->user_error($error) if $error; if (defined(my $id_or_name = $self->param('id'))) { @@ -142,7 +142,7 @@ sub get { my $user = $self->bugzilla->login; $user->id || return $self->user_error('login_required'); - my ($params, $error) = merge_request_params($self); + my ($params, $error) = merge_request_params($self, ['ids', 'names']); return $self->user_error($error) if $error; if (defined(my $id_or_name = $self->param('id'))) { diff --git a/qa/t/rest_group_get.t b/qa/t/rest_group_get.t index 39365df2a7..6070350441 100644 --- a/qa/t/rest_group_get.t +++ b/qa/t/rest_group_get.t @@ -44,6 +44,27 @@ $t->get_ok( $url . "rest/group/$group_id" => {'X-Bugzilla-API-Key' => $admin_api_key}) ->status_is(200)->json_is('/groups/0/name', 'secret-group'); +# A repeated ids parameter must return every group asked for, not just the last +my $second_group = { + name => 'secret-group-two', + description => 'Also too secret for you!', + is_active => true +}; +$t->post_ok($url + . 'rest/group' => {'X-Bugzilla-API-Key' => $admin_api_key} => json => + $second_group)->status_is(201)->json_has('/id'); + +my $second_group_id = $t->tx->res->json->{id}; + +$t->get_ok($url + . "rest/group?ids=$group_id&ids=$second_group_id" => + {'X-Bugzilla-API-Key' => $admin_api_key})->status_is(200); + +my @returned_ids + = sort { $a <=> $b } map { $_->{id} } @{$t->tx->res->json->{groups}}; +is_deeply(\@returned_ids, [sort { $a <=> $b } ($group_id, $second_group_id)], + 'a repeated ids parameter returns both groups'); + # Create a new user and add it to the new group my $new_user = { email => 'group_test_user@mozilla.bugs',