From 3891b63a1eb52076337885487f251a10580a4a85 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Wed, 27 Apr 2016 18:50:13 +0200 Subject: Bug 218917 - Allow the login name to be different from the email address Original patch by Gervase Markham r=gerv a=dkl --- Bugzilla/API/1_0/Resource/Bug.pm | 21 ++- Bugzilla/API/1_0/Resource/Bugzilla.pm | 4 +- Bugzilla/API/1_0/Resource/Group.pm | 10 +- Bugzilla/API/1_0/Resource/Product.pm | 4 +- Bugzilla/API/1_0/Resource/User.pm | 34 +++-- Bugzilla/API/1_0/Util.pm | 37 ++++-- Bugzilla/Auth.pm | 12 ++ Bugzilla/Auth/Verify.pm | 54 ++++---- Bugzilla/Auth/Verify/LDAP.pm | 10 +- Bugzilla/Auth/Verify/RADIUS.pm | 4 +- Bugzilla/Config.pm | 39 +++++- Bugzilla/Config/Auth.pm | 13 +- Bugzilla/Config/BugChange.pm | 3 +- Bugzilla/Config/Common.pm | 47 +++++-- Bugzilla/DB/Schema.pm | 5 +- Bugzilla/FlagType.pm | 4 +- Bugzilla/Group.pm | 6 +- Bugzilla/Hook.pm | 8 +- Bugzilla/Install.pm | 33 ++++- Bugzilla/Install/DB.pm | 25 ++++ Bugzilla/Template.pm | 2 +- Bugzilla/Token.pm | 39 ++++-- Bugzilla/User.pm | 240 +++++++++++++++++++++++----------- Bugzilla/Util.pm | 10 +- Bugzilla/WebService/Bug.pm | 31 +++-- Bugzilla/WebService/Bugzilla.pm | 4 +- Bugzilla/WebService/Group.pm | 9 +- Bugzilla/WebService/Product.pm | 4 +- Bugzilla/WebService/Server/JSONRPC.pm | 7 +- Bugzilla/WebService/Server/XMLRPC.pm | 8 +- Bugzilla/WebService/User.pm | 51 +++++--- 31 files changed, 523 insertions(+), 255 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/API/1_0/Resource/Bug.pm b/Bugzilla/API/1_0/Resource/Bug.pm index 781ac3176..b0182a5e3 100644 --- a/Bugzilla/API/1_0/Resource/Bug.pm +++ b/Bugzilla/API/1_0/Resource/Bug.pm @@ -509,7 +509,7 @@ sub _translate_comment { my $comment_hash = { id => as_int($comment->id), bug_id => as_int($comment->bug_id), - creator => as_email($comment->author->login), + creator => as_login($comment->author->login), time => as_datetime($comment->creation_ts), creation_time => as_datetime($comment->creation_ts), is_private => as_boolean($comment->is_private), @@ -1431,7 +1431,7 @@ sub _bug_to_hash { $item{alias} = as_string_array($bug->alias); } if (filter_wants $params, 'assigned_to') { - $item{'assigned_to'} = as_email($bug->assigned_to->login); + $item{'assigned_to'} = as_login($bug->assigned_to->login); $item{'assigned_to_detail'} = $self->_user_to_hash($bug->assigned_to, $params, undef, 'assigned_to'); } if (filter_wants $params, 'blocks') { @@ -1444,14 +1444,14 @@ sub _bug_to_hash { $item{component} = as_string($bug->component); } if (filter_wants $params, 'cc') { - $item{'cc'} = as_email_array($bug->cc); + $item{'cc'} = as_login_array($bug->cc); $item{'cc_detail'} = [ map { $self->_user_to_hash($_, $params, undef, 'cc') } @{ $bug->cc_users } ]; } if (filter_wants $params, 'creation_time') { $item{'creation_time'} = as_datetime($bug->creation_ts); } if (filter_wants $params, 'creator') { - $item{'creator'} = as_email($bug->reporter->login); + $item{'creator'} = as_login($bug->reporter->login); $item{'creator_detail'} = $self->_user_to_hash($bug->reporter, $params, undef, 'creator'); } if (filter_wants $params, 'depends_on') { @@ -1477,7 +1477,7 @@ sub _bug_to_hash { } if (filter_wants $params, 'qa_contact') { my $qa_login = $bug->qa_contact ? $bug->qa_contact->login : ''; - $item{'qa_contact'} = as_email($qa_login); + $item{'qa_contact'} = as_login($qa_login); if ($bug->qa_contact) { $item{'qa_contact_detail'} = $self->_user_to_hash($bug->qa_contact, $params, undef, 'qa_contact'); } @@ -1546,8 +1546,7 @@ sub _user_to_hash { my $item = filter $filters, { id => as_int($user->id), real_name => as_string($user->name), - name => as_email($user->login), - email => as_email($user->email), + name => as_login($user->login), }, $types, $prefix; return $item; } @@ -1571,7 +1570,7 @@ sub _attachment_to_hash { # creator requires an extra lookup, so we only send them if # the filter wants them. if (filter_wants $filters, 'creator', $types, $prefix) { - $item->{'creator'} = as_email($attach->attacher->login); + $item->{'creator'} = as_login($attach->attacher->login); } if (filter_wants $filters, 'data', $types, $prefix) { @@ -1603,7 +1602,7 @@ sub _flag_to_hash { foreach my $field (qw(setter requestee)) { my $field_id = $field . "_id"; - $item->{$field} = as_email($flag->$field->login) + $item->{$field} = as_login($flag->$field->login) if $flag->$field_id; } @@ -2672,10 +2671,6 @@ C The 'real' name for this user, if any. C The user's Bugzilla login. -=item C - -C The user's email address. Currently this is the same value as the name. - =back =back diff --git a/Bugzilla/API/1_0/Resource/Bugzilla.pm b/Bugzilla/API/1_0/Resource/Bugzilla.pm index f5bb2f417..fc2e15e90 100644 --- a/Bugzilla/API/1_0/Resource/Bugzilla.pm +++ b/Bugzilla/API/1_0/Resource/Bugzilla.pm @@ -69,7 +69,6 @@ use constant PARAMETERS_LOGGED_IN => qw( defaultseverity duplicate_or_move_bug_status emailregexpdesc - emailsuffix letsubmitterchoosemilestone letsubmitterchoosepriority mailfrom @@ -82,6 +81,7 @@ use constant PARAMETERS_LOGGED_IN => qw( requirelogin search_allow_no_criteria urlbase + use_email_as_login use_see_also useclassification usemenuforusers @@ -449,7 +449,6 @@ A logged-in user can access the following parameters (listed alphabetically): C, C, C, - C, C, C, C, @@ -462,6 +461,7 @@ A logged-in user can access the following parameters (listed alphabetically): C, C, C, + C, C, C, C, diff --git a/Bugzilla/API/1_0/Resource/Group.pm b/Bugzilla/API/1_0/Resource/Group.pm index 43a2521fb..f3b55b3fd 100644 --- a/Bugzilla/API/1_0/Resource/Group.pm +++ b/Bugzilla/API/1_0/Resource/Group.pm @@ -247,8 +247,8 @@ sub _get_group_membership { map {{ id => as_int($_->id), real_name => as_string($_->name), - name => as_string($_->login), - email => as_string($_->email), + name => as_login($_->login), + email => as_email($_->email), can_login => as_boolean($_->is_enabled), email_enabled => as_boolean($_->email_enabled), login_denied_text => as_string($_->disabledtext), @@ -571,12 +571,12 @@ C The actual name of the user. =item email -C The email address of the user. +C If you are in the editusers group, returns the email address of the +user, else returns nothing. =item name -C The login name of the user. Note that in some situations this is -different than their email. +C The login name of the user. =item can_login diff --git a/Bugzilla/API/1_0/Resource/Product.pm b/Bugzilla/API/1_0/Resource/Product.pm index d3576a734..02581b0ed 100644 --- a/Bugzilla/API/1_0/Resource/Product.pm +++ b/Bugzilla/API/1_0/Resource/Product.pm @@ -322,9 +322,9 @@ sub _component_to_hash { name => as_string($component->name), description => as_string($component->description), default_assigned_to => - as_email($component->default_assignee->login), + as_login($component->default_assignee->login), default_qa_contact => - as_email($component->default_qa_contact ? + as_login($component->default_qa_contact ? $component->default_qa_contact->login : ""), sort_key => 0, # sort_key is returned to match Bug.fields is_active => as_boolean($component->is_active), diff --git a/Bugzilla/API/1_0/Resource/User.pm b/Bugzilla/API/1_0/Resource/User.pm index a214bd81f..101a70529 100644 --- a/Bugzilla/API/1_0/Resource/User.pm +++ b/Bugzilla/API/1_0/Resource/User.pm @@ -53,13 +53,11 @@ use constant PUBLIC_METHODS => qw( ); use constant MAPPED_FIELDS => { - email => 'login', full_name => 'name', login_denied_text => 'disabledtext', }; use constant MAPPED_RETURNS => { - login_name => 'email', realname => 'full_name', disabledtext => 'login_denied_text', }; @@ -158,8 +156,11 @@ sub offer_account_by_email { my $email = trim($params->{email}) || ThrowCodeError('param_required', { param => 'email' }); + my $login = Bugzilla->params->{use_email_as_login} ? $email : trim($params->{login}); + $login or ThrowCodeError('param_required', { param => 'login' }); + Bugzilla->user->check_account_creation_enabled; - Bugzilla->user->check_and_send_account_creation_confirmation($email); + Bugzilla->user->check_and_send_account_creation_confirmation($login, $email); return undef; } @@ -174,11 +175,16 @@ sub create { my $email = trim($params->{email}) || ThrowCodeError('param_required', { param => 'email' }); + + my $login = Bugzilla->params->{use_email_as_login} ? $email : trim($params->{login}); + $login or ThrowCodeError('param_required', { param => 'login' }); + my $realname = trim($params->{full_name}); my $password = trim($params->{password}) || '*'; my $user = Bugzilla::User->create({ - login_name => $email, + login_name => $login, + email => $email, realname => $realname, cryptpassword => $password }); @@ -225,7 +231,7 @@ sub get { @users = map { filter $params, { id => as_int($_->id), real_name => as_string($_->name), - name => as_email($_->login), + name => as_login($_->login), } } @$in_group; return { users => \@users }; @@ -277,12 +283,12 @@ sub get { my $user_info = filter $params, { id => as_int($user->id), real_name => as_string($user->name), - name => as_email($user->login), - email => as_email($user->email), + name => as_login($user->login), can_login => as_boolean($user->is_enabled ? 1 : 0), }; if (Bugzilla->user->in_group('editusers')) { + $user_info->{email} = as_email($user->email), $user_info->{email_enabled} = as_boolean($user->email_enabled); $user_info->{login_denied_text} = as_string($user->disabledtext); } @@ -651,6 +657,8 @@ This is the recommended way to create a Bugzilla account. =item C (string) - the email to send the offer to. +=item C (string) - the login of the user account. + =back =item B (nothing) @@ -699,6 +707,11 @@ are the same as below. =item C (string) - The email address for the new user. +=item C (string) - The login name for the new user. +If the installation has the C parameter switched on, then +this parameter is ignored, and the value of the C parameter will +be used as the login name for the new account. + =item C (string) B - The user's full name. Will be set to empty if not specified. @@ -768,7 +781,7 @@ C Contains ids of user to update. =item C -C Contains email/login of user to update. +C Contains login of user to update. =item C @@ -996,8 +1009,7 @@ 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. +C The login name of the user. =item can_login @@ -1083,7 +1095,7 @@ C The CGI parameters for the saved report. B: If you are not logged in to Bugzilla when you call this function, you will only be returned the C, C, and C items. If you are logged in and not in editusers group, you will only be returned the C, C, -C, C, C, and C items. The groups returned are +C, C, and C items. The groups returned are filtered based on your permission to bless each group. The C and C items are only returned if you are querying your own account, even if you are in the editusers group. diff --git a/Bugzilla/API/1_0/Util.pm b/Bugzilla/API/1_0/Util.pm index ce4487c1f..13c3eebac 100644 --- a/Bugzilla/API/1_0/Util.pm +++ b/Bugzilla/API/1_0/Util.pm @@ -33,9 +33,10 @@ our @EXPORT = qw( as_datetime as_double as_email - as_email_array as_int as_int_array + as_login + as_login_array as_name_array as_string as_string_array @@ -367,8 +368,8 @@ sub as_string { defined $_[0] ? $_[0] . '' : JSON::null } # array types -sub as_email_array { [ map { as_email($_) } @{ $_[0] // [] } ] } sub as_int_array { [ map { as_int($_) } @{ $_[0] // [] } ] } +sub as_login_array { [ map { as_login($_) } @{ $_[0] // [] } ] } sub as_name_array { [ map { as_string($_->name) } @{ $_[0] // [] } ] } sub as_string_array { [ map { as_string($_) } @{ $_[0] // [] } ] } @@ -380,10 +381,14 @@ sub as_datetime { : JSON::null; } -sub as_email { +sub as_login { defined $_[0] - ? ( Bugzilla->params->{webservice_email_filter} ? email_filter($_[0]) : $_[0] . '' ) - : JSON::null + ? ( Bugzilla->params->{use_email_as_login} ? email_filter($_[0]) : $_[0] . '' ) + : JSON::null; +} + +sub as_email { + defined($_[0]) && Bugzilla->user->in_group('editusers') ? $_[0] . '' : JSON::null; } sub as_base64 { @@ -496,13 +501,9 @@ double value. If parameter is undefined, returns JSON::null. =head2 as_email -Takes an email address as a parameter if filters it if C is -enabled in the system settings. If parameter is undefined, returns JSON::null. - -=head2 as_email_array - -Similar to C, but takes an array reference to a list of values and -returns an array reference with the converted values. +Takes an email address as a parameter. If the user is in the editusers group, +it returns the email address, unchanged. If the parameter is undefined or the +user is not in the editusers group, it returns JSON::null. =head2 as_int @@ -514,6 +515,18 @@ value. If parameter is undefined, returns JSON::null. Similar to C, but takes an array reference to a list of values and returns an array reference with the converted values. +=head2 as_login + +Takes a login name as a parameter. If C is enabled and the +user is logged out, it returns the local part of the email address (the part +before '@'). Else it returns the full login name. If parameter is undefined, +returns JSON::null. + +=head2 as_login_array + +Similar to C, but takes an array reference to a list of values and +returns an array reference with the converted values. + =head2 as_name_array Takes a list of L values and returns an array of new values diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 399f68d24..d693bf12d 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -136,6 +136,10 @@ sub extern_id_used { || $self->{_verifier}->extern_id_used; } +sub can_change_login { + return $_[0]->user_can_create_account; +} + sub can_change_email { return $_[0]->user_can_create_account; } @@ -412,6 +416,14 @@ Returns: C if users are allowed to create new Bugzilla accounts, Description: Whether or not current login system uses extern_id. +=item C + +Description: Whether or not the current login system allows users to + change their own login. +Params: None +Returns: C if users can change their own login, + C otherwise. + =item C Description: Whether or not the current login system allows users to diff --git a/Bugzilla/Auth/Verify.pm b/Bugzilla/Auth/Verify.pm index ef5b749b1..20318b3a5 100644 --- a/Bugzilla/Auth/Verify.pm +++ b/Bugzilla/Auth/Verify.pm @@ -36,7 +36,8 @@ sub create_or_update_user { my $dbh = Bugzilla->dbh; my $extern_id = $params->{extern_id}; - my $username = $params->{bz_username} || $params->{username}; + my $login = $params->{bz_username} || $params->{username}; + my $email = Bugzilla->params->{use_email_as_login} ? $login : $params->{email}; my $password = $params->{password} || '*'; my $real_name = $params->{realname} || ''; my $user_id = $params->{user_id}; @@ -44,7 +45,7 @@ sub create_or_update_user { # A passed-in user_id always overrides anything else, for determining # what account we should return. if (!$user_id) { - my $username_user_id = login_to_id($username || ''); + my $login_user_id = login_to_id($login || ''); my $extern_user_id; if ($extern_id) { trick_taint($extern_id); @@ -52,26 +53,26 @@ sub create_or_update_user { FROM profiles WHERE extern_id = ?', undef, $extern_id); } - # If we have both a valid extern_id and a valid username, and they are + # If we have both a valid extern_id and a valid login, and they are # not the same id, then we have a conflict. - if ($username_user_id && $extern_user_id - && $username_user_id ne $extern_user_id) + if ($login_user_id && $extern_user_id + && $login_user_id ne $extern_user_id) { my $extern_name = Bugzilla::User->new($extern_user_id)->login; return { failure => AUTH_ERROR, error => "extern_id_conflict", details => {extern_id => $extern_id, extern_user => $extern_name, - username => $username} }; + username => $login} }; } - # If we have a valid username, but no valid id, + # If we have a valid login, but no valid id, # then we have to create the user. This happens when we're - # passed only a username, and that username doesn't exist already. - if ($username && !$username_user_id && !$extern_user_id) { - validate_email_syntax($username) - || return { failure => AUTH_ERROR, + # passed only a login, and that login doesn't exist already. + if ($login && !$login_user_id && !$extern_user_id) { + validate_email_syntax($email) + || return { failure => AUTH_ERROR, error => 'auth_invalid_email', - details => {addr => $username} }; + details => {addr => $email} }; # Usually we'd call validate_password, but external authentication # systems might follow different standards than ours. So in this # place here, we call trick_taint without checks. @@ -79,23 +80,24 @@ sub create_or_update_user { # 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, + my $user = Bugzilla::User->create({ + login_name => $login, + email => $email, cryptpassword => $password, realname => $real_name}); - $username_user_id = $user->id; + $login_user_id = $user->id; } - # If we have a valid username id and an extern_id, but no valid + # If we have a valid login id and an extern_id, but no valid # extern_user_id, then we have to set the user's extern_id. - if ($extern_id && $username_user_id && !$extern_user_id) { + if ($extern_id && $login_user_id && !$extern_user_id) { $dbh->do('UPDATE profiles SET extern_id = ? WHERE userid = ?', - undef, $extern_id, $username_user_id); - Bugzilla->memcached->clear({ table => 'profiles', id => $username_user_id }); + undef, $extern_id, $login_user_id); + Bugzilla->memcached->clear({ table => 'profiles', id => $login_user_id }); } # Finally, at this point, one of these will give us a valid user id. - $user_id = $extern_user_id || $username_user_id; + $user_id = $extern_user_id || $login_user_id; } # If we still don't have a valid user_id, then we weren't passed @@ -109,11 +111,15 @@ sub create_or_update_user { # Now that we have a valid User, we need to see if any data has to be updated. my $changed = 0; - if ($username && lc($user->login) ne lc($username)) { - validate_email_syntax($username) + if ($email && lc($user->email) ne lc($email)) { + validate_email_syntax($email) || return { failure => AUTH_ERROR, error => 'auth_invalid_email', - details => {addr => $username} }; - $user->set_login($username); + details => {addr => $email} }; + $user->set_email($email); + $changed = 1; + } + if ($login && lc($user->login) ne lc($login)) { + $user->set_login($login); $changed = 1; } if ($real_name && $user->name ne $real_name) { diff --git a/Bugzilla/Auth/Verify/LDAP.pm b/Bugzilla/Auth/Verify/LDAP.pm index cd2e64370..2ff38a217 100644 --- a/Bugzilla/Auth/Verify/LDAP.pm +++ b/Bugzilla/Auth/Verify/LDAP.pm @@ -99,23 +99,21 @@ sub check_credentials { my @emails = $user_entry->get_value($mail_attr); # Default to the first email address returned. - $params->{bz_username} = $emails[0]; + $params->{email} = $emails[0]; if (@emails > 1) { # Cycle through the adresses and check if they're Bugzilla logins. # Use the first one that returns a valid id. foreach my $email (@emails) { - if ( login_to_id($email) ) { - $params->{bz_username} = $email; + if ( email_to_id($email) ) { + $params->{email} = $email; last; } } } - - } else { - $params->{bz_username} = $username; } + $params->{bz_username} = $username; $params->{realname} ||= $user_entry->get_value("displayName"); $params->{realname} ||= $user_entry->get_value("cn"); diff --git a/Bugzilla/Auth/Verify/RADIUS.pm b/Bugzilla/Auth/Verify/RADIUS.pm index 60be52a07..163058c54 100644 --- a/Bugzilla/Auth/Verify/RADIUS.pm +++ b/Bugzilla/Auth/Verify/RADIUS.pm @@ -46,8 +46,10 @@ sub check_credentials { Bugzilla->params->{'RADIUS_NAS_IP'} || undef) || return { failure => AUTH_LOGINFAILED }; + $params->{bz_username} = $username; + # Build the user account's e-mail address. - $params->{bz_username} = $username . $address_suffix; + $params->{email} = $username . $address_suffix; return $params; } diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index 7a7ba3a3e..79b468f0c 100644 --- a/Bugzilla/Config.pm +++ b/Bugzilla/Config.pm @@ -29,7 +29,10 @@ use File::Basename; # when it shouldn't %Bugzilla::Config::EXPORT_TAGS = ( - admin => [qw(update_params SetParam write_params)], + admin => [qw(update_params + SetParam + call_param_onchange_handlers + write_params)], ); Exporter::export_ok_tags('admin'); @@ -78,7 +81,7 @@ sub param_panels { sub SetParam { my ($name, $value) = @_; - _load_params unless %params; + _load_params() unless %params; die "Unknown param $name" unless (exists $params{$name}); my $entry = $params{$name}; @@ -96,6 +99,19 @@ sub SetParam { Bugzilla->params->{$name} = $value; } +sub call_param_onchange_handlers { + my ($changes) = @_; + + _load_params() unless %params; + + foreach my $name (@$changes) { + my $param = $params{$name}; + if (exists $param->{'onchange'}) { + $param->{'onchange'}->(Bugzilla->params->{$name}); + } + } +} + sub update_params { my ($params) = @_; my $answer = Bugzilla->installation_answers; @@ -211,7 +227,7 @@ sub update_params { # --- DEFAULTS FOR NEW PARAMS --- - _load_params unless %params; + _load_params() unless %params; foreach my $name (keys %params) { my $item = $params{$name}; unless (exists $param->{$name}) { @@ -228,8 +244,10 @@ sub update_params { } } - # Bug 452525: OR based groups are on by default for new installations - $param->{'or_groups'} = 1 if $new_install; + if ($new_install) { + $param->{'or_groups'} = 1; + $param->{'use_email_as_login'} = 0; + } # --- REMOVE OLD PARAMS --- @@ -341,6 +359,7 @@ Bugzilla::Config - Configuration parameters for Bugzilla update_params(); SetParam($param, $value); + call_param_onchange_handlers(\@changes); write_params(); =head1 DESCRIPTION @@ -371,6 +390,16 @@ Prints out information about what it's doing, if it makes any changes. May prompt the user for input, if certain required parameters are not specified. +=item C + +Expects a list of parameter names. +For each parameter, checks whether there is a change handler defined, +and if so, calls it. + +Params: C<\@changes> (required) - A list of parameter names. + +Returns: nothing + =item C Description: Writes the parameters to disk. diff --git a/Bugzilla/Config/Auth.pm b/Bugzilla/Config/Auth.pm index 219e834ee..326c4cd3f 100644 --- a/Bugzilla/Config/Auth.pm +++ b/Bugzilla/Config/Auth.pm @@ -74,12 +74,6 @@ sub get_param_list { default => '0' }, - { - name => 'webservice_email_filter', - type => 'b', - default => 0 - }, - { name => 'emailregexp', type => 't', @@ -95,9 +89,10 @@ sub get_param_list { }, { - name => 'emailsuffix', - type => 't', - default => '' + name => 'use_email_as_login', + type => 'b', + default => '1', + onchange => \&change_use_email_as_login }, { diff --git a/Bugzilla/Config/BugChange.pm b/Bugzilla/Config/BugChange.pm index b8a80dd94..b6cd7ae43 100644 --- a/Bugzilla/Config/BugChange.pm +++ b/Bugzilla/Config/BugChange.pm @@ -38,7 +38,8 @@ sub get_param_list { type => 's', choices => \@closed_bug_statuses, default => $closed_bug_statuses[0], - checker => \&check_bug_status + checker => \&check_bug_status, + onchange => \&change_duplicate_or_move_bug_status }, { diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm index 6e89fdcae..e6e0d4a23 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -29,6 +29,8 @@ use parent qw(Exporter); check_bug_status check_smtp_auth check_theschwartz_available check_maxattachmentsize check_email check_smtp_ssl check_comment_taggers_group check_smtp_server check_resolution + + change_use_email_as_login change_duplicate_or_move_bug_status ); # Checking functions for the various values @@ -362,18 +364,36 @@ sub check_comment_taggers_group { return check_group($group_name); } +# Change handler functions for various parameters + +# If use_email_as_login is turned on, update all login names to be email +# addresses. +sub change_use_email_as_login { + my $newvalue = shift; + if ($newvalue) { + Bugzilla->dbh->do('UPDATE profiles SET login_name = email'); + } +} + +sub change_duplicate_or_move_bug_status { + my $newvalue = shift; + Bugzilla::Status::add_missing_bug_status_transitions($newvalue); +} + # OK, here are the parameter definitions themselves. # # Each definition is a hash with keys: # -# name - name of the param -# desc - description of the param (for editparams.cgi) -# type - see below -# choices - (optional) see below -# default - default value for the param -# checker - (optional) checking function for validating parameter entry -# It is called with the value of the param as the first arg and a -# reference to the param's hash as the second argument +# name - name of the param +# desc - description of the param (for editparams.cgi) +# type - see below +# choices - (optional) see below +# default - default value for the param +# checker - (optional) checking function for validating parameter entry. +# It is called with the value of the param as the first arg +# and a reference to the param's hash as the second argument. +# onchange - (optional) handling function for parameter changes. +# The argument is the new value of the param. # # The type value can be one of the following: # @@ -467,6 +487,17 @@ Checks that the value is a valid regexp Checks that the required modules for comment tagging are installed, and that a valid group is provided. +=item C + +Change handler for "use_email_as_login" parameter - if param is changed to +true, updates login_name field to be the value of the email field for all +users. + +=item C + +Change handler for "duplicate_or_move_bug_status" parameter - if param is +changed, insert all missing transitions to the new bug status. + =back =head1 B diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index ca3839ca6..76ea3c0c8 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -930,6 +930,7 @@ use constant ABSTRACT_SCHEMA => { userid => {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1}, login_name => {TYPE => 'varchar(255)', NOTNULL => 1}, + email => {TYPE => 'varchar(255)', NOTNULL => 1}, cryptpassword => {TYPE => 'varchar(128)'}, realname => {TYPE => 'varchar(255)', NOTNULL => 1, DEFAULT => "''"}, @@ -948,7 +949,9 @@ use constant ABSTRACT_SCHEMA => { profiles_login_name_idx => {FIELDS => ['login_name'], TYPE => 'UNIQUE'}, profiles_extern_id_idx => {FIELDS => ['extern_id'], - TYPE => 'UNIQUE'} + TYPE => 'UNIQUE'}, + profiles_email_idx => {FIELDS => ['email'], + TYPE => 'UNIQUE'} ], }, diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index cbf831bc5..644a02e7f 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -301,8 +301,8 @@ sub _check_cc_list { my @addresses = split(/[,\s]+/, $cc_list); my $addr_spec = $Email::Address::addr_spec; - # We do not call check_email_syntax() because these addresses do not - # require to match 'emailregexp' and do not depend on 'emailsuffix'. + # We do not call check_email_syntax() because these addresses are not + # required to match 'emailregexp'. foreach my $address (@addresses) { ($address !~ /\P{ASCII}/ && $address =~ /^$addr_spec$/) || ThrowUserError('illegal_email_address', diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm index 9ea851c20..61c085c0e 100644 --- a/Bugzilla/Group.pm +++ b/Bugzilla/Group.pm @@ -322,7 +322,7 @@ sub _rederive_regexp { my ($self) = @_; my $dbh = Bugzilla->dbh; - my $sth = $dbh->prepare("SELECT userid, login_name, group_id + my $sth = $dbh->prepare("SELECT userid, email, group_id FROM profiles LEFT JOIN user_group_map ON user_group_map.user_id = profiles.userid @@ -337,8 +337,8 @@ sub _rederive_regexp { AND grant_type = ? and isbless = 0"); $sth->execute($self->id, GRANT_REGEXP); my $regexp = $self->user_regexp; - while (my ($uid, $login, $present) = $sth->fetchrow_array) { - if ($regexp ne '' and $login =~ /$regexp/i) { + while (my ($uid, $email, $present) = $sth->fetchrow_array) { + if ($regexp ne '' and $email =~ /$regexp/i) { $sthadd->execute($uid, $self->id, GRANT_REGEXP) unless $present; } else { $sthdel->execute($uid, $self->id, GRANT_REGEXP) if $present; diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index 8d0976d9c..3f902cab0 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -1631,10 +1631,14 @@ Params: =over +=item C + +The email address of the new account. + =item C -The login of the new account. This is usually an email address, unless the -C parameter is not empty. +The login of the new account. This will be the same as the email address if +the "use_email_as_login" parameter is set, otherwise it may be different. =back diff --git a/Bugzilla/Install.pm b/Bugzilla/Install.pm index e1ed65c36..37c87a2b1 100644 --- a/Bugzilla/Install.pm +++ b/Bugzilla/Install.pm @@ -308,27 +308,47 @@ sub create_admin { my $template = Bugzilla->template; my $admin_group = new Bugzilla::Group({ name => 'admin' }); - my $admin_inheritors = + my $admin_inheritors = Bugzilla::Group->flatten_group_membership($admin_group->id); my $admin_group_ids = join(',', @$admin_inheritors); my ($admin_count) = $dbh->selectrow_array( - "SELECT COUNT(*) FROM user_group_map + "SELECT COUNT(*) FROM user_group_map WHERE group_id IN ($admin_group_ids)"); return if $admin_count; my %answer = %{Bugzilla->installation_answers}; - my $login = $answer{'ADMIN_EMAIL'}; + my $login = $answer{'ADMIN_LOGIN'}; + my $email = $answer{'ADMIN_EMAIL'}; my $password = $answer{'ADMIN_PASSWORD'}; my $full_name = $answer{'ADMIN_REALNAME'}; - if (!$login || !$password || !$full_name) { + if (!($login || Bugzilla->params->{'use_email_as_login'}) + || !$email + || !$password) + { say "\n" . get_text('install_admin_setup') . "\n"; } - while (!$login) { + while (!$email) { print get_text('install_admin_get_email') . ' '; + $email = ; + chomp $email; + eval { Bugzilla::User->check_email($email); }; + if ($@) { + say $@; + undef $email; + } + } + + # Make sure the email address is used as login when required. + if (Bugzilla->params->{'use_email_as_login'}) { + $login = $email; + } + + while (!$login) { + print get_text('install_admin_get_login') . ' '; $login = ; chomp $login; eval { Bugzilla::User->check_login_name($login); }; @@ -349,7 +369,8 @@ sub create_admin { get_text('install_admin_get_password')); } - my $admin = Bugzilla::User->create({ login_name => $login, + my $admin = Bugzilla::User->create({ login_name => $login, + email => $email, realname => $full_name, cryptpassword => $password }); make_admin($admin); diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index a6ca1e90f..b7ca7c657 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -748,6 +748,10 @@ sub update_table_definitions { # 2015-12-16 LpSolit@gmail.com - Bug 1232578 _sanitize_audit_log_table(); + # 2016-04-27 wurblzap@gmail.com and gerv@gerv.net - Bug 218917 + # Split login_name into login_name and email columns + _split_login_and_email($old_params); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -3957,6 +3961,27 @@ sub _sanitize_audit_log_table { } } +sub _split_login_and_email { + my ($old_params) = (@_); + + my $dbh = Bugzilla->dbh; + $dbh->bz_add_column('profiles', 'email', + {TYPE => 'varchar(255)', NOTNULL => 1}, ''); + $dbh->do('UPDATE profiles SET email = login_name'); + + # This change obsoletes the 'emailsuffix' parameter. If it is in use, + # append it to all the values in the 'email' column. + my $suffix = $old_params->{'emailsuffix'}; + if ($suffix) { + $dbh->do('UPDATE profiles SET email = ' . $dbh->sql_string_concat('email', '?'), + undef, $suffix); + } + + $dbh->bz_add_index('profiles', 'profiles_email_idx', + {TYPE => 'UNIQUE', FIELDS => ['email']}); +} + + 1; __END__ diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index 618d80cfa..dea207f21 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -1170,7 +1170,7 @@ sub create { my $cache = Bugzilla->request_cache; return $cache->{login_not_email} //= - ($params->{emailsuffix} + (!$params->{use_email_as_login} || ($params->{user_verify_class} =~ /LDAP/ && $params->{LDAPmailattribute}) || ($params->{user_verify_class} =~ /RADIUS/ && $params->{RADIUS_email_suffix})) ? 1 : 0; diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm index b6f07468e..4f522bda9 100644 --- a/Bugzilla/Token.pm +++ b/Bugzilla/Token.pm @@ -89,30 +89,32 @@ sub check_auth_delegation_token { # Creates and sends a token to create a new user account. # It assumes that the login has the correct format and is not already in use. sub issue_new_user_account_token { - my $login_name = shift; + my ($login, $email) = @_; my $dbh = Bugzilla->dbh; my $template = Bugzilla->template; my $vars = {}; - # Is there already a pending request for this login name? If yes, do not throw + # Is there already a pending request for this email? If yes, do not throw # an error because the user may have lost their email with the token inside. # But to prevent using this way to mailbomb an email address, make sure # the last request is old enough before sending a new email (default: 10 minutes). + my $regexp = "^$email:"; my $pending_requests = $dbh->selectrow_array( 'SELECT COUNT(*) FROM tokens WHERE tokentype = ? - AND ' . $dbh->sql_istrcmp('eventdata', '?') . ' + AND ' . $dbh->sql_regexp('eventdata', $dbh->quote($regexp)) . ' AND issuedate > ' . $dbh->sql_date_math('NOW()', '-', ACCOUNT_CHANGE_INTERVAL, 'MINUTE'), - undef, ('account', $login_name)); + undef, 'account'); ThrowUserError('too_soon_for_new_token', {'type' => 'account'}) if $pending_requests; - my ($token, $token_ts) = _create_token(undef, 'account', $login_name); + my ($token, $token_ts) = _create_token(undef, 'account', "$email:$login"); - $vars->{'email'} = $login_name . Bugzilla->params->{'emailsuffix'}; + $vars->{'login'} = $login; + $vars->{'email'} = $email; $vars->{'expiration_ts'} = ctime($token_ts + MAX_TOKEN_AGE * 86400); $vars->{'token'} = $token; @@ -131,15 +133,15 @@ sub IssueEmailChangeToken { my $new_email = shift; my $user = Bugzilla->user; - my ($token, $token_ts) = _create_token($user->id, 'emailold', $user->login . ":$new_email"); - my $newtoken = _create_token($user->id, 'emailnew', $user->login . ":$new_email"); + my ($token, $token_ts) = _create_token($user->id, 'emailold', $user->email . ":$new_email"); + my $newtoken = _create_token($user->id, 'emailnew', $user->email . ":$new_email"); # Mail the user the token along with instructions for using it. my $template = Bugzilla->template_inner($user->setting('lang')); my $vars = {}; - $vars->{'newemailaddress'} = $new_email . Bugzilla->params->{'emailsuffix'}; + $vars->{'newemailaddress'} = $new_email; $vars->{'expiration_ts'} = ctime($token_ts + MAX_TOKEN_AGE * 86400); # First send an email to the new address. If this one doesn't exist, @@ -335,7 +337,17 @@ sub Cancel { # is no entry in the 'profiles' table. my $user = new Bugzilla::User($userid); - $vars->{'emailaddress'} = $userid ? $user->email : $eventdata; + if ($userid) { + $vars->{'emailaddress'} = $user->email; + $vars->{'login'} = $user->login; + } + else { + # Be careful! Some logins may contain ":" in them. + my ($email, $login) = split(':', $eventdata, 2); + $vars->{'emailaddress'} = $email; + $vars->{'login'} = $login; + } + $vars->{'remoteaddress'} = remote_ip(); $vars->{'token'} = $token; $vars->{'tokentype'} = $tokentype; @@ -511,7 +523,7 @@ Bugzilla::Token - Provides different routines to manage tokens. use Bugzilla::Token; - Bugzilla::Token::issue_new_user_account_token($login_name); + Bugzilla::Token::issue_new_user_account_token($login, $email); Bugzilla::Token::IssueEmailChangeToken($user, $new_email); Bugzilla::Token::IssuePasswordToken($user); Bugzilla::Token::DeletePasswordTokens($user_id, $reason); @@ -539,14 +551,15 @@ Bugzilla::Token - Provides different routines to manage tokens. Returns: The token. -=item C +=item C Description: Creates and sends a token per email to the email address requesting a new user account. It doesn't check whether the user account already exists. The user will have to use this token to confirm the creation of their user account. - Params: $login_name - The new login name requested by the user. + Params: $login - The new login name requested by the user. + $email - The email address to be associated with the account. Returns: Nothing. It throws an error if the same user made the same request in the last few minutes. diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 91defebb4..c1e3adee3 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -32,7 +32,7 @@ use URI; use URI::QueryParam; use parent qw(Bugzilla::Object Exporter); -@Bugzilla::User::EXPORT = qw(is_available_username +@Bugzilla::User::EXPORT = qw(is_available_email email_to_id login_to_id validate_password validate_password_check USER_MATCH_MULTIPLE USER_MATCH_FAILED USER_MATCH_SUCCESS MATCH_SKIP_CONFIRM @@ -50,12 +50,13 @@ use constant MATCH_SKIP_CONFIRM => 1; use constant DEFAULT_USER => { 'userid' => 0, - 'realname' => '', 'login_name' => '', + 'email' => '', + 'realname' => '', 'showmybugslink' => 0, 'disabledtext' => '', 'disable_mail' => 0, - 'is_enabled' => 1, + 'is_enabled' => 1, }; use constant DB_TABLE => 'profiles'; @@ -69,6 +70,7 @@ sub DB_COLUMNS { return ( 'profiles.userid', 'profiles.login_name', + 'profiles.email', 'profiles.realname', 'profiles.mybugslink AS showmybugslink', 'profiles.disabledtext', @@ -88,9 +90,10 @@ use constant VALIDATORS => { disable_mail => \&_check_disable_mail, disabledtext => \&_check_disabledtext, login_name => \&check_login_name, + email => \&check_email, realname => \&_check_realname, extern_id => \&_check_extern_id, - is_enabled => \&_check_is_enabled, + is_enabled => \&_check_is_enabled, }; sub UPDATE_COLUMNS { @@ -99,6 +102,7 @@ sub UPDATE_COLUMNS { disable_mail disabledtext login_name + email realname extern_id is_enabled @@ -108,7 +112,8 @@ sub UPDATE_COLUMNS { }; use constant VALIDATOR_DEPENDENCIES => { - is_enabled => ['disabledtext'], + is_enabled => ['disabledtext'], + login_name => ['email'], }; use constant EXTRA_REQUIRED_FIELDS => qw(is_enabled); @@ -222,7 +227,7 @@ sub update { my $dbh = Bugzilla->dbh; $self->_update_groups($group_changes, $changes); - if (exists $changes->{login_name}) { + if (exists $changes->{email}) { # Delete all the tokens related to the userid $dbh->do('DELETE FROM tokens WHERE userid = ?', undef, $self->id) unless $options->{keep_tokens}; @@ -231,15 +236,16 @@ sub update { } # Logout the user if necessary. - Bugzilla->logout_user($self) + Bugzilla->logout_user($self) if (!$options->{keep_session} && (exists $changes->{login_name} + || exists $changes->{email} || exists $changes->{disabledtext} || exists $changes->{cryptpassword})); # XXX Can update profiles_activity here as soon as it understands # field names like login_name. - + return $changes; } @@ -266,23 +272,53 @@ sub _check_extern_id { return $extern_id; } -# This is public since createaccount.cgi needs to use it before issuing -# a token for account creation. sub check_login_name { - my ($invocant, $name) = @_; - $name = trim($name); - $name || ThrowUserError('user_login_required'); - check_email_syntax($name); + my ($invocant, $login, undef, $data) = @_; + + # No control characters + $login = clean_text($login); + $login || ThrowUserError('user_login_required'); + # No whitespace + $login !~ /\s/ || ThrowUserError('login_illegal_character'); + + # No @ sign unless login is email (VALIDATOR_DEPENDENCIES means + # this will be set already) + if ($login =~ /@/) { + my $email = ref($invocant) ? $invocant->email : $data->{email}; + # We should really use fc() instead of lc(), but this requires Perl 5.16. + ThrowUserError('login_at_sign_disallowed') unless lc($login) eq lc($email); + } + # We set the max length to 127 to ensure logins aren't truncated when + # inserted into the tokens.eventdata field. + length($login) <= 127 or ThrowUserError('login_too_long'); + + trick_taint($login); + + # Check the login name if it's a new user, or if we're changing the login name. + if (!ref($invocant) || lc($invocant->login) ne lc($login)) { + if (login_to_id($login)) { + ThrowUserError('account_exists', { login => $login }); + } + } - # Check the name if it's a new user, or if we're changing the name. - if (!ref($invocant) || lc($invocant->login) ne lc($name)) { - my @params = ($name); - push(@params, $invocant->login) if ref($invocant); - is_available_username(@params) - || ThrowUserError('account_exists', { email => $name }); + return $login; +} + +sub check_email { + my ($invocant, $email) = @_; + $email = clean_text($email); + $email || ThrowUserError('email_required'); + + check_email_syntax($email); + + # Check the email if it's a new user, or if we're changing the email. + my $old_email = ref($invocant) ? $invocant->email : undef; + if (!defined($old_email) || lc($old_email) ne lc($email)) { + is_available_email($email, $old_email) + || ThrowUserError('account_exists', { email => $email }); } - return $name; + return $email; } sub _check_password { @@ -325,6 +361,12 @@ sub set_login { delete $self->{nick}; } +sub set_email { + my ($self, $email) = @_; + $self->set('email', $email); + $self->set_login($email) if Bugzilla->params->{'use_email_as_login'}; +} + sub set_name { my ($self, $name) = @_; $self->set('realname', $name); @@ -470,7 +512,7 @@ sub update_last_seen_date { sub name { $_[0]->{realname}; } sub login { $_[0]->{login_name}; } sub extern_id { $_[0]->{extern_id}; } -sub email { $_[0]->login . Bugzilla->params->{'emailsuffix'}; } +sub email { $_[0]->{email}; } sub disabledtext { $_[0]->{'disabledtext'}; } sub is_enabled { $_[0]->{'is_enabled'} ? 1 : 0; } sub showmybugslink { $_[0]->{showmybugslink}; } @@ -502,14 +544,17 @@ sub authorizer { # Generate a string to identify the user by name + login if the user # has a name or by login only if they don't. +# +# See also get_userlist(), which constructs pseudo-Bugzilla::Users, including +# the 'identity' value. sub identity { my $self = shift; return "" unless $self->id; if (!defined $self->{identity}) { - $self->{identity} = - $self->name ? $self->name . " <" . $self->login. ">" : $self->login; + $self->{identity} = + $self->name ? $self->name . " (" . $self->login. ")" : $self->login; } return $self->{identity}; @@ -521,6 +566,7 @@ sub nick { return "" unless $self->id; if (!defined $self->{nick}) { + # This has the correct result even if the login does not contain an @. $self->{nick} = (split(/@/, $self->login, 2))[0]; } @@ -1730,8 +1776,8 @@ sub can_bless { } sub match { - # Generates a list of users whose login name (email address) or real name - # matches a substring or wildcard. + # Generates a list of users whose login name or real name matches a + # substring or wildcard. # This is also called if matches are disabled (for error checking), but # in this case only the exact match code will end up running. @@ -1780,8 +1826,10 @@ sub match { @users = @{Bugzilla::User->new_from_list($user_ids)}; } else { # try an exact match - # Exact matches don't care if a user is disabled. + # Exact matches don't care if a user is disabled, and match + # login names only. trick_taint($str); + my $user_id = $dbh->selectrow_array('SELECT userid FROM profiles WHERE ' . $dbh->sql_istrcmp('login_name', '?'), undef, $str); @@ -2253,7 +2301,7 @@ sub get_userlist { while (my($login, $name, $visible) = $sth->fetchrow_array) { push @userlist, { login => $login, - identity => $name ? "$name <$login>" : $login, + identity => $name ? "$name ($login)" : $login, visible => $visible, }; } @@ -2375,17 +2423,15 @@ sub check_current_password { # Subroutines # ############### -sub is_available_username { - my ($username, $old_username) = @_; +sub is_available_email { + my ($email, $old_email) = @_; - if(login_to_id($username) != 0) { - return 0; - } + return 0 if email_to_id($email); my $dbh = Bugzilla->dbh; - # $username is safe because it is only used in SELECT placeholders. - trick_taint($username); - # Reject if the new login is part of an email change which is + # $email is safe because it is only used in SELECT placeholders. + trick_taint($email); + # Reject if the new email is part of an email change which is # still in progress # # substring/locate stuff: bug 165221; this used to use regexes, but that @@ -2401,13 +2447,13 @@ sub is_available_username { OR (tokentype = 'emailnew' AND SUBSTRING(eventdata, (" . $dbh->sql_position(q{':'}, 'eventdata') . "+ 1), LENGTH(eventdata)) = ?)", - undef, ($username, $username)); + undef, ($email, $email)); if ($eventdata) { # Allow thru owner of token - if ($old_username - && (($tokentype eq 'emailnew' && $eventdata eq "$old_username:$username") - || ($tokentype eq 'emailold' && $eventdata eq "$username:$old_username"))) + if ($old_email + && (($tokentype eq 'emailnew' && $eventdata eq "$old_email:$email") + || ($tokentype eq 'emailold' && $eventdata eq "$email:$old_email"))) { return 1; } @@ -2429,24 +2475,27 @@ sub check_account_creation_enabled { } sub check_and_send_account_creation_confirmation { - my ($self, $login) = @_; + my ($self, $login, $email) = @_; + my $class = ref($self) || $self; my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction; - $login = $self->check_login_name($login); + $email = $class->check_email($email); + $login = $class->check_login_name($login, undef, { email => $email }); my $creation_regexp = Bugzilla->params->{'createemailregexp'}; - if ($login !~ /$creation_regexp/i) { + if ($email !~ /$creation_regexp/i) { ThrowUserError('account_creation_restricted'); } # Allow extensions to do extra checks. - Bugzilla::Hook::process('user_check_account_creation', { login => $login }); + Bugzilla::Hook::process('user_check_account_creation', + { login => $login, email => $email }); # Create and send a token for this new account. require Bugzilla::Token; - Bugzilla::Token::issue_new_user_account_token($login); + Bugzilla::Token::issue_new_user_account_token($login, $email); $dbh->bz_commit_transaction; } @@ -2458,20 +2507,17 @@ sub login_to_id { my $dbh = Bugzilla->dbh; my $cache = Bugzilla->request_cache->{user_login_to_id} ||= {}; - # We cache lookups because this function showed up as taking up a + # We cache lookups because this function showed up as taking up a # significant amount of time in profiles of xt/search.t. However, - # for users that don't exist, we re-do the check every time, because - # otherwise we break is_available_username. + # for users that don't exist, we re-do the check every time. my $user_id; if (defined $cache->{$login}) { $user_id = $cache->{$login}; } else { - # No need to validate $login -- it will be used by the following SELECT - # statement only, so it's safe to simply trick_taint. trick_taint($login); $user_id = $dbh->selectrow_array( - "SELECT userid FROM profiles + "SELECT userid FROM profiles WHERE " . $dbh->sql_istrcmp('login_name', '?'), undef, $login); $cache->{$login} = $user_id; } @@ -2485,6 +2531,24 @@ sub login_to_id { } } +sub email_to_id { + my ($email, $throw_error) = @_; + my $dbh = Bugzilla->dbh; + trick_taint($email); + my $user_id = $dbh->selectrow_array("SELECT userid FROM profiles WHERE " . + $dbh->sql_istrcmp('email', '?'), + undef, $email); + if ($user_id) { + return $user_id; + } + elsif ($throw_error) { + ThrowUserError('invalid_email', { email => $email }); + } + else { + return 0; + } +} + sub validate_password { my $check = validate_password_check(@_); ThrowUserError($check) if $check; @@ -2532,14 +2596,15 @@ Bugzilla::User - Object for a Bugzilla user my $user = new Bugzilla::User($id); - my @get_selectable_classifications = + my @get_selectable_classifications = $user->get_selectable_classifications; # Class Functions - $user = Bugzilla::User->create({ - login_name => $username, - realname => $realname, - cryptpassword => $plaintext_password, + $user = Bugzilla::User->create({ + login_name => $username, + email => $email, + realname => $realname, + cryptpassword => $plaintext_password, disabledtext => $disabledtext, disable_mail => 0}); @@ -2595,6 +2660,27 @@ database changes and so on. =back +=head2 Validators + +=over + +=item C + +Returns the sanitized email address if that email address is well formatted +and not already used by another user account. Else an error is thrown. + +=item C + +Returns the sanitized login name if: + +1) it is not already used by another user account; and +2) it contains no forbidden characters, which means no whitespace characters; and +3) it contains no @ character (unless it exactly matches the email address of the account). + +Else an error is thrown. + +=back + =head2 Saved and Shared Queries =over @@ -2734,8 +2820,7 @@ Returns the login name for this user. =item C -Returns the user's email address. Currently this is the same value as the -login. +Returns the user's email address. =item C @@ -2754,10 +2839,10 @@ otherwise. =item C -Returns a user "nickname" -- i.e. a shorter, not-necessarily-unique name by -which to identify the user. Currently the part of the user's email address -before the at sign (@), but that could change, especially if we implement -usernames not dependent on email address. +Usually, this is an alias for C. +If email addresses are used as logins, though, then this is the part of the +user's email address before the at sign (@). In this case, nicks are not +unique. =item C @@ -3141,7 +3226,8 @@ called "statically," just like a normal procedural function. The same as L. Params: login_name - B The login name for the new user. - realname - The full name for the new user. + email - B The email address of the new user. + realname - The full name of 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 @@ -3162,30 +3248,29 @@ user with that username. Returns a C object. Checks that users can create new user accounts, and throws an error if user creation is disabled. -=item C +=item C If the user request for a new account passes validation checks, an email is sent to this user for confirmation. Otherwise an error is thrown indicating why the request has been rejected. -=item C +=item C -Returns a boolean indicating whether or not the supplied username is +Returns a boolean indicating whether or not the supplied email address is already taken in Bugzilla. -Params: $username (scalar, string) - The full login name of the username - that you are checking. - $old_username (scalar, string) - If you are checking an email-change - token, insert the "old" username that the user is changing from, - here. Then, as long as it's the right user for that token, they - can change their username to $username. (That is, this function +Params: $email (scalar, string) - The email address that you are checking. + $old_email (scalar, string) - If you are checking an email-change + token, insert the "old" address that the user is changing from, + here. Then, as long as it's the right user for that token, he + can change his email to $email. (That is, this function will return a boolean true value). =item C Takes a login name of a Bugzilla user and changes that into a numeric ID for that user. This ID can then be passed to Bugzilla::User::new to -create a new user. +create a new user object. If no valid user exists with that login name, then the function returns 0. However, if $throw_error is set, the function will throw a user error @@ -3197,6 +3282,11 @@ of a user, but you don't want the full weight of Bugzilla::User. However, consider using a Bugzilla::User object instead of this function if you need more information about the user than just their ID. +=item C + +Same as C, but operates on an email address instead of a login +name. + =item C Returns true if a password is valid (i.e. meets Bugzilla's @@ -3282,8 +3372,6 @@ L =item groups_with_icon -=item check_login_name - =item set_extern_id =item mail_settings @@ -3300,6 +3388,8 @@ L =item set_login +=item set_email + =item set_password =item last_seen_date diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 06022ce7c..e673a920e 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -690,9 +690,8 @@ sub generate_random_password { } sub validate_email_syntax { - my ($addr) = @_; + my ($email) = @_; my $match = Bugzilla->params->{'emailregexp'}; - my $email = $addr . Bugzilla->params->{'emailsuffix'}; # This regexp follows RFC 2822 section 3.4.1. my $addr_spec = $Email::Address::addr_spec; # RFC 2822 section 2.1 specifies that email addresses must @@ -700,7 +699,7 @@ sub validate_email_syntax { # Email::Address::addr_spec doesn't enforce this. # We set the max length to 127 to ensure addresses aren't truncated when # inserted into the tokens.eventdata field. - if ($addr =~ /$match/ + if ($email =~ /$match/ && $email !~ /\P{ASCII}/ && $email =~ /^$addr_spec$/ && length($email) <= 127) @@ -713,11 +712,10 @@ sub validate_email_syntax { } sub check_email_syntax { - my ($addr) = @_; + my ($email) = @_; unless (validate_email_syntax(@_)) { - my $email = $addr . Bugzilla->params->{'emailsuffix'}; - ThrowUserError('illegal_email_address', { addr => $email }); + ThrowUserError('illegal_email_address', { email => $email }); } } diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 2279395a1..d137000a9 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -366,7 +366,7 @@ sub _translate_comment { my $comment_hash = { id => $self->type('int', $comment->id), bug_id => $self->type('int', $comment->bug_id), - creator => $self->type('email', $comment->author->login), + creator => $self->type('login', $comment->author->login), time => $self->type('dateTime', $comment->creation_ts), creation_time => $self->type('dateTime', $comment->creation_ts), is_private => $self->type('boolean', $comment->is_private), @@ -1308,7 +1308,7 @@ sub _bug_to_hash { $item{alias} = [ map { $self->type('string', $_) } @{ $bug->alias } ]; } if (filter_wants $params, 'assigned_to') { - $item{'assigned_to'} = $self->type('email', $bug->assigned_to->login); + $item{'assigned_to'} = $self->type('login', $bug->assigned_to->login); $item{'assigned_to_detail'} = $self->_user_to_hash($bug->assigned_to, $params, undef, 'assigned_to'); } if (filter_wants $params, 'blocks') { @@ -1322,7 +1322,7 @@ sub _bug_to_hash { $item{component} = $self->type('string', $bug->component); } if (filter_wants $params, 'cc') { - my @cc = map { $self->type('email', $_) } @{ $bug->cc }; + my @cc = map { $self->type('login', $_) } @{ $bug->cc }; $item{'cc'} = \@cc; $item{'cc_detail'} = [ map { $self->_user_to_hash($_, $params, undef, 'cc') } @{ $bug->cc_users } ]; } @@ -1330,7 +1330,7 @@ sub _bug_to_hash { $item{'creation_time'} = $self->type('dateTime', $bug->creation_ts); } if (filter_wants $params, 'creator') { - $item{'creator'} = $self->type('email', $bug->reporter->login); + $item{'creator'} = $self->type('login', $bug->reporter->login); $item{'creator_detail'} = $self->_user_to_hash($bug->reporter, $params, undef, 'creator'); } if (filter_wants $params, 'depends_on') { @@ -1361,7 +1361,7 @@ sub _bug_to_hash { } if (filter_wants $params, 'qa_contact') { my $qa_login = $bug->qa_contact ? $bug->qa_contact->login : ''; - $item{'qa_contact'} = $self->type('email', $qa_login); + $item{'qa_contact'} = $self->type('login', $qa_login); if ($bug->qa_contact) { $item{'qa_contact_detail'} = $self->_user_to_hash($bug->qa_contact, $params, undef, 'qa_contact'); } @@ -1433,8 +1433,7 @@ sub _user_to_hash { my $item = filter $filters, { id => $self->type('int', $user->id), real_name => $self->type('string', $user->name), - name => $self->type('email', $user->login), - email => $self->type('email', $user->email), + name => $self->type('login', $user->login), }, $types, $prefix; return $item; } @@ -1455,10 +1454,10 @@ sub _attachment_to_hash { is_patch => $self->type('int', $attach->ispatch), }, $types, $prefix; - # creator requires an extra lookup, so we only send them if - # the filter wants them. + # creator requires an extra lookup, so we only send it if + # the filter wants it. if (filter_wants $filters, 'creator', $types, $prefix) { - $item->{'creator'} = $self->type('email', $attach->attacher->login); + $item->{'creator'} = $self->type('login', $attach->attacher->login); } if (filter_wants $filters, 'data', $types, $prefix) { @@ -1490,7 +1489,7 @@ sub _flag_to_hash { foreach my $field (qw(setter requestee)) { my $field_id = $field . "_id"; - $item->{$field} = $self->type('email', $flag->$field->login) + $item->{$field} = $self->type('login', $flag->$field->login) if $flag->$field_id; } @@ -2575,10 +2574,6 @@ C The 'real' name for this user, if any. C The user's Bugzilla login. -=item C - -C The user's email address. Currently this is the same value as the name. - =back =back @@ -2698,7 +2693,11 @@ in Bugzilla B<4.4>. =item REST API call added in Bugzilla B<5.0>. -=item In Bugzilla B<5.0>, the following items were added to the bugs return value: C, C, C. +=item In Bugzilla B<5.0>, the following items were added to the bugs return +value: C, C, C. + +=item Since Bugzilla B<6.0>, the email address of users is private and is no +longer returned as part of the user detail hash. =back diff --git a/Bugzilla/WebService/Bugzilla.pm b/Bugzilla/WebService/Bugzilla.pm index 67e40a15f..c0df4754f 100644 --- a/Bugzilla/WebService/Bugzilla.pm +++ b/Bugzilla/WebService/Bugzilla.pm @@ -62,7 +62,6 @@ use constant PARAMETERS_LOGGED_IN => qw( defaultseverity duplicate_or_move_bug_status emailregexpdesc - emailsuffix letsubmitterchoosemilestone letsubmitterchoosepriority mailfrom @@ -75,6 +74,7 @@ use constant PARAMETERS_LOGGED_IN => qw( resolution_forbidden_with_open_blockers search_allow_no_criteria urlbase + use_email_as_login use_see_also useclassification usemenuforusers @@ -417,7 +417,6 @@ A logged-in user can access the following parameters (listed alphabetically): C, C, C, - C, C, C, C, @@ -430,6 +429,7 @@ A logged-in user can access the following parameters (listed alphabetically): C, C, C, + C, C, C, C, diff --git a/Bugzilla/WebService/Group.pm b/Bugzilla/WebService/Group.pm index ec09604c4..a9539f83d 100644 --- a/Bugzilla/WebService/Group.pm +++ b/Bugzilla/WebService/Group.pm @@ -210,8 +210,8 @@ sub _get_group_membership { map {{ id => $self->type('int', $_->id), real_name => $self->type('string', $_->name), - name => $self->type('string', $_->login), - email => $self->type('string', $_->email), + name => $self->type('login', $_->login), + email => $self->type('email', $_->email), can_login => $self->type('boolean', $_->is_enabled), email_enabled => $self->type('boolean', $_->email_enabled), login_denied_text => $self->type('string', $_->disabledtext), @@ -282,7 +282,7 @@ name of the group. =item C -C A regular expression. Any user whose Bugzilla username matches +C A regular expression. Any user whose Bugzilla email address matches this regular expression will automatically be granted membership in this group. =item C @@ -553,8 +553,7 @@ 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. +C The login name of the user. =item can_login diff --git a/Bugzilla/WebService/Product.pm b/Bugzilla/WebService/Product.pm index 879d8eac1..66bd32808 100644 --- a/Bugzilla/WebService/Product.pm +++ b/Bugzilla/WebService/Product.pm @@ -271,9 +271,9 @@ sub _component_to_hash { description => $self->type('string' , $component->description), default_assigned_to => - $self->type('email', $component->default_assignee->login), + $self->type('login', $component->default_assignee->login), default_qa_contact => - $self->type('email', $component->default_qa_contact ? + $self->type('login', $component->default_qa_contact ? $component->default_qa_contact->login : ""), sort_key => # sort_key is returned to match Bug.fields 0, diff --git a/Bugzilla/WebService/Server/JSONRPC.pm b/Bugzilla/WebService/Server/JSONRPC.pm index 93d315fde..b488c8de5 100644 --- a/Bugzilla/WebService/Server/JSONRPC.pm +++ b/Bugzilla/WebService/Server/JSONRPC.pm @@ -222,8 +222,11 @@ sub type { utf8::encode($value) if utf8::is_utf8($value); $retval = encode_base64($value, ''); } - elsif ($type eq 'email' && Bugzilla->params->{'webservice_email_filter'}) { - $retval = email_filter($value); + elsif ($type eq 'login') { + $retval = Bugzilla->params->{'use_email_as_login'} ? email_filter($retval) : "$retval"; + } + elsif ($type eq 'email') { + $retval = Bugzilla->user->in_group('editusers') ? "$retval" : ''; } return $retval; diff --git a/Bugzilla/WebService/Server/XMLRPC.pm b/Bugzilla/WebService/Server/XMLRPC.pm index f697f0927..c6461abd6 100644 --- a/Bugzilla/WebService/Server/XMLRPC.pm +++ b/Bugzilla/WebService/Server/XMLRPC.pm @@ -35,11 +35,13 @@ BEGIN { $value = Bugzilla::WebService::Server->datetime_format_outbound($value); $value =~ s/-//g; } + elsif ($type eq 'login') { + $type = 'string'; + $value = email_filter($value) if Bugzilla->params->{'use_email_as_login'}; + } elsif ($type eq 'email') { $type = 'string'; - if (Bugzilla->params->{'webservice_email_filter'}) { - $value = email_filter($value); - } + $value = '' unless Bugzilla->user->in_group('editusers'); } return XMLRPC::Data->type($type)->value($value); }; diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm index 079e0782e..d69df5056 100644 --- a/Bugzilla/WebService/User.pm +++ b/Bugzilla/WebService/User.pm @@ -43,13 +43,11 @@ use constant PUBLIC_METHODS => qw( ); use constant MAPPED_FIELDS => { - email => 'login', full_name => 'name', login_denied_text => 'disabledtext', }; use constant MAPPED_RETURNS => { - login_name => 'email', realname => 'full_name', disabledtext => 'login_denied_text', }; @@ -67,7 +65,7 @@ sub login { return $self->_login_to_hash($user); } - # Username and password params are required + # Login name and password params are required foreach my $param ("login", "password") { (defined $params->{$param} || defined $params->{'Bugzilla_' . $param}) || ThrowCodeError('param_required', { param => $param }); @@ -103,8 +101,11 @@ sub offer_account_by_email { my $email = trim($params->{email}) || ThrowCodeError('param_required', { param => 'email' }); + my $login = Bugzilla->params->{use_email_as_login} ? $email : trim($params->{login}); + $login or ThrowCodeError('param_required', { param => 'login' }); + Bugzilla->user->check_account_creation_enabled; - Bugzilla->user->check_and_send_account_creation_confirmation($email); + Bugzilla->user->check_and_send_account_creation_confirmation($login, $email); return undef; } @@ -119,11 +120,16 @@ sub create { my $email = trim($params->{email}) || ThrowCodeError('param_required', { param => 'email' }); + + my $login = Bugzilla->params->{use_email_as_login} ? $email : trim($params->{login}); + $login or ThrowCodeError('param_required', { param => 'login' }); + my $realname = trim($params->{full_name}); my $password = trim($params->{password}) || '*'; my $user = Bugzilla::User->create({ - login_name => $email, + login_name => $login, + email => $email, realname => $realname, cryptpassword => $password }); @@ -170,7 +176,7 @@ sub get { @users = map { filter $params, { id => $self->type('int', $_->id), real_name => $self->type('string', $_->name), - name => $self->type('email', $_->login), + name => $self->type('login', $_->login), } } @$in_group; return { users => \@users }; @@ -222,12 +228,12 @@ sub get { my $user_info = filter $params, { id => $self->type('int', $user->id), real_name => $self->type('string', $user->name), - name => $self->type('email', $user->login), - email => $self->type('email', $user->email), + name => $self->type('login', $user->login), can_login => $self->type('boolean', $user->is_enabled ? 1 : 0), }; if (Bugzilla->user->in_group('editusers')) { + $user_info->{email} = $self->type('email', $user->email), $user_info->{email_enabled} = $self->type('boolean', $user->email_enabled); $user_info->{login_denied_text} = $self->type('string', $user->disabledtext); } @@ -605,10 +611,15 @@ and real name. This is the recommended way to create a Bugzilla account. -=item B +=item B =over +=item C (string) - the login name for the new account. +If the installation has the C parameter switched on, then +this parameter is ignored, and the value of the C parameter will +be used as the login name for the new account. + =item C (string) - the email to send the offer to. =back @@ -659,7 +670,12 @@ are the same as below. =over -=item C (string) - The email address for the new user. +=item C (string) - the login name for the new account. +If the installation has the C parameter switched on, then +this parameter is ignored, and the value of the C parameter will +be used as the login name for the new account. + +=item C (string) - The email address for the new account's user. =item C (string) B - The user's full name. Will be set to empty if not specified. @@ -675,7 +691,7 @@ resetting their password) or by the administrator. =item B -A hash containing one item, C, the numeric id of the user that was +A hash containing one item, C, the numeric id of the user account that was created. =item B @@ -737,7 +753,7 @@ C Contains ids of user to update. =item C -C Contains email/login of user to update. +C Contains login name of user to update. =item C @@ -745,8 +761,10 @@ C The new name of the user. =item C -C The email of the user. Note that email used to login to bugzilla. -Also note that you can only update one user at a time when changing the +C The email address of the user. It may be required that this is the +same as the login name. If you send different values in that case, the results +are undefined. +Note that you can only update one user at a time when changing the login name / email. (An error will be thrown if you try to update this field for multiple users at once.) @@ -967,8 +985,7 @@ 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. +C The login name of the user. =item can_login @@ -1054,7 +1071,7 @@ C The CGI parameters for the saved report. B: If you are not logged in to Bugzilla when you call this function, you will only be returned the C, C, and C items. If you are logged in and not in editusers group, you will only be returned the C, C, -C, C, C, and C items. The groups returned are +C, C, and C items. The groups returned are filtered based on your permission to bless each group. The C and C items are only returned if you are querying your own account, even if you are in the editusers group. -- cgit v1.2.3-24-g4f1b