Patchwork bugzilla: support Bugzilla 4.4.3+ API login token authentication (issue4257)

login
register
mail settings
Submitter Jim Hague
Date May 23, 2014, 4:42 p.m.
Message ID <1522f121693156c814fd.1400863371@hagrid.drewett.net>
Download mbox | patch
Permalink /patch/4856/
State Accepted
Headers show

Comments

Jim Hague - May 23, 2014, 4:42 p.m.
# HG changeset patch
# User Jim Hague <jim.hague@acm.org>
# Date 1400862544 -3600
#      Fri May 23 17:29:04 2014 +0100
# Branch stable
# Node ID 1522f121693156c814fd1d037c3dbc57a3639030
# Parent  54d7657d7d1e6a62315eea53f4498657e766bb60
bugzilla: support Bugzilla 4.4.3+ API login token authentication (issue4257)

Bugzilla 4.4.3 and later remove the old cookie based session authentication
from the Web Services API and replace it with a login token. The session
can now also be restricted to the originating IP.

Add the necessary to the extension so it works with 4.4.3 and later.
Augie Fackler - May 26, 2014, 4:13 p.m.
On Fri, May 23, 2014 at 05:42:51PM +0100, Jim Hague wrote:
> # HG changeset patch
> # User Jim Hague <jim.hague@acm.org>
> # Date 1400862544 -3600
> #      Fri May 23 17:29:04 2014 +0100
> # Branch stable
> # Node ID 1522f121693156c814fd1d037c3dbc57a3639030
> # Parent  54d7657d7d1e6a62315eea53f4498657e766bb60
> bugzilla: support Bugzilla 4.4.3+ API login token authentication (issue4257)

Looks reasonable, queued for stable. Thanks!

>
> Bugzilla 4.4.3 and later remove the old cookie based session authentication
> from the Web Services API and replace it with a login token. The session
> can now also be restricted to the originating IP.
>
> Add the necessary to the extension so it works with 4.4.3 and later.
>
> diff -r 54d7657d7d1e -r 1522f1216931 hgext/bugzilla.py
> --- a/hgext/bugzilla.py	Mon May 05 16:54:15 2014 +0200
> +++ b/hgext/bugzilla.py	Fri May 23 17:29:04 2014 +0100
> @@ -1,7 +1,7 @@
>  # bugzilla.py - bugzilla integration for mercurial
>  #
>  # Copyright 2006 Vadim Gelfer <vadim.gelfer@gmail.com>
> -# Copyright 2011-2 Jim Hague <jim.hague@acm.org>
> +# Copyright 2011-4 Jim Hague <jim.hague@acm.org>
>  #
>  # This software may be used and distributed according to the terms of the
>  # GNU General Public License version 2 or any later version.
> @@ -523,7 +523,7 @@
>
>      The regular xmlrpclib transports ignore cookies. Which causes
>      a bit of a problem when you need a cookie-based login, as with
> -    the Bugzilla XMLRPC interface.
> +    the Bugzilla XMLRPC interface prior to 4.4.3.
>
>      So this is a helper for defining a Transport which looks for
>      cookies being set in responses and saves them to add to all future
> @@ -620,7 +620,9 @@
>          ver = self.bzproxy.Bugzilla.version()['version'].split('.')
>          self.bzvermajor = int(ver[0])
>          self.bzverminor = int(ver[1])
> -        self.bzproxy.User.login({'login': user, 'password': passwd})
> +        login = self.bzproxy.User.login({'login': user, 'password': passwd,
> +                                         'restrict_login': True})
> +        self.bztoken = login.get('token', '')
>
>      def transport(self, uri):
>          if urlparse.urlparse(uri, "http")[0] == "https":
> @@ -631,13 +633,15 @@
>      def get_bug_comments(self, id):
>          """Return a string with all comment text for a bug."""
>          c = self.bzproxy.Bug.comments({'ids': [id],
> -                                       'include_fields': ['text']})
> +                                       'include_fields': ['text'],
> +                                       'token': self.bztoken})
>          return ''.join([t['text'] for t in c['bugs'][str(id)]['comments']])
>
>      def filter_real_bug_ids(self, bugs):
>          probe = self.bzproxy.Bug.get({'ids': sorted(bugs.keys()),
>                                        'include_fields': [],
>                                        'permissive': True,
> +                                      'token': self.bztoken,
>                                        })
>          for badbug in probe['faults']:
>              id = badbug['id']
> @@ -662,6 +666,7 @@
>              if 'fix' in newstate:
>                  args['status'] = self.fixstatus
>                  args['resolution'] = self.fixresolution
> +            args['token'] = self.bztoken
>              self.bzproxy.Bug.update(args)
>          else:
>              if 'fix' in newstate:
> @@ -719,10 +724,12 @@
>          than the subject line, and leave a blank line after it.
>          '''
>          user = self.map_committer(committer)
> -        matches = self.bzproxy.User.get({'match': [user]})
> +        matches = self.bzproxy.User.get({'match': [user],
> +                                         'token': self.bztoken})
>          if not matches['users']:
>              user = self.ui.config('bugzilla', 'user', 'bugs')
> -            matches = self.bzproxy.User.get({'match': [user]})
> +            matches = self.bzproxy.User.get({'match': [user],
> +                                             'token': self.bztoken})
>              if not matches['users']:
>                  raise util.Abort(_("default bugzilla user %s email not found") %
>                                   user)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - May 27, 2014, midnight
On Mon, 2014-05-26 at 12:13 -0400, Augie Fackler wrote:
> On Fri, May 23, 2014 at 05:42:51PM +0100, Jim Hague wrote:
> > # HG changeset patch
> > # User Jim Hague <jim.hague@acm.org>
> > # Date 1400862544 -3600
> > #      Fri May 23 17:29:04 2014 +0100
> > # Branch stable
> > # Node ID 1522f121693156c814fd1d037c3dbc57a3639030
> > # Parent  54d7657d7d1e6a62315eea53f4498657e766bb60
> > bugzilla: support Bugzilla 4.4.3+ API login token authentication (issue4257)
> 
> Looks reasonable, queued for stable. Thanks!
> 
> >
> > Bugzilla 4.4.3 and later remove the old cookie based session authentication
> > from the Web Services API and replace it with a login token. The session
> > can now also be restricted to the originating IP.
> >
> > Add the necessary to the extension so it works with 4.4.3 and later.

Technically, this is a regression for Bugzilla (we broke all our
clients!) but a feature for us (we added 4.4.3 support!). So it's not
100% appropriate for OUR stable branch..

Are we sure this is safe against older Bugzilla?
Augie Fackler - May 27, 2014, 12:55 a.m.
On May 26, 2014, at 8:00 PM, Matt Mackall <mpm@selenic.com> wrote:

> On Mon, 2014-05-26 at 12:13 -0400, Augie Fackler wrote:
>> On Fri, May 23, 2014 at 05:42:51PM +0100, Jim Hague wrote:
>>> # HG changeset patch
>>> # User Jim Hague <jim.hague@acm.org>
>>> # Date 1400862544 -3600
>>> #      Fri May 23 17:29:04 2014 +0100
>>> # Branch stable
>>> # Node ID 1522f121693156c814fd1d037c3dbc57a3639030
>>> # Parent  54d7657d7d1e6a62315eea53f4498657e766bb60
>>> bugzilla: support Bugzilla 4.4.3+ API login token authentication (issue4257)
>> 
>> Looks reasonable, queued for stable. Thanks!
>> 
>>> 
>>> Bugzilla 4.4.3 and later remove the old cookie based session authentication
>>> from the Web Services API and replace it with a login token. The session
>>> can now also be restricted to the originating IP.
>>> 
>>> Add the necessary to the extension so it works with 4.4.3 and later.
> 
> Technically, this is a regression for Bugzilla (we broke all our
> clients!) but a feature for us (we added 4.4.3 support!). So it's not
> 100% appropriate for OUR stable branch..

I figured that working with bz outweighed the usual ruling.

> Are we sure this is safe against older Bugzilla?

My interpretation of the log message was that it should be. Jim?

> 
> -- 
> Mathematics is the supreme nostalgia of our time.
> 
>
Jim Hague - May 27, 2014, 1:51 p.m.
On Monday 26 May 2014 17:00:12 Matt Mackall wrote:
> Technically, this is a regression for Bugzilla (we broke all our
> clients!) but a feature for us (we added 4.4.3 support!). So it's not
> 100% appropriate for OUR stable branch..

Granted. I wasn't sure whether to go for stable or not.

> Are we sure this is safe against older Bugzilla?

I don't have a large selection of older Bugzillas to test against. I've tried 
it against 4.2.5, and it's fine. The additional new argument to the Bugzilla 
API calls is just ignored. I've checked the source for 3.4.13 and unrecognised 
arguments are ignored there, too, so I'm pretty confident it's safe.

The patch is the minimal possible. If I could drop XMLRPC support for Bugzilla 
3.4, I could rework to pass user/pass on each API call and remove the horrible 
hack for supporting cookies in XMLRPC transports. This would work on all 
Bugzilla from 3.6 onwards. 3.6 has been out of support for some time. 

Mind you, if allowed to drop support for 3.4, I'd first nuke the older access 
methods that go direct to the database.

Patch

diff -r 54d7657d7d1e -r 1522f1216931 hgext/bugzilla.py
--- a/hgext/bugzilla.py	Mon May 05 16:54:15 2014 +0200
+++ b/hgext/bugzilla.py	Fri May 23 17:29:04 2014 +0100
@@ -1,7 +1,7 @@ 
 # bugzilla.py - bugzilla integration for mercurial
 #
 # Copyright 2006 Vadim Gelfer <vadim.gelfer@gmail.com>
-# Copyright 2011-2 Jim Hague <jim.hague@acm.org>
+# Copyright 2011-4 Jim Hague <jim.hague@acm.org>
 #
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
@@ -523,7 +523,7 @@ 
 
     The regular xmlrpclib transports ignore cookies. Which causes
     a bit of a problem when you need a cookie-based login, as with
-    the Bugzilla XMLRPC interface.
+    the Bugzilla XMLRPC interface prior to 4.4.3.
 
     So this is a helper for defining a Transport which looks for
     cookies being set in responses and saves them to add to all future
@@ -620,7 +620,9 @@ 
         ver = self.bzproxy.Bugzilla.version()['version'].split('.')
         self.bzvermajor = int(ver[0])
         self.bzverminor = int(ver[1])
-        self.bzproxy.User.login({'login': user, 'password': passwd})
+        login = self.bzproxy.User.login({'login': user, 'password': passwd,
+                                         'restrict_login': True})
+        self.bztoken = login.get('token', '')
 
     def transport(self, uri):
         if urlparse.urlparse(uri, "http")[0] == "https":
@@ -631,13 +633,15 @@ 
     def get_bug_comments(self, id):
         """Return a string with all comment text for a bug."""
         c = self.bzproxy.Bug.comments({'ids': [id],
-                                       'include_fields': ['text']})
+                                       'include_fields': ['text'],
+                                       'token': self.bztoken})
         return ''.join([t['text'] for t in c['bugs'][str(id)]['comments']])
 
     def filter_real_bug_ids(self, bugs):
         probe = self.bzproxy.Bug.get({'ids': sorted(bugs.keys()),
                                       'include_fields': [],
                                       'permissive': True,
+                                      'token': self.bztoken,
                                       })
         for badbug in probe['faults']:
             id = badbug['id']
@@ -662,6 +666,7 @@ 
             if 'fix' in newstate:
                 args['status'] = self.fixstatus
                 args['resolution'] = self.fixresolution
+            args['token'] = self.bztoken
             self.bzproxy.Bug.update(args)
         else:
             if 'fix' in newstate:
@@ -719,10 +724,12 @@ 
         than the subject line, and leave a blank line after it.
         '''
         user = self.map_committer(committer)
-        matches = self.bzproxy.User.get({'match': [user]})
+        matches = self.bzproxy.User.get({'match': [user],
+                                         'token': self.bztoken})
         if not matches['users']:
             user = self.ui.config('bugzilla', 'user', 'bugs')
-            matches = self.bzproxy.User.get({'match': [user]})
+            matches = self.bzproxy.User.get({'match': [user],
+                                             'token': self.bztoken})
             if not matches['users']:
                 raise util.Abort(_("default bugzilla user %s email not found") %
                                  user)