diff --git a/refstack/api/constants.py b/refstack/api/constants.py index 8d773334..2ed89adf 100644 --- a/refstack/api/constants.py +++ b/refstack/api/constants.py @@ -20,6 +20,7 @@ END_DATE = 'end_date' CPID = 'cpid' PAGE = 'page' SIGNED = 'signed' +VERIFICATION_STATUS = 'verification_status' OPENID = 'openid' USER_PUBKEYS = 'pubkeys' @@ -50,6 +51,10 @@ USER_OPENID = 'user_openid' USER = 'user' SHARED_TEST_RUN = 'shared' +# Test verification statuses +TEST_NOT_VERIFIED = 0 +TEST_VERIFIED = 1 + # Roles ROLE_USER = 'user' ROLE_OWNER = 'owner' diff --git a/refstack/api/controllers/products.py b/refstack/api/controllers/products.py index f44a76e5..56df4500 100644 --- a/refstack/api/controllers/products.py +++ b/refstack/api/controllers/products.py @@ -19,6 +19,7 @@ import json import uuid from oslo_config import cfg +from oslo_db.exception import DBReferenceError from oslo_log import log import pecan from pecan.secure import secure @@ -112,8 +113,11 @@ class VersionsController(validation.BaseRestControllerWithValidation): not api_utils.check_user_is_foundation_admin()): pecan.abort(403, 'Forbidden.') - - db.delete_product_version(version_id) + try: + db.delete_product_version(version_id) + except DBReferenceError: + pecan.abort(400, 'Unable to delete. There are still tests ' + 'associated to this product version.') pecan.response.status = 204 @@ -262,6 +266,9 @@ class ProductsController(validation.BaseRestControllerWithValidation): if (not api_utils.check_user_is_foundation_admin() and not api_utils.check_user_is_product_admin(id)): pecan.abort(403, 'Forbidden.') - - db.delete_product(id) + try: + db.delete_product(id) + except DBReferenceError: + pecan.abort(400, 'Unable to delete. There are still tests ' + 'associated to versions of this product.') pecan.response.status = 204 diff --git a/refstack/api/controllers/results.py b/refstack/api/controllers/results.py index 96fe2f89..9d6ef6c1 100644 --- a/refstack/api/controllers/results.py +++ b/refstack/api/controllers/results.py @@ -76,6 +76,10 @@ class MetadataController(rest.RestController): @pecan.expose('json') def post(self, test_id, key): """Save value for key in test run metadata.""" + test = db.get_test(test_id) + if test['verification_status'] == const.TEST_VERIFIED: + pecan.abort(403, 'Can not add/alter a new metadata key for a ' + 'verified test run.') db.save_test_meta_item(test_id, key, pecan.request.body) pecan.response.status = 201 @@ -84,6 +88,10 @@ class MetadataController(rest.RestController): @pecan.expose('json') def delete(self, test_id, key): """Delete key from test run metadata.""" + test = db.get_test(test_id) + if test['verification_status'] == const.TEST_VERIFIED: + pecan.abort(403, 'Can not delete a metadata key for a ' + 'verified test run.') db.delete_test_meta_item(test_id, key) pecan.response.status = 204 @@ -104,7 +112,8 @@ class ResultsController(validation.BaseRestControllerWithValidation): test_info = db.get_test( test_id, allowed_keys=['id', 'cpid', 'created_at', 'duration_seconds', 'meta', - 'product_version_id'] + 'product_version_id', + 'verification_status'] ) else: test_info = db.get_test(test_id) @@ -143,6 +152,10 @@ class ResultsController(validation.BaseRestControllerWithValidation): @api_utils.check_permissions(level=const.ROLE_OWNER) def delete(self, test_id): """Delete test run.""" + test = db.get_test(test_id) + if test['verification_status'] == const.TEST_VERIFIED: + pecan.abort(403, 'Can not delete a verified test run.') + db.delete_test(test_id) pecan.response.status = 204 @@ -162,7 +175,8 @@ class ResultsController(validation.BaseRestControllerWithValidation): const.START_DATE, const.END_DATE, const.CPID, - const.SIGNED + const.SIGNED, + const.VERIFICATION_STATUS ] filters = api_utils.parse_input_params(expected_input_params) @@ -205,7 +219,14 @@ class ResultsController(validation.BaseRestControllerWithValidation): def put(self, test_id, **kw): """Update a test result.""" test_info = {'id': test_id} + is_foundation_admin = api_utils.check_user_is_foundation_admin() + if 'product_version_id' in kw: + test = db.get_test(test_id) + if test['verification_status'] == const.TEST_VERIFIED: + pecan.abort(403, 'Can not update product_version_id for a ' + 'verified test run.') + if kw['product_version_id']: # Verify that the user is a member of the product's vendor. version = db.get_product_version(kw['product_version_id']) @@ -218,13 +239,34 @@ class ResultsController(validation.BaseRestControllerWithValidation): # is_vendor_admin to True. is_vendor_admin = True kw['product_version_id'] = None - is_foundation_admin = api_utils.check_user_is_foundation_admin() if not is_vendor_admin and not is_foundation_admin: pecan.abort(403, 'Forbidden.') test_info['product_version_id'] = kw['product_version_id'] + if 'verification_status' in kw: + if not is_foundation_admin: + pecan.abort(403, 'You do not have permission to change a ' + 'verification status.') + + if kw['verification_status'] not in (0, 1): + pecan.abort(400, 'Invalid verification_status value: %d' % + kw['verification_status']) + + # Check pre-conditions are met to mark a test verified. + if (kw['verification_status'] == 1 and + not (db.get_test_meta_key(test_id, 'target') and + db.get_test_meta_key(test_id, 'guideline') and + db.get_test_meta_key(test_id, const.SHARED_TEST_RUN))): + + pecan.abort(403, 'In order to mark a test verified, the ' + 'test must be shared and have been ' + 'associated to a guideline and target ' + 'program.') + + test_info['verification_status'] = kw['verification_status'] + test = db.update_test(test_info) pecan.response.status = 201 return test diff --git a/refstack/api/controllers/vendors.py b/refstack/api/controllers/vendors.py index f26e7b78..8b554f96 100644 --- a/refstack/api/controllers/vendors.py +++ b/refstack/api/controllers/vendors.py @@ -19,6 +19,7 @@ import json import six from oslo_config import cfg +from oslo_db.exception import DBReferenceError from oslo_log import log import pecan from pecan import rest @@ -196,7 +197,11 @@ class VendorsController(validation.BaseRestControllerWithValidation): pecan.abort(403, 'Forbidden.') _check_is_not_foundation(vendor_id) - db.delete_organization(vendor_id) + try: + db.delete_organization(vendor_id) + except DBReferenceError: + pecan.abort(400, 'Unable to delete. There are still tests ' + 'associated to products for this vendor.') pecan.response.status = 204 @secure(api_utils.is_authenticated) diff --git a/refstack/db/migrations/alembic/versions/59df512e82f_add_verification_status.py b/refstack/db/migrations/alembic/versions/59df512e82f_add_verification_status.py new file mode 100644 index 00000000..89021d84 --- /dev/null +++ b/refstack/db/migrations/alembic/versions/59df512e82f_add_verification_status.py @@ -0,0 +1,28 @@ +"""Add verification_status field to test. + +Revision ID: 59df512e82f +Revises: 23843be3da52 +Create Date: 2016-09-26 11:51:08.955006 + +""" + +# revision identifiers, used by Alembic. +revision = '59df512e82f' +down_revision = '23843be3da52' +MYSQL_CHARSET = 'utf8' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + """Upgrade DB.""" + op.add_column('test', sa.Column('verification_status', + sa.Integer, + nullable=False, + default=0)) + + +def downgrade(): + """Downgrade DB.""" + op.drop_column('test', 'verification_status') diff --git a/refstack/db/sqlalchemy/api.py b/refstack/db/sqlalchemy/api.py index 9e240dd3..e00a0748 100644 --- a/refstack/db/sqlalchemy/api.py +++ b/refstack/db/sqlalchemy/api.py @@ -166,7 +166,7 @@ def update_test(test_info): if test is None: raise NotFound('Test result with id %s not found' % _id) - keys = ['product_version_id'] + keys = ['product_version_id', 'verification_status'] for key in keys: if key in test_info: setattr(test, key, test_info[key]) @@ -238,6 +238,11 @@ def _apply_filters_for_query(query, filters): if cpid: query = query.filter(models.Test.cpid == cpid) + verification_status = filters.get(api_const.VERIFICATION_STATUS) + if verification_status: + query = query.filter(models.Test.verification_status == + verification_status) + signed = api_const.SIGNED in filters # If we only want to get the user's test results. if signed: @@ -442,6 +447,12 @@ def delete_organization(organization_id): """delete organization by id.""" session = get_session() with session.begin(): + product_ids = (session + .query(models.Product.id) + .filter_by(organization_id=organization_id)) + (session.query(models.ProductVersion). + filter(models.ProductVersion.product_id.in_(product_ids)). + delete(synchronize_session=False)) (session.query(models.Product). filter_by(organization_id=organization_id). delete(synchronize_session=False)) diff --git a/refstack/db/sqlalchemy/models.py b/refstack/db/sqlalchemy/models.py index 4a4d964a..9b6f7211 100644 --- a/refstack/db/sqlalchemy/models.py +++ b/refstack/db/sqlalchemy/models.py @@ -62,6 +62,7 @@ class Test(BASE, RefStackBase): # pragma: no cover product_version_id = sa.Column(sa.String(36), sa.ForeignKey('product_version.id'), nullable=True, unique=False) + verification_status = sa.Column(sa.Integer, nullable=False, default=0) @property def _extra_keys(self): @@ -78,7 +79,7 @@ class Test(BASE, RefStackBase): # pragma: no cover def default_allowed_keys(self): """Default keys.""" return ('id', 'created_at', 'duration_seconds', 'meta', - 'product_version_id') + 'product_version_id', 'verification_status') class TestResults(BASE, RefStackBase): # pragma: no cover diff --git a/refstack/tests/api/test_results.py b/refstack/tests/api/test_results.py index 4c102fd8..90dab0bd 100644 --- a/refstack/tests/api/test_results.py +++ b/refstack/tests/api/test_results.py @@ -89,7 +89,8 @@ class TestResultsEndpoint(api.FunctionalTest): """Test results endpoint with put request.""" results = json.dumps(FAKE_TESTS_RESULT) test_response = self.post_json(self.URL, params=results) - url = self.URL + test_response.get('test_id') + test_id = test_response.get('test_id') + url = self.URL + test_id user_info = { 'openid': 'test-open-id', @@ -126,6 +127,39 @@ class TestResultsEndpoint(api.FunctionalTest): get_response = self.get_json(url) self.assertIsNone(get_response['product_version_id']) + # Test when test verification preconditions are not met. + body = {'verification_status': api_const.TEST_VERIFIED} + put_response = self.put_json(url, expect_errors=True, + params=json.dumps(body)) + self.assertEqual(403, put_response.status_code) + + # Share the test run. + db.save_test_meta_item(test_id, api_const.SHARED_TEST_RUN, True) + put_response = self.put_json(url, expect_errors=True, + params=json.dumps(body)) + self.assertEqual(403, put_response.status_code) + + # Now associate guideline and target program. Now we should be + # able to mark a test verified. + db.save_test_meta_item(test_id, 'target', 'platform') + db.save_test_meta_item(test_id, 'guideline', '2016.01.json') + put_response = self.put_json(url, params=json.dumps(body)) + self.assertEqual(api_const.TEST_VERIFIED, + put_response['verification_status']) + + # Unshare the test, and check that we can mark it not verified. + db.delete_test_meta_item(test_id, api_const.SHARED_TEST_RUN) + body = {'verification_status': api_const.TEST_NOT_VERIFIED} + put_response = self.put_json(url, params=json.dumps(body)) + self.assertEqual(api_const.TEST_NOT_VERIFIED, + put_response['verification_status']) + + # Test when verification_status value is invalid. + body = {'verification_status': 111} + put_response = self.put_json(url, expect_errors=True, + params=json.dumps(body)) + self.assertEqual(400, put_response.status_code) + # Check test owner can put. mock_check_foundation.return_value = False mock_check_owner.return_value = True @@ -135,6 +169,12 @@ class TestResultsEndpoint(api.FunctionalTest): self.assertEqual(version_response['id'], get_response['product_version_id']) + # Test non-Foundation user can't change verification_status. + body = {'verification_status': 1} + put_response = self.put_json(url, expect_errors=True, + params=json.dumps(body)) + self.assertEqual(403, put_response.status_code) + # Test unauthorized put. mock_check_foundation.return_value = False mock_check_owner.return_value = False @@ -287,3 +327,22 @@ class TestResultsEndpoint(api.FunctionalTest): url = '/v1/results?end_date=1000-01-01 12:00:00' filtering_results = self.get_json(url) self.assertEqual([], filtering_results['results']) + + @mock.patch('refstack.api.utils.check_owner') + def test_delete(self, mock_check_owner): + results = json.dumps(FAKE_TESTS_RESULT) + test_response = self.post_json(self.URL, params=results) + test_id = test_response.get('test_id') + url = self.URL + test_id + + mock_check_owner.return_value = True + + # Test can't delete verified test run. + db.update_test({'id': test_id, 'verification_status': 1}) + resp = self.delete(url, expect_errors=True) + self.assertEqual(403, resp.status_code) + + # Test can delete verified test run. + db.update_test({'id': test_id, 'verification_status': 0}) + resp = self.delete(url, expect_errors=True) + self.assertEqual(204, resp.status_code) diff --git a/refstack/tests/unit/test_api.py b/refstack/tests/unit/test_api.py index 2779ad7f..430ede9c 100644 --- a/refstack/tests/unit/test_api.py +++ b/refstack/tests/unit/test_api.py @@ -141,7 +141,8 @@ class ResultsControllerTestCase(BaseControllerTestCase): mock_get_test.assert_called_once_with( 'fake_arg', allowed_keys=['id', 'cpid', 'created_at', 'duration_seconds', 'meta', - 'product_version_id'] + 'product_version_id', + 'verification_status'] ) @mock.patch('refstack.db.store_results') @@ -248,7 +249,8 @@ class ResultsControllerTestCase(BaseControllerTestCase): const.START_DATE, const.END_DATE, const.CPID, - const.SIGNED + const.SIGNED, + const.VERIFICATION_STATUS, ] page_number = 1 total_pages_number = 10 @@ -286,13 +288,21 @@ class ResultsControllerTestCase(BaseControllerTestCase): db_get_test.assert_called_once_with(page_number, per_page, filters) + @mock.patch('refstack.db.get_test') @mock.patch('refstack.db.delete_test') - def test_delete(self, mock_db_delete): + def test_delete(self, mock_db_delete, mock_get_test): self.mock_get_user_role.return_value = const.ROLE_OWNER + self.controller.delete('test_id') self.assertEqual(204, self.mock_response.status) + + # Verified test deletion attempt should raise error. + mock_get_test.return_value = {'verification_status': + const.TEST_VERIFIED} + self.assertRaises(webob.exc.HTTPError, + self.controller.delete, 'test_id') + self.mock_get_user_role.return_value = const.ROLE_USER - self.mock_abort.side_effect = webob.exc.HTTPError() self.assertRaises(webob.exc.HTTPError, self.controller.delete, 'test_id') @@ -634,9 +644,13 @@ class MetadataControllerTestCase(BaseControllerTestCase): self.mock_get_user_role.return_value = const.ROLE_FOUNDATION self.assertEqual(42, self.controller.get_one('test_id', 'user')) + @mock.patch('refstack.db.get_test') @mock.patch('refstack.db.save_test_meta_item') - def test_post(self, mock_save_test_meta_item): + def test_post(self, mock_save_test_meta_item, mock_get_test): self.mock_get_user_role.return_value = const.ROLE_OWNER + mock_get_test.return_value = { + 'verification_status': const.TEST_NOT_VERIFIED + } # Test trying to post a valid key. self.controller.post('test_id', 'shared') @@ -654,9 +668,13 @@ class MetadataControllerTestCase(BaseControllerTestCase): self.assertRaises(webob.exc.HTTPError, self.controller.post, 'test_id', 'shared') + @mock.patch('refstack.db.get_test') @mock.patch('refstack.db.delete_test_meta_item') - def test_delete(self, mock_delete_test_meta_item): + def test_delete(self, mock_delete_test_meta_item, mock_get_test): self.mock_get_user_role.return_value = const.ROLE_OWNER + mock_get_test.return_value = { + 'verification_status': const.TEST_NOT_VERIFIED + } self.controller.delete('test_id', 'shared') self.assertEqual(204, self.mock_response.status) mock_delete_test_meta_item.assert_called_once_with('test_id', 'shared')