Skip to content

bugzilla
bugzilla
Commit d2c2138b authored by Simon Green's avatar Simon Green
Browse files
Options

Bug 442013 - Create Bugzilla::User->set_groups and set_bless_groups and have editusers.cgi use them

r=justdave, a=glob
parent 2f75161d
Hide whitespace changes
Inline Side-by-side
Showing with 239 additions and 116 deletions
Bugzilla/User.pm
View file @ d2c2138b
...@@ -144,12 +144,80 @@ sub super_user { ...@@ -144,12 +144,80 @@ sub super_user {
return $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 { sub update {
my $self = shift; my $self = shift;
my $options = shift; my $options = shift;
my $group_changes = delete $self->{_group_changes};
my $changes = $self->SUPER::update(@_); my $changes = $self->SUPER::update(@_);
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
$self->_update_groups($group_changes, $changes);
if (exists $changes->{login_name}) { if (exists $changes->{login_name}) {
# Delete all the tokens related to the userid # Delete all the tokens related to the userid
...@@ -266,6 +334,111 @@ sub set_disabledtext { ...@@ -266,6 +334,111 @@ sub set_disabledtext {
$_[0]->set('is_enabled', $_[1] ? 0 : 1); $_[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 { sub update_last_seen_date {
my $self = shift; my $self = shift;
return unless $self->id; return unless $self->id;
...@@ -2834,6 +3007,37 @@ User is in the cc list for the bug. ...@@ -2834,6 +3007,37 @@ User is in the cc list for the bug.
=back =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 =back
=head1 CLASS FUNCTIONS =head1 CLASS FUNCTIONS
......
editusers.cgi
View file @ d2c2138b
...@@ -248,7 +248,7 @@ if ($action eq 'search') { ...@@ -248,7 +248,7 @@ if ($action eq 'search') {
# Lock tables during the check+update session. # Lock tables during the check+update session.
$dbh->bz_start_transaction(); $dbh->bz_start_transaction();
$editusers || $user->can_see_user($otherUser) $editusers || $user->can_see_user($otherUser)
|| ThrowUserError('auth_failure', {reason => "not_visible", || ThrowUserError('auth_failure', {reason => "not_visible",
action => "modify", action => "modify",
...@@ -256,6 +256,10 @@ if ($action eq 'search') { ...@@ -256,6 +256,10 @@ if ($action eq 'search') {
$vars->{'loginold'} = $otherUser->login; $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 # Update profiles table entry; silently skip doing this if the user
# is not authorized. # is not authorized.
my $changes = {}; my $changes = {};
...@@ -268,87 +272,12 @@ if ($action eq 'search') { ...@@ -268,87 +272,12 @@ if ($action eq 'search') {
$otherUser->set_disable_mail($cgi->param('disable_mail')); $otherUser->set_disable_mail($cgi->param('disable_mail'));
$otherUser->set_extern_id($cgi->param('extern_id')) $otherUser->set_extern_id($cgi->param('extern_id'))
if defined($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. # Update bless groups
# Skip silently if this is not the case. my @bless_ids = grep { s/bless_// } keys %{ Bugzilla->cgi->Vars };
if ($editusers) { $otherUser->set_bless_groups({ set => \@bless_ids });
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" })
} }
# XXX: should create profiles_activity entries for blesser changes. $changes = $otherUser->update();
$dbh->bz_commit_transaction(); $dbh->bz_commit_transaction();
...@@ -357,11 +286,7 @@ if ($action eq 'search') { ...@@ -357,11 +286,7 @@ if ($action eq 'search') {
delete_token($token); delete_token($token);
$vars->{'message'} = 'account_updated'; $vars->{'message'} = 'account_updated';
$vars->{'changed_fields'} = [keys %$changes]; $vars->{'changes'} = \%$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;
# We already display the updated page. We have to recreate a token now. # We already display the updated page. We have to recreate a token now.
$vars->{'token'} = issue_session_token('edit_user'); $vars->{'token'} = issue_session_token('edit_user');
......
template/en/default/global/messages.html.tmpl
View file @ d2c2138b
...@@ -26,14 +26,12 @@ ...@@ -26,14 +26,12 @@
canceled. canceled.
[% ELSIF message_tag == "account_updated" %] [% ELSIF message_tag == "account_updated" %]
[% IF changed_fields.size [% IF changes.size %]
+ groups_added_to.size + groups_removed_from.size
+ groups_granted_rights_to_bless.size + groups_denied_rights_to_bless.size %]
[% title = "User $loginold updated" %] [% title = "User $loginold updated" %]
The following changes have been made to the user account The following changes have been made to the user account
[%+ loginold FILTER html %]: [%+ loginold FILTER html %]:
<ul> <ul>
[% FOREACH field = changed_fields %] [% FOREACH field = changes.keys %]
<li> <li>
[% IF field == 'login_name' %] [% IF field == 'login_name' %]
The login is now [% otheruser.login FILTER html %]. The login is now [% otheruser.login FILTER html %].
...@@ -53,35 +51,31 @@ ...@@ -53,35 +51,31 @@
[% ELSE %] [% ELSE %]
[% terms.Bug %]mail has been enabled. [% terms.Bug %]mail has been enabled.
[% END %] [% 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 %] [% END %]
</li> </li>
[% END %] [% 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> </ul>
[% ELSE %] [% ELSE %]
[% title = "User $otheruser.login not changed" %] [% title = "User $otheruser.login not changed" %]
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment