| Message ID | 20170525084718.9586-1-andrew.donnellan@au1.ibm.com |
|---|---|
| State | RFC |
| Headers | show |
On Thu, 2017-05-25 at 18:47 +1000, 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 <andrew.donnellan@au1.ibm.com> Yup, totally onboard with this, even for 2.0. I'd be in favor of removing Basic Auth if we add this. I can take documentation if you want to handle the other items. I imagine tests shouldn't be too arduous either. Think this is something you could wire up in the next few days? :) Stephen > --- > > This is an RFC; I haven't written any tests or documentation, UI's a > bit > ugly, need to split patches. > --- > 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.</p> > </div> > > <div class="box"> > -<h2>Authentication</h2> > -<a href="{% url 'password_change' %}">Change password</a> > + <h2>Authentication</h2> > + <form method="post" action="{%url 'generate_token' %}"> > + {% csrf_token %} > + <table> > + <tr> > + <th>Password:</th> > + <td><a href="{% url 'password_change' %}">Change > password</a> > + </tr> > + <tr> > + <th>API Token:</th> > + <td> > + {% if api_token %} > + {{ api_token }} > + <input type="submit" value="Regenerate token" /> > + {% else %} > + <input type="submit" value="Generate token" /> > + {% endif %} > + </td> > + </tr> > + </table> > + </form> > </div> > > </div> > 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 api_settings.DEFAULT_AUTHENTICATION_CLASSES: > + if auth == SessionAuthentication: > + continue > + auth_result = auth().authenticate(request) > + if auth_result: > + return auth_result[0] > except AuthenticationFailed: > pass > return request.user > diff --git a/patchwork/views/user.py b/patchwork/views/user.py > index 375d3d9..53e2ea8 100644 > --- a/patchwork/views/user.py > +++ b/patchwork/views/user.py > @@ -42,6 +42,8 @@ from patchwork.models import Project > from patchwork.models import State > from patchwork.views import generic_list > > +if settings.ENABLE_REST_API: > + from rest_framework.authtoken.models import Token > > def register(request): > context = {} > @@ -126,6 +128,11 @@ def profile(request): > .extra(select={'is_optout': optout_query}) > context['linked_emails'] = people > context['linkform'] = EmailForm() > + if settings.ENABLE_REST_API: > + try: > + context['api_token'] = > Token.objects.get(user=request.user) > + except Token.DoesNotExist: > + pass > > return render(request, 'patchwork/profile.html', context) > > @@ -232,3 +239,15 @@ def todo_list(request, project_id): > context['action_required_states'] = \ > State.objects.filter(action_required=True).all() > return render(request, 'patchwork/todo-list.html', context) > + > +@login_required > +def generate_token(request): > + if not settings.ENABLE_REST_API: > + raise RuntimeError('REST API not enabled') > + try: > + t = Token.objects.get(user=request.user) > + t.delete() > + except Token.DoesNotExist: > + pass > + Token.objects.create(user=request.user) > + return HttpResponseRedirect(reverse('user-profile'))
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 <andrew.donnellan@au1.ibm.com> > > --- > > 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.</p> > </div> > > <div class="box"> > -<h2>Authentication</h2> > -<a href="{% url 'password_change' %}">Change password</a> > + <h2>Authentication</h2> > + <form method="post" action="{%url 'generate_token' %}"> > + {% csrf_token %} > + <table> > + <tr> > + <th>Password:</th> > + <td><a href="{% url 'password_change' %}">Change password</a> > + </tr> > + <tr> > + <th>API Token:</th> > + <td> > + {% if api_token %} > + {{ api_token }} > + <input type="submit" value="Regenerate token" /> > + {% else %} > + <input type="submit" value="Generate token" /> > + {% endif %} > + </td> > + </tr> > + </table> > + </form> > </div> > > </div> > 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 api_settings.DEFAULT_AUTHENTICATION_CLASSES: > + if auth == SessionAuthentication: > + continue > + auth_result = auth().authenticate(request) > + if auth_result: > + return auth_result[0] > except AuthenticationFailed: > pass I think the try/except block should be inside the loop when calling authenticate(). > return request.user > diff --git a/patchwork/views/user.py b/patchwork/views/user.py > index 375d3d9..53e2ea8 100644 > --- a/patchwork/views/user.py > +++ b/patchwork/views/user.py > @@ -42,6 +42,8 @@ from patchwork.models import Project > from patchwork.models import State > from patchwork.views import generic_list > > +if settings.ENABLE_REST_API: > + from rest_framework.authtoken.models import Token > > def register(request): > context = {} > @@ -126,6 +128,11 @@ def profile(request): > .extra(select={'is_optout': optout_query}) > context['linked_emails'] = people > context['linkform'] = EmailForm() > + if settings.ENABLE_REST_API: > + try: > + context['api_token'] = Token.objects.get(user=request.user) > + except Token.DoesNotExist: > + pass > > return render(request, 'patchwork/profile.html', context) > > @@ -232,3 +239,15 @@ def todo_list(request, project_id): > context['action_required_states'] = \ > State.objects.filter(action_required=True).all() > return render(request, 'patchwork/todo-list.html', context) > + > +@login_required > +def generate_token(request): > + if not settings.ENABLE_REST_API: > + raise RuntimeError('REST API not enabled') What about disabling the url in patchwork/urls.py instead ? > + try: > + t = Token.objects.get(user=request.user) > + t.delete() > + except Token.DoesNotExist: > + pass This block could be replaced by: Token.objects.filter(user=request.user).delete() > + Token.objects.create(user=request.user) > + return HttpResponseRedirect(reverse('user-profile')) >
On 27/05/17 09:32, Philippe Pepiot wrote: > Thanks for adding this ! Aside my comments below, I tested a bit and it > worked fine. Thanks for the review, I'll fix all those comments in the next version.
On Thu, 2017-05-25 at 22:26 +0100, Stephen Finucane wrote: > On Thu, 2017-05-25 at 18:47 +1000, 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 <andrew.donnellan@au1.ibm.com> > FWIW it looks good to me. > Yup, totally onboard with this, even for 2.0. I'd be in favor of > removing Basic Auth if we add this. I can take documentation if you > want to handle the other items. I imagine tests shouldn't be too > arduous either. What exactly does BA get used for at the moment? - Russell > > Think this is something you could wire up in the next few days? :) > > Stephen > > > --- > > > > This is an RFC; I haven't written any tests or documentation, UI's a > > bit > > ugly, need to split patches. > > --- > > 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.</p> > > </div> > > > > <div class="box"> > > -<h2>Authentication</h2> > > -<a href="{% url 'password_change' %}">Change password</a> > > + <h2>Authentication</h2> > > + <form method="post" action="{%url 'generate_token' %}"> > > + {% csrf_token %} > > + <table> > > + <tr> > > + <th>Password:</th> > > + <td><a href="{% url 'password_change' %}">Change > > password</a> > > + </tr> > > + <tr> > > + <th>API Token:</th> > > + <td> > > + {% if api_token %} > > + {{ api_token }} > > + <input type="submit" value="Regenerate token" /> > > + {% else %} > > + <input type="submit" value="Generate token" /> > > + {% endif %} > > + </td> > > + </tr> > > + </table> > > + </form> > > </div> > > > > </div> > > 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 api_settings.DEFAULT_AUTHENTICATION_CLASSES: > > + if auth == SessionAuthentication: > > + continue > > + auth_result = auth().authenticate(request) > > + if auth_result: > > + return auth_result[0] > > except AuthenticationFailed: > > pass > > return request.user > > diff --git a/patchwork/views/user.py b/patchwork/views/user.py > > index 375d3d9..53e2ea8 100644 > > --- a/patchwork/views/user.py > > +++ b/patchwork/views/user.py > > @@ -42,6 +42,8 @@ from patchwork.models import Project > > from patchwork.models import State > > from patchwork.views import generic_list > > > > +if settings.ENABLE_REST_API: > > + from rest_framework.authtoken.models import Token > > > > def register(request): > > context = {} > > @@ -126,6 +128,11 @@ def profile(request): > > .extra(select={'is_optout': optout_query}) > > context['linked_emails'] = people > > context['linkform'] = EmailForm() > > + if settings.ENABLE_REST_API: > > + try: > > + context['api_token'] = > > Token.objects.get(user=request.user) > > + except Token.DoesNotExist: > > + pass > > > > return render(request, 'patchwork/profile.html', context) > > > > @@ -232,3 +239,15 @@ def todo_list(request, project_id): > > context['action_required_states'] = \ > > State.objects.filter(action_required=True).all() > > return render(request, 'patchwork/todo-list.html', context) > > + > > +@login_required > > +def generate_token(request): > > + if not settings.ENABLE_REST_API: > > + raise RuntimeError('REST API not enabled') > > + try: > > + t = Token.objects.get(user=request.user) > > + t.delete() > > + except Token.DoesNotExist: > > + pass > > + Token.objects.create(user=request.user) > > + return HttpResponseRedirect(reverse('user-profile')) > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On 30/05/17 16:26, Russell Currey wrote:
> What exactly does BA get used for at the moment?
REST requests that update status, e.g. sending a PATCH request on a
patch to change status from "new" to "under-review", or something like that.
On Tue, 2017-05-30 at 16:35 +1000, Andrew Donnellan wrote: > On 30/05/17 16:26, Russell Currey wrote: > > What exactly does BA get used for at the moment? > > REST requests that update status, e.g. sending a PATCH request on a > patch to change status from "new" to "under-review", or something > like that. You also need it to access the '/users' resource, though this is a bit useless now that we expose much of this information inline with certain resources ('/patches', for example) I'm contemplating either removing auth for '/users', or enforcing it for _all_ endpoints. I'm still on the fence with this though. Stephen
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.</p> </div> <div class="box"> -<h2>Authentication</h2> -<a href="{% url 'password_change' %}">Change password</a> + <h2>Authentication</h2> + <form method="post" action="{%url 'generate_token' %}"> + {% csrf_token %} + <table> + <tr> + <th>Password:</th> + <td><a href="{% url 'password_change' %}">Change password</a> + </tr> + <tr> + <th>API Token:</th> + <td> + {% if api_token %} + {{ api_token }} + <input type="submit" value="Regenerate token" /> + {% else %} + <input type="submit" value="Generate token" /> + {% endif %} + </td> + </tr> + </table> + </form> </div> </div> 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 api_settings.DEFAULT_AUTHENTICATION_CLASSES: + if auth == SessionAuthentication: + continue + auth_result = auth().authenticate(request) + if auth_result: + return auth_result[0] except AuthenticationFailed: pass return request.user diff --git a/patchwork/views/user.py b/patchwork/views/user.py index 375d3d9..53e2ea8 100644 --- a/patchwork/views/user.py +++ b/patchwork/views/user.py @@ -42,6 +42,8 @@ from patchwork.models import Project from patchwork.models import State from patchwork.views import generic_list +if settings.ENABLE_REST_API: + from rest_framework.authtoken.models import Token def register(request): context = {} @@ -126,6 +128,11 @@ def profile(request): .extra(select={'is_optout': optout_query}) context['linked_emails'] = people context['linkform'] = EmailForm() + if settings.ENABLE_REST_API: + try: + context['api_token'] = Token.objects.get(user=request.user) + except Token.DoesNotExist: + pass return render(request, 'patchwork/profile.html', context) @@ -232,3 +239,15 @@ def todo_list(request, project_id): context['action_required_states'] = \ State.objects.filter(action_required=True).all() return render(request, 'patchwork/todo-list.html', context) + +@login_required +def generate_token(request): + if not settings.ENABLE_REST_API: + raise RuntimeError('REST API not enabled') + try: + t = Token.objects.get(user=request.user) + t.delete() + except Token.DoesNotExist: + pass + Token.objects.create(user=request.user) + return HttpResponseRedirect(reverse('user-profile'))
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 <andrew.donnellan@au1.ibm.com> --- This is an RFC; I haven't written any tests or documentation, UI's a bit ugly, need to split patches. --- 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(-)