From 99cf7cd1bbcb579051a6ad95113d2c2b4676ee27 Mon Sep 17 00:00:00 2001 From: Nikolai Kristiansen Date: Sun, 20 Oct 2013 00:18:00 +0200 Subject: [PATCH] Membership: Permissions, cleanups, green tests --- dusken/api/member.py | 4 +++ dusken/api/membership.py | 15 ++++---- dusken/authorization.py | 54 +++++++++++++++++++++++++++-- dusken/fixtures/membership.json | 8 ++--- dusken/fixtures/membershiptype.json | 2 +- dusken/models.py | 23 ++++++++---- dusken/tests/membership.py | 7 ++-- 7 files changed, 89 insertions(+), 24 deletions(-) diff --git a/dusken/api/member.py b/dusken/api/member.py index 698e6db..94e6ac7 100644 --- a/dusken/api/member.py +++ b/dusken/api/member.py @@ -84,7 +84,11 @@ class MemberResource(ModelResource): + len(Member.objects.filter(email=bundle.data['email'])) \ + len(Member.objects.filter(phone_number=bundle.data['phone_number'])) if member_count > 0: + # TODO Specify what data is missing raise ImmediateHttpResponse(HttpForbidden("User with given data already exists")) + + # Quick hack to allow new users to be created anonymously (unauthenticated) + bundle._anonymous_request_allowed = True return bundle def obj_update(self, bundle, **kwargs): diff --git a/dusken/api/membership.py b/dusken/api/membership.py index 162b883..128e201 100644 --- a/dusken/api/membership.py +++ b/dusken/api/membership.py @@ -2,7 +2,8 @@ from django.conf.urls import url from dusken.api.member import MemberResource from dusken.models import * from tastypie import fields -from tastypie.authorization import Authorization +from dusken.authorization import MyDjangoAuthorization +from dusken.authentication import MyApiKeyAuthentication from tastypie.resources import ModelResource, ALL, ALL_WITH_RELATIONS @@ -19,7 +20,8 @@ class MembershipResource(ModelResource): resource_name = 'membership' list_allowed_methods = [ 'get', 'post' ] detail_allowed_methods = [ 'get', 'patch' ] - authorization = Authorization() # TODO: for dev (VERY INSECURE) + authorization = MyDjangoAuthorization() + authentication = MyApiKeyAuthentication() filtering = { 'member' : ALL_WITH_RELATIONS, 'expires' : [ 'exact','range','gt','gte','lt','lte' ], #TODO: Doesn't work. @@ -41,8 +43,7 @@ class MembershipResource(ModelResource): def hydrate(self, bundle): if 'membership_type' in bundle.data: - bundle.data['mtype'] = bundle.data['membership_type'] - del bundle.data['membership_type'] + bundle.obj.membership_type = MembershipType.objects.get(id=bundle.data['membership_type']) if 'member' in bundle.data: bundle.data['member'] = Member.objects.get(id=bundle.data['member']) @@ -51,13 +52,13 @@ class MembershipResource(ModelResource): def dehydrate(self, bundle): bundle.data['member'] = bundle.obj.member.id - bundle.data['membership_type'] = bundle.obj.mtype.id + bundle.data['membership_type'] = bundle.obj.membership_type.id bundle.data['payment'] = None if not bundle.obj.payment else bundle.obj.payment.id return bundle def obj_update(self, bundle, request, **kwargs): member_id = bundle.data.get('member') - membership_type = bundle.data.get('member') + membership_type = bundle.data.get('membership_type') start_date = bundle.data.get('start_date') if None in (member_id, membership_type, start_date): @@ -75,7 +76,7 @@ class MembershipTypeResource(ModelResource): resource_name = "membership/type" list_allowed_methods = [ 'get', 'post' ] detail_allowed_methods = [ 'get', 'delete' ] - authorization = Authorization() # TODO + authorization = MyDjangoAuthorization() def delete_detail(self, request, **kwarg): #TODO set is_active flag to False diff --git a/dusken/authorization.py b/dusken/authorization.py index 7192caa..c044fd5 100644 --- a/dusken/authorization.py +++ b/dusken/authorization.py @@ -1,8 +1,57 @@ from tastypie.authorization import DjangoAuthorization from tastypie.exceptions import Unauthorized +# TODO add django-guardian for object level/row level permissions, then + +""" + Create + ###### + + All registered users should be allowed to create: + + * Membership + + Unregistered users should be able to create: + + * Member + + Update + ###### + + All registered users should be allowed to update: + + + Delete + ###### + + All registered users should be allowed to delete: + +""" + class MyDjangoAuthorization(DjangoAuthorization): + def create_detail(self, object_list, bundle): + # Special case for creating a new user + if hasattr(bundle, '_anonymous_request_allowed'): + return True + + klass = self.base_checks(bundle.request, bundle.obj.__class__) + + if klass is False: + raise Unauthorized("You are not allowed to access that resource.") + + # Skip permission check if the request is made by the object owner + # Note: tastypie.authorization.DjangoAuthorization does not have this check + if bundle.obj.owner() == bundle.request.user: + return True + + permission = '%s.add_%s' % (klass._meta.app_label, klass._meta.module_name) + + if not bundle.request.user.has_perm(permission): + raise Unauthorized("You are not allowed to access that resource.") + + return True + def update_detail(self, object_list, bundle): klass = self.base_checks(bundle.request, bundle.obj.__class__) @@ -10,8 +59,8 @@ class MyDjangoAuthorization(DjangoAuthorization): raise Unauthorized("You are not allowed to access that resource.") # Skip permission check if the request is made by the object owner - # Note: Standard DjangoAuthorization does not have this check - if bundle.obj == bundle.request.user: + # Note: tastypie.authorization.DjangoAuthorization does not have this check + if bundle.obj.owner() == bundle.request.user: return True permission = '%s.change_%s' % (klass._meta.app_label, klass._meta.module_name) @@ -20,3 +69,4 @@ class MyDjangoAuthorization(DjangoAuthorization): raise Unauthorized("You are not allowed to access that resource.") return True + diff --git a/dusken/fixtures/membership.json b/dusken/fixtures/membership.json index d74d562..bf4d19b 100644 --- a/dusken/fixtures/membership.json +++ b/dusken/fixtures/membership.json @@ -7,7 +7,7 @@ "updated": "2013-09-11T18:45:13.341Z", "created": "2013-09-11T18:45:13.341Z", "payment": null, - "mtype": 1, + "membership_type": 1, "start_date": "2013-09-11" } }, { @@ -18,7 +18,7 @@ "updated": "2013-09-11T18:45:30.084Z", "created": "2013-09-11T18:45:30.084Z", "payment": null, - "mtype": 1, + "membership_type": 1, "start_date": "2013-09-15" } }, { @@ -29,8 +29,8 @@ "updated": "2013-09-11T18:45:53.522Z", "created": "2013-09-11T18:45:53.522Z", "payment": null, - "mtype": 2, + "membership_type": 2, "start_date": "2013-09-11" } } -] \ No newline at end of file +] diff --git a/dusken/fixtures/membershiptype.json b/dusken/fixtures/membershiptype.json index 935727c..fac7934 100644 --- a/dusken/fixtures/membershiptype.json +++ b/dusken/fixtures/membershiptype.json @@ -19,4 +19,4 @@ "end_date": "2013-09-11" } } -] \ No newline at end of file +] diff --git a/dusken/models.py b/dusken/models.py index ee4d82a..f48e8a0 100644 --- a/dusken/models.py +++ b/dusken/models.py @@ -21,33 +21,36 @@ class Member(django.contrib.auth.models.AbstractUser): phone_number = models.CharField(max_length=30, unique=True, null=True, blank=True) date_of_birth = models.DateField(blank=True, null=True) legacy_id = models.IntegerField(unique=True, null=True, blank=True) - address = models.ForeignKey('dusken.Address', null=True, blank=True) + address = models.ForeignKey('dusken.Address', null=True, blank=True) # TODO change to ManyToManyField place_of_study = models.ManyToManyField('dusken.PlaceOfStudy', null=True, blank=True) created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) + def owner(self): + return self + class Membership(BaseModel): def __unicode__(self): return u"{member}: {fromdate}".format(member=self.member, fromdate=self.start_date) start_date = models.DateField() - mtype = models.ForeignKey('dusken.MembershipType', db_column='type') + membership_type = models.ForeignKey('dusken.MembershipType') payment = models.ForeignKey('dusken.Payment', unique=True, null=True, blank=True) member = models.ForeignKey('dusken.Member') def expires(self): - return self.mtype.end_date + return self.membership_type.end_date + + def owner(self): + return self.member -''' - TODO: should validate end_day_of_month and end_month (use datetime exceptions) -''' class MembershipType(BaseModel): def __unicode__(self): return u"{}".format(self.name) name = models.CharField(max_length=50, unique=True) - end_date = models.DateField() + end_date = models.DateField(null=True, blank=True) # TODO move to Membership (no new Membership types every year) is_active = models.BooleanField(default=True) does_not_expire = models.BooleanField(default=False) @@ -77,6 +80,9 @@ class FacebookAuth(BaseModel): token_expires = models.DateTimeField(null=True, blank=True) member = models.OneToOneField('dusken.Member', null=True, blank=True) + def owner(self): + return self.member + class GoogleAuth(BaseModel): def __unicode__(self): @@ -86,6 +92,9 @@ class GoogleAuth(BaseModel): token_expires = models.DateTimeField(null=True, blank=True) member = models.OneToOneField('dusken.Member', null=True, blank=True) + def owner(self): + return self.member + class Address(BaseModel): class Meta: diff --git a/dusken/tests/membership.py b/dusken/tests/membership.py index 1d18cec..17eac9c 100644 --- a/dusken/tests/membership.py +++ b/dusken/tests/membership.py @@ -34,6 +34,7 @@ class MembershipTest(ResourceTestCase): authentication=self.creds ) + # Help set format and add authentication credentials def do_post_request(self, url, data): return self.api_client.post( url, @@ -70,7 +71,7 @@ class MembershipTest(ResourceTestCase): resp = self.do_post_request(self.membership_url, data) - self.assertValidJSONResponse(resp) + self.assertHttpCreated(resp) def test_patch_membership(self): data = { @@ -83,8 +84,8 @@ class MembershipTest(ResourceTestCase): format='json', data=data, authentication=self.creds) - - self.assertValidJSONResponse(resp) + + self.assertHttpAccepted(resp) # Note: Patch requests should return HTTP 202 Accepted #################################################################### ### JSON Responses