summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSimon Green <sgreen@redhat.com>2014-06-06 01:47:27 +0200
committerSimon Green <sgreen@redhat.com>2014-06-06 01:47:27 +0200
commitd2c2138b20b7a3c8e05fda35d3116862fcda210b (patch)
treef05c24ab1104b85c326b4d6b0cd116892122502b
parent2f75161d4f6fcd97e24cef94027664ced163d833 (diff)
downloadbugzilla-d2c2138b20b7a3c8e05fda35d3116862fcda210b.tar.gz
bugzilla-d2c2138b20b7a3c8e05fda35d3116862fcda210b.tar.xz
Bug 442013 - Create Bugzilla::User->set_groups and set_bless_groups and have editusers.cgi use them
r=justdave, a=glob
-rw-r--r--Bugzilla/User.pm206
-rwxr-xr-xeditusers.cgi95
-rw-r--r--template/en/default/global/messages.html.tmpl54
3 files changed, 239 insertions, 116 deletions
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 81ce228e7..7aeb9f8ee 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -144,12 +144,80 @@ sub super_user {
return $user;
}
+sub _update_groups {
+ my $self = shift;
+ my $group_changes = shift;
+ my $changes = shift;
+ my $dbh = Bugzilla->dbh;
+
+ # Update group settings.
+ my $sth_add_mapping = $dbh->prepare(
+ qq{INSERT INTO user_group_map (
+ user_id, group_id, isbless, grant_type
+ ) VALUES (
+ ?, ?, ?, ?
+ )
+ });
+ my $sth_remove_mapping = $dbh->prepare(
+ qq{DELETE FROM user_group_map
+ WHERE user_id = ?
+ AND group_id = ?
+ AND isbless = ?
+ AND grant_type = ?
+ });
+
+ foreach my $is_bless (keys %$group_changes) {
+ my ($removed, $added) = @{$group_changes->{$is_bless}};
+
+ foreach my $group (@$removed) {
+ $sth_remove_mapping->execute(
+ $self->id, $group->id, $is_bless, GRANT_DIRECT
+ );
+ }
+ foreach my $group (@$added) {
+ $sth_add_mapping->execute(
+ $self->id, $group->id, $is_bless, GRANT_DIRECT
+ );
+ }
+
+ if (! $is_bless) {
+ my $query = qq{
+ INSERT INTO profiles_activity
+ (userid, who, profiles_when, fieldid, oldvalue, newvalue)
+ VALUES ( ?, ?, now(), ?, ?, ?)
+ };
+
+ $dbh->do(
+ $query, undef,
+ $self->id, Bugzilla->user->id,
+ get_field_id('bug_group'),
+ join(', ', map { $_->name } @$removed),
+ join(', ', map { $_->name } @$added)
+ );
+ }
+ else {
+ # XXX: should create profiles_activity entries for blesser changes.
+ }
+
+ Bugzilla->memcached->clear_config({ key => 'user_groups.' . $self->id });
+
+ my $type = $is_bless ? 'bless_groups' : 'groups';
+ $changes->{$type} = [
+ [ map { $_->name } @$removed ],
+ [ map { $_->name } @$added ],
+ ];
+ }
+}
+
sub update {
my $self = shift;
my $options = shift;
-
+
+ my $group_changes = delete $self->{_group_changes};
+
my $changes = $self->SUPER::update(@_);
my $dbh = Bugzilla->dbh;
+ $self->_update_groups($group_changes, $changes);
if (exists $changes->{login_name}) {
# Delete all the tokens related to the userid
@@ -266,6 +334,111 @@ sub set_disabledtext {
$_[0]->set('is_enabled', $_[1] ? 0 : 1);
}
+sub set_groups {
+ my $self = shift;
+ $self->_set_groups(GROUP_MEMBERSHIP, @_);
+}
+
+sub set_bless_groups {
+ my $self = shift;
+
+ # The person making the change needs to be in the editusers group
+ Bugzilla->user->in_group('editusers')
+ || ThrowUserError("auth_failure", {group => "editusers",
+ reason => "cant_bless",
+ action => "edit",
+ object => "users"});
+
+ $self->_set_groups(GROUP_BLESS, @_);
+}
+
+sub _set_groups {
+ my $self = shift;
+ my $is_bless = shift;
+ my $changes = shift;
+ my $dbh = Bugzilla->dbh;
+
+ # The person making the change is $user, $self is the person being changed
+ my $user = Bugzilla->user;
+
+ # Input is a hash of arrays. Key is 'set', 'add' or 'remove'. The array
+ # is a list of group ids and/or names.
+
+ # First turn the arrays into group objects.
+ $changes = $self->_set_groups_to_object($changes);
+
+ # Get a list of the groups the user currently is a member of
+ my $ids = $dbh->selectcol_arrayref(
+ q{SELECT DISTINCT group_id
+ FROM user_group_map
+ WHERE user_id = ? AND isbless = ? AND grant_type = ?},
+ undef, $self->id, $is_bless, GRANT_DIRECT);
+
+ my $new_groups = my $current_groups = Bugzilla::Group->new_from_list($ids);
+
+ # Record the changes
+ if (exists $changes->{set}) {
+ $new_groups = $changes->{set};
+
+ # We need to check the user has bless rights on the existing groups
+ # If they don't, then we need to add them back to new_groups
+ foreach my $group (@$current_groups) {
+ if (! $user->can_bless($group->id)) {
+ push @$new_groups, $group
+ unless grep { $_->id eq $group->id } @$new_groups;
+ }
+ }
+ }
+ else {
+ foreach my $group (@{$changes->{removed} // []}) {
+ @$new_groups = grep { $_->id ne $group->id } @$new_groups;
+ }
+ foreach my $group (@{$changes->{added} // []}) {
+ push @$new_groups, $group
+ unless grep { $_->id eq $group->id } @$new_groups;
+ }
+ }
+
+ # Stash the changes, so self->update can actually make them
+ my @diffs = diff_arrays($current_groups, $new_groups, 'id');
+ if (scalar(@{$diffs[0]}) || scalar(@{$diffs[1]})) {
+ $self->{_group_changes}{$is_bless} = \@diffs;
+ }
+}
+
+sub _set_groups_to_object {
+ my $self = shift;
+ my $changes = shift;
+ my $user = Bugzilla->user;
+
+ foreach my $key (keys %$changes) {
+ # Check we were given an array
+ unless (ref($changes->{$key}) eq 'ARRAY') {
+ ThrowCodeError(
+ 'param_invalid',
+ { param => $changes->{$key}, function => $key }
+ );
+ }
+
+ # Go through the array, and turn items into group objects
+ my @groups = ();
+ foreach my $value (@{$changes->{$key}}) {
+ my $type = $value =~ /^\d+$/ ? 'id' : 'name';
+ my $group = Bugzilla::Group->new({$type => $value});
+
+ if (! $group || ! $user->can_bless($group->id)) {
+ ThrowUserError('auth_failure',
+ { group => $value, reason => 'cant_bless',
+ action => 'edit', object => 'users' });
+ }
+ push @groups, $group;
+ }
+ $changes->{$key} = \@groups;
+ }
+
+ return $changes;
+}
+
sub update_last_seen_date {
my $self = shift;
return unless $self->id;
@@ -2834,6 +3007,37 @@ User is in the cc list for the bug.
=back
+=item C<set_groups>
+
+C<hash> These specify the groups that this user is directly a member of.
+To set these, you should pass a hash as the value. The hash may contain
+the following fields:
+
+=over
+
+=item C<add> An array of C<int>s or C<string>s. The group ids or group names
+that the user should be added to.
+
+=item C<remove> An array of C<int>s or C<string>s. The group ids or group names
+that the user should be removed from.
+
+=item C<set> An array of C<int>s or C<string>s. An exact set of group ids
+and group names that the user should be a member of. NOTE: This does not
+remove groups from the user where the person making the change does not
+have the bless privilege for.
+
+If you specify C<set>, then C<add> and C<remove> will be ignored. A group in
+both the C<add> and C<remove> list will be added. Specifying a group that the
+user making the change does not have bless rights will generate an error.
+
+=back
+
+=item C<set_bless_groups>
+
+C<hash> - This is the same as set_groups, but affects what groups a user
+has direct membership to bless that group. It takes the same inputs as
+set_groups.
+
=back
=head1 CLASS FUNCTIONS
diff --git a/editusers.cgi b/editusers.cgi
index 650784d5d..3ce22068e 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -248,7 +248,7 @@ if ($action eq 'search') {
# Lock tables during the check+update session.
$dbh->bz_start_transaction();
-
+
$editusers || $user->can_see_user($otherUser)
|| ThrowUserError('auth_failure', {reason => "not_visible",
action => "modify",
@@ -256,6 +256,10 @@ if ($action eq 'search') {
$vars->{'loginold'} = $otherUser->login;
+ # Update groups
+ my @group_ids = grep { s/group_// } keys %{ Bugzilla->cgi->Vars };
+ $otherUser->set_groups({ set => \@group_ids });
+
# Update profiles table entry; silently skip doing this if the user
# is not authorized.
my $changes = {};
@@ -268,87 +272,12 @@ if ($action eq 'search') {
$otherUser->set_disable_mail($cgi->param('disable_mail'));
$otherUser->set_extern_id($cgi->param('extern_id'))
if defined($cgi->param('extern_id'));
- $changes = $otherUser->update();
- }
-
- # Update group settings.
- my $sth_add_mapping = $dbh->prepare(
- qq{INSERT INTO user_group_map (
- user_id, group_id, isbless, grant_type
- ) VALUES (
- ?, ?, ?, ?
- )
- });
- my $sth_remove_mapping = $dbh->prepare(
- qq{DELETE FROM user_group_map
- WHERE user_id = ?
- AND group_id = ?
- AND isbless = ?
- AND grant_type = ?
- });
-
- my @groupsAddedTo;
- my @groupsRemovedFrom;
- my @groupsGrantedRightsToBless;
- my @groupsDeniedRightsToBless;
-
- # Regard only groups the user is allowed to bless and skip all others
- # silently.
- # XXX: checking for existence of each user_group_map entry
- # would allow to display a friendlier error message on page reloads.
- userDataToVars($otherUserID);
- my $permissions = $vars->{'permissions'};
- foreach my $blessable (@{$user->bless_groups()}) {
- my $id = $blessable->id;
- my $name = $blessable->name;
-
- # Change memberships.
- my $groupid = $cgi->param("group_$id") || 0;
- if ($groupid != $permissions->{$id}->{'directmember'}) {
- if (!$groupid) {
- $sth_remove_mapping->execute(
- $otherUserID, $id, 0, GRANT_DIRECT);
- push(@groupsRemovedFrom, $name);
- } else {
- $sth_add_mapping->execute(
- $otherUserID, $id, 0, GRANT_DIRECT);
- push(@groupsAddedTo, $name);
- }
- }
- # Only members of the editusers group may change bless grants.
- # Skip silently if this is not the case.
- if ($editusers) {
- my $groupid = $cgi->param("bless_$id") || 0;
- if ($groupid != $permissions->{$id}->{'directbless'}) {
- if (!$groupid) {
- $sth_remove_mapping->execute(
- $otherUserID, $id, 1, GRANT_DIRECT);
- push(@groupsDeniedRightsToBless, $name);
- } else {
- $sth_add_mapping->execute(
- $otherUserID, $id, 1, GRANT_DIRECT);
- push(@groupsGrantedRightsToBless, $name);
- }
- }
- }
- }
- if (@groupsAddedTo || @groupsRemovedFrom) {
- $dbh->do(qq{INSERT INTO profiles_activity (
- userid, who,
- profiles_when, fieldid,
- oldvalue, newvalue
- ) VALUES (
- ?, ?, now(), ?, ?, ?
- )
- },
- undef,
- ($otherUserID, $userid,
- get_field_id('bug_group'),
- join(', ', @groupsRemovedFrom), join(', ', @groupsAddedTo)));
- Bugzilla->memcached->clear_config({ key => "user_groups.$otherUserID" })
+ # Update bless groups
+ my @bless_ids = grep { s/bless_// } keys %{ Bugzilla->cgi->Vars };
+ $otherUser->set_bless_groups({ set => \@bless_ids });
}
- # XXX: should create profiles_activity entries for blesser changes.
+ $changes = $otherUser->update();
$dbh->bz_commit_transaction();
@@ -357,11 +286,7 @@ if ($action eq 'search') {
delete_token($token);
$vars->{'message'} = 'account_updated';
- $vars->{'changed_fields'} = [keys %$changes];
- $vars->{'groups_added_to'} = \@groupsAddedTo;
- $vars->{'groups_removed_from'} = \@groupsRemovedFrom;
- $vars->{'groups_granted_rights_to_bless'} = \@groupsGrantedRightsToBless;
- $vars->{'groups_denied_rights_to_bless'} = \@groupsDeniedRightsToBless;
+ $vars->{'changes'} = \%$changes;
# We already display the updated page. We have to recreate a token now.
$vars->{'token'} = issue_session_token('edit_user');
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index 34c678e4f..f47a1d6ec 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -26,14 +26,12 @@
canceled.
[% ELSIF message_tag == "account_updated" %]
- [% IF changed_fields.size
- + groups_added_to.size + groups_removed_from.size
- + groups_granted_rights_to_bless.size + groups_denied_rights_to_bless.size %]
+ [% IF changes.size %]
[% title = "User $loginold updated" %]
The following changes have been made to the user account
[%+ loginold FILTER html %]:
<ul>
- [% FOREACH field = changed_fields %]
+ [% FOREACH field = changes.keys %]
<li>
[% IF field == 'login_name' %]
The login is now [% otheruser.login FILTER html %].
@@ -53,35 +51,31 @@
[% ELSE %]
[% terms.Bug %]mail has been enabled.
[% END %]
+ [% ELSIF field == 'groups' %]
+ [% IF changes.groups.1.size %]
+ The account has been added to the
+ [%+ changes.groups.1.join(', ') FILTER html %]
+ group[% 's' IF changes.groups.1.size > 1 %].
+ [% END %]
+ [% IF changes.groups.0.size %]
+ The account has been removed from the
+ [%+ changes.groups.0.join(', ') FILTER html %]
+ group[% 's' IF changes.groups.0.size > 1 %].
+ [% END %]
+ [% ELSIF field == 'bless_groups' %]
+ [% IF changes.bless_groups.1.size %]
+ The account has been granted rights to bless the
+ [%+ changes.bless_groups.1.join(', ') FILTER html %]
+ group[% 's' IF changes.bless_groups.1.size > 1 %].
+ [% END %]
+ [% IF changes.bless_groups.0.size %]
+ The account has been denied rights to bless the
+ [%+ changes.bless_groups.0.join(', ') FILTER html %]
+ group[% 's' IF changes.bless_groups.0.size > 1 %].
+ [% END %]
[% END %]
</li>
[% END %]
- [% IF groups_added_to.size %]
- <li>
- The account has been added to the following group[% 's' IF groups_added_to.size > 1 %]:
- [%+ groups_added_to.join(', ') FILTER html %]
- </li>
- [% END %]
- [% IF groups_removed_from.size %]
- <li>
- The account has been removed from the following group[% 's' IF groups_removed_from.size > 1 %]:
- [%+ groups_removed_from.join(', ') FILTER html %]
- </li>
- [% END %]
- [% IF groups_granted_rights_to_bless.size %]
- <li>
- The account has been granted rights to bless the
- [%+ groups_granted_rights_to_bless.join(', ') FILTER html %]
- group[% 's' IF groups_granted_rights_to_bless.size > 1 %].
- </li>
- [% END %]
- [% IF groups_denied_rights_to_bless.size %]
- <li>
- The account has been denied rights to bless the
- [%+ groups_denied_rights_to_bless.join(', ') FILTER html %]
- group[% 's' IF groups_denied_rights_to_bless.size > 1 %].
- </li>
- [% END %]
</ul>
[% ELSE %]
[% title = "User $otheruser.login not changed" %]