From 24cd4938a5da26938b67eec52196eba67d34915a Mon Sep 17 00:00:00 2001
From: David Ignacio <deignacio@gmail.com>
Date: Fri, 22 Jun 2012 00:15:43 -0500
Subject: [PATCH] correct roles_* decorator signature expectations

Having multiple RoleNeed objects in a Permission does not require
all to be satisfied in order to .can(), but will return True if
any are present.  This makes the previous roles_required logic more
elegant for roles_accepted.  roles_required decorator needs to check
all permissions individually and return only if all permissions exist
---
 example/app.py               |  6 ++++++
 flask_security/decorators.py | 21 ++++++++++-----------
 tests/functional_tests.py    | 10 ++++++++++
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/example/app.py b/example/app.py
index ee2e38e..e0beb90 100644
--- a/example/app.py
+++ b/example/app.py
@@ -28,6 +28,7 @@ def create_roles():
 def create_users():
     for u in  (('matt@lp.com', 'password', ['admin'], True),
                ('joe@lp.com', 'password', ['editor'], True),
+               ('dave@lp.com', 'password', ['admin', 'editor'], True),
                ('jill@lp.com', 'password', ['author'], True),
                ('tiya@lp.com', 'password', [], False)):
         current_app.security.datastore.create_user(
@@ -96,6 +97,11 @@ def create_app(auth_config):
     def admin():
         return render_template('index.html', content='Admin Page')
 
+    @app.route('/admin_and_editor')
+    @roles_required('admin', 'editor')
+    def admin_and_editor():
+        return render_template('index.html', content='Admin and Editor Page')
+
     @app.route('/admin_or_editor')
     @roles_accepted('admin', 'editor')
     def admin_or_editor():
diff --git a/flask_security/decorators.py b/flask_security/decorators.py
index 63ef946..8a9b618 100644
--- a/flask_security/decorators.py
+++ b/flask_security/decorators.py
@@ -93,7 +93,7 @@ def roles_required(*roles):
 
     :param args: The required roles.
     """
-    perm = Permission(*[RoleNeed(role) for role in roles])
+    perms = [Permission(RoleNeed(role)) for role in roles]
 
     def wrapper(fn):
         @wraps(fn)
@@ -102,12 +102,12 @@ def roles_required(*roles):
                 login_view = app.security.login_manager.login_view
                 return redirect(login_url(login_view, request.url))
 
-            if perm.can():
-                return fn(*args, **kwargs)
-
-            app.logger.debug('Identity does not provide the '
-                                     'roles: %s' % [r for r in roles])
-            return redirect(request.referrer or '/')
+            for perm in perms:
+                if not perm.can():
+                    app.logger.debug('Identity does not provide the '
+                                             'roles: %s' % [r for r in roles])
+                    return redirect(request.referrer or '/')
+            return fn(*args, **kwargs)
         return decorated_view
     return wrapper
 
@@ -126,7 +126,7 @@ def roles_accepted(*roles):
 
     :param args: The possible roles.
     """
-    perms = [Permission(RoleNeed(role)) for role in roles]
+    perm = Permission(*[RoleNeed(role) for role in roles])
 
     def wrapper(fn):
         @wraps(fn)
@@ -135,9 +135,8 @@ def roles_accepted(*roles):
                 login_view = app.security.login_manager.login_view
                 return redirect(login_url(login_view, request.url))
 
-            for perm in perms:
-                if perm.can():
-                    return fn(*args, **kwargs)
+            if perm.can():
+                return fn(*args, **kwargs)
 
             r1 = [r for r in roles]
             r2 = [r.name for r in current_user.roles]
diff --git a/tests/functional_tests.py b/tests/functional_tests.py
index b3100ab..c824cfc 100644
--- a/tests/functional_tests.py
+++ b/tests/functional_tests.py
@@ -84,6 +84,16 @@ class DefaultSecurityTests(SecurityTest):
         r = self._get('/admin', follow_redirects=True)
         self.assertIn('<input id="next"', r.data)
 
+    def test_multiple_role_required(self):
+        for user in ("matt@lp.com", "joe@lp.com"):
+            self.authenticate(user)
+            r = self._get("/admin_and_editor", follow_redirects=True)
+            self.assertIsHomePage(r.data)
+
+        self.authenticate('dave@lp.com')
+        r = self._get("/admin_and_editor")
+        self.assertIn('Admin and Editor Page', r.data)
+
     def test_token_auth_via_querystring_valid_token(self):
         r = self._get('/token?auth_token=123abc')
         self.assertIn('Token Authentication', r.data)