From c05a49f8c9e566d49210dd34e1ccffe907f0ad46 Mon Sep 17 00:00:00 2001 From: kevgliss Date: Tue, 11 Oct 2016 17:24:15 -0700 Subject: [PATCH] Fixes an issuer where a member of a role is not able to add new users to said role. (#445) --- lemur/auth/permissions.py | 10 ++++---- lemur/auth/service.py | 4 ++-- lemur/certificates/views.py | 2 +- lemur/roles/views.py | 21 ++++++++--------- lemur/tests/test_roles.py | 47 ++++++++++++++++++++++++++++++++++++- lemur/tests/vectors.py | 8 +++++-- 6 files changed, 70 insertions(+), 22 deletions(-) diff --git a/lemur/auth/permissions.py b/lemur/auth/permissions.py index 5bd55624..36fc87e1 100644 --- a/lemur/auth/permissions.py +++ b/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']) diff --git a/lemur/auth/service.py b/lemur/auth/service.py index a0d86887..3f3d5610 100644 --- a/lemur/auth/service.py +++ b/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'): diff --git a/lemur/certificates/views.py b/lemur/certificates/views.py index 37363b0c..008383a2 100644 --- a/lemur/certificates/views.py +++ b/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: diff --git a/lemur/roles/views.py b/lemur/roles/views.py index 32fdf80e..875a7c49 100644 --- a/lemur/roles/views.py +++ b/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): diff --git a/lemur/tests/test_roles.py b/lemur/tests/test_roles.py index 58b355e1..912546df 100644 --- a/lemur/tests/test_roles.py +++ b/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), diff --git a/lemur/tests/vectors.py b/lemur/tests/vectors.py index 0d76c7ff..d029bec2 100644 --- a/lemur/tests/vectors.py +++ b/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"""