Skip to content

bugzilla
bugzilla
Commit 810ac042 authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org
Browse files
Options

Bug 352243: Make editusers.cgi use Bugzilla::User for basic user updates

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=justdave
parent ea2d2a47
Hide whitespace changes
Inline Side-by-side
Showing with 127 additions and 91 deletions
Bugzilla/Object.pm
View file @ 810ac042
......@@ -149,17 +149,28 @@ sub update {
my $dbh = Bugzilla->dbh;
my $table = $self->DB_TABLE;
my $id_field = $self->ID_FIELD;
my $old_self = $self->new($self->id);
my $columns = join(', ', map {"$_ = ?"} $self->UPDATE_COLUMNS);
my @values;
my (@update_columns, @values, %changes);
foreach my $column ($self->UPDATE_COLUMNS) {
my $value = $self->{$column};
trick_taint($value) if defined $value;
push(@values, $value);
if ($old_self->{$column} ne $self->{$column}) {
my $value = $self->{$column};
trick_taint($value) if defined $value;
push(@values, $value);
push(@update_columns, $column);
# We don't use $value because we don't want to detaint this for
# the caller.
$changes{$column} = [$old_self->{$column}, $self->{$column}];
}
}
my $columns = join(', ', map {"$_ = ?"} @update_columns);
$dbh->do("UPDATE $table SET $columns WHERE $id_field = ?", undef,
@values, $self->id);
@values, $self->id) if @values;
return \%changes;
}
###############################
......@@ -452,9 +463,27 @@ data into the database. Returns a newly created object.
=item C<update>
=over
=item B<Description>
Saves the values currently in this object to the database.
Only the fields specified in L</UPDATE_COLUMNS> will be
updated. Returns nothing and takes no parameters.
updated, and they will only be updated if their values have changed.
=item B<Params> (none)
=item B<Returns>
A hashref showing what changed during the update. The keys are the column
names from L</UPDATE_COLUMNS>. If a field was not changed, it will not be
in the hash at all. If the field was changed, the key will point to an arrayref.
The first item of the arrayref will be the old value, and the second item
will be the new value.
If there were no changes, we return a reference to an empty hash.
=back
=back
......
Bugzilla/User.pm
View file @ 810ac042
......@@ -67,8 +67,8 @@ use constant MATCH_SKIP_CONFIRM => 1;
use constant DEFAULT_USER => {
'id' => 0,
'name' => '',
'login' => '',
'realname' => '',
'login_name' => '',
'showmybugslink' => 0,
'disabledtext' => '',
'disable_mail' => 0,
......@@ -82,8 +82,8 @@ use constant DB_TABLE => 'profiles';
# fixed one day.
use constant DB_COLUMNS => (
'profiles.userid AS id',
'profiles.login_name AS login',
'profiles.realname AS name',
'profiles.login_name',
'profiles.realname',
'profiles.mybugslink AS showmybugslink',
'profiles.disabledtext',
'profiles.disable_mail',
......@@ -101,6 +101,18 @@ use constant VALIDATORS => {
realname => \&_check_realname,
};
sub UPDATE_COLUMNS {
my $self = shift;
my @cols = qw(
disable_mail
disabledtext
login_name
realname
);
push(@cols, 'cryptpassword') if exists $self->{cryptpassword};
return @cols;
};
################################################################################
# Functions
################################################################################
......@@ -117,6 +129,29 @@ sub new {
return $class->SUPER::new(@_);
}
sub update {
my $self = shift;
my $changes = $self->SUPER::update(@_);
my $dbh = Bugzilla->dbh;
if (exists $changes->{login_name}) {
# If we changed the login, silently delete any tokens.
$dbh->do('DELETE FROM tokens WHERE userid = ?', undef, $self->id);
# And rederive regex groups
$self->derive_regexp_groups();
}
# Logout the user if necessary.
Bugzilla->logout_user($self)
if (exists $changes->{login_name} || exists $changes->{disabledtext}
|| exists $changes->{cryptpassword});
# XXX Can update profiles_activity here as soon as it understands
# field names like login_name.
return $changes;
}
################################################################################
# Validators
################################################################################
......@@ -127,13 +162,18 @@ sub _check_disabledtext { return trim($_[1]) || ''; }
# This is public since createaccount.cgi needs to use it before issuing
# a token for account creation.
sub check_login_name_for_creation {
my ($self, $name) = @_;
my ($invocant, $name) = @_;
$name = trim($name);
$name || ThrowUserError('user_login_required');
validate_email_syntax($name)
|| ThrowUserError('illegal_email_address', { addr => $name });
is_available_username($name)
|| ThrowUserError('account_exists', { email => $name });
# Check the name if it's a new user, or if we're changing the name.
if (!ref($invocant) || $invocant->login ne $name) {
is_available_username($name)
|| ThrowUserError('account_exists', { email => $name });
}
return $name;
}
......@@ -153,12 +193,36 @@ sub _check_password {
sub _check_realname { return trim($_[1]) || ''; }
################################################################################
# Mutators
################################################################################
sub set_disabledtext { $_[0]->set('disabledtext', $_[1]); }
sub set_disable_mail { $_[0]->set('disable_mail', $_[1]); }
sub set_login {
my ($self, $login) = @_;
$self->set('login_name', $login);
delete $self->{identity};
delete $self->{nick};
}
sub set_name {
my ($self, $name) = @_;
$self->set('realname', $name);
delete $self->{identity};
}
sub set_password { $_[0]->set('cryptpassword', $_[1]); }
################################################################################
# Methods
################################################################################
# Accessors for user attributes
sub login { $_[0]->{login}; }
sub email { $_[0]->{login} . Bugzilla->params->{'emailsuffix'}; }
sub name { $_[0]->{realname}; }
sub login { $_[0]->{login_name}; }
sub email { $_[0]->login . Bugzilla->params->{'emailsuffix'}; }
sub disabledtext { $_[0]->{'disabledtext'}; }
sub is_disabled { $_[0]->disabledtext ? 1 : 0; }
sub showmybugslink { $_[0]->{showmybugslink}; }
......@@ -187,7 +251,7 @@ sub identity {
if (!defined $self->{identity}) {
$self->{identity} =
$self->{name} ? "$self->{name} <$self->{login}>" : $self->{login};
$self->name ? $self->name . " <" . $self->login. ">" : $self->login;
}
return $self->{identity};
......@@ -199,7 +263,7 @@ sub nick {
return "" unless $self->id;
if (!defined $self->{nick}) {
$self->{nick} = (split(/@/, $self->{login}, 2))[0];
$self->{nick} = (split(/@/, $self->login, 2))[0];
}
return $self->{nick};
......@@ -767,7 +831,7 @@ sub derive_regexp_groups {
AND isbless = 0
AND grant_type = ?});
while (my ($group, $regexp, $present) = $sth->fetchrow_array()) {
if (($regexp ne '') && ($self->{login} =~ m/$regexp/i)) {
if (($regexp ne '') && ($self->login =~ m/$regexp/i)) {
$group_insert->execute($id, $group, GRANT_REGEXP) unless $present;
} else {
$group_delete->execute($id, $group, GRANT_REGEXP) if $present;
......@@ -1101,10 +1165,11 @@ sub match_field {
# skip confirmation for exact matches
if ((scalar(@{$users}) == 1)
&& (lc(@{$users}[0]->{'login'}) eq lc($query)))
&& (lc(@{$users}[0]->login) eq lc($query)))
{
$cgi->append(-name=>$field,
-values=>[@{$users}[0]->{'login'}]);
-values=>[@{$users}[0]->login]);
next;
}
......@@ -1117,7 +1182,7 @@ sub match_field {
if (scalar(@{$users}) == 1) { # exactly one match
$cgi->append(-name=>$field,
-values=>[@{$users}[0]->{'login'}]);
-values=>[@{$users}[0]->login]);
$need_confirm = 1 if $params->{'confirmuniqueusermatch'};
......@@ -1282,7 +1347,7 @@ sub wants_bug_mail {
#
# We do them separately because if _any_ of them are set, we don't want
# the mail.
if ($wants_mail && $changer && ($self->{'login'} eq $changer)) {
if ($wants_mail && $changer && ($self->login eq $changer)) {
$wants_mail &= $self->wants_mail([EVT_CHANGED_BY_ME], $relationship);
}
......
editusers.cgi
View file @ 810ac042
......@@ -226,9 +226,6 @@ if ($action eq 'search') {
my $otherUser = check_user($otherUserID, $otherUserLogin);
$otherUserID = $otherUser->id;
my $logoutNeeded = 0;
my @changedFields;
# Lock tables during the check+update session.
$dbh->bz_lock_tables('profiles WRITE',
'profiles_activity WRITE',
......@@ -245,73 +242,19 @@ if ($action eq 'search') {
action => "modify",
object => "user"});
my $login = $cgi->param('login');
my $password = $cgi->param('password');
my $realname = trim($cgi->param('name') || '');
my $disabledtext = trim($cgi->param('disabledtext') || '');
my $disable_mail = $cgi->param('disable_mail') ? 1 : 0;
$vars->{'loginold'} = $otherUser->login;
# Update profiles table entry; silently skip doing this if the user
# is not authorized.
my %changes;
if ($editusers) {
my @values;
if ($login ne $otherUser->login) {
# Validating untaints for us.
$login || ThrowUserError('user_login_required');
validate_email_syntax($login)
|| ThrowUserError('illegal_email_address', {addr => $login});
is_available_username($login)
|| ThrowUserError('account_exists', {email => $login});
push(@changedFields, 'login_name');
push(@values, $login);
$logoutNeeded = 1;
# Since we change the login, silently delete any tokens.
$dbh->do('DELETE FROM tokens WHERE userid = ?', {}, $otherUserID);
}
if ($realname ne $otherUser->name) {
# The real name may be anything; we use a placeholder for our
# INSERT, and we rely on displaying code to FILTER html.
trick_taint($realname);
push(@changedFields, 'realname');
push(@values, $realname);
}
if ($password) {
# Validating untaints for us.
validate_password($password) if $password;
push(@changedFields, 'cryptpassword');
push(@values, bz_crypt($password));
$logoutNeeded = 1;
}
if ($disabledtext ne $otherUser->disabledtext) {
# The disable text may be anything; we use a placeholder for our
# INSERT, and we rely on displaying code to FILTER html.
trick_taint($disabledtext);
push(@changedFields, 'disabledtext');
push(@values, $disabledtext);
$logoutNeeded = 1;
}
if ($disable_mail != $otherUser->email_disabled) {
push(@changedFields, 'disable_mail');
push(@values, $disable_mail);
}
if (@changedFields) {
push (@values, $otherUserID);
$logoutNeeded && Bugzilla->logout_user($otherUser);
$dbh->do('UPDATE profiles SET ' .
join(' = ?,', @changedFields).' = ? ' .
'WHERE userid = ?',
undef, @values);
# XXX: should create profiles_activity entries.
#
# We create a new user object here because it needs to
# read information that may have changed since this
# script started.
my $newprofile = new Bugzilla::User($otherUserID);
$newprofile->derive_regexp_groups();
}
$otherUser->set_login($cgi->param('login'));
$otherUser->set_name($cgi->param('name'));
$otherUser->set_password($cgi->param('password'))
if $cgi->param('password');
$otherUser->set_disabledtext($cgi->param('disabledtext'));
$otherUser->set_disable_mail($cgi->param('disable_mail'));
%changes = %{$otherUser->update()};
}
# Update group settings.
......@@ -399,8 +342,7 @@ if ($action eq 'search') {
delete_token($token);
$vars->{'message'} = 'account_updated';
$vars->{'loginold'} = $otherUser->login;
$vars->{'changed_fields'} = \@changedFields;
$vars->{'changed_fields'} = [keys %changes];
$vars->{'groups_added_to'} = \@groupsAddedTo;
$vars->{'groups_removed_from'} = \@groupsRemovedFrom;
$vars->{'groups_granted_rights_to_bless'} = \@groupsGrantedRightsToBless;
......
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