Fixes an issuer where a member of a role is not able to add new users to said role. (#445)

lemur/auth/permissions.py

@@ -36,14 +36,14 @@ class CertificatePermission(Permission):
         super(CertificatePermission, self).__init__(*needs)
 
 
-RoleUser = namedtuple('role', ['method', 'value'])
-ViewRoleCredentialsNeed = partial(RoleUser, 'roleView')
+RoleMember = namedtuple('role', ['method', 'value'])
+RoleMemberNeed = partial(RoleMember, 'member')
 
 
-class ViewRoleCredentialsPermission(Permission):
+class RoleMemberPermission(Permission):
     def __init__(self, role_id):
-        need = ViewRoleCredentialsNeed(role_id)
-        super(ViewRoleCredentialsPermission, self).__init__(need, RoleNeed('admin'))
+        needs = [RoleNeed('admin'), RoleMemberNeed(role_id)]
+        super(RoleMemberPermission, self).__init__(*needs)
 
 
 AuthorityCreator = namedtuple('authority', ['method', 'value'])

lemur/auth/service.py

@@ -29,7 +29,7 @@ from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicNumbers
 
 from lemur.users import service as user_service
 from lemur.auth.permissions import CertificateCreatorNeed, \
-    AuthorityCreatorNeed, ViewRoleCredentialsNeed
+    AuthorityCreatorNeed, RoleMemberNeed
 
 
 def get_rsa_public_key(n, e):
@@ -155,8 +155,8 @@ def on_identity_loaded(sender, identity):
     # identity with the roles that the user provides
     if hasattr(user, 'roles'):
         for role in user.roles:
-            identity.provides.add(ViewRoleCredentialsNeed(role.name))
             identity.provides.add(RoleNeed(role.name))
+            identity.provides.add(RoleMemberNeed(role.id))
 
     # apply ownership for authorities
     if hasattr(user, 'authorities'):

lemur/certificates/views.py

@@ -880,7 +880,7 @@ class CertificateExport(AuthenticatedResource):
                 if permission.can():
                     extension, passphrase, data = plugin.export(cert.body, cert.chain, cert.private_key, options)
                 else:
-                    return dict(message='You are not authorized to export this certificate'), 403
+                    return dict(message='You are not authorized to export this certificate.'), 403
             else:
                 return dict(message='Unable to export certificate, plugin: {0} requires a private key but no key was found.'.format(plugin.slug))
         else:

lemur/roles/views.py

@@ -8,12 +8,12 @@
 
 """
 from flask import Blueprint
-from flask import make_response, jsonify, abort, g
+from flask import make_response, jsonify
 from flask.ext.restful import reqparse, Api
 
 from lemur.roles import service
 from lemur.auth.service import AuthenticatedResource
-from lemur.auth.permissions import ViewRoleCredentialsPermission, admin_permission
+from lemur.auth.permissions import RoleMemberPermission, admin_permission
 from lemur.common.utils import paginated_parser
 
 from lemur.common.schema import validate_schema
@@ -171,14 +171,14 @@ class RoleViewCredentials(AuthenticatedResource):
            :statuscode 200: no error
            :statuscode 403: unauthenticated
         """
-        permission = ViewRoleCredentialsPermission(role_id)
+        permission = RoleMemberPermission(role_id)
         if permission.can():
             role = service.get(role_id)
             response = make_response(jsonify(username=role.username, password=role.password), 200)
             response.headers['cache-control'] = 'private, max-age=0, no-cache, no-store'
             response.headers['pragma'] = 'no-cache'
             return response
-        abort(403)
+        return dict(message='You are not authorized to view the credentials for this role.'), 403
 
 
 class Roles(AuthenticatedResource):
@@ -220,12 +220,11 @@ class Roles(AuthenticatedResource):
            :statuscode 403: unauthenticated
         """
         # we want to make sure that we cannot view roles that we are not members of
-        if not g.current_user.is_admin:
-            user_role_ids = set([r.id for r in g.current_user.roles])
-            if role_id not in user_role_ids:
-                return dict(message="You are not allowed to view a role which you are not a member of"), 403
+        permission = RoleMemberPermission(role_id)
+        if permission.can():
+            return service.get(role_id)
 
-        return service.get(role_id)
+        return dict(message="You are not allowed to view a role which you are not a member of."), 403
 
     @validate_schema(role_input_schema, role_output_schema)
     def put(self, role_id, data=None):
@@ -265,10 +264,10 @@ class Roles(AuthenticatedResource):
            :statuscode 200: no error
            :statuscode 403: unauthenticated
         """
-        permission = ViewRoleCredentialsPermission(role_id)
+        permission = RoleMemberPermission(role_id)
         if permission.can():
             return service.update(role_id, data['name'], data.get('description'), data.get('users'))
-        abort(403)
+        return dict(message='You are not authorized to modify this role.'), 403
 
     @admin_permission.require(http_exception=403)
     def delete(self, role_id):

lemur/tests/test_roles.py

@@ -1,7 +1,8 @@
+import json
 import pytest
 
 from lemur.roles.views import *  # noqa
-from lemur.tests.factories import RoleFactory, AuthorityFactory, CertificateFactory
+from lemur.tests.factories import RoleFactory, AuthorityFactory, CertificateFactory, UserFactory
 
 
 from .vectors import VALID_ADMIN_HEADER_TOKEN, VALID_USER_HEADER_TOKEN
@@ -65,6 +66,50 @@ def test_role_put(client, token, status):
     assert client.put(api.url_for(Roles, role_id=1), data={}, headers=token).status_code == status
 
 
+@pytest.mark.parametrize("token,status", [
+    (VALID_USER_HEADER_TOKEN, 403),
+    (VALID_ADMIN_HEADER_TOKEN, 200),
+    ('', 401)
+])
+def test_role_put_with_data(client, session, token, status):
+    user = UserFactory()
+    role = RoleFactory()
+    session.commit()
+
+    data = {
+        'users': [
+            {'id': user.id}
+        ],
+        'id': role.id,
+        'name': role.name
+    }
+
+    assert client.put(api.url_for(Roles, role_id=role.id), data=json.dumps(data), headers=token).status_code == status
+
+
+def test_role_put_with_data_and_user(client, session):
+    from lemur.auth.service import create_token
+    user = UserFactory()
+    role = RoleFactory(users=[user])
+    user1 = UserFactory()
+    session.commit()
+
+    headers = {
+        'Authorization': 'Basic ' + create_token(user),
+        'Content-Type': 'application/json'
+    }
+
+    data = {
+        'users': [
+            {'id': user1.id}
+        ],
+        'id': role.id,
+        'name': role.name
+    }
+
+    assert client.put(api.url_for(Roles, role_id=role.id), data=json.dumps(data), headers=headers).status_code == 200
+
+
 @pytest.mark.parametrize("token,status", [
     (VALID_USER_HEADER_TOKEN, 403),
     (VALID_ADMIN_HEADER_TOKEN, 200),

lemur/tests/vectors.py

@@ -1,11 +1,15 @@
 from lemur.common.utils import parse_certificate
 
 VALID_USER_HEADER_TOKEN = {
-    'Authorization': 'Basic ' + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE0MzUyMzMzNjksInN1YiI6MSwiZXhwIjoxNTIxNTQ2OTY5fQ.1qCi0Ip7mzKbjNh0tVd3_eJOrae3rNa_9MCVdA4WtQI'}
+    'Authorization': 'Basic ' + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE0MzUyMzMzNjksInN1YiI6MSwiZXhwIjoxNTIxNTQ2OTY5fQ.1qCi0Ip7mzKbjNh0tVd3_eJOrae3rNa_9MCVdA4WtQI',
+    'Content-Type': 'application/json'
+}
 
 
 VALID_ADMIN_HEADER_TOKEN = {
-    'Authorization': 'Basic ' + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE0MzUyNTAyMTgsInN1YiI6MiwiZXhwIjoxNTIxNTYzODE4fQ.6mbq4-Ro6K5MmuNiTJBB153RDhlM5LGJBjI7GBKkfqA'}
+    'Authorization': 'Basic ' + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE0MzUyNTAyMTgsInN1YiI6MiwiZXhwIjoxNTIxNTYzODE4fQ.6mbq4-Ro6K5MmuNiTJBB153RDhlM5LGJBjI7GBKkfqA',
+    'Content-Type': 'application/json'
+}
 
 
 INTERNAL_VALID_LONG_STR = b"""