Bug 1541555 - Add facility for requiring an API Key to always come from the same...

diff --git a/Bugzilla/Auth/Login/APIKey.pm b/Bugzilla/Auth/Login/APIKey.pm
index 43032c58459058b971e52539bc8f071345189b79..f6d2dea38678b47c6668881a72d3c98c8d0209b8 100644 (file)
--- a/Bugzilla/Auth/Login/APIKey.pm
+++ b/Bugzilla/Auth/Login/APIKey.pm
@@ -47,18 +47,25 @@ sub get_login_info {
     return {failure => AUTH_NODATA};
   }
 
-  my $api_key = Bugzilla::User::APIKey->new({name => $api_key_text});
+  my $api_key   = Bugzilla::User::APIKey->new({name => $api_key_text});
+  my $remote_ip = remote_ip();
 
   if (!$api_key or $api_key->api_key ne $api_key_text) {
 
     # The second part checks the correct capitalisation. Silly MySQL
     ThrowUserError("api_key_not_valid");
   }
+  elsif ( $api_key->sticky
+    && $api_key->last_used_ip
+    && $api_key->last_used_ip ne $remote_ip)
+  {
+    ThrowUserError("api_key_not_valid");
+  }
   elsif ($api_key->revoked) {
     ThrowUserError('api_key_revoked');
   }
 
-  $api_key->update_last_used();
+  $api_key->update_last_used($remote_ip);
   $self->set_app_id($api_key->app_id);
 
   return {user_id => $api_key->user_id};

diff --git a/Bugzilla/User/APIKey.pm b/Bugzilla/User/APIKey.pm
index a30c6718fb4e8cf7e563bafc1347807a78c50fff..86f7ba2db34a32f678efd38ddb5453e34d7d3fdd 100644 (file)
--- a/Bugzilla/User/APIKey.pm
+++ b/Bugzilla/User/APIKey.pm
@@ -14,7 +14,7 @@ use warnings;
 use base qw(Bugzilla::Object);
 
 use Bugzilla::User;
-use Bugzilla::Util qw(generate_random_password trim remote_ip);
+use Bugzilla::Util qw(generate_random_password trim);
 use Bugzilla::Error;
 
 #####################################################################
@@ -31,14 +31,16 @@ use constant DB_COLUMNS => qw(
   revoked
   last_used
   last_used_ip
+  sticky
 );
 
-use constant UPDATE_COLUMNS => qw(description revoked last_used last_used_ip);
+use constant UPDATE_COLUMNS => qw(description revoked last_used last_used_ip sticky);
 use constant VALIDATORS     => {
   api_key     => \&_check_api_key,
   app_id      => \&_check_app_id,
   description => \&_check_description,
   revoked     => \&Bugzilla::Object::check_boolean,
+  sticky      => \&Bugzilla::Object::check_boolean,
 };
 use constant LIST_ORDER => 'id';
 use constant NAME_FIELD => 'api_key';
@@ -60,6 +62,7 @@ sub description  { return $_[0]->{description} }
 sub revoked      { return $_[0]->{revoked} }
 sub last_used    { return $_[0]->{last_used} }
 sub last_used_ip { return $_[0]->{last_used_ip} }
+sub sticky       { return $_[0]->{sticky} }
 
 # Helpers
 sub user {
@@ -69,17 +72,17 @@ sub user {
 }
 
 sub update_last_used {
-  my $self = shift;
-  my $timestamp
-    = shift || Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
+  my ($self, $remote_ip) = @_;
+  my $timestamp = Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
   $self->set('last_used',    $timestamp);
-  $self->set('last_used_ip', remote_ip());
+  $self->set('last_used_ip', $remote_ip);
   $self->update;
 }
 
 # Setters
 sub set_description { $_[0]->set('description', $_[1]); }
 sub set_revoked     { $_[0]->set('revoked',     $_[1]); }
+sub set_sticky      { $_[0]->set('sticky',      $_[1]); }
 
 # Validators
 sub _check_api_key { return generate_random_password(40); }

diff --git a/docs/en/rst/using/preferences.rst b/docs/en/rst/using/preferences.rst
index d61dc4f8a44760a5b557347f20d2061d7f34434e..cfce75d6dc1f9b83abaeb799d070b8627a1b0e38 100644 (file)
--- a/docs/en/rst/using/preferences.rst
+++ b/docs/en/rst/using/preferences.rst
@@ -132,10 +132,15 @@ change your password everywhere.
 You can create more than one API key if required. Each API key has an optional
 description which can help you record what it is used for.
 
-On this page, you can unrevoke, revoke and change the description of existing
+On this page, you can unrevoke, revoke, make sticky, and change the description of existing
 API keys for your login. A revoked key means that it cannot be used. The
 description is optional and purely for your information.
 
+Sticky API keys may only be used from one IP address, which reduces the risk
+of the key being leaked. The IP address is the one the key was last used
+from. The expected workflow is that the sticky bit will be set once your application
+(or script) is setup. The sticky attribute may only be set, it can't ever be unset.
+
 You can also create a new API key by selecting the checkbox under the 'New
 API key' section of the page.
 

diff --git a/template/en/default/account/prefs/apikey.html.tmpl b/template/en/default/account/prefs/apikey.html.tmpl
index 19cec468fffb58e9e2215b0a093f6b5b190ac2c2..3a27b0a9c2fa86b4f13cdffeda08f44cd8af8429 100644 (file)
--- a/template/en/default/account/prefs/apikey.html.tmpl
+++ b/template/en/default/account/prefs/apikey.html.tmpl
@@ -14,8 +14,9 @@
 <p>
   API keys are used to authenticate WebService API calls. You can create more than
   one API key if required. Each API key has an optional description which can help
-  you record what each key is used for.<br>
-  <br>
+  you record what each key is used for.
+</p>
+<p>
   Documentation on how to log in is available
   <a href="https://bmo.readthedocs.io/en/latest/api/core/v1/general.html#authentication">
     here</a>.
@@ -24,13 +25,14 @@
 <h3>Existing API keys</h3>
 
 <p>You can update the description, and revoke or unrevoke existing API keys
-here.</p>
+here. Sticky keys may only be used from the last IP that used the API key, and cannot be unset.</p>
 
 <table id="email_prefs">
   <tr class="column_header">
     <th>API key</th>
     <th>Description (optional)</th>
     <th>Last used</th>
+    <th>Sticky</th>
     <th>Revoked</th>
   </tr>
 
@@ -51,6 +53,12 @@ here.</p>
       [% ELSE %]
         <td class="center"><i>never used</i></td>
       [% END %]
+      <td class="center">
+      <input type="checkbox" value="1"
+          name="sticky_[% api_key.id FILTER html %]"
+          id="sticky_[% api_key.id FILTER html %]"
+        [% IF api_key.sticky %] checked="checked" disabled="disabled" [% END %]>
+      </td>
       <td class="center">
         <input type="checkbox" value="1"
           name="revoked_[% api_key.id FILTER html %]"

diff --git a/userprefs.cgi b/userprefs.cgi
index a308726f6953e80f6fb29f0b313a40fdbeb90233..fdd921536f669597b14f8af74ac5ba576d090cf9 100755 (executable)
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -850,8 +850,12 @@ sub SaveApiKey {
   foreach my $api_key (@$api_keys) {
     my $description = $cgi->param('description_' . $api_key->id);
     my $revoked     = !!$cgi->param('revoked_' . $api_key->id);
+    my $sticky      = !!$cgi->param('sticky_' . $api_key->id);
 
-    if ($description ne $api_key->description || $revoked != $api_key->revoked) {
+    if ( $description ne $api_key->description
+      || $revoked != $api_key->revoked
+      || $sticky != $api_key->sticky)
+    {
       if ($user->mfa && !$revoked && $api_key->revoked) {
         push @mfa_events,
           {
@@ -862,7 +866,9 @@ sub SaveApiKey {
           };
       }
       else {
-        $api_key->set_all({description => $description, revoked => $revoked,});
+        $sticky = 1 if $api_key->sticky;
+        $api_key->set_all(
+          {description => $description, revoked => $revoked, sticky => $sticky});
         $api_key->update();
         if ($revoked) {
           Bugzilla->log_user_request(undef, undef, 'api-key-revoke');