From e48d6205f32611e4162410f59c8d89efdf0a8033 Mon Sep 17 00:00:00 2001 From: Kirill Abramov Date: Thu, 26 Apr 2018 16:08:54 +0200 Subject: [PATCH 1/7] Fixed response messages' consistency --- src/api/handlers/projects/collaborators.py | 2 +- src/api/handlers/projects/secrets.py | 10 +++++----- src/api/handlers/projects/tokens.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/api/handlers/projects/collaborators.py b/src/api/handlers/projects/collaborators.py index 738e30a..c486cb7 100644 --- a/src/api/handlers/projects/collaborators.py +++ b/src/api/handlers/projects/collaborators.py @@ -68,7 +68,7 @@ def post(self, project_id): g.db.commit() - return OK('Successfully added user') + return OK('Successfully added user.') @ns.route('//collaborators/') diff --git a/src/api/handlers/projects/secrets.py b/src/api/handlers/projects/secrets.py index faa8a6d..0079799 100644 --- a/src/api/handlers/projects/secrets.py +++ b/src/api/handlers/projects/secrets.py @@ -37,14 +37,14 @@ def post(self, project_id): b = request.get_json() if not Secrets.name_pattern.match(b['name']): - abort(400, 'Secret name must be not empty alphanumeric string') + abort(400, 'Secret name must be not empty alphanumeric string.') result = g.db.execute_one_dict(''' SELECT COUNT(*) as cnt FROM secret WHERE project_id = %s ''', [project_id]) if result['cnt'] > 50: - abort(400, 'Too many secrets') + abort(400, 'Too many secrets.') r = g.db.execute_one(''' SELECT count(*) FROM secret @@ -52,7 +52,7 @@ def post(self, project_id): ''', [project_id, b['name']]) if r[0] > 0: - abort(400, 'Secret with this name already exist') + abort(400, 'Secret with this name already exist.') g.db.execute(''' INSERT INTO secret (project_id, name, value) VALUES(%s, %s, %s) @@ -60,7 +60,7 @@ def post(self, project_id): g.db.commit() - return OK('Successfully added secret') + return OK('Successfully added secret.') @ns.route('//secrets/') @@ -72,4 +72,4 @@ def delete(self, project_id, secret_id): ''', [project_id, secret_id]) g.db.commit() - return OK('Successfully deleted secret') + return OK('Successfully deleted secret.') diff --git a/src/api/handlers/projects/tokens.py b/src/api/handlers/projects/tokens.py index 0b8bbb5..3ad024c 100644 --- a/src/api/handlers/projects/tokens.py +++ b/src/api/handlers/projects/tokens.py @@ -51,7 +51,7 @@ def post(self, project_id): g.db.commit() - return OK('Successfully added token', {'token': token}) + return OK('Successfully added token.', {'token': token}) @ns.route('//tokens/') class Token(Resource): @@ -59,7 +59,7 @@ class Token(Resource): @auth_required(['user']) def delete(self, project_id, token_id): if not validate_uuid4(token_id): - abort(400, "Invalid project-token uuid") + abort(400, "Invalid project-token uuid.") num_tokens = g.db.execute_one(""" SELECT COUNT(*) FROM auth_token @@ -75,4 +75,4 @@ def delete(self, project_id, token_id): """, [project_id, token_id]) g.db.commit() - return OK('Successfully deleted token') + return OK('Successfully deleted token.') From 45d2324b5328c82b0e392ece8fc2670ed9c33d6d Mon Sep 17 00:00:00 2001 From: Kirill Abramov Date: Thu, 26 Apr 2018 17:01:55 +0200 Subject: [PATCH 2/7] New CLI API: secrets API improvements. * Added more conditions and appropriate messages on secrets deletion to check: - if provided secret_id is a valid uuid4; - if secret with provided secret_id is exists. * Stylystic fixes. --- src/api/handlers/projects/secrets.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/api/handlers/projects/secrets.py b/src/api/handlers/projects/secrets.py index 0079799..f6a1c7a 100644 --- a/src/api/handlers/projects/secrets.py +++ b/src/api/handlers/projects/secrets.py @@ -2,6 +2,7 @@ from flask_restplus import Resource, fields import re +from pyinfrabox.utils import validate_uuid4 from pyinfraboxutils.ibflask import auth_required, OK from pyinfraboxutils.ibrestplus import api from api.namespaces import project as ns @@ -39,24 +40,24 @@ def post(self, project_id): if not Secrets.name_pattern.match(b['name']): abort(400, 'Secret name must be not empty alphanumeric string.') - result = g.db.execute_one_dict(''' + result = g.db.execute_one_dict(""" SELECT COUNT(*) as cnt FROM secret WHERE project_id = %s - ''', [project_id]) + """, [project_id]) if result['cnt'] > 50: abort(400, 'Too many secrets.') - r = g.db.execute_one(''' + r = g.db.execute_one(""" SELECT count(*) FROM secret WHERE project_id = %s AND name = %s - ''', [project_id, b['name']]) + """, [project_id, b['name']]) if r[0] > 0: abort(400, 'Secret with this name already exist.') - g.db.execute(''' + g.db.execute(""" INSERT INTO secret (project_id, name, value) VALUES(%s, %s, %s) - ''', [project_id, b['name'], b['value']]) + """, [project_id, b['name'], b['value']]) g.db.commit() @@ -67,9 +68,20 @@ def post(self, project_id): class Secret(Resource): @auth_required(['user']) def delete(self, project_id, secret_id): - g.db.execute(''' + if not validate_uuid4(secret_id): + abort(400, "Invalid secret uuid.") + + num_secrets = g.db.execute_one(""" + SELECT COUNT(*) FROM secret + WHERE project_id = %s and id = %s + """, [project_id, secret_id])[0] + + if num_secrets == 0: + return abort(400, 'Such secret does not exist.') + + g.db.execute(""" DELETE FROM secret WHERE project_id = %s and id = %s - ''', [project_id, secret_id]) + """, [project_id, secret_id]) g.db.commit() return OK('Successfully deleted secret.') From f4b9ca13a1b40393e1c559dc3bce22562960cce3 Mon Sep 17 00:00:00 2001 From: Kirill Abramov Date: Thu, 26 Apr 2018 19:16:42 +0200 Subject: [PATCH 3/7] API: added method to get all projects for user. --- src/api/handlers/project.py | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/api/handlers/project.py b/src/api/handlers/project.py index f3dcd67..7938d34 100644 --- a/src/api/handlers/project.py +++ b/src/api/handlers/project.py @@ -54,17 +54,43 @@ def get_badge(url): 'public': fields.Boolean }) +@ns.route('') +class Projects(Resource): + + @auth_required(['user']) + def get(self): + b = request.get_json() + username = b['username'] + + user_id = g.db.execute_one(""" + SELECT id + FROM "user" + WHERE username = %s + """, [username])[0]['id'] + + response = g.db.execute_many_dict(""" + SELECT name, id, type, public + FROM project p + INNER JOIN collaborators c + WHERE c.project_id = p.id + AND c.owner = true + AND c.user_id = %s + """, [user_id]) + + return response + + @ns.route('/') class Project(Resource): @auth_required(['user', 'project']) @api.marshal_with(project_model) def get(self, project_id): - p = g.db.execute_one_dict(''' + p = g.db.execute_one_dict(""" SELECT name, id, type, public FROM project WHERE id = %s - ''', [project_id]) + """, [project_id]) return p @ns.route('//state.svg') @@ -73,9 +99,9 @@ class State(Resource): @nocache def get(self, project_id): - p = g.db.execute_one_dict(''' + p = g.db.execute_one_dict(""" SELECT type FROM project WHERE id = %s - ''', [project_id]) + """, [project_id]) project_type = p['type'] From 58783b0ea69d38d17afc1013d1748ced4b589240 Mon Sep 17 00:00:00 2001 From: Kirill Abramov Date: Thu, 26 Apr 2018 19:19:58 +0200 Subject: [PATCH 4/7] Fixed response messages' consistency --- src/api/handlers/projects/collaborators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/handlers/projects/collaborators.py b/src/api/handlers/projects/collaborators.py index c486cb7..bdb9e06 100644 --- a/src/api/handlers/projects/collaborators.py +++ b/src/api/handlers/projects/collaborators.py @@ -102,4 +102,4 @@ def delete(self, project_id, user_id): g.db.commit() - return OK('Successfully removed user') + return OK('Successfully removed user.') From d80b9d65e5290e99a2549ecce393e85be67082e5 Mon Sep 17 00:00:00 2001 From: Kirill Abramov Date: Thu, 26 Apr 2018 19:29:14 +0200 Subject: [PATCH 5/7] Tests expected messages fixes. --- infrabox/test/api/collaborators_test.py | 4 ++-- infrabox/test/api/secrets_test.py | 8 ++++---- infrabox/test/api/tokens_test.py | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/infrabox/test/api/collaborators_test.py b/infrabox/test/api/collaborators_test.py index 5a142e0..8e18e3e 100644 --- a/infrabox/test/api/collaborators_test.py +++ b/infrabox/test/api/collaborators_test.py @@ -20,7 +20,7 @@ def test_collaborators_root(self): r = TestClient.post('api/v1/projects/%s/collaborators' % self.project_id, data=self.test_collaborator_data, headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Successfully added user') + self.assertEqual(r['message'], 'Successfully added user.') self.assertEqual(r['status'], 200) # test unauthorized @@ -54,7 +54,7 @@ def test_collaborators_delete(self): % (self.project_id, self.collaborator_id), headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Successfully removed user') + self.assertEqual(r['message'], 'Successfully removed user.') self.assertEqual(r['status'], 200) # check if database does not contain this collaborator anymore diff --git a/infrabox/test/api/secrets_test.py b/infrabox/test/api/secrets_test.py index 05650fa..7219678 100644 --- a/infrabox/test/api/secrets_test.py +++ b/infrabox/test/api/secrets_test.py @@ -27,19 +27,19 @@ def test_secrets_root(self): # test secret creation r = TestClient.post('api/v1/projects/%s/secrets/' % self.project_id, data=self.test_secret_data, headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Successfully added secret') + self.assertEqual(r['message'], 'Successfully added secret.') self.assertEqual(r['status'], 200) # test invalid secret data handling for invalid_secret_data in self.invalid_test_secret_data: r = TestClient.post('api/v1/projects/%s/secrets/' % self.project_id, data=invalid_secret_data, headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Secret name must be not empty alphanumeric string') + self.assertEqual(r['message'], 'Secret name must be not empty alphanumeric string.') # test name already exist r = TestClient.post('api/v1/projects/%s/secrets/' % self.project_id, data=self.test_secret_data, headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Secret with this name already exist') + self.assertEqual(r['message'], 'Secret with this name already exist.') # test secret receiving r = TestClient.get('api/v1/projects/%s/secrets' % self.project_id, @@ -61,7 +61,7 @@ def test_secret_delete(self): r = TestClient.delete('api/v1/projects/%s/secrets/%s' % (self.project_id, secret_id), headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Successfully deleted secret') + self.assertEqual(r['message'], 'Successfully deleted secret.') self.assertEqual(r['status'], 200) r = TestClient.execute_one("""SELECT count(*) FROM secret WHERE id = %s""", [secret_id]) diff --git a/infrabox/test/api/tokens_test.py b/infrabox/test/api/tokens_test.py index e4362ae..b425c77 100644 --- a/infrabox/test/api/tokens_test.py +++ b/infrabox/test/api/tokens_test.py @@ -20,7 +20,7 @@ def test_tokens_root(self): # test token creation r = TestClient.post('api/v1/projects/%s/tokens' % self.project_id, data=self.test_token_data, headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Successfully added token') + self.assertEqual(r['message'], 'Successfully added token.') self.assertEqual(r['status'], 200) # test token receiving @@ -47,7 +47,7 @@ def test_tokens_delete(self): r = TestClient.delete('api/v1/projects/%s/tokens/%s' % (self.project_id, token_id), headers=TestClient.get_user_authorization(self.user_id)) - self.assertEqual(r['message'], 'Successfully deleted token') + self.assertEqual(r['message'], 'Successfully deleted token.') self.assertEqual(r['status'], 200) r = TestClient.execute_one("""SELECT count(*) FROM auth_token WHERE id = '%s'""" % token_id) From 69a93cfcc9bcf64361ffa5fbaefb52a5a625faeb Mon Sep 17 00:00:00 2001 From: Kirill Abramov Date: Fri, 27 Apr 2018 15:34:46 +0200 Subject: [PATCH 6/7] New CLI API: project API improvements. * Removed redundant code to get projects list in `handlers/project.py` file which duplicated `handlers/projects/project.py`; * Updated `project_model` schema to have also `public` field; * When returning project from query, return `public` field also. --- src/api/handlers/project.py | 26 -------------------------- src/api/handlers/projects/projects.py | 14 ++++++++------ 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/src/api/handlers/project.py b/src/api/handlers/project.py index 7938d34..1299b72 100644 --- a/src/api/handlers/project.py +++ b/src/api/handlers/project.py @@ -54,32 +54,6 @@ def get_badge(url): 'public': fields.Boolean }) -@ns.route('') -class Projects(Resource): - - @auth_required(['user']) - def get(self): - b = request.get_json() - username = b['username'] - - user_id = g.db.execute_one(""" - SELECT id - FROM "user" - WHERE username = %s - """, [username])[0]['id'] - - response = g.db.execute_many_dict(""" - SELECT name, id, type, public - FROM project p - INNER JOIN collaborators c - WHERE c.project_id = p.id - AND c.owner = true - AND c.user_id = %s - """, [user_id]) - - return response - - @ns.route('/') class Project(Resource): diff --git a/src/api/handlers/projects/projects.py b/src/api/handlers/projects/projects.py index 524431b..4da5ba5 100644 --- a/src/api/handlers/projects/projects.py +++ b/src/api/handlers/projects/projects.py @@ -16,7 +16,8 @@ project_model = api.model('Project', { 'id': fields.String(required=True), 'name': fields.String(required=True), - 'type': fields.String(required=True) + 'type': fields.String(required=True), + 'public': fields.String(required=True) }) add_project_schema = { @@ -38,13 +39,14 @@ class Projects(Resource): @auth_required(['user'], check_project_access=False) @api.marshal_list_with(project_model) def get(self): - projects = g.db.execute_many_dict(''' - SELECT p.id, p.name, p.type FROM project p + projects = g.db.execute_many_dict(""" + SELECT p.id, p.name, p.type, p.public + FROM project p INNER JOIN collaborator co ON co.project_id = p.id AND %s = co.user_id ORDER BY p.name - ''', [g.token['user']['id']]) + """, [g.token['user']['id']]) return projects @@ -200,7 +202,7 @@ class ProjectName(Resource): @api.marshal_with(project_model) def get(self, project_name): project = g.db.execute_one_dict(''' - SELECT id, name, type + SELECT id, name, type, public FROM project WHERE name = %s ''', [project_name]) @@ -218,7 +220,7 @@ class Project(Resource): @api.marshal_with(project_model) def get(self, project_id): project = g.db.execute_one_dict(''' - SELECT p.id, p.name, p.type + SELECT p.id, p.name, p.type, p.public FROM project p WHERE id = %s ''', [project_id]) From 6b4b5f8e6f7376b7faae9131428546a4451ec45c Mon Sep 17 00:00:00 2001 From: Kirill Abramov Date: Fri, 27 Apr 2018 17:01:57 +0200 Subject: [PATCH 7/7] New CLI API: project API improvements. * On project deletion: - validate project id before executing anything with it; - check if project with provided id exists. --- src/api/handlers/projects/projects.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/api/handlers/projects/projects.py b/src/api/handlers/projects/projects.py index 4da5ba5..2c34d79 100644 --- a/src/api/handlers/projects/projects.py +++ b/src/api/handlers/projects/projects.py @@ -5,6 +5,7 @@ from flask_restplus import Resource, fields from pyinfraboxutils import get_logger +from pyinfrabox.utils import validate_uuid4 from pyinfraboxutils.ibrestplus import api from pyinfraboxutils.ibflask import auth_required, OK @@ -229,14 +230,15 @@ def get(self, project_id): @auth_required(['user'], check_project_owner=True) def delete(self, project_id): + if not validate_uuid4(project_id): + abort(400, "Invalid project uuid.") - project = g.db.execute_one_dict(''' + project = g.db.execute_one_dict(""" DELETE FROM project WHERE id = %s RETURNING type - ''', [project_id]) + """, [project_id]) if not project: - abort(404) - + abort(400, 'Project with such an id does not exist.') if project['type'] == 'github': repo = g.db.execute_one_dict('''