From 3120e71d44a272228c0393bfe8be3d4653f2cd82 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Sat, 26 Aug 2006 05:10:38 +0000 Subject: Bug 349349: Use ->create from Bugzilla::Object instead of insert_new_user for Bugzilla::User Patch By Max Kanat-Alexander r=LpSolit, a=myk --- Bugzilla/Auth/Verify.pm | 9 +++- Bugzilla/DB/Schema.pm | 6 ++- Bugzilla/Install/DB.pm | 6 +++ Bugzilla/Object.pm | 4 +- Bugzilla/User.pm | 133 +++++++++++++++++++++++++++++------------------- checksetup.pl | 6 ++- contrib/syncLDAP.pl | 5 +- createaccount.cgi | 11 +--- editusers.cgi | 38 +++----------- token.cgi | 32 +++--------- 10 files changed, 125 insertions(+), 125 deletions(-) diff --git a/Bugzilla/Auth/Verify.pm b/Bugzilla/Auth/Verify.pm index 952998caf..52cebb5ea 100644 --- a/Bugzilla/Auth/Verify.pm +++ b/Bugzilla/Auth/Verify.pm @@ -77,8 +77,13 @@ sub create_or_update_user { || return { failure => AUTH_ERROR, error => 'auth_invalid_email', details => {addr => $username} }; - insert_new_user($username, $real_name, $password); - $username_user_id = login_to_id($username); + # XXX Theoretically this could fail with an error, but the fix for + # that is too involved to be done right now. + my $user = Bugzilla::User->create({ + login_name => $username, + cryptpassword => $password, + realname => $real_name}); + $username_user_id = $user->id; } # If we have a valid username id and an extern_id, but no valid diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index adac0c6d8..3967c8282 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -611,8 +611,10 @@ use constant ABSTRACT_SCHEMA => { PRIMARYKEY => 1}, login_name => {TYPE => 'varchar(255)', NOTNULL => 1}, cryptpassword => {TYPE => 'varchar(128)'}, - realname => {TYPE => 'varchar(255)'}, - disabledtext => {TYPE => 'MEDIUMTEXT', NOTNULL => 1}, + realname => {TYPE => 'varchar(255)', NOTNULL => 1, + DEFAULT => "''"}, + disabledtext => {TYPE => 'MEDIUMTEXT', NOTNULL => 1, + DEFAULT => "''"}, disable_mail => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}, mybugslink => {TYPE => 'BOOLEAN', NOTNULL => 1, diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index bf1fbcccf..06c46e1ec 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -485,6 +485,12 @@ sub update_table_definitions { $dbh->bz_add_index('bugs', 'bugs_short_desc_idx', [qw(short_desc)]); } + # The profiles table was missing some defaults. + $dbh->bz_alter_column('profiles', 'disabledtext', + {TYPE => 'MEDIUMTEXT', NOTNULL => 1, DEFAULT => "''"}); + $dbh->bz_alter_column('profiles', 'realname', + {TYPE => 'varchar(255)', NOTNULL => 1, DEFAULT => "''"}); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index fa6c4e9cb..219658a92 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -153,7 +153,7 @@ sub create { chop($qmarks); $dbh->do("INSERT INTO $table (" . join(', ', @field_names) . ") VALUES ($qmarks)", undef, @values); - my $id = $dbh->bz_last_key($table, 'id'); + my $id = $dbh->bz_last_key($table, $class->ID_FIELD); return $class->new($id); } @@ -303,7 +303,7 @@ Params: C<$params> - hashref - A value to put in each database Returns: The Object just created in the database. Notes: In order for this function to work in your subclass, - your subclass's C field must be of C + your subclass's L must be of C type in the database. Your subclass also must define L and L. diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 54d84020f..c037f317a 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -49,7 +49,7 @@ use Bugzilla::Classification; use Bugzilla::Field; use base qw(Bugzilla::Object Exporter); -@Bugzilla::User::EXPORT = qw(insert_new_user is_available_username +@Bugzilla::User::EXPORT = qw(is_available_username login_to_id user_id_to_login validate_password UserInGroup USER_MATCH_MULTIPLE USER_MATCH_FAILED USER_MATCH_SUCCESS @@ -92,6 +92,16 @@ use constant DB_COLUMNS => ( use constant NAME_FIELD => 'login_name'; use constant ID_FIELD => 'userid'; +use constant REQUIRED_CREATE_FIELDS => qw(login_name cryptpassword); + +use constant VALIDATORS => { + cryptpassword => \&_check_password, + disable_mail => \&_check_disable_mail, + disabledtext => \&_check_disabledtext, + login_name => \&check_login_name_for_creation, + realname => \&_check_realname, +}; + ################################################################################ # Functions ################################################################################ @@ -108,6 +118,45 @@ sub new { return $class->SUPER::new(@_); } +################################################################################ +# Validators +################################################################################ + +sub _check_disable_mail { return $_[0] ? 1 : 0; } +sub _check_disabledtext { return trim($_[0]) || ''; } + +# This is public since createaccount.cgi needs to use it before issuing +# a token for account creation. +sub check_login_name_for_creation { + my ($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 }); + return $name; +} + +sub _check_password { + my ($pass) = @_; + + # If the password is '*', do not encrypt it or validate it further--we + # are creating a user who should not be able to log in using DB + # authentication. + return $pass if $pass eq '*'; + + validate_password($pass); + my $cryptpassword = bz_crypt($pass); + return $cryptpassword; +} + +sub _check_realname { return trim($_[0]) || ''; } + +################################################################################ +# Methods +################################################################################ + # Accessors for user attributes sub login { $_[0]->{login}; } sub email { $_[0]->{login} . Bugzilla->params->{'emailsuffix'}; } @@ -1292,36 +1341,18 @@ sub get_userlist { return $self->{'userlist'}; } -sub insert_new_user { - my ($username, $realname, $password, $disabledtext, $disable_mail) = (@_); +sub create { + my $invocant = shift; + my $class = ref($invocant) || $invocant; my $dbh = Bugzilla->dbh; - $disabledtext ||= ''; - $disable_mail ||= 0; + $dbh->bz_lock_tables('profiles WRITE', 'profiles_activity WRITE', + 'user_group_map WRITE', 'email_setting WRITE', 'groups READ', + 'tokens READ', 'fielddefs READ'); - # If not specified, generate a new random password for the user. - # If the password is '*', do not encrypt it; we are creating a user - # based on the ENV auth method. - $password ||= generate_random_password(); - my $cryptpassword = ($password ne '*') ? bz_crypt($password) : $password; - - # XXX - These should be moved into is_available_username or validate_email_syntax - # At the least, they shouldn't be here. They're safe for now, though. - trick_taint($username); - trick_taint($realname); - - # Insert the new user record into the database. - $dbh->do("INSERT INTO profiles - (login_name, realname, cryptpassword, disabledtext, - disable_mail) - VALUES (?, ?, ?, ?, ?)", - undef, - ($username, $realname, $cryptpassword, $disabledtext, - $disable_mail)); + my $user = $class->SUPER::create(@_); # Turn on all email for the new user - my $new_userid = $dbh->bz_last_key('profiles', 'userid'); - foreach my $rel (RELATIONSHIPS) { foreach my $event (POS_EVENTS, NEG_EVENTS) { # These "exceptions" define the default email preferences. @@ -1332,16 +1363,15 @@ sub insert_new_user { next if (($event == EVT_CC) && ($rel != REL_REPORTER)); $dbh->do('INSERT INTO email_setting (user_id, relationship, event) - VALUES (?, ?, ?)', undef, ($new_userid, $rel, $event)); + VALUES (?, ?, ?)', undef, ($user->id, $rel, $event)); } } foreach my $event (GLOBAL_EVENTS) { $dbh->do('INSERT INTO email_setting (user_id, relationship, event) - VALUES (?, ?, ?)', undef, ($new_userid, REL_ANY, $event)); + VALUES (?, ?, ?)', undef, ($user->id, REL_ANY, $event)); } - my $user = new Bugzilla::User($new_userid); $user->derive_regexp_groups(); # Add the creation date to the profiles_activity table. @@ -1355,6 +1385,8 @@ sub insert_new_user { VALUES (?, ?, NOW(), ?, NOW())', undef, ($user->id, $who, $creation_date_fieldid)); + $dbh->bz_unlock_tables(); + # Return the newly created user account. return $user; } @@ -1461,7 +1493,12 @@ Bugzilla::User - Object for a Bugzilla user $user->get_selectable_classifications; # Class Functions - $user = insert_new_user($username, $realname, $password, $disabledtext); + $user = Bugzilla::User->create({ + login_name => $username, + realname => $realname, + cryptpassword => $plaintext_password, + disabledtext => $disabledtext, + disable_mail => 0}); =head1 DESCRIPTION @@ -1797,27 +1834,21 @@ called "statically," just like a normal procedural function. =over 4 -=item C - -Creates a new user in the database. - -Params: $username (scalar, string) - The login name for the new user. - $realname (scalar, string) - The full name for the new user. - $password (scalar, string) - Optional. The password for the new user; - if not given, a random password will be - generated. - $disabledtext (scalar, string) - Optional. The disable text for the new - user; if not given, it will be empty. - If given, the user will be disabled, - meaning the account will be - unavailable for login. - $disable_mail (scalar, boolean) - Optional, defaults to 0. - If 1, bug-related mail will not be - sent to this user; if 0, mail will - be sent depending on the user's - email preferences. - -Returns: The Bugzilla::User object representing the new user account. +=item C + +The same as L. + +Params: login_name - B The login name for the new user. + realname - The full name for the new user. + cryptpassword - B The password for the new user. + Even though the name says "crypt", you should just specify + a plain-text password. If you specify '*', the user will not + be able to log in using DB authentication. + disabledtext - The disable-text for the new user. If given, the user + will be disabled, meaning he cannot log in. Defaults to an + empty string. + disable_mail - If 1, bug-related mail will not be sent to this user; + if 0, mail will be sent depending on the user's email preferences. =item C diff --git a/checksetup.pl b/checksetup.pl index c0b206bed..7f3c6783f 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -326,7 +326,6 @@ import Bugzilla::Util qw(bz_crypt trim html_quote is_7bit_clean clean_text url_quote); require Bugzilla::User; -import Bugzilla::User qw(insert_new_user); require Bugzilla::Bug; import Bugzilla::Bug qw(is_open_state); @@ -756,7 +755,10 @@ if ($sth->rows == 0) { $SIG{QUIT} = 'DEFAULT'; $SIG{TERM} = 'DEFAULT'; - insert_new_user($login, $realname, $pass1); + Bugzilla::User->create({ + login_name => $login, + realname => $realname, + cryptpassword => $pass1}); } # Put the admin in each group if not already diff --git a/contrib/syncLDAP.pl b/contrib/syncLDAP.pl index 32d01f150..72ea91798 100755 --- a/contrib/syncLDAP.pl +++ b/contrib/syncLDAP.pl @@ -273,7 +273,10 @@ if($readonly == 0) { print "Phase 3: creating new users... " unless $quiet; if($nocreate == 0) { while( my ($key, $value) = each(%create_users) ) { - insert_new_user($key, @$value{'realname'}); + Bugzilla::User->create({ + login_name => $key, + realname => @$value{'realname'}, + password => '*'}); } print "done!\n" unless $quiet; } diff --git a/createaccount.cgi b/createaccount.cgi index 6f325347e..f58b8402b 100755 --- a/createaccount.cgi +++ b/createaccount.cgi @@ -60,18 +60,9 @@ unless ($createexp) { my $login = $cgi->param('login'); if (defined($login)) { - validate_email_syntax($login) - || ThrowUserError('illegal_email_address', {addr => $login}); - + $login = Bugzilla::User::check_login_name_for_creation($login); $vars->{'login'} = $login; - if (!is_available_username($login)) { - # Account already exists - $template->process("account/exists.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - exit; - } - if ($login !~ /$createexp/) { ThrowUserError("account_creation_disabled"); } diff --git a/editusers.cgi b/editusers.cgi index 0ce3a95ce..4663b18ee 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -191,36 +191,14 @@ if ($action eq 'search') { action => "add", object => "users"}); - 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; - - # Lock tables during the check+creation session. - $dbh->bz_lock_tables('profiles WRITE', 'profiles_activity WRITE', - 'email_setting WRITE', 'user_group_map WRITE', - 'groups READ', 'tokens READ', 'fielddefs READ'); - - # Validity checks - $login || ThrowUserError('user_login_required'); - validate_email_syntax($login) - || ThrowUserError('illegal_email_address', {addr => $login}); - is_available_username($login) - || ThrowUserError('account_exists', {email => $login}); - validate_password($password); - - # Login and password are validated now, and realname and disabledtext - # are allowed to contain anything - trick_taint($login); - trick_taint($realname); - trick_taint($password); - trick_taint($disabledtext); - - insert_new_user($login, $realname, $password, $disabledtext, $disable_mail); - my $new_user_id = $dbh->bz_last_key('profiles', 'userid'); - $dbh->bz_unlock_tables(); - userDataToVars($new_user_id); + my $new_user = Bugzilla::User->create({ + login_name => $cgi->param('login'), + cryptpassword => $cgi->param('password'), + realname => $cgi->param('name'), + disabledtext => $cgi->param('disabledtext'), + disable_mail => $cgi->param('disable_mail')}); + + userDataToVars($new_user->id); $vars->{'message'} = 'account_created'; $template->process('admin/users/edit.html.tmpl', $vars) diff --git a/token.cgi b/token.cgi index 6b72dfa36..30913642e 100755 --- a/token.cgi +++ b/token.cgi @@ -369,31 +369,13 @@ sub request_create_account { sub confirm_create_account { my (undef, undef, $login_name) = Bugzilla::Token::GetTokenData($::token); - (defined $cgi->param('passwd1') && defined $cgi->param('passwd2')) - || ThrowUserError('new_password_missing'); - validate_password($cgi->param('passwd1'), $cgi->param('passwd2')); - - my $realname = $cgi->param('realname'); - my $password = $cgi->param('passwd1'); - - $dbh->bz_lock_tables('profiles WRITE', 'profiles_activity WRITE', - 'email_setting WRITE', 'user_group_map WRITE', - 'groups READ', 'tokens READ', 'fielddefs READ'); - - # The email syntax may have changed since the initial creation request. - validate_email_syntax($login_name) - || ThrowUserError('illegal_email_address', {addr => $login_name}); - # Also, maybe that this user account has already been created meanwhile. - is_available_username($login_name) - || ThrowUserError('account_exists', {email => $login_name}); - - # Login and password are validated now, and realname is allowed to - # contain anything. - trick_taint($realname); - trick_taint($password); - - my $otheruser = insert_new_user($login_name, $realname, $password); - $dbh->bz_unlock_tables(); + validate_password($cgi->param('passwd1') || '', + $cgi->param('passwd2') || ''); + + my $otheruser = Bugzilla::User->create({ + login_name => $login_name, + realname => $cgi->param('realname'), + cryptpassword => $cgi->param('passwd1')}); # Now delete this token. Bugzilla::Token::DeleteToken($::token); -- cgit v1.2.3-24-g4f1b