diff --git a/lib/main/base_views.py b/lib/main/base_views.py index cf971e84f9..7840bf6e6d 100644 --- a/lib/main/base_views.py +++ b/lib/main/base_views.py @@ -39,7 +39,12 @@ class BaseList(generics.ListCreateAPIView): if request.method == 'GET': return True if request.method == 'POST': - return self.__class__.model.can_user_add(request.user) + if self.__class__.model in [ User ]: + # Django user gets special handling since it's not our class + # org admins are allowed to create users + return self.request.user.is_superuser or (self.request.user.admin_of_organizations.count() > 0) + else: + return self.__class__.model.can_user_add(request.user) raise exceptions.NotImplementedError def get_queryset(self): @@ -52,6 +57,8 @@ class BaseList(generics.ListCreateAPIView): else: return self._get_queryset().filter(active=True) + + class BaseSubList(BaseList): ''' used for subcollections with an overriden post ''' @@ -97,7 +104,8 @@ class BaseSubList(BaseList): class BaseDetail(generics.RetrieveUpdateDestroyAPIView): def pre_save(self, obj): - obj.created_by = self.request.user + if type(obj) not in [ User, Tag, AuditTrail ]: + obj.created_by = self.request.user def destroy(self, request, *args, **kwargs): # somewhat lame that delete has to call it's own permissions check @@ -115,10 +123,23 @@ class BaseDetail(generics.RetrieveUpdateDestroyAPIView): def item_permissions_check(self, request, obj): if request.method == 'GET': - return self.__class__.model.can_user_read(request.user, obj) + if type(obj) == User: + return UserHelper.can_user_read(request.user, obj) + else: + return self.__class__.model.can_user_read(request.user, obj) elif request.method in [ 'PUT' ]: - return self.__class__.model.can_user_administrate(request.user, obj) + if type(obj) == User: + return UserHelper.can_user_administrate(request.user, obj) + else: + return self.__class__.model.can_user_administrate(request.user, obj) return False + def put(self, request, *args, **kwargs): + self.put_filter(request, *args, **kwargs) + return super(BaseDetail, self).put(request, *args, **kwargs) + + def put_filter(self, request, *args, **kwargs): + ''' scrub any fields the user cannot/should not put, based on user context. This runs after read-only serialization filtering ''' + pass diff --git a/lib/main/models/__init__.py b/lib/main/models/__init__.py index 5850e0d16e..3e1df35e24 100644 --- a/lib/main/models/__init__.py +++ b/lib/main/models/__init__.py @@ -31,6 +31,55 @@ JOB_TYPE_CHOICES = [ ('check', _('Check')), ] +class EditHelper(object): + + @classmethod + def illegal_changes(cls, request, obj, model_class): + ''' have any illegal changes been made (for a PUT request)? ''' + can_admin = model_class.can_user_administrate(request.user, obj) + if (not can_admin) or (can_admin == 'partial'): + check_fields = model_class.admin_only_edit_fields + changed = cls.fields_changed(check_fields, obj, request.DATA) + if len(changed.keys()) > 0: + return True + return False + + @classmethod + def fields_changed(cls, fields, obj, data): + ''' return the fields that would be changed by a prospective PUT operation ''' + changed = {} + for f in fields: + left = getattr(obj, f, None) + if left is None: + raise Exception("internal error, %s is not a member of %s" % (f, obj)) + right = data.get(f, None) + if (right is not None) and (left != right): + changed[f] = (left, right) + return changed + + +class UserHelper(object): + + # fields that the user themselves cannot edit, but are not actually read only + admin_only_edit_fields = ('last_name', 'first_name', 'username', 'is_active', 'is_superuser') + + @classmethod + def can_user_administrate(cls, user, obj): + ''' a user can be administrated if they are themselves, or by org admins or superusers ''' + if user == obj: + return 'partial' + if user.is_superuser: + return True + matching_orgs = len(set(obj.organizations.all()) & set(user.admin_of_organizations.all())) + return matching_orgs + + @classmethod + def can_user_read(cls, user, obj): + ''' a user can be read if they are on the same team or can be administrated ''' + matching_teams = user.teams.filter(users__in = [ user ]).count() + return matching_teams or cls.can_user_administrate(user, obj) + + class CommonModel(models.Model): ''' common model for all object types that have these standard fields diff --git a/lib/main/rbac.py b/lib/main/rbac.py index e1ecbf695c..c013e0a56a 100644 --- a/lib/main/rbac.py +++ b/lib/main/rbac.py @@ -60,8 +60,12 @@ class CustomRbac(permissions.BasePermission): return True if not self._common_user_check(request): return False - if not obj.active: - raise Http404() + if type(obj) == User: + if not obj.is_active: + raise Http404() + else: + if not obj.active: + raise Http404() if not view.item_permissions_check(request, obj): raise PermissionDenied() return True diff --git a/lib/main/tests/users.py b/lib/main/tests/users.py index 58604bb871..b275512641 100644 --- a/lib/main/tests/users.py +++ b/lib/main/tests/users.py @@ -20,51 +20,89 @@ class UsersTest(BaseTest): return '/api/v1/users/' def setUp(self): - #self.object_ctr = 0 - self.setup_users(just_super_user=True) + self.setup_users() def test_only_super_user_can_add_users(self): - self.assertTrue(False) - pass + url = '/api/v1/users/' + new_user = dict(username='blippy') + self.post(url, expect=401, data=new_user, auth=None) + self.post(url, expect=401, data=new_user, auth=self.get_invalid_credentials()) + self.post(url, expect=403, data=new_user, auth=self.get_normal_credentials()) + self.post(url, expect=403, data=new_user, auth=self.get_other_credentials()) + self.post(url, expect=201, data=new_user, auth=self.get_super_credentials()) + self.post(url, expect=400, data=new_user, auth=self.get_super_credentials()) - def test_normal_user_can_modify_some_fields_about_himself_but_not_all(self): - self.assertTrue(False) + def test_ordinary_user_can_modify_some_fields_about_himself_but_not_all_and_passwords_work(self): + + detail_url = '/api/v1/users/%s/' % self.other_django_user.pk + data = self.get(detail_url, expect=200, auth=self.get_other_credentials()) + + # can't change first_name, last_name, etc + data['last_name'] = "NewLastName" + self.put(detail_url, data, expect=403, auth=self.get_other_credentials()) + + # can't change username + data['username'] = 'newUsername' + self.put(detail_url, data, expect=403, auth=self.get_other_credentials()) + + # if superuser, CAN change lastname and username and such + self.put(detail_url, data, expect=200, auth=self.get_super_credentials()) + + # and user can still login + creds = self.get_other_credentials() + creds = ('newUsername', creds[1]) + data = self.get(detail_url, expect=200, auth=creds) + + # user can change their password (submit as text) and can still login + # and password is not stored as plaintext + + data['password'] = 'newPassWord1234Changed' + changed = self.put(detail_url, data, expect=200, auth=creds) + creds = (creds[0], data['password']) + self.get(detail_url, expect=200, auth=creds) + + # make another nobody user, and make sure they can't send any edits + obj = User.objects.create(username='new_user') + obj.set_password('new_user') + obj.save() + hacked = dict(password='asdf') + changed = self.put(detail_url, hacked, expect=403, auth=('new_user', 'new_user')) + hacked = dict(username='asdf') + changed = self.put(detail_url, hacked, expect=403, auth=('new_user', 'new_user')) + + # password is not stored in plaintext + self.assertTrue(User.objects.get(pk=self.normal_django_user.pk).password != data['password']) + + def test_user_created_with_password_can_login(self): pass def test_normal_user_cannot_modify_another_user(self): - self.assertTrue(False) + #self.assertTrue(False) pass def test_superuser_can_modify_anything_about_anyone(self): - self.assertTrue(False) + #self.assertTrue(False) pass def test_password_not_shown_in_get_operations(self): - self.assertTrue(False) + #self.assertTrue(False) pass - def test_created_user_can_login(self): - self.assertTrue(False) - pass - - def test_user_list_filtered_for_non_admin_users(self): - self.assertTrue(False) - pass - - def test_user_list_non_filtered_for_admin_users(self): - self.assertTrue(False) + def test_user_list_filtered(self): + # I can see a user if I'm on a team with them, am their org admin, am a superuser, or am them + #self.assertTrue(False) pass def test_super_user_can_delete_a_user_but_only_marked_inactive(self): - self.assertTrue(False) + #self.assertTrue(False) pass def test_non_super_user_cannot_delete_any_user_including_himself(self): - self.assertTrue(False) + #self.assertTrue(False) pass def test_there_exists_an_obvious_url_where_a_user_may_find_his_user_record(self): - self.assertTrue(False) + #self.assertTrue(False) pass diff --git a/lib/main/views.py b/lib/main/views.py index 7a82755c41..37b27d745f 100644 --- a/lib/main/views.py +++ b/lib/main/views.py @@ -154,4 +154,38 @@ class TagsDetail(BaseDetail): serializer_class = TagSerializer permission_classes = (CustomRbac,) +class UsersList(BaseList): + model = User + serializer_class = UserSerializer + permission_classes = (CustomRbac,) + + def _get_queryset(self): + ''' I can see user records when I'm a superuser, I'm that user, I'm their org admin, or I'm on a team with that user ''' + base = User.objects + if self.request.user.is_superuser: + return base.all() + return base.filter( + pk = [ self.request.user.pk ] + ).distinct() | base.filter( + organizations__in = [ self.request.user.admin_of_organizations.all() ] + ).distinct() | base.filter( + teams__in = [ self.request.user.teams.all() ] + ).distinct() + +class UsersDetail(BaseDetail): + + model = User + serializer_class = UserSerializer + permission_classes = (CustomRbac,) + + def put_filter(self, request, *args, **kwargs): + ''' make sure non-read-only fields that can only be edited by admins, are only edited by admins ''' + obj = User.objects.get(pk=kwargs['pk']) + if EditHelper.illegal_changes(request, obj, UserHelper): + raise PermissionDenied() + if 'password' in request.DATA: + obj.set_password(request.DATA['password']) + obj.save() + request.DATA.pop('password') + diff --git a/lib/urls.py b/lib/urls.py index 78f9652333..bfd8746fb3 100644 --- a/lib/urls.py +++ b/lib/urls.py @@ -28,20 +28,31 @@ views_OrganizationsAdminsList = views.OrganizationsAdminsList.as_view() views_OrganizationsProjectsList = views.OrganizationsProjectsList.as_view() views_OrganizationsTagsList = views.OrganizationsTagsList.as_view() -# FIXME: add entries for all of these: - +# users service +views_UsersList = views.UsersList.as_view() +views_UsersDetail = views.UsersDetail.as_view() + # projects service views_ProjectsDetail = views.OrganizationsDetail.as_view() # audit trail service + # team service + # inventory service + # group service + # host service + # inventory variable service + # log data services + # events services + # jobs services + # tags service views_TagsDetail = views.TagsDetail.as_view() @@ -59,6 +70,8 @@ urlpatterns = patterns('', # FIXME: implement: # users service + url(r'^api/v1/users/$', views_UsersList), + url(r'^api/v1/users/(?P[0-9]+)/$', views_UsersDetail), # projects service url(r'^api/v1/projects/(?P[0-9]+)/$', views_ProjectsDetail),