From 872a80d19cc36247ec326a59900d9cb298394742 Mon Sep 17 00:00:00 2001 From: Luc Rocher Date: Tue, 2 May 2017 15:15:05 +0200 Subject: [PATCH] Add initial authentication and authorization for views and API --- apps/core/management/commands/init_groups.py | 15 +++-- apps/rp/api/mixins.py | 51 +++++++++++++++++ apps/rp/api/views.py | 57 ++----------------- apps/rp/migrations/0014_auto_20170501_1611.py | 25 ++++++++ apps/rp/models/article.py | 21 ++++--- apps/rp/templates/rp/article_list.html | 9 ++- apps/rp/urls.py | 12 ++-- apps/rp/views/articles.py | 6 +- apps/userprofile/admin.py | 8 ++- project/settings/api.py | 4 ++ project/settings/auth.py | 2 +- 11 files changed, 136 insertions(+), 74 deletions(-) create mode 100644 apps/rp/api/mixins.py create mode 100644 apps/rp/migrations/0014_auto_20170501_1611.py diff --git a/apps/core/management/commands/init_groups.py b/apps/core/management/commands/init_groups.py index acf489d..96b0e96 100644 --- a/apps/core/management/commands/init_groups.py +++ b/apps/core/management/commands/init_groups.py @@ -3,13 +3,20 @@ from django.contrib.auth.models import Group, Permission groups = ["jedi", "padawan"] permissions = { - "jedi": [], - "padawans": [] + "jedi": [ + "can_change_status", "can_change_priority", "can_vote", "can_edit" + ], + "padawan": ["can_vote", "add_article"] } class Command(BaseCommand): - help = "Adds initial groups for the application (jedi and padawans)" + help = "Adds initial groups for the application (jedis and padawans)" def handle(self, *args, **options): - pass + + for g in groups: + print("Creating group '{}'".format(g)) + new_group, created = Group.objects.get_or_create(name=g) + for p in permissions[g]: + new_group.permissions.add(Permission.objects.get(codename=p)) diff --git a/apps/rp/api/mixins.py b/apps/rp/api/mixins.py new file mode 100644 index 0000000..91646bd --- /dev/null +++ b/apps/rp/api/mixins.py @@ -0,0 +1,51 @@ +from django_fsm import has_transition_perm, can_proceed +from rest_framework.decorators import detail_route +from rest_framework.exceptions import PermissionDenied +from rest_framework.response import Response + +import inspect + + +def get_transition_viewset_method(model, transition_name): + @detail_route(methods=['post']) + def inner_func(self, request, pk=None, *args, **kwargs): + object = self.get_object() + transition_method = getattr(object, transition_name) + + if not can_proceed(transition_method): + raise PermissionDenied + + if not has_transition_perm(transition_method, request.user): + raise PermissionDenied + + if 'by' in inspect.getargspec(transition_method).args: + transition_method(*args, by=request.user, **kwargs) + else: + transition_method(*args, **kwargs) + + if self.save_after_transition: + object.save() + + serializer = self.get_serializer(object) + return Response(serializer.data) + + return inner_func + + +def get_viewset_transition_actions_mixin(model): + """ + Automatically generate methods for Django REST Framework from transition + rules. + """ + instance = model() + + class Mixin(object): + save_after_transition = True + + transitions = instance.get_all_status_transitions() + transition_names = set(x.name for x in transitions) + for transition_name in transition_names: + setattr(Mixin, transition_name, + get_transition_viewset_method(model, transition_name)) + + return Mixin diff --git a/apps/rp/api/views.py b/apps/rp/api/views.py index fc118f2..91c696b 100644 --- a/apps/rp/api/views.py +++ b/apps/rp/api/views.py @@ -1,61 +1,12 @@ from rest_framework import viewsets -from rest_framework.decorators import detail_route -from rest_framework.response import Response from rp.models import Article from .serializers import ArticleSerializer +from .mixins import get_viewset_transition_actions_mixin +ArticleMixin = get_viewset_transition_actions_mixin(Article) -class ArticleViewSet(viewsets.ModelViewSet): + +class ArticleViewSet(ArticleMixin, viewsets.ModelViewSet): queryset = Article.objects.all() serializer_class = ArticleSerializer - - def response_serialized_object(self, object): - return Response(self.serializer_class(object).data) - - @detail_route(methods=["post"], url_path="publish") - def publish(self, request, pk=None): - article = self.get_object() - article.publish() - article.save() - return self.response_serialized_object(article) - - @detail_route(methods=["post"], url_path="reject") - def reject(self, request, pk=None): - article = self.get_object() - article.reject() - article.save() - return self.response_serialized_object(article) - - @detail_route(methods=["post"], url_path="recover") - def recover(self, request, pk=None): - article = self.get_object() - article.recover() - article.save() - return self.response_serialized_object(article) - - @detail_route(methods=["post"], url_path="upvote") - def upvote(self, request, pk=None): - article = self.get_object() - article.upvote(user_object=request.user.username) - return self.response_serialized_object(article) - - @detail_route(methods=["post"], url_path="downvote") - def downvote(self, request, pk=None): - article = self.get_object() - article.downvote(user_object=request.user.username) - return self.response_serialized_object(article) - - @detail_route(methods=["post"], url_path="priority/on") - def priority_on(self, request, pk=None, priority=True): - article = self.get_object() - article.set_priority(True) - article.save() - return self.response_serialized_object(article) - - @detail_route(methods=["post"], url_path="priority/off") - def priority_off(self, request, pk=None, priority=True): - article = self.get_object() - article.set_priority(False) - article.save() - return self.response_serialized_object(article) diff --git a/apps/rp/migrations/0014_auto_20170501_1611.py b/apps/rp/migrations/0014_auto_20170501_1611.py new file mode 100644 index 0000000..3eb468e --- /dev/null +++ b/apps/rp/migrations/0014_auto_20170501_1611.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11 on 2017-05-01 16:11 +from __future__ import unicode_literals + +from django.db import migrations +import django_fsm + + +class Migration(migrations.Migration): + + dependencies = [ + ('rp', '0013_auto_20170428_1341'), + ] + + operations = [ + migrations.AlterModelOptions( + name='article', + options={'permissions': (('can_change_status', 'Can change article status'), ('can_change_priority', 'Can change article priority'), ('can_vote', 'Can vote articles'), ('can_edit', 'Can edit articles')), 'verbose_name': 'Article', 'verbose_name_plural': 'Articles'}, + ), + migrations.AlterField( + model_name='article', + name='status', + field=django_fsm.FSMField(choices=[('NEW', 'New'), ('DRAFT', 'Draft'), ('PUBLISHED', 'Published'), ('REJECTED', 'Rejected')], default='NEW', max_length=50, protected=True), + ), + ] diff --git a/apps/rp/models/article.py b/apps/rp/models/article.py index 75fe3e0..ac25976 100644 --- a/apps/rp/models/article.py +++ b/apps/rp/models/article.py @@ -75,6 +75,7 @@ class Article(VoteMixin): ("can_change_status", "Can change article status"), ("can_change_priority", "Can change article priority"), ("can_vote", "Can vote articles"), + ("can_edit", "Can edit articles") ) def __str__(self): @@ -82,7 +83,8 @@ class Article(VoteMixin): # Finite state logic - @transition(field=status, source='DRAFT', target='PUBLISHED') + @transition(field=status, source='DRAFT', target='PUBLISHED', + permission="can_change_status") def publish(self): self.published_at = datetime.now() @@ -98,14 +100,19 @@ class Article(VoteMixin): @transition(field=status, source='DRAFT', target='DRAFT', permission="can_change_priority") - def set_priority(self, value): - self.priority = value + def set_priority(self): + self.priority = True + + @transition(field=status, source='DRAFT', target='DRAFT', + permission="can_change_priority") + def unset_priority(self): + self.priority = False @transition(field=status, source='DRAFT', target='DRAFT') @transition(field=status, source='NEW', target=RETURN_VALUE('NEW', 'DRAFT'), permission="can_vote") - def upvote(self, user_object): - super(Article, self).upvote(user_object) + def upvote(self, by=None): + super(Article, self).upvote(by) if self.und_score >= ARTICLE_SCORE_THRESHOLD: return 'DRAFT' else: @@ -114,8 +121,8 @@ class Article(VoteMixin): @transition(field=status, source='NEW', target='NEW', permission="can_vote") @transition(field=status, source='DRAFT', target='DRAFT', permission="can_vote") - def downvote(self, user_object): - super(Article, self).downvote(user_object) + def downvote(self, by=None): + super(Article, self).downvote(by) # Content extraction diff --git a/apps/rp/templates/rp/article_list.html b/apps/rp/templates/rp/article_list.html index bef1da3..262a017 100644 --- a/apps/rp/templates/rp/article_list.html +++ b/apps/rp/templates/rp/article_list.html @@ -218,8 +218,13 @@ } function call_priority(id, flag) { - var url = "/api/articles/" + id + "/priority/" + (flag ? "on/" : "off/"); - $.post(url, {'priority': flag}, function response(data) { + if(flag) { + var url = "/api/articles/" + id + "/set_priority/"; + } else { + var url = "/api/articles/" + id + "/unset_priority/"; + } + + $.post(url, function response(data) { $("#priority_" + id).toggleClass("fa-star").toggleClass("fa-star-o"); }); } diff --git a/apps/rp/urls.py b/apps/rp/urls.py index cb1a236..ce39af7 100644 --- a/apps/rp/urls.py +++ b/apps/rp/urls.py @@ -1,30 +1,32 @@ +from django.contrib.auth.decorators import login_required from django.conf.urls import url + from rp.views.articles import ArticleListFlux, ArticleEdit, ArticleDetailView urlpatterns = [ url( r"^article/list/(?P\w+)", - ArticleListFlux.as_view(), + login_required(ArticleListFlux.as_view()), name="article-list" ), url( r"^article/list", - ArticleListFlux.as_view(), + login_required(ArticleListFlux.as_view()), name="article-list" ), url( r"^article/edit/(?P\d+)", - ArticleEdit.as_view(), + login_required(ArticleEdit.as_view()), name="article-edit" ), url( r"^article/view/(?P\d+)", - ArticleDetailView.as_view(), + login_required(ArticleDetailView.as_view()), name="article-view" ), url( r"^article/preview/(?P\d+)", - ArticleDetailView.as_view(preview=True), + login_required(ArticleDetailView.as_view(preview=True)), name="article-preview" ) ] diff --git a/apps/rp/views/articles.py b/apps/rp/views/articles.py index 3534c33..8260096 100644 --- a/apps/rp/views/articles.py +++ b/apps/rp/views/articles.py @@ -5,6 +5,8 @@ from django.utils.translation import ugettext_lazy as _ from django.urls import reverse, reverse_lazy from django import forms +from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin + from crispy_forms.helper import FormHelper from crispy_forms.layout import Layout, Field, Div, HTML from crispy_forms.bootstrap import AppendedText @@ -47,8 +49,10 @@ class ArticleDetailView(DetailView): return context -class ArticleEdit(UpdateView): +class ArticleEdit(PermissionRequiredMixin, UpdateView): model = Article + permission_required = 'can_edit' + fields = ['screenshot', 'url', 'lang', 'title', 'tags', 'extracts'] success_url = reverse_lazy("rp:article-list") diff --git a/apps/userprofile/admin.py b/apps/userprofile/admin.py index 7820168..e6dee28 100644 --- a/apps/userprofile/admin.py +++ b/apps/userprofile/admin.py @@ -22,13 +22,19 @@ class UserProfileInline(admin.StackedInline): class UserProfileAdmin(UserAdmin): inlines = [UserProfileInline] + + list_display = ("username", "email", "first_name", "last_name", "is_staff", "get_groups") + fieldsets = ( (None, {"fields": ("username", "password")}), (_("Personal info"), {"fields": ("first_name", "last_name", "email")}), (_("Permissions"), { - "fields": ("is_active", "is_staff", "is_superuser")}), + "fields": ("is_active", "is_staff", "is_superuser", "groups")}), ) + def get_groups(self, obj): + return ", ".join(sorted([g.name for g in obj.groups.all()])) + get_groups.short_description = _("Groups") admin.site.unregister(User) admin.site.register(User, UserProfileAdmin) diff --git a/project/settings/api.py b/project/settings/api.py index 59064a8..8a753f6 100644 --- a/project/settings/api.py +++ b/project/settings/api.py @@ -6,4 +6,8 @@ REST_FRAMEWORK = { "DEFAULT_PAGINATION_CLASS": ("rest_framework.pagination." "PageNumberPagination"), "PAGE_SIZE": 20, + + "DEFAULT_PERMISSION_CLASSES": ( + "rest_framework.permissions.IsAuthenticated", + ) } diff --git a/project/settings/auth.py b/project/settings/auth.py index 538f33a..078c4e4 100644 --- a/project/settings/auth.py +++ b/project/settings/auth.py @@ -4,7 +4,7 @@ User registration and login related settings AUTH_USER_MODEL = "auth.User" EXTENDED_USER_MODEL = "userprofile.Profile" -LOGIN_URL = "login" +LOGIN_URL = "/accounts/login" AUTHENTICATION_BACKENDS = [ -- GitLab