Merge pull request #1385 Add check if user has permission to add a pr…

…oject to a profile.

CHANGES.rst

@@ -4,7 +4,10 @@ Changelog for onadata
 1.13.3 (Unreleased)
 --------------------
 
-- Nothing changed yet
+- Add check if user has permission to add a project to a profile
+  `Issue 1385 <https://github.com/onaio/onadata/pull/1385>`_
+  [ukanga]
+
 
 1.13.2 (2018-04-11)
 --------------------

onadata/apps/api/management/commands/apply_can_add_project_perms.py

@@ -0,0 +1,55 @@
+# -*- coding: utf-8 -*-
+"""
+Command apply_can_add_project_perms - applys can_add_project permission to all
+users who have can_add_xform permission to a user/organization profile.
+
+This was necessary because we previously (April 2018) did not have
+can_add_project permission on the UserProfile and OrganizationProfile classes.
+An attempt on doing this in migrations seems not to be a recommended approach.
+"""
+from django.core.management.base import BaseCommand
+from django.utils.translation import gettext as _
+from guardian.shortcuts import assign_perm
+
+from onadata.apps.api.models import OrganizationProfile
+from onadata.apps.main.models import UserProfile
+
+
+def org_can_add_project_permission():
+    """
+    Set 'can_add_project' permission to all users who have 'can_add_xform'
+    permission in the organization profile.
+    """
+    organizations = OrganizationProfile.objects.all()
+
+    for organization in organizations.iterator():
+        permissions = organization.orgprofileuserobjectpermission_set.filter(
+            permission__codename='can_add_xform')
+        for permission in permissions:
+            assign_perm('can_add_project', permission.user, organization)
+
+
+def user_can_add_project_permission():
+    """
+    Set 'can_add_project' permission to all users who have 'can_add_xform'
+    permission in the user profile.
+    """
+    users = UserProfile.objects.all()
+
+    for user in users.iterator():
+        permissions = user.userprofileuserobjectpermission_set.filter(
+            permission__codename='can_add_xform')
+        for permission in permissions:
+            assign_perm('can_add_project', permission.user, user)
+
+
+class Command(BaseCommand):
+    """
+    Command apply_can_add_preject_perms - applys can_add_project permission to
+    all users who have can_add_xform permission to a user/organization profile.
+    """
+    help = _(u"Apply can_add_project permissions")
+
+    def handle(self, *args, **options):
+        user_can_add_project_permission()
+        org_can_add_project_permission()

onadata/apps/api/migrations/0003_auto_20180425_0754.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.11 on 2018-04-25 11:54
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0002_auto_20151014_0909'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='organizationprofile',
+            options={
+                'permissions':
+                (('can_add_project', 'Can add a project to an organization'),
+                 ('can_add_xform',
+                  'Can add/upload an xform to an organization'),
+                 ('view_organizationprofile', 'Can view organization profile'))
+            }, ),
+    ]

onadata/apps/api/models/organization_profile.py

@@ -1,24 +1,36 @@
-from django.db import models
-from django.contrib.auth.models import Permission
-from django.contrib.auth.models import User
+# -*- coding: utf-8 -*-
+"""
+OrganizationProfile module.
+"""
+from django.contrib.auth.models import Permission, User
 from django.contrib.contenttypes.models import ContentType
-from django.db.models.signals import post_save, post_delete
-from guardian.shortcuts import get_perms_for_model, assign_perm
-from guardian.models import UserObjectPermissionBase
-from guardian.models import GroupObjectPermissionBase
+from django.db import models
+from django.db.models.signals import post_delete, post_save
+from django.utils.encoding import python_2_unicode_compatible
+
+from guardian.models import GroupObjectPermissionBase, UserObjectPermissionBase
+from guardian.shortcuts import assign_perm, get_perms_for_model
 
 from onadata.apps.api.models.team import Team
 from onadata.apps.main.models import UserProfile
-from onadata.libs.utils.cache_tools import safe_delete, IS_ORG
+from onadata.libs.utils.cache_tools import IS_ORG, safe_delete
 
 
+# pylint: disable=invalid-name,unused-argument
 def org_profile_post_delete_callback(sender, instance, **kwargs):
+    """
+    Signal handler to delete the organization user object.
+    """
     # delete the org_user too
     instance.user.delete()
     safe_delete('{}{}'.format(IS_ORG, instance.pk))
 
 
 def create_owner_team_and_permissions(sender, instance, created, **kwargs):
+    """
+    Signal handler that creates the Owner team and assigns group and user
+    permissions.
+    """
     if created:
         team = Team.objects.create(
             name=Team.OWNER_TEAM_NAME, organization=instance.user,
@@ -37,10 +49,11 @@ def create_owner_team_and_permissions(sender, instance, created, **kwargs):
             if instance.creator:
                 assign_perm(perm.codename, instance.creator, instance)
 
-            if instance.created_by:
+            if instance.created_by and instance.created_by != instance.creator:
                 assign_perm(perm.codename, instance.created_by, instance)
 
 
+@python_2_unicode_compatible
 class OrganizationProfile(UserProfile):
 
     """Organization: Extends the user profile for organization specific info
@@ -57,15 +70,19 @@ class OrganizationProfile(UserProfile):
     class Meta:
         app_label = 'api'
         permissions = (
-            ('can_add_xform', "Can add/upload an xform to organization"),
+            ('can_add_project', "Can add a project to an organization"),
+            ('can_add_xform', "Can add/upload an xform to an organization"),
             ('view_organizationprofile', "Can view organization profile"),
         )
 
     is_organization = models.BooleanField(default=True)
     # Other fields here
     creator = models.ForeignKey(User)
 
-    def save(self, *args, **kwargs):
+    def __str__(self):
+        return u'%s[%s]' % (self.name, self.user.username)
+
+    def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
         super(OrganizationProfile, self).save(*args, **kwargs)
 
     def remove_user_from_organization(self, user):
@@ -97,6 +114,7 @@ def is_organization_owner(self, user):
                     dispatch_uid='org_profile_post_delete_callback')
 
 
+# pylint: disable=model-no-explicit-unicode
 class OrgProfileUserObjectPermission(UserObjectPermissionBase):
     """Guardian model to create direct foreign keys."""
     content_object = models.ForeignKey(OrganizationProfile)

onadata/apps/api/tests/viewsets/test_project_viewset.py

@@ -60,7 +60,7 @@ def get_latest_tags(project):
 class TestProjectViewSet(TestAbstractViewSet):
 
     def setUp(self):
-        super(self.__class__, self).setUp()
+        super(TestProjectViewSet, self).setUp()
         self.view = ProjectViewSet.as_view({
             'get': 'list',
             'post': 'create'
@@ -244,6 +244,44 @@ def test_projects_create(self):
             self.assertEqual(self.user, project.created_by)
             self.assertEqual(self.user, project.organization)
 
+    def test_project_create_other_account(self):  # pylint: disable=C0103
+        """
+        Test that a user cannot create a project in a different user account
+        without the right permission.
+        """
+        alice_data = {'username': 'alice', 'email': 'alice@localhost.com'}
+        alice_profile = self._create_user_profile(alice_data)
+        bob = self.user
+        self._login_user_and_profile(alice_data)
+        data = {
+            "name": "Example Project",
+            "owner": "http://testserver/api/v1/users/bob",  # Bob
+        }
+
+        # Alice should not be able to create the form in bobs account.
+        request = self.factory.post('/projects', data=data, **self.extra)
+        response = self.view(request)
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.data, {
+            'owner': [u'You do not have permmission to create a project in '
+                      'this organization.']})
+        self.assertEqual(Project.objects.count(), 0)
+
+        # Give Alice the permission to create a project in Bob's account.
+        ManagerRole.add(alice_profile.user, bob.profile)
+        request = self.factory.post('/projects', data=data, **self.extra)
+        response = self.view(request)
+        self.assertEqual(response.status_code, 201)
+
+        projects = Project.objects.all()
+        self.assertEqual(len(projects), 1)
+
+        for project in projects:
+            # Created by Alice
+            self.assertEqual(alice_profile.user, project.created_by)
+            # But under Bob's account
+            self.assertEqual(bob, project.organization)
+
     def test_create_duplicate_project(self):
         """
         Test creating a project with the same name
@@ -1495,8 +1533,16 @@ def test_move_project_owner(self):
         request = self.factory.patch('/', data=data_patch, **self.extra)
         response = view(request, pk=projectid)
 
-        self.project.refresh_from_db()
+        # bob cannot move project if he does not have can_add_project project
+        # permission on alice's account.c
+        self.assertEqual(response.status_code, 400)
+
+        # Give bob permission.
+        ManagerRole.add(self.user, alice_profile)
+        request = self.factory.patch('/', data=data_patch, **self.extra)
+        response = view(request, pk=projectid)
         self.assertEqual(response.status_code, 200)
+        self.project.refresh_from_db()
         self.assertEquals(self.project.organization, alice)
         self.assertTrue(OwnerRole.user_has_role(alice, self.project))
 

onadata/apps/main/migrations/0008_auto_20180425_0754.py

@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.11 on 2018-04-25 11:54
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('main', '0007_auto_20160418_0525'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='userprofile',
+            options={
+                'permissions':
+                (('can_add_project', 'Can add a project to an organization'),
+                 ('can_add_xform', 'Can add/upload an xform to user profile'),
+                 ('view_profile', 'Can view user profile'))
+            }, ),
+    ]

onadata/apps/main/models/user_profile.py

@@ -78,6 +78,7 @@ def save(self, force_insert=False, force_update=False, using=None,
     class Meta:
         app_label = 'main'
         permissions = (
+            ('can_add_project', "Can add a project to an organization"),
             ('can_add_xform', "Can add/upload an xform to user profile"),
             ('view_profile', "Can view user profile"),
         )

onadata/libs/permissions.py

@@ -25,12 +25,14 @@
 CAN_ADD_USERPROFILE = 'add_userprofile'
 CAN_CHANGE_USERPROFILE = 'change_userprofile'
 CAN_DELETE_USERPROFILE = 'delete_userprofile'
+CAN_ADD_PROJECT_TO_PROFILE = 'can_add_project'
 CAN_ADD_XFORM_TO_PROFILE = 'can_add_xform'
 CAN_VIEW_PROFILE = 'view_profile'
 
 # Organization Permissions
 CAN_VIEW_ORGANIZATION_PROFILE = 'view_organizationprofile'
 CAN_ADD_ORGANIZATION_PROFILE = 'add_organizationprofile'
+CAN_ADD_ORGANIZATION_PROJECT = 'can_add_project'
 CAN_ADD_ORGANIZATION_XFORM = 'can_add_xform'
 CAN_CHANGE_ORGANIZATION_PROFILE = 'change_organizationprofile'
 CAN_DELETE_ORGANIZATION_PROFILE = 'delete_organizationprofile'
@@ -263,14 +265,16 @@ class ManagerRole(Role):
     class_to_permissions = {
         MergedXForm: [CAN_VIEW_MERGED_XFORM],
         OrganizationProfile:
-        [CAN_ADD_XFORM_TO_PROFILE, CAN_VIEW_ORGANIZATION_PROFILE],
+        [CAN_ADD_ORGANIZATION_PROJECT, CAN_ADD_ORGANIZATION_XFORM,
+         CAN_VIEW_ORGANIZATION_PROFILE],
         Project: [
             CAN_ADD_PROJECT, CAN_ADD_PROJECT_XFORM,
             CAN_ADD_SUBMISSIONS_PROJECT, CAN_CHANGE_PROJECT,
             CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT, CAN_VIEW_PROJECT_ALL,
             CAN_VIEW_PROJECT_DATA
         ],
-        UserProfile: [CAN_ADD_XFORM_TO_PROFILE, CAN_VIEW_PROFILE],
+        UserProfile: [CAN_ADD_PROJECT_TO_PROFILE, CAN_ADD_XFORM_TO_PROFILE,
+                      CAN_VIEW_PROFILE],
         XForm: [
             CAN_ADD_SUBMISSIONS, CAN_ADD_XFORM, CAN_CHANGE_XFORM,
             CAN_DELETE_SUBMISSION, CAN_DELETE_XFORM, CAN_EXPORT_XFORM,
@@ -298,7 +302,8 @@ class OwnerRole(Role):
         ],
         MergedXForm: [CAN_VIEW_MERGED_XFORM],
         OrganizationProfile: [
-            CAN_ADD_XFORM_TO_PROFILE, CAN_ADD_ORGANIZATION_PROFILE,
+            CAN_ADD_ORGANIZATION_PROJECT, CAN_ADD_ORGANIZATION_XFORM,
+            CAN_ADD_ORGANIZATION_PROFILE, CAN_ADD_ORGANIZATION_PROJECT,
             CAN_ADD_ORGANIZATION_XFORM, CAN_CHANGE_ORGANIZATION_PROFILE,
             CAN_DELETE_ORGANIZATION_PROFILE, CAN_VIEW_ORGANIZATION_PROFILE,
             IS_ORGANIZATION_OWNER
@@ -311,8 +316,9 @@ class OwnerRole(Role):
             CAN_VIEW_PROJECT_ALL, CAN_VIEW_PROJECT_DATA
         ],
         UserProfile: [
-            CAN_ADD_XFORM_TO_PROFILE, CAN_ADD_USERPROFILE,
-            CAN_CHANGE_USERPROFILE, CAN_DELETE_USERPROFILE, CAN_VIEW_PROFILE
+            CAN_ADD_PROJECT_TO_PROFILE, CAN_ADD_XFORM_TO_PROFILE,
+            CAN_ADD_USERPROFILE, CAN_CHANGE_USERPROFILE,
+            CAN_DELETE_USERPROFILE, CAN_VIEW_PROFILE
         ],
         XForm: [
             CAN_ADD_SUBMISSIONS, CAN_ADD_XFORM, CAN_CHANGE_XFORM,