Merge pull request #146 from vintasoftware/gracefull-fail-when-unregi…

…tering-not-registered-module

Gracefull fail when unregitering not registered module

docs/views_utils.rst

@@ -12,6 +12,8 @@ They are meant to be used on function based views.
 
 Accepts the same arguments as ``has_role`` function and raises PermissionDenied in case it returns ``False``.
 You can pass an optional key word argument ``redirect_to_login`` to overhide the ``ROLEPERMISSIONS_REDIRECT_TO_LOGIN`` setting.
+You can also pass an optional key word argument ``redirect_url`` to specify the view to return in case of permission denied. 
+``redirect_url`` takes precedence over ``redirect_to_login`` and ``ROLEPERMISSIONS_REDIRECT_TO_LOGIN``.
 
 .. code-block:: python
 
@@ -26,6 +28,8 @@ You can pass an optional key word argument ``redirect_to_login`` to overhide the
 
 Accepts the same arguments as ``has_permission`` function and raises PermissionDenied in case it returns ``False``.
 You can pass an optional key word argument ``redirect_to_login`` to overhide the ``ROLEPERMISSIONS_REDIRECT_TO_LOGIN`` setting.
+You can also pass an optional key word argument ``redirect_url`` to specify the view to return in case of permission denied. 
+``redirect_url`` takes precedence over ``redirect_to_login`` and ``ROLEPERMISSIONS_REDIRECT_TO_LOGIN``.
 
 .. code-block:: python
 
@@ -45,7 +49,8 @@ They are meant to be used on class based views.
 
 Add ``HasRoleMixin`` mixin to the desired CBV (class based view) and use the ``allowed_roles`` attribute to set the roles that can access the view.
 ``allowed_roles`` attribute will be passed to ``has_role`` function, and PermissionDenied will be raised in case it returns ``False``.
-You can set an optional ``redirect_to_login`` attribute to overhide the ``ROLEPERMISSIONS_REDIRECT_TO_LOGIN`` setting.
+You can set an optional ``redirect_to_login`` attribute to overhide the ``ROLEPERMISSIONS_REDIRECT_TO_LOGIN`` setting. Just like ``has_role_decorator`` set 
+an optional ``redirect_url`` to specify the view to return incase of permission denied. ``redirect_url`` overrides ``redirect_to_login`` and ``ROLEPERMISSIONS_REDIRECT_TO_LOGIN``.
 
 
 .. code-block:: python

rolepermissions/admin.py

@@ -2,6 +2,7 @@
 from django.contrib import admin, auth
 from django.contrib.auth.models import Group
 from django.contrib.auth.admin import UserAdmin
+from django.contrib.admin.sites import NotRegistered
 from rolepermissions import roles
 
 ROLEPERMISSIONS_REGISTER_ADMIN = getattr(settings, 'ROLEPERMISSIONS_REGISTER_ADMIN', False)
@@ -37,5 +38,8 @@ class RolePermissionsUserAdmin(RolePermissionsUserAdminMixin, UserAdmin):
 
 
 if ROLEPERMISSIONS_REGISTER_ADMIN:
-    admin.site.unregister(UserModel)
+    try:
+        admin.site.unregister(UserModel)
+    except NotRegistered:
+        pass
     admin.site.register(UserModel, RolePermissionsUserAdmin)

rolepermissions/decorators.py

@@ -5,20 +5,24 @@
 from django.conf import settings
 from django.contrib.auth.views import redirect_to_login as dj_redirect_to_login
 from django.core.exceptions import PermissionDenied
+from django.shortcuts import redirect as dj_redirect
 
 from rolepermissions.checkers import has_role, has_permission
 from rolepermissions.utils import user_is_authenticated
 
 
-def has_role_decorator(role, redirect_to_login=None):
+def _role_permission_checker(function, subject, redirect_to_login, redirect_url):
     def request_decorator(dispatch):
         @wraps(dispatch)
         def wrapper(request, *args, **kwargs):
             user = request.user
             if user_is_authenticated(user):
-                if has_role(user, role):
+                if function(user, subject):
                     return dispatch(request, *args, **kwargs)
 
+            if redirect_url:
+                return dj_redirect(redirect_url)
+
             redirect = redirect_to_login
             if redirect is None:
                 redirect = getattr(
@@ -30,21 +34,9 @@ def wrapper(request, *args, **kwargs):
     return request_decorator
 
 
-def has_permission_decorator(permission_name, redirect_to_login=None):
-    def request_decorator(dispatch):
-        @wraps(dispatch)
-        def wrapper(request, *args, **kwargs):
-            user = request.user
-            if user_is_authenticated(user):
-                if has_permission(user, permission_name):
-                    return dispatch(request, *args, **kwargs)
+def has_role_decorator(role, redirect_to_login=None, redirect_url=None):
+    return _role_permission_checker(has_role, role, redirect_to_login, redirect_url)
 
-            redirect = redirect_to_login
-            if redirect is None:
-                redirect = getattr(
-                    settings, 'ROLEPERMISSIONS_REDIRECT_TO_LOGIN', False)
-            if redirect:
-                return dj_redirect_to_login(request.get_full_path())
-            raise PermissionDenied
-        return wrapper
-    return request_decorator
+
+def has_permission_decorator(permission_name, redirect_to_login=None, redirect_url=None):
+    return _role_permission_checker(has_permission, permission_name, redirect_to_login, redirect_url)

tests/test_decorators.py

@@ -140,6 +140,32 @@ def render_to_response(self, context, **response_kwargs):
         return HttpResponse("Test")
 
 
+class PermissionOverhiddenRedirectViewRedirectUrl(DetailView):
+
+    @method_decorator(has_permission_decorator('permission2', redirect_url='/new_redirect'))
+    def dispatch(self, request, *args, **kwargs):
+        return super(PermissionOverhiddenRedirectViewRedirectUrl, self).dispatch(request, *args, **kwargs)
+
+    def get_object(self):
+        return True
+
+    def render_to_response(self, context, **response_kwargs):
+        return HttpResponse("Test")
+
+
+class RoleOverhiddenRedirectViewRedirectUrl(DetailView):
+
+    @method_decorator(has_role_decorator('permission2', redirect_url='/new_redirect'))
+    def dispatch(self, request, *args, **kwargs):
+        return super(RoleOverhiddenRedirectViewRedirectUrl, self).dispatch(request, *args, **kwargs)
+
+    def get_object(self):
+        return True
+
+    def render_to_response(self, context, **response_kwargs):
+        return HttpResponse("Test")
+
+
 class HasPermissionDecoratorTests(TestCase):
 
     def setUp(self):
@@ -174,7 +200,7 @@ def test_permission_denied(self):
 @override_settings(
     ROLEPERMISSIONS_REDIRECT_TO_LOGIN=True, LOGIN_URL='/login/',
     ROOT_URLCONF='tests.mock_urls')
-class RedirectToLoginTests(TestCase):
+class RedirectTests(TestCase):
 
     def setUp(self):
         self.user = mommy.make(get_user_model())
@@ -213,6 +239,20 @@ def test_role_overhiding_setting(self):
         with self.assertRaises(PermissionDenied):
             RoleOverhiddenRedirectView.as_view()(request)
 
+    def test_role_overhiding_new_redirect(self):
+        request = self.request
+
+        response = RoleOverhiddenRedirectViewRedirectUrl.as_view()(request)
+        self.assertEquals(response.status_code, 302)
+        self.assertIn('/new_redirect', response['Location'])
+
+    def test_permission_overhiding_new_redirect(self):
+        request = self.request
+
+        response = PermissionOverhiddenRedirectViewRedirectUrl.as_view()(request)
+        self.assertEquals(response.status_code, 302)
+        self.assertIn('/new_redirect', response['Location'])
+
 
 @override_settings(
     ROLEPERMISSIONS_REDIRECT_TO_LOGIN=False, LOGIN_URL='/login/',