From 7f790be1e48df43c9c3af69179044a792b1293e9 Mon Sep 17 00:00:00 2001 From: kevgliss Date: Tue, 10 May 2016 14:19:24 -0700 Subject: [PATCH] Marsmallowing users (#312) --- lemur/tests/test_users.py | 101 ++++++++++++++++++++++++++++++++++++++ lemur/users/models.py | 7 --- lemur/users/schemas.py | 35 +++++++++++++ lemur/users/service.py | 11 +---- lemur/users/views.py | 75 ++++++++-------------------- 5 files changed, 158 insertions(+), 71 deletions(-) create mode 100644 lemur/tests/test_users.py create mode 100644 lemur/users/schemas.py diff --git a/lemur/tests/test_users.py b/lemur/tests/test_users.py new file mode 100644 index 00000000..e709d2ce --- /dev/null +++ b/lemur/tests/test_users.py @@ -0,0 +1,101 @@ +import pytest + +from lemur.users.views import * # noqa + + +from .vectors import VALID_ADMIN_HEADER_TOKEN, VALID_USER_HEADER_TOKEN + + +def test_user_input_schema(client): + from lemur.users.schemas import UserInputSchema + + input_data = { + 'username': 'example', + 'password': '1233432', + 'email': 'example@example.com' + } + + data, errors = UserInputSchema().load(input_data) + + assert not errors + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 200), + (VALID_ADMIN_HEADER_TOKEN, 200), + ('', 401) +]) +def test_user_get(client, token, status): + assert client.get(api.url_for(Users, user_id=1), headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 405), + (VALID_ADMIN_HEADER_TOKEN, 405), + ('', 405) +]) +def test_user_post_(client, token, status): + assert client.post(api.url_for(Users, user_id=1), data={}, headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 403), + (VALID_ADMIN_HEADER_TOKEN, 400), + ('', 401) +]) +def test_user_put(client, token, status): + assert client.put(api.url_for(Users, user_id=1), data={}, headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 405), + (VALID_ADMIN_HEADER_TOKEN, 405), + ('', 405) +]) +def test_user_delete(client, token, status): + assert client.delete(api.url_for(Users, user_id=1), headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 405), + (VALID_ADMIN_HEADER_TOKEN, 405), + ('', 405) +]) +def test_user_patch(client, token, status): + assert client.patch(api.url_for(Users, user_id=1), data={}, headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 403), + (VALID_ADMIN_HEADER_TOKEN, 400), + ('', 401) +]) +def test_user_list_post_(client, token, status): + assert client.post(api.url_for(UsersList), data={}, headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 404), + (VALID_ADMIN_HEADER_TOKEN, 404), + ('', 401) +]) +def test_user_list_get(client, token, status): + assert client.get(api.url_for(UsersList), headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 405), + (VALID_ADMIN_HEADER_TOKEN, 405), + ('', 405) +]) +def test_user_list_delete(client, token, status): + assert client.delete(api.url_for(UsersList), headers=token).status_code == status + + +@pytest.mark.parametrize("token,status", [ + (VALID_USER_HEADER_TOKEN, 405), + (VALID_ADMIN_HEADER_TOKEN, 405), + ('', 405) +]) +def test_user_list_patch(client, token, status): + assert client.patch(api.url_for(UsersList), data={}, headers=token).status_code == status diff --git a/lemur/users/models.py b/lemur/users/models.py index ab20c982..c859a36e 100644 --- a/lemur/users/models.py +++ b/lemur/users/models.py @@ -77,12 +77,5 @@ class User(db.Model): if role.name == 'admin': return True - def as_dict(self): - return {c.name: getattr(self, c.name) for c in self.__table__.columns} - - def serialize(self): - blob = self.as_dict() - return blob - listen(User, 'before_insert', hash_password) diff --git a/lemur/users/schemas.py b/lemur/users/schemas.py new file mode 100644 index 00000000..6a5bfe7e --- /dev/null +++ b/lemur/users/schemas.py @@ -0,0 +1,35 @@ +""" +.. module: lemur.users.schemas + :platform: unix + :copyright: (c) 2015 by Netflix Inc., see AUTHORS for more + :license: Apache, see LICENSE for more details. +.. moduleauthor:: Kevin Glisson +""" +from marshmallow import fields +from lemur.common.schema import LemurInputSchema, LemurOutputSchema +from lemur.schemas import AssociatedRoleSchema, AssociatedCertificateSchema, AssociatedAuthoritySchema + + +class UserInputSchema(LemurInputSchema): + username = fields.String(required=True) + email = fields.Email(required=True) + password = fields.String(required=True) # TODO add complexity requirements + active = fields.Boolean() + roles = fields.Nested(AssociatedRoleSchema, many=True) + certificates = fields.Nested(AssociatedCertificateSchema, many=True) + authorities = fields.Nested(AssociatedAuthoritySchema, many=True) + + +class UserOutputSchema(LemurOutputSchema): + username = fields.String() + email = fields.Email() + password = fields.String() + active = fields.Boolean() + roles = fields.Nested(AssociatedRoleSchema, many=True) + certificates = fields.Nested(AssociatedCertificateSchema, many=True) + authorities = fields.Nested(AssociatedAuthoritySchema, many=True) + + +user_input_schema = UserInputSchema() +user_output_schema = UserOutputSchema() +users_output_schema = UserOutputSchema(many=True) diff --git a/lemur/users/service.py b/lemur/users/service.py index 79b77a14..38772ba1 100644 --- a/lemur/users/service.py +++ b/lemur/users/service.py @@ -129,19 +129,10 @@ def render(args): """ query = database.session_query(User) - sort_by = args.pop('sort_by') - sort_dir = args.pop('sort_dir') - page = args.pop('page') - count = args.pop('count') filt = args.pop('filter') if filt: terms = filt.split(';') query = database.filter(query, User, terms) - query = database.find_all(query, User, args) - - if sort_by and sort_dir: - query = database.sort(query, User, sort_by, sort_dir) - - return database.paginate(query, page, count) + database.sort_and_page(query, User, args) diff --git a/lemur/users/views.py b/lemur/users/views.py index 2df6e8d7..8d76c89a 100644 --- a/lemur/users/views.py +++ b/lemur/users/views.py @@ -6,52 +6,32 @@ .. moduleauthor:: Kevin Glisson """ from flask import g, Blueprint -from flask.ext.restful import reqparse, Api, fields +from flask.ext.restful import reqparse, Api + +from lemur.common.schema import validate_schema +from lemur.common.utils import paginated_parser + +from lemur.auth.service import AuthenticatedResource +from lemur.auth.permissions import admin_permission from lemur.users import service from lemur.certificates import service as certificate_service from lemur.roles import service as role_service -from lemur.auth.service import AuthenticatedResource -from lemur.auth.permissions import admin_permission -from lemur.common.utils import marshal_items, paginated_parser + +from lemur.users.schemas import user_input_schema, user_output_schema, users_output_schema mod = Blueprint('users', __name__) api = Api(mod) -FIELDS = { - 'username': fields.String, - 'active': fields.Boolean, - 'email': fields.String, - 'profileImage': fields.String(attribute='profile_picture'), - 'id': fields.Integer, -} - - -def roles(values): - """ - Validate that the passed in roles exist. - - :param values: - :return: :raise ValueError: - """ - rs = [] - for role in values: - r = role_service.get(role['id']) - if not r: - raise ValueError("Role {0} does not exist".format(role['name'])) - rs.append(r) - return rs - - class UsersList(AuthenticatedResource): """ Defines the 'users' endpoint """ def __init__(self): self.reqparse = reqparse.RequestParser() super(UsersList, self).__init__() - @marshal_items(FIELDS) + @validate_schema(None, users_output_schema) def get(self): """ .. http:get:: /users @@ -109,8 +89,8 @@ class UsersList(AuthenticatedResource): return service.render(args) @admin_permission.require(http_exception=403) - @marshal_items(FIELDS) - def post(self): + @validate_schema(user_input_schema, user_output_schema) + def post(self, data=None): """ .. http:post:: /users @@ -155,14 +135,7 @@ class UsersList(AuthenticatedResource): :reqheader Authorization: OAuth token to authenticate :statuscode 200: no error """ - self.reqparse.add_argument('username', type=str, location='json', required=True) - self.reqparse.add_argument('email', type=str, location='json', required=True) - self.reqparse.add_argument('password', type=str, location='json', default=None) - self.reqparse.add_argument('active', type=bool, default=True, location='json') - self.reqparse.add_argument('roles', type=roles, default=[], location='json') - - args = self.reqparse.parse_args() - return service.create(args['username'], args['password'], args['email'], args['active'], None, args['roles']) + return service.create(data['username'], data['password'], data['email'], data['active'], None, data['roles']) class Users(AuthenticatedResource): @@ -170,7 +143,7 @@ class Users(AuthenticatedResource): self.reqparse = reqparse.RequestParser() super(Users, self).__init__() - @marshal_items(FIELDS) + @validate_schema(None, user_output_schema) def get(self, user_id): """ .. http:get:: /users/1 @@ -207,8 +180,8 @@ class Users(AuthenticatedResource): return service.get(user_id) @admin_permission.require(http_exception=403) - @marshal_items(FIELDS) - def put(self, user_id): + @validate_schema(user_input_schema, user_output_schema) + def put(self, user_id, data=None): """ .. http:put:: /users/1 @@ -248,13 +221,7 @@ class Users(AuthenticatedResource): :reqheader Authorization: OAuth token to authenticate :statuscode 200: no error """ - self.reqparse.add_argument('username', type=str, location='json', required=True) - self.reqparse.add_argument('email', type=str, location='json', required=True) - self.reqparse.add_argument('active', type=bool, location='json', required=True) - self.reqparse.add_argument('roles', type=roles, default=[], location='json', required=True) - - args = self.reqparse.parse_args() - return service.update(user_id, args['username'], args['email'], args['active'], None, args['roles']) + return service.update(user_id, data['username'], data['email'], data['active'], None, data['roles']) class CertificateUsers(AuthenticatedResource): @@ -262,7 +229,7 @@ class CertificateUsers(AuthenticatedResource): self.reqparse = reqparse.RequestParser() super(CertificateUsers, self).__init__() - @marshal_items(FIELDS) + @validate_schema(None, user_output_schema) def get(self, certificate_id): """ .. http:get:: /certificates/1/creator @@ -304,7 +271,7 @@ class RoleUsers(AuthenticatedResource): self.reqparse = reqparse.RequestParser() super(RoleUsers, self).__init__() - @marshal_items(FIELDS) + @validate_schema(None, user_output_schema) def get(self, role_id): """ .. http:get:: /roles/1/users @@ -357,7 +324,7 @@ class Me(AuthenticatedResource): def __init__(self): super(Me, self).__init__() - @marshal_items(FIELDS) + @validate_schema(None, user_output_schema) def get(self): """ .. http:get:: /auth/me @@ -391,7 +358,7 @@ class Me(AuthenticatedResource): :reqheader Authorization: OAuth token to authenticate :statuscode 200: no error """ - return g.current_user.as_dict() + return g.current_user api.add_resource(Me, '/auth/me', endpoint='me')