Re: [RFC PATCH] REST: enable token authentication

[Philippe Pepiot]

On 05/25/2017 10:47 AM, Andrew Donnellan wrote:
> Token authentication is generally viewed as a more secure option for API
> authentication than storing a username and password.
> 
> Django REST Framework gives us a TokenAuthentication class and an authtoken
> app that we can use to generate random tokens and authenticate to API
> endpoints.
> 
> Enable DRF's token support and add options to the user profile view to view
> or regenerate tokens.
> 
> Signed-off-by: Andrew Donnellan 
> 
> ---
> 
> This is an RFC; I haven't written any tests or documentation, UI's a bit
> ugly, need to split patches.
> ---

Hi Andrew,

Thanks for adding this ! Aside my comments below, I tested a bit and it
worked fine.


>  patchwork/settings/base.py |  6 ++
>  patchwork/signals.py   | 10 ++
>  patchwork/templates/patchwork/profile.html | 23 +--
>  patchwork/urls.py  |  4 
>  patchwork/views/bundle.py  | 12 
>  patchwork/views/user.py| 19 +++
>  6 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py
> index 26c75c9..6fd98a7 100644
> --- a/patchwork/settings/base.py
> +++ b/patchwork/settings/base.py
> @@ -143,6 +143,7 @@ try:
>  
>  INSTALLED_APPS += [
>  'rest_framework',
> +'rest_framework.authtoken',
>  'django_filters',
>  ]
>  except ImportError:
> @@ -158,6 +159,11 @@ REST_FRAMEWORK = {
>  'rest_framework.filters.SearchFilter',
>  'rest_framework.filters.OrderingFilter',
>  ),
> +'DEFAULT_AUTHENTICATION_CLASSES': (
> +'rest_framework.authentication.SessionAuthentication',
> +'rest_framework.authentication.BasicAuthentication',
> +'rest_framework.authentication.TokenAuthentication',
> +),
>  'SEARCH_PARAM': 'q',
>  'ORDERING_PARAM': 'order',
>  }
> diff --git a/patchwork/signals.py b/patchwork/signals.py
> index 208685c..f335525 100644
> --- a/patchwork/signals.py
> +++ b/patchwork/signals.py
> @@ -19,6 +19,7 @@
>  
>  from datetime import datetime as dt
>  
> +from django.conf import settings
>  from django.db.models.signals import post_save
>  from django.db.models.signals import pre_save
>  from django.dispatch import receiver
> @@ -239,3 +240,12 @@ def create_series_completed_event(sender, instance, 
> created, **kwargs):
>  
>  if instance.series.received_all:
>  create_event(instance.series)
> +
> +
> +if settings.ENABLE_REST_API:
> +from rest_framework.authtoken.models import Token
> +@receiver(post_save, sender=settings.AUTH_USER_MODEL)
> +def create_user_created_event(sender, instance=None, created=False,
> +  **kwargs):
> +if created:
> +Token.objects.create(user=instance)
> diff --git a/patchwork/templates/patchwork/profile.html 
> b/patchwork/templates/patchwork/profile.html
> index f976195..c7be044 100644
> --- a/patchwork/templates/patchwork/profile.html
> +++ b/patchwork/templates/patchwork/profile.html
> @@ -133,8 +133,27 @@ address.
>  
>  
>  
> -Authentication
> -Change password
> +  Authentication
> +  
> +{% csrf_token %}
> +
> +  
> + Password:
> + Change password
> +  
> +  
> + API Token:
> + 
> +   {% if api_token %}
> +   {{ api_token }}
> +   
> +   {% else %}
> +   
> +   {% endif %}
> + 
> +  
> +
> +  
>  
>  
>  
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 1871c9a..aa49b4d 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -101,6 +101,10 @@ urlpatterns = [
>  auth_views.password_reset_complete,
>  name='password_reset_complete'),
>  
> +# token change
> +url(r'^user/generate-token/$', user_views.generate_token,
> +name='generate_token'),
> +
>  # login/logout
>  url(r'^user/login/$', auth_views.login,
>  {'template_name': 'patchwork/login.html'},
> diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
> index 387b7c6..bb331f4 100644
> --- a/patchwork/views/bundle.py
> +++ b/patchwork/views/bundle.py
> @@ -36,17 +36,21 @@ from patchwork.views import generic_list
>  from patchwork.views.utils import bundle_to_mbox
>  
>  if settings.ENABLE_REST_API:
> -from rest_framework.authentication import BasicAuthentication  # noqa
> +from rest_framework.authentication import SessionAuthentication
>  from rest_framework.exceptions import AuthenticationFailed
> +from rest_framework.settings import api_settings
>  
>  
>  def rest_auth(request):
>  if not settings.ENABLE_REST_API:
>  return request.user
>  try:
> -auth_result = BasicAuthentication().authenticate(request)
> -if auth_result:
> -return auth_result[0]
> +for auth in 

Re: [PATCH 2/2] tests: Add tests for viewing private bundles

[Stephen Finucane]

On Thu, 2017-05-25 at 17:38 +1000, Andrew Donnellan wrote:
> Add some tests to check that owners can view their private bundles
> while
> other authenticated users can't.
> 
> Signed-off-by: Andrew Donnellan 
> 
> ---
> 
> I'm not very familiar with writing Django tests, please flame away

For both of these:

Reviewed-by: Stephen Finucane 

and applied. Sorry for missing this. I think I added tests but they
clearly weren't up to scratch. I guess these will all be replaced if we
switch to token auth?

Stephen
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH] docker: increase database connection timeout

[Stephen Finucane]

On Thu, 2017-05-25 at 16:01 +1000, Daniel Axtens wrote:
> Andrew Donnellan  writes:
> 
> > When starting the Docker environment, if the web container can't
> > see the
> > database immediately, it waits 5 seconds, tries again, then waits
> > 15
> > seconds more to account for first-time start-ups where it takes a
> > bit
> > longer for the database to be initialised.
> > 
> > Some of us, unfortunately, have slow computers with slow mechanical
> > hard
> > drives which take just a bit longer. Increase the second timeout
> > from 15
> > seconds to 60 seconds, testing every 5 seconds.
> > 
> > Cc: Daniel Axtens 
> 
> Acked-by: Daniel Axtens 
> 
> LGTM - thanks Andrew for the fix and the reminder we don't all have
> PCI-connected NVMe :P

Boo spinning rust.

Reviewed-by: Stephen Finucane 

and applied.

Stephen
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 1/3] views: Enable downloading of cover mboxes

[Stephen Finucane]

Signed-off-by: Stephen Finucane 
---
 patchwork/models.py  |  5 -
 patchwork/urls.py|  2 ++
 patchwork/views/cover.py | 13 +
 patchwork/views/utils.py | 45 +++--
 4 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/patchwork/models.py b/patchwork/models.py
index ee66ace..cba3582 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -362,7 +362,10 @@ class SeriesMixin(object):
 
 
 class CoverLetter(SeriesMixin, Submission):
-pass
+
+@models.permalink
+def get_mbox_url(self):
+return ('cover-mbox', (), {'cover_id': self.id})
 
 
 @python_2_unicode_compatible
diff --git a/patchwork/urls.py b/patchwork/urls.py
index 1871c9a..be996c0 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -61,6 +61,8 @@ urlpatterns = [
 # cover views
 url(r'^cover/(?P\d+)/$', cover_views.cover_detail,
 name='cover-detail'),
+url(r'^cover/(?P\d+)/mbox/$', cover_views.cover_mbox,
+name='cover-mbox'),
 
 # comment views
 url(r'^comment/(?P\d+)/$', comment_views.comment,
diff --git a/patchwork/views/cover.py b/patchwork/views/cover.py
index 21bfde2..054e8e5 100644
--- a/patchwork/views/cover.py
+++ b/patchwork/views/cover.py
@@ -18,6 +18,7 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 from django.http import Http404
+from django.http import HttpResponse
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.shortcuts import render_to_response
@@ -25,6 +26,7 @@ from django.shortcuts import render_to_response
 from patchwork.compat import reverse
 from patchwork.models import CoverLetter
 from patchwork.models import Submission
+from patchwork.views.utils import cover_to_mbox
 
 
 def cover_detail(request, cover_id):
@@ -44,3 +46,14 @@ def cover_detail(request, cover_id):
 }
 
 return render_to_response('patchwork/submission.html', context)
+
+
+def cover_mbox(request, cover_id):
+cover = get_object_or_404(CoverLetter, id=cover_id)
+
+response = HttpResponse(content_type='text/plain')
+response.write(cover_to_mbox(cover))
+response['Content-Disposition'] = 'attachment; filename=' + \
+cover.filename.replace(';', '').replace('\n', '')
+
+return response
diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py
index 7198c2d..5528ee7 100644
--- a/patchwork/views/utils.py
+++ b/patchwork/views/utils.py
@@ -30,6 +30,7 @@ from django.http import Http404
 from django.utils import six
 
 from patchwork.models import Comment
+from patchwork.models import Patch
 from patchwork.models import Series
 
 
@@ -43,20 +44,24 @@ class PatchMbox(MIMENonMultipart):
 encode_7or8bit(self)
 
 
-def patch_to_mbox(patch):
-"""Get an mbox representation of a single patch.
+def _submission_to_mbox(submission):
+"""Get an mbox representation of a single Submission.
+
+Handles both Patch and CoverLetter objects.
 
 Arguments:
-patch: The Patch object to convert.
+submission: The Patch object to convert.
 
 Returns:
 A string for the mbox file.
 """
+is_patch = isinstance(submission, Patch)
+
 postscript_re = re.compile('\n-{2,3} ?\n')
 body = ''
 
-if patch.content:
-body = patch.content.strip() + "\n"
+if submission.content:
+body = submission.content.strip() + "\n"
 
 parts = postscript_re.split(body, 1)
 if len(parts) == 2:
@@ -67,31 +72,31 @@ def patch_to_mbox(patch):
 postscript = ''
 
 # TODO(stephenfin): Make this use the tags infrastructure
-for comment in Comment.objects.filter(submission=patch):
+for comment in Comment.objects.filter(submission=submission):
 body += comment.patch_responses
 
 if postscript:
 body += '---\n' + postscript + '\n'
 
-if patch.diff:
-body += '\n' + patch.diff
+if is_patch and submission.diff:
+body += '\n' + submission.diff
 
-delta = patch.date - datetime.datetime.utcfromtimestamp(0)
+delta = submission.date - datetime.datetime.utcfromtimestamp(0)
 utc_timestamp = delta.seconds + delta.days * 24 * 3600
 
 mail = PatchMbox(body)
-mail['Subject'] = patch.name
+mail['Subject'] = submission.name
 mail['X-Patchwork-Submitter'] = email.utils.formataddr((
-str(Header(patch.submitter.name, mail.patch_charset)),
-patch.submitter.email))
-mail['X-Patchwork-Id'] = str(patch.id)
-if patch.delegate:
-mail['X-Patchwork-Delegate'] = str(patch.delegate.email)
-mail['Message-Id'] = patch.msgid
-mail.set_unixfrom('From patchwork ' + patch.date.ctime())
+str(Header(submission.submitter.name, mail.patch_charset)),
+submission.submitter.email))
+mail['X-Patchwork-Id'] = str(submission.id)
+if is_patch and submission.delegate:
+mail['X-Patchwork-Delegate'] = str(submission.delegate.email)
+ 

Re: [PATCH] docs/api: change POST to PATCH in REST API parameters example

[Stephen Finucane]

On Thu, 2017-05-25 at 18:05 +1000, Andrew Donnellan wrote:
> api/rest.rst gives an example of how to POST parameters to the
> PatchDetail
> view at api/patches/. However, the endpoint in question
> doesn't
> support POST - you need to use PUT or PATCH. Change it to PATCH.
> 
> Signed-off-by: Andrew Donnellan 

Yup - my bad.

Acked-by: Stephen Finucane 

and applied.

> ---
> 
> I have no idea whether it's *meant* to support POST...

We do not - we want/need Patchwork to do the parsing itself, rather
than allow arbitrary creation of "patches". It might make sense to
allow uploading raw mbox files (superuser only) in the future if people
request it, but this would be a different endpoint (/upload?).

> ---
>  docs/api/rest.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/api/rest.rst b/docs/api/rest.rst
> index 1750913..8c8fd95 100644
> --- a/docs/api/rest.rst
> +++ b/docs/api/rest.rst
> @@ -155,7 +155,7 @@ parameters should be passed as form-encoded data:
>  
>  .. code-block:: shell
>  
> -$ curl -X POST -F 'state=under-review' \
> +$ curl -X PATCH -F 'state=under-review' \
>    'https://patchwork.example.com/api/patches/123'
>  
>  Authentication

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH] docs: Correct pre-release regex

[Stephen Finucane]

On Fri, 2017-05-26 at 09:29 +0100, Stephen Finucane wrote:
> There's no dot before the rc version.
> 
> Signed-off-by: Stephen Finucane 
> Fixes: b02c43d ("docs: Add pre-release regex")

I'm going to go right ahead and merge this as a noddy fix that I've
validated locally.

Stephen
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 2/3] REST: Expose cover mbox link via REST API

[Stephen Finucane]

Signed-off-by: Stephen Finucane 
---
 patchwork/api/cover.py   | 6 +-
 patchwork/tests/test_rest_api.py | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
index 797cadf..f69d329 100644
--- a/patchwork/api/cover.py
+++ b/patchwork/api/cover.py
@@ -38,10 +38,14 @@ class CoverLetterListSerializer(HyperlinkedModelSerializer):
 submitter = PersonSerializer(read_only=True)
 series = SeriesSerializer(many=True, read_only=True)
 
+def get_mbox(self, instance):
+request = self.context.get('request')
+return request.build_absolute_uri(instance.get_mbox_url())
+
 class Meta:
 model = CoverLetter
 fields = ('id', 'url', 'project', 'msgid', 'date', 'name', 'submitter',
-  'series')
+  'mbox', 'series')
 read_only_fields = fields
 extra_kwargs = {
 'url': {'view_name': 'api-cover-detail'},
diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
index 8d64625..abffd17 100644
--- a/patchwork/tests/test_rest_api.py
+++ b/patchwork/tests/test_rest_api.py
@@ -457,6 +457,7 @@ class TestCoverLetterAPI(APITestCase):
 def assertSerialized(self, cover_obj, cover_json):
 self.assertEqual(cover_obj.id, cover_json['id'])
 self.assertEqual(cover_obj.name, cover_json['name'])
+self.assertIn(cover_obj.get_mbox_url(), cover_json['mbox'])
 
 # nested fields
 
-- 
2.9.4

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 0/3] Allow downloading of cover letter mboxes

[Stephen Finucane]

While we haven't totally integrated cover letters in the web UI yet (planned
for 2.1), we do expose these in a limited way and fully expose them via the
REST API. For the latter case, it should be possible to download cover letters
in mbox format for users who want them. For the former case, it already looks
like you can do this, but the links are broken and need to be fixed. This
series works on addressing both issues.

Stephen Finucane (3):
  views: Enable downloading of cover mboxes
  REST: Expose cover mbox link via REST API
  views: Display correct download links for covers

 patchwork/api/cover.py |  6 ++-
 patchwork/models.py|  5 ++-
 .../templates/patchwork/download_buttons.html  |  6 +++
 patchwork/tests/test_rest_api.py   |  1 +
 patchwork/urls.py  |  2 +
 patchwork/views/cover.py   | 13 +++
 patchwork/views/utils.py   | 45 +-
 7 files changed, 58 insertions(+), 20 deletions(-)

-- 
2.9.4

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 3/3] views: Display correct download links for covers

[Stephen Finucane]

This means using the correct link for the cover mbox and not displaying
one for the non-existent diff.

Signed-off-by: Stephen Finucane 
---
 patchwork/templates/patchwork/download_buttons.html | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/patchwork/templates/patchwork/download_buttons.html 
b/patchwork/templates/patchwork/download_buttons.html
index df392b3..1322eed 100644
--- a/patchwork/templates/patchwork/download_buttons.html
+++ b/patchwork/templates/patchwork/download_buttons.html
@@ -1,10 +1,16 @@
 
+  {% if submission.diff %}
   diff
   mbox
+  {% else %}
+  mbox
+  {% endif %}
   {% if submission.series.all|length == 1 %}
   https://lists.ozlabs.org/listinfo/patchwork

[PATCH] docs: Correct pre-release regex

[Stephen Finucane]

There's no dot before the rc version.

Signed-off-by: Stephen Finucane 
Fixes: b02c43d ("docs: Add pre-release regex")
---
 releasenotes/config.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/releasenotes/config.yaml b/releasenotes/config.yaml
index ff1d7e2..1fb8a5b 100644
--- a/releasenotes/config.yaml
+++ b/releasenotes/config.yaml
@@ -1,4 +1,4 @@
 ---
 earliest_version: v1.1.0
 release_tag_re: 'v\d\.\d\.\d(rc\d+)?'
-pre_release_tag_re: '(?P-rc(?:\.\d)*)$'
+pre_release_tag_re: '(?P-rc(?:\d)*)$'
-- 
2.9.4

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork