From: Kohei Yoshino Date: Thu, 14 Feb 2019 21:43:07 +0000 (-0500) Subject: Bug 1402894 - Remove "Restrict this session to this IP" option from login page X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=141489aec3eca106eba1891a38f7513796fee0fa;p=thirdparty%2Fbugzilla.git Bug 1402894 - Remove "Restrict this session to this IP" option from login page --- diff --git a/Bugzilla/App/Plugin/Glue.pm b/Bugzilla/App/Plugin/Glue.pm index 6e28528a8..b8b0c20bd 100644 --- a/Bugzilla/App/Plugin/Glue.pm +++ b/Bugzilla/App/Plugin/Glue.pm @@ -99,19 +99,13 @@ sub register { my $login_cookie = $c->cookie("Bugzilla_logincookie"); my $user_id = $c->cookie("Bugzilla_login"); - my $ip_addr = $c->tx->remote_address; return $c->bugzilla->login_redirect_if_required($type) unless ($login_cookie && $user_id); my $db_cookie = Bugzilla->dbh->selectrow_array( - q{ - SELECT cookie - FROM logincookies - WHERE cookie = ? - AND userid = ? - AND (restrict_ipaddr = 0 OR ipaddr = ?) - }, undef, ($login_cookie, $user_id, $ip_addr) + 'SELECT cookie FROM logincookies WHERE cookie = ? AND userid = ?', + undef, ($login_cookie, $user_id) ); if (defined $db_cookie && secure_compare($login_cookie, $db_cookie)) { diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index c40b3582e..868d04c03 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -93,7 +93,7 @@ sub login { my $cgi = Bugzilla->cgi; my $uri = URI->new($cgi->self_url); foreach - my $param (qw( Bugzilla_remember Bugzilla_restrictlogin GoAheadAndLogIn )) + my $param (qw( Bugzilla_remember GoAheadAndLogIn )) { $uri->query_param_delete($param); } @@ -101,7 +101,6 @@ sub login { user => $user, type => $type, reason => 'Logging in as ' . $user->identity, - restrictlogin => $params->{Bugzilla_restrictlogin}, remember => $params->{Bugzilla_remember}, url => $uri->as_string, postback => @@ -119,8 +118,6 @@ sub mfa_verified { my $params = Bugzilla->input_params; $self->{_info_getter}->{successful} = Bugzilla::Auth::Login::CGI->new(); - $params->{Bugzilla_restrictlogin} = $event->{restrictlogin} - if defined $event->{restrictlogin}; $params->{Bugzilla_remember} = $event->{remember} if defined $event->{remember}; $self->_handle_login_result({user => $user}, $event->{type}); diff --git a/Bugzilla/Auth/Login/Cookie.pm b/Bugzilla/Auth/Login/Cookie.pm index c71eb74fd..46a47f4c5 100644 --- a/Bugzilla/Auth/Login/Cookie.pm +++ b/Bugzilla/Auth/Login/Cookie.pm @@ -85,23 +85,16 @@ sub get_login_info { ($user_id, $login_cookie) = ($token->{'user_id'}, $token->{'login_token'}); } - my $ip_addr = remote_ip(); - if ($login_cookie && $user_id) { # Anything goes for these params - they're just strings which # we're going to verify against the db - trick_taint($ip_addr); trick_taint($login_cookie); detaint_natural($user_id); my $db_cookie = $dbh->selectrow_array( - 'SELECT cookie - FROM logincookies - WHERE cookie = ? - AND userid = ? - AND (restrict_ipaddr = 0 OR ipaddr = ?)', - undef, ($login_cookie, $user_id, $ip_addr) + 'SELECT cookie FROM logincookies WHERE cookie = ? AND userid = ?', + undef, ($login_cookie, $user_id) ); # If the cookie is valid, return a valid username. diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm index 65e6a1541..105c79ca2 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -39,13 +39,9 @@ sub persist_login { my $ip_addr = remote_ip(); trick_taint($ip_addr); - my $restrict = $input_params->{Bugzilla_restrictlogin} ? 1 : 0; - $dbh->do( - "INSERT INTO logincookies (cookie, userid, ipaddr, lastused, restrict_ipaddr) - VALUES (?, ?, ?, NOW(), ?)", undef, $login_cookie, $user->id, - $ip_addr, $restrict - ); + $dbh->do('INSERT INTO logincookies (cookie, userid, ipaddr, lastused) + VALUES (?, ?, ?, NOW())', undef, $login_cookie, $user->id, $ip_addr); # Issuing a new cookie is a good time to clean up the old # cookies. diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index ee96220cf..9356a1391 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -1138,7 +1138,6 @@ use constant ABSTRACT_SCHEMA => { ipaddr => {TYPE => 'varchar(40)'}, lastused => {TYPE => 'DATETIME', NOTNULL => 1}, id => {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1}, - restrict_ipaddr => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 0}, ], INDEXES => [ logincookies_lastused_idx => ['lastused'], diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index f9142fa4a..72a1cab6f 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -744,8 +744,6 @@ sub update_table_definitions { $dbh->bz_add_column('user_api_keys', 'last_used_ip', {TYPE => 'varchar(40)'}); - _add_restrict_ipaddr(); - $dbh->bz_add_column('profiles', 'password_change_required', {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}); $dbh->bz_add_column('profiles', 'password_change_reason', @@ -782,6 +780,9 @@ sub update_table_definitions { _add_oauth2_jwt_support(); + # Bug 1402894 - kohei.yoshino@gmail.com + $dbh->bz_drop_column('logincookies', 'restrict_ipaddr'); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -4176,16 +4177,6 @@ sub _fix_disable_mail { Bugzilla->dbh->do("UPDATE profiles SET disable_mail = 1 WHERE is_enabled = 0"); } -sub _add_restrict_ipaddr { - my $dbh = Bugzilla->dbh; - return if $dbh->bz_column_info('logincookies', 'restrict_ipaddr'); - - $dbh->bz_add_column('logincookies', 'restrict_ipaddr', - {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 0}); - $dbh->do( - "UPDATE logincookies SET restrict_ipaddr = 1 WHERE ipaddr IS NOT NULL"); -} - sub _migrate_group_owners { my $dbh = Bugzilla->dbh; return if $dbh->bz_column_info('groups', 'owner_user_id'); diff --git a/Bugzilla/User/Session.pm b/Bugzilla/User/Session.pm index 652a866c1..6583d72f0 100644 --- a/Bugzilla/User/Session.pm +++ b/Bugzilla/User/Session.pm @@ -24,7 +24,6 @@ use constant DB_COLUMNS => qw( lastused ipaddr id - restrict_ipaddr ); use constant UPDATE_COLUMNS => qw(); @@ -46,6 +45,5 @@ sub userid { return $_[0]->{userid} } sub cookie { return $_[0]->{cookie} } sub lastused { return $_[0]->{lastused} } sub ipaddr { return $_[0]->{ipaddr} } -sub restrict_ipaddr { return $_[0]->{restrict_ipaddr} } 1; diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm index 4a0264f9c..82d27b99d 100644 --- a/Bugzilla/WebService.pm +++ b/Bugzilla/WebService.pm @@ -193,19 +193,15 @@ WebService method to perform a login: =item C (string) - That user's password. -=item C (boolean) - Optional. If true, -then your login will only be valid for your IP address. - =item C (boolean) - Optional. If true, then the cookie sent back to you with the method response will not expire. =back -The C and C options -are only used when you have also specified C and -C. This value will be deprecated in the release -after Bugzilla 5.0 and you will be required to pass the Bugzilla_login +The C option is only used when you have also specified +C and C. This value will be deprecated in the +release after Bugzilla 5.0 and you will be required to pass the Bugzilla_login and Bugzilla_password for every call. Note that Bugzilla will return HTTP cookies along with the method diff --git a/docs/en/rst/api/core/v1/general.rst b/docs/en/rst/api/core/v1/general.rst index 3d078ad7a..6227e1825 100644 --- a/docs/en/rst/api/core/v1/general.rst +++ b/docs/en/rst/api/core/v1/general.rst @@ -111,13 +111,8 @@ name type description ====================== ======= ============================================== **Bugzilla_login** string A user's login name. **Bugzilla_password** string That user's password. -Bugzilla_restrictlogin boolean If true, then your login will only be - valid for your IP address. ====================== ======= ============================================== -The ``Bugzilla_restrictlogin`` option is only used when you have also -specified ``Bugzilla_login`` and ``Bugzilla_password``. - There is also a deprecated method of authentication described below that will be removed in the version after Bugzilla 5.0. diff --git a/docs/en/rst/api/core/v1/user.rst b/docs/en/rst/api/core/v1/user.rst index 1565425e5..6a01df0a1 100644 --- a/docs/en/rst/api/core/v1/user.rst +++ b/docs/en/rst/api/core/v1/user.rst @@ -28,9 +28,6 @@ name type description ============== ======= ======================================================== **login** string The user's login name. **password** string The user's password. -restrict_login boolean If set to a true value, the token returned by this - method will only be valid from the IP address which - called this method. ============== ======= ======================================================== **Response** diff --git a/sanitycheck.cgi b/sanitycheck.cgi index ffc936021..71ad9eab0 100755 --- a/sanitycheck.cgi +++ b/sanitycheck.cgi @@ -73,7 +73,7 @@ else { # Only check the token if we are running this script from the # web browser and a parameter is passed to the script. # XXX - Maybe these two parameters should be deleted once logged in? - $cgi->delete('GoAheadAndLogIn', 'Bugzilla_restrictlogin', 'hooks_only'); + $cgi->delete('GoAheadAndLogIn', 'hooks_only'); if (scalar($cgi->param())) { my $token = $cgi->param('token'); check_hash_token($token, ['sanitycheck']); diff --git a/skins/standard/global.css b/skins/standard/global.css index 949a46549..3737731dd 100644 --- a/skins/standard/global.css +++ b/skins/standard/global.css @@ -1037,7 +1037,7 @@ input.required, select.required, span.required_explanation { font-weight: bold; } -#login .field-restrict, #login .field-remember { +#login .field-remember { margin-left: 7em; } diff --git a/skins/standard/mobile.css b/skins/standard/mobile.css index f112c2a3b..e777cabd8 100644 --- a/skins/standard/mobile.css +++ b/skins/standard/mobile.css @@ -25,7 +25,7 @@ only screen and (max-device-width : 720px) { padding-bottom: 0px; } - #login .field-restrict, #login .field-remember { + #login .field-remember { margin-left: 0px; } #login .field-submit { diff --git a/t/mojo-oauth2.t b/t/mojo-oauth2.t index e1e59abe6..d91fe458b 100644 --- a/t/mojo-oauth2.t +++ b/t/mojo-oauth2.t @@ -67,7 +67,6 @@ $t->post_ok( '/login' => {Referer => $referer} => form => { Bugzilla_login => $oauth_login, Bugzilla_password => $oauth_password, - Bugzilla_restrictlogin => 1, GoAheadAndLogIn => 1, client_id => $oauth_client->{client_id}, response_type => 'code', diff --git a/template/en/default/account/auth/login.html.tmpl b/template/en/default/account/auth/login.html.tmpl index f1e084ecb..53ea9b283 100644 --- a/template/en/default/account/auth/login.html.tmpl +++ b/template/en/default/account/auth/login.html.tmpl @@ -69,15 +69,7 @@ [% END %] [% PROCESS "global/hidden-fields.html.tmpl" - exclude="^Bugzilla_(login|password|restrictlogin)$" %] - -
- - -
+ exclude="^Bugzilla_(login|password)$" %]
Last used IP Address - IP Restriction Logout @@ -43,7 +42,6 @@ [% session.lastused FILTER time %] [% session.ipaddr OR "Unknown" FILTER html %] - [% session.restrict_ipaddr ? "Restricted" : "Unrestricted" FILTER html %] [% IF session.current %] (current) @@ -53,4 +51,4 @@ [% END %] [% END %] - \ No newline at end of file +