summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorValentin Deniaud <vdeniaud@entrouvert.com>2019-10-23 09:50:40 (GMT)
committerValentin Deniaud <vdeniaud@entrouvert.com>2019-11-13 15:25:22 (GMT)
commit8560d251023e2ff353bc275e4c1e5d389fd8061b (patch)
tree86967219b66b340e1a272d1c6349daf9313f4f39
parent0f64603d98d141ba55dd05c5d9277a6e517da64f (diff)
downloadauthentic-wip/37159-manager-validation-incomplete-de.zip
authentic-wip/37159-manager-validation-incomplete-de.tar.gz
authentic-wip/37159-manager-validation-incomplete-de.tar.bz2
manager: add missing field validation (#37159)wip/37159-manager-validation-incomplete-de
Which makes widget validation useless, so we remove it.
-rw-r--r--src/authentic2/manager/forms.py35
-rw-r--r--src/authentic2/manager/role_views.py11
-rw-r--r--src/authentic2/manager/service_views.py3
-rw-r--r--src/authentic2/manager/user_views.py31
-rw-r--r--src/authentic2/manager/widgets.py46
-rw-r--r--tests/test_a2_rbac.py1
-rw-r--r--tests/test_manager.py95
7 files changed, 148 insertions, 74 deletions
diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py
index 851f1b0..2af8b85 100644
--- a/src/authentic2/manager/forms.py
+++ b/src/authentic2/manager/forms.py
@@ -104,18 +104,20 @@ class LimitQuerysetFormMixin(FormWithRequest):
and name in self.field_view_permissions:
perms = self.field_view_permissions[name]
else:
- app_label = qs.model._meta.app_label
- model_name = qs.model._meta.model_name
- perm = '%s.search_%s' % (app_label, model_name)
- qs = self.request.user.filter_by_perm(perm, qs)
+ perms = ('search',)
+ app_label = qs.model._meta.app_label
+ model_name = qs.model._meta.model_name
+ perms = ['%s.%s_%s' % (app_label, perm, model_name) for perm in perms]
+ qs = self.request.user.filter_by_perm(perms, qs)
field.queryset = qs
if not qs.exists():
# This should not happen, but could, so log it as error to find it later
- logger.error(u'user has no search permissions on model %s with roles %s',
- qs.model, list(self.request.user.roles_and_parents()))
+ logger.error(u'user does not have %s permissions on model %s with roles %s',
+ perms, qs.model, list(self.request.user.roles_and_parents()))
-class ChooseUserForm(CssClass, forms.Form):
+class ChooseUserForm(LimitQuerysetFormMixin, CssClass, forms.Form):
+
user = fields.ChooseUserField(label=_('Add an user'))
action = forms.CharField(initial='add', widget=forms.HiddenInput)
@@ -127,24 +129,31 @@ class ChooseUserForm(CssClass, forms.Form):
self.fields['user'].queryset = self.fields['user'].queryset.filter(ou=ou)
-class ChooseRoleForm(CssClass, forms.Form):
+class ChooseRoleForm(LimitQuerysetFormMixin, CssClass, forms.Form):
+
role = fields.ChooseRoleField(label=_('Add a role'))
action = forms.CharField(initial='add', widget=forms.HiddenInput)
-class UsersForm(CssClass, forms.Form):
+class UsersForm(LimitQuerysetFormMixin, CssClass, forms.Form):
+
users = fields.ChooseUsersField(label=_('Add some users'))
-class RolesForm(CssClass, forms.Form):
+class RolesForm(LimitQuerysetFormMixin, CssClass, forms.Form):
+
roles = fields.ChooseRolesField(label=_('Add some roles'))
-class RolesForChangeForm(CssClass, forms.Form):
- roles = fields.ChooseRolesForChangeField(label=_('Add some roles'))
+class RolesForChangeForm(LimitQuerysetFormMixin, CssClass, forms.Form):
+ field_view_permissions = {'roles': ['change']}
+
+ roles = fields.ChooseAllRolesField(label=_('Add some roles'))
+
+class ChooseUserRoleForm(LimitQuerysetFormMixin, CssClass, forms.Form):
+ field_view_permissions = {'role': ['change']}
-class ChooseUserRoleForm(CssClass, FormWithRequest, forms.Form):
role = fields.ChooseUserRoleField(label=_('Add a role'))
action = forms.CharField(initial='add', widget=forms.HiddenInput)
diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py
index 7df448a..659eea6 100644
--- a/src/authentic2/manager/role_views.py
+++ b/src/authentic2/manager/role_views.py
@@ -276,7 +276,8 @@ members_export = RoleMembersExportView.as_view()
class RoleAddChildView(views.AjaxFormViewMixin, views.TitleMixin,
- views.PermissionMixin, SingleObjectMixin, FormView):
+ views.PermissionMixin, views.RequestContextData,
+ SingleObjectMixin, FormView):
title = _('Add child role')
model = get_role_model()
form_class = forms.RolesForm
@@ -300,7 +301,7 @@ add_child = RoleAddChildView.as_view()
class RoleAddParentView(views.AjaxFormViewMixin, views.TitleMixin,
- SingleObjectMixin, FormView):
+ views.RequestContextData, SingleObjectMixin, FormView):
title = _('Add parent role')
model = get_role_model()
form_class = forms.RolesForChangeForm
@@ -382,7 +383,8 @@ remove_parent = RoleRemoveParentView.as_view()
class RoleAddAdminRoleView(views.AjaxFormViewMixin, views.TitleMixin,
- views.PermissionMixin, SingleObjectMixin, FormView):
+ views.PermissionMixin, views.RequestContextData,
+ SingleObjectMixin, FormView):
title = _('Add admin role')
model = get_role_model()
form_class = forms.RolesForm
@@ -434,7 +436,8 @@ remove_admin_role = RoleRemoveAdminRoleView.as_view()
class RoleAddAdminUserView(views.AjaxFormViewMixin, views.TitleMixin,
- views.PermissionMixin, SingleObjectMixin, FormView):
+ views.PermissionMixin, views.RequestContextData,
+ SingleObjectMixin, FormView):
title = _('Add admin user')
model = get_role_model()
form_class = forms.UsersForm
diff --git a/src/authentic2/manager/service_views.py b/src/authentic2/manager/service_views.py
index 501f0e5..2edaf72 100644
--- a/src/authentic2/manager/service_views.py
+++ b/src/authentic2/manager/service_views.py
@@ -34,7 +34,8 @@ class ServicesView(views.HideOUColumnMixin, views.BaseTableView):
listing = ServicesView.as_view()
-class ServiceView(views.SimpleSubTableView, role_views.RoleViewMixin, views.MediaMixin, views.FormView):
+class ServiceView(views.SimpleSubTableView, role_views.RoleViewMixin,
+ views.MediaMixin, views.RequestContextData, views.FormView):
search_form_class = forms.NameSearchForm
model = Service
pk_url_kwarg = 'service_pk'
diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index 5faebe9..e12f71e 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -578,23 +578,20 @@ class UserRolesView(HideOUColumnMixin, BaseSubTableView):
user = self.object
role = form.cleaned_data['role']
action = form.cleaned_data['action']
- if self.request.user.has_perm('a2_rbac.change_role', role):
- if action == 'add':
- if user.roles.filter(pk=role.pk):
- messages.warning(
- self.request,
- _('User {user} has already the role {role}.')
- .format(user=user, role=role))
- else:
- user.roles.add(role)
- hooks.call_hooks('event', name='manager-add-role-member',
- user=self.request.user, role=role, member=user)
- elif action == 'remove':
- user.roles.remove(role)
- hooks.call_hooks('event', name='manager-remove-role-member', user=self.request.user,
- role=role, member=user)
- else:
- messages.warning(self.request, _('You are not authorized'))
+ if action == 'add':
+ if user.roles.filter(pk=role.pk):
+ messages.warning(
+ self.request,
+ _('User {user} has already the role {role}.')
+ .format(user=user, role=role))
+ else:
+ user.roles.add(role)
+ hooks.call_hooks('event', name='manager-add-role-member',
+ user=self.request.user, role=role, member=user)
+ elif action == 'remove':
+ user.roles.remove(role)
+ hooks.call_hooks('event', name='manager-remove-role-member', user=self.request.user,
+ role=role, member=user)
return super(UserRolesView, self).form_valid(form)
def get_search_form_kwargs(self):
diff --git a/src/authentic2/manager/widgets.py b/src/authentic2/manager/widgets.py
index d456e7d..23512bc 100644
--- a/src/authentic2/manager/widgets.py
+++ b/src/authentic2/manager/widgets.py
@@ -21,7 +21,6 @@ from django_select2.forms import ModelSelect2Widget, ModelSelect2MultipleWidget
from django.contrib.auth import get_user_model
from django.utils import six
-from django_rbac.backends import DjangoRBACBackend
from django_rbac.utils import get_role_model, get_ou_model
from authentic2.models import Service
@@ -46,31 +45,6 @@ class SplitTermMixin(object):
return qs
-class SecurityCheckMixin(SplitTermMixin):
- operations = ['change', 'add', 'view', 'delete']
-
- @property
- def perms(self):
- model = self.queryset.model
- app_label = model._meta.app_label
- model_name = model._meta.model_name
- return ['%s.%s_%s' % (app_label, perm, model_name)
- for perm in self.operations]
-
- def security_check(self, request, *args, **kwargs):
- return request.user.is_authenticated() \
- and request.user.has_perm_any(self.perms)
-
- def filter_queryset(self, term, queryset=None):
- '''Only search visible objects'''
- if not hasattr(self, 'view'):
- return []
- request = self.view.request
- qs = super(SecurityCheckMixin, self).filter_queryset(term, queryset=queryset)
- rbac_backend = DjangoRBACBackend()
- return rbac_backend.filter_by_perm(request.user, self.perms, qs)
-
-
class RoleLabelMixin(object):
def label_from_instance(self, obj):
label = six.text_type(obj)
@@ -80,7 +54,7 @@ class RoleLabelMixin(object):
return label
-class ChooseUserWidget(SecurityCheckMixin, ModelSelect2Widget):
+class ChooseUserWidget(SplitTermMixin, ModelSelect2Widget):
model = get_user_model()
search_fields = [
'username__icontains', 'first_name__icontains',
@@ -91,7 +65,7 @@ class ChooseUserWidget(SecurityCheckMixin, ModelSelect2Widget):
return utils.label_from_user(user)
-class ChooseUsersWidget(SecurityCheckMixin, ModelSelect2MultipleWidget):
+class ChooseUsersWidget(SplitTermMixin, ModelSelect2MultipleWidget):
model = get_user_model()
search_fields = [
'username__icontains', 'first_name__icontains',
@@ -102,7 +76,7 @@ class ChooseUsersWidget(SecurityCheckMixin, ModelSelect2MultipleWidget):
return utils.label_from_user(user)
-class ChooseRoleWidget(RoleLabelMixin, SecurityCheckMixin, ModelSelect2Widget):
+class ChooseRoleWidget(RoleLabelMixin, SplitTermMixin, ModelSelect2Widget):
queryset = get_role_model().objects.exclude(slug__startswith='_')
split_term_operator = operator.__and__
search_fields = [
@@ -112,7 +86,7 @@ class ChooseRoleWidget(RoleLabelMixin, SecurityCheckMixin, ModelSelect2Widget):
]
-class ChooseRolesWidget(RoleLabelMixin, SecurityCheckMixin, ModelSelect2MultipleWidget):
+class ChooseRolesWidget(RoleLabelMixin, SplitTermMixin, ModelSelect2MultipleWidget):
queryset = get_role_model().objects.exclude(slug__startswith='_')
split_term_operator = operator.__and__
search_fields = [
@@ -122,9 +96,8 @@ class ChooseRolesWidget(RoleLabelMixin, SecurityCheckMixin, ModelSelect2Multiple
]
-class ChooseRolesForChangeWidget(RoleLabelMixin, SecurityCheckMixin, ModelSelect2MultipleWidget):
- operations = ['change']
- queryset = get_role_model().objects.all()
+class ChooseAllRolesWidget(RoleLabelMixin, SplitTermMixin, ModelSelect2MultipleWidget):
+ model = get_role_model()
split_term_operator = 'AND'
search_fields = [
'name__icontains',
@@ -133,7 +106,7 @@ class ChooseRolesForChangeWidget(RoleLabelMixin, SecurityCheckMixin, ModelSelect
]
-class ChooseOUWidget(SecurityCheckMixin, ModelSelect2Widget):
+class ChooseOUWidget(SplitTermMixin, ModelSelect2Widget):
model = get_ou_model()
split_term_operator = 'AND'
search_fields = [
@@ -141,15 +114,14 @@ class ChooseOUWidget(SecurityCheckMixin, ModelSelect2Widget):
]
-class ChooseServiceWidget(SecurityCheckMixin, ModelSelect2Widget):
+class ChooseServiceWidget(SplitTermMixin, ModelSelect2Widget):
model = Service
search_fields = [
'name__icontains',
]
-class ChooseUserRoleWidget(RoleLabelMixin, SecurityCheckMixin, ModelSelect2Widget):
- operations = ['change']
+class ChooseUserRoleWidget(RoleLabelMixin, SplitTermMixin, ModelSelect2Widget):
model = get_role_model()
search_fields = [
'name__icontains',
diff --git a/tests/test_a2_rbac.py b/tests/test_a2_rbac.py
index cb7756c..b8549e5 100644
--- a/tests/test_a2_rbac.py
+++ b/tests/test_a2_rbac.py
@@ -420,6 +420,7 @@ def test_admin_role_user_view(settings, app, admin, simple_user, ou1, user_ou1,
# with A2_RBAC_ROLE_ADMIN_RESTRICT_TO_OU_USERS after a reload of the admin
# page, we should only see user from the same OU as the role
settings.A2_RBAC_ROLE_ADMIN_RESTRICT_TO_OU_USERS = True
+ role_ou1.get_admin_role()
response = app.get('/manage/roles/')
response = response.click('role_ou1')
select2_url = response.pyquery('select#id_user')[0].attrib['data-ajax--url']
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 518b289..586fc7c 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -25,8 +25,11 @@ from webtest import Upload
from authentic2.a2_rbac.utils import get_default_ou
-from django_rbac.utils import get_ou_model, get_role_model
+from django_rbac.models import VIEW_OP, CHANGE_OP
+from django_rbac.utils import (get_ou_model, get_role_model,
+ get_permission_model, get_operation)
from django.contrib.auth import get_user_model
+from django.contrib.contenttypes.models import ContentType
from django.utils.six.moves.urllib.parse import urlparse
from utils import login, get_link_from_mail
@@ -831,7 +834,7 @@ def test_roles_widget(admin, app, db):
Role.objects.create(ou=la_bedoule, name=u'Administrateur')
Role.objects.create(ou=cuges, name=u'Administrateur')
- form = ChooseRoleForm()
+ form = ChooseRoleForm(request=None)
assert form.as_p()
field_id = signing.dumps(id(form.fields['role'].widget))
url = reverse('django_select2-json')
@@ -896,3 +899,91 @@ def test_manager_edit_user_email_verified(app, simple_user, superuser_or_admin):
user = User.objects.get(id=simple_user.id)
assert not user.email_verified
+
+
+def test_manager_permission_inheritance(app, simple_user, admin, simple_role):
+ admin_role = Role.objects.get(slug='_a2-manager')
+ view_role_perm = get_permission_model().objects.create(
+ operation=get_operation(VIEW_OP), ou=simple_role.ou,
+ target_ct=ContentType.objects.get_for_model(Role),
+ target_id=simple_role.pk)
+ simple_role.permissions.add(view_role_perm)
+ simple_user.roles.add(simple_role)
+ login(app, simple_user, '/manage/')
+
+ response = app.get('/manage/roles/%s/add-parent/' % simple_role.pk)
+ form = response.form
+ form['roles'].force_value(admin_role.pk)
+ response = form.submit()
+
+ assert response.status_code == 200
+ assert not admin_role in simple_role.parents()
+
+
+def test_widget_fields_validation(app, simple_user, simple_role):
+ '''Verify that fields corresponding to widget implement queryset restrictions.'''
+ from authentic2.manager.forms import (ChooseUserForm, ChooseRoleForm,
+ UsersForm, RolesForm, ChooseUserRoleForm,
+ RolesForChangeForm)
+ error_message = 'Select a valid choice'
+
+ class DummyRequest:
+ user = simple_user
+ request = DummyRequest()
+
+ visible_role = Role.objects.create(name='visible_role', ou=simple_user.ou)
+ visible_user = User.objects.create(username='visible_user', ou=simple_user.ou)
+ forbidden_role = Role.objects.create(name='forbidden_role', ou=simple_user.ou)
+ forbidden_user = User.objects.create(username='forbidden_user', ou=simple_user.ou)
+
+ view_role_perm = get_permission_model().objects.create(
+ operation=get_operation(VIEW_OP), ou=visible_role.ou,
+ target_ct=ContentType.objects.get_for_model(Role),
+ target_id=visible_role.pk)
+ view_user_perm = get_permission_model().objects.create(
+ operation=get_operation(VIEW_OP), ou=visible_user.ou,
+ target_ct=ContentType.objects.get_for_model(User),
+ target_id=visible_user.pk)
+ simple_role.permissions.add(view_role_perm)
+ simple_role.permissions.add(view_user_perm)
+ simple_user.roles.add(simple_role)
+
+ form = ChooseUserForm(request=request, data={'user': visible_user.pk, 'action': 'add'})
+ assert form.is_valid()
+ form = ChooseUserForm(request=request, data={'user': forbidden_user.pk, 'action': 'add'})
+ assert error_message in form.errors['user'][0]
+
+ form = ChooseRoleForm(request=request, data={'role': visible_role.pk, 'action': 'add'})
+ assert form.is_valid()
+ form = ChooseRoleForm(request=request, data={'role': forbidden_role.pk, 'action': 'add'})
+ assert error_message in form.errors['role'][0]
+
+ form = UsersForm(request=request, data={'users': [visible_user.pk]})
+ assert form.is_valid()
+ form = UsersForm(request=request, data={'users': [forbidden_user.pk]})
+ assert error_message in form.errors['users'][0]
+
+ form = RolesForm(request=request, data={'roles': [visible_role.pk]})
+ assert form.is_valid()
+ form = RolesForm(request=request, data={'roles': [forbidden_role.pk]})
+ assert error_message in form.errors['roles'][0]
+
+ # For those we need change permission
+ form = RolesForChangeForm(request=request, data={'roles': [visible_role.pk]})
+ assert error_message in form.errors['roles'][0]
+
+ form = ChooseUserRoleForm(request=request, data={'role': visible_role.pk, 'action': 'add'})
+ assert error_message in form.errors['role'][0]
+
+ change_role_perm = get_permission_model().objects.create(
+ operation=get_operation(CHANGE_OP), ou=visible_role.ou,
+ target_ct=ContentType.objects.get_for_model(Role),
+ target_id=visible_role.pk)
+ simple_role.permissions.add(change_role_perm)
+ del simple_user._rbac_perms_cache
+
+ form = RolesForChangeForm(request=request, data={'roles': [visible_role.pk]})
+ assert form.is_valid()
+
+ form = ChooseUserRoleForm(request=request, data={'role': visible_role.pk, 'action': 'add'})
+ assert form.is_valid()