[PATCH 3/5] REST: Enable token auth support

[Andrew Donnellan]

On 10/06/17 03:25, Stephen Finucane 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 this support and add some tests to validate correct
> behavior.
>
> Signed-off-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
> Signed-off-by: Stephen Finucane <stephen at that.guru>

Changes from my RFC (that I spotted on first reading):

* add token property on Person
* add tests
* split out regenerate_token()

All looks good to me!


> ---
>  patchwork/models.py             | 13 +++++++++++++
>  patchwork/settings/base.py      |  6 ++++++
>  patchwork/tests/test_bundles.py | 24 ++++++++++++++++++++++++
>  patchwork/views/bundle.py       | 18 +++++++++++-------
>  patchwork/views/utils.py        | 14 ++++++++++++++
>  5 files changed, 68 insertions(+), 7 deletions(-)
>
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 943a601..dcb4c55 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -37,6 +37,9 @@ from patchwork.compat import is_authenticated
>  from patchwork.fields import HashField
>  from patchwork.hasher import hash_diff
>
> +if settings.ENABLE_REST_API:
> +    from rest_framework.authtoken.models import Token
> +
>
>  @python_2_unicode_compatible
>  class Person(models.Model):
> @@ -162,6 +165,16 @@ class UserProfile(models.Model):
>      def n_todo_patches(self):
>          return self.todo_patches().count()
>
> +    @property
> +    def token(self):
> +        if not settings.ENABLE_REST_API:
> +            return
> +
> +        try:
> +            return Token.objects.get(user=self.user)
> +        except Token.DoesNotExist:
> +            return
> +
>      def todo_patches(self, project=None):
>          # filter on project, if necessary
>          if project:
> 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/tests/test_bundles.py b/patchwork/tests/test_bundles.py
> index cdc7ee0..4461e32 100644
> --- a/patchwork/tests/test_bundles.py
> +++ b/patchwork/tests/test_bundles.py
> @@ -37,6 +37,7 @@ from patchwork.tests.utils import create_bundle
>  from patchwork.tests.utils import create_patches
>  from patchwork.tests.utils import create_project
>  from patchwork.tests.utils import create_user
> +from patchwork.views import utils as view_utils
>
>
>  def bundle_url(bundle):
> @@ -311,6 +312,7 @@ class BundlePrivateViewTest(BundleTestBase):
>          self.assertEqual(response.status_code, 404)
>
>
> + at unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
>  class BundlePrivateViewMboxTest(BundlePrivateViewTest):
>
>      """Ensure that non-owners can't view private bundle mboxes"""
> @@ -342,6 +344,28 @@ class BundlePrivateViewMboxTest(BundlePrivateViewTest):
>          response = self.client.get(self.url, HTTP_AUTHORIZATION=auth_string)
>          self.assertEqual(response.status_code, 404)
>
> +    def test_private_bundle_mbox_token_auth(self):
> +        self.client.logout()
> +
> +        # create tokens for both users
> +        for user in [self.user, self.other_user]:
> +            view_utils.regenerate_token(user)
> +
> +        def _get_auth_string(user):
> +            return 'Token {}'.format(str(user.profile.token))
> +
> +        # Check we can view as owner
> +        auth_string = _get_auth_string(self.user)
> +        response = self.client.get(self.url, HTTP_AUTHORIZATION=auth_string)
> +
> +        self.assertEqual(response.status_code, 200)
> +        self.assertContains(response, self.patches[0].name)
> +
> +        # Check we can't view as another user
> +        auth_string = _get_auth_string(self.other_user)
> +        response = self.client.get(self.url, HTTP_AUTHORIZATION=auth_string)
> +        self.assertEqual(response.status_code, 404)
> +
>
>  class BundleCreateFromListTest(BundleTestBase):
>
> diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
> index 387b7c6..2d18571 100644
> --- a/patchwork/views/bundle.py
> +++ b/patchwork/views/bundle.py
> @@ -36,19 +36,23 @@ 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]
> -    except AuthenticationFailed:
> -        pass
> +    for auth in api_settings.DEFAULT_AUTHENTICATION_CLASSES:
> +        if auth == SessionAuthentication:
> +            continue
> +        try:
> +            auth_result = auth().authenticate(request)
> +            if auth_result:
> +                return auth_result[0]
> +        except AuthenticationFailed:
> +            pass
>      return request.user
>
>
> diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py
> index 5528ee7..84682b8 100644
> --- a/patchwork/views/utils.py
> +++ b/patchwork/views/utils.py
> @@ -26,6 +26,7 @@ from email.parser import HeaderParser
>  import email.utils
>  import re
>
> +from django.conf import settings
>  from django.http import Http404
>  from django.utils import six
>
> @@ -33,6 +34,9 @@ from patchwork.models import Comment
>  from patchwork.models import Patch
>  from patchwork.models import Series
>
> +if settings.ENABLE_REST_API:
> +    from rest_framework.authtoken.models import Token
> +
>
>  class PatchMbox(MIMENonMultipart):
>      patch_charset = 'utf-8'
> @@ -181,3 +185,13 @@ def series_to_mbox(series):
>          mbox.append(patch_to_mbox(dep.patch))
>
>      return '\n'.join(mbox)
> +
> +
> +def regenerate_token(user):
> +    """Generate (or regenerate) user API tokens.
> +
> +    Arguments:
> +        user: The User object to generate a token for.
> +    """
> +    Token.objects.filter(user=user).delete()
> +    Token.objects.create(user=user)
>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited