diff --git a/refstack-ui/app/components/results-report/partials/reportDetails.html b/refstack-ui/app/components/results-report/partials/reportDetails.html index 56afe240..40de3c24 100644 --- a/refstack-ui/app/components/results-report/partials/reportDetails.html +++ b/refstack-ui/app/components/results-report/partials/reportDetails.html @@ -42,6 +42,7 @@ report page. diff --git a/refstack/api/app.py b/refstack/api/app.py index 2579a40d..38b94c1c 100644 --- a/refstack/api/app.py +++ b/refstack/api/app.py @@ -124,7 +124,8 @@ class JSONErrorHook(pecan.hooks.PecanHook): elif isinstance(exc, webob.exc.HTTPError): return webob.Response( body=json.dumps({'code': exc.status_int, - 'title': exc.title}), + 'title': exc.title, + 'detail': exc.detail}), status=exc.status_int, content_type='application/json' ) diff --git a/refstack/api/constants.py b/refstack/api/constants.py index 2a0b8687..95135e78 100644 --- a/refstack/api/constants.py +++ b/refstack/api/constants.py @@ -41,7 +41,7 @@ CSRF_TOKEN = 'csrf_token' USER_OPENID = 'user_openid' # Test metadata fields -PUBLIC_KEY = 'public_key' +USER = 'user' SHARED_TEST_RUN = 'shared' # Roles diff --git a/refstack/api/controllers/results.py b/refstack/api/controllers/results.py index 8ecf6c32..5e658fb7 100644 --- a/refstack/api/controllers/results.py +++ b/refstack/api/controllers/results.py @@ -92,12 +92,17 @@ class ResultsController(validation.BaseRestControllerWithValidation): """Handler for storing item. Should return new item id.""" test_ = test.copy() if pecan.request.headers.get('X-Public-Key'): + key = pecan.request.headers.get('X-Public-Key').strip().split()[1] if 'meta' not in test_: test_['meta'] = {} - test_['meta'][const.PUBLIC_KEY] = \ - pecan.request.headers.get('X-Public-Key') + pubkey = db.get_pubkey(key) + if not pubkey: + pecan.abort(400, 'User with specified key not found. ' + 'Please log into the RefStack server to ' + 'upload your key.') + + test_['meta'][const.USER] = pubkey.openid test_id = db.store_results(test_) - LOG.debug(test_) return {'test_id': test_id, 'url': parse.urljoin(CONF.ui_url, CONF.api.test_results_url) % test_id} diff --git a/refstack/api/utils.py b/refstack/api/utils.py index 4162d325..e9ad94d0 100644 --- a/refstack/api/utils.py +++ b/refstack/api/utils.py @@ -59,8 +59,8 @@ def _get_input_params_from_request(expected_params): def parse_input_params(expected_input_params): """Parse input parameters from request. - :param expecred_params: (array) Expected input - params specified in constants. + :param expected_input_params: (array) Expected input + params specified in constants. """ raw_filters = _get_input_params_from_request(expected_input_params) filters = copy.deepcopy(raw_filters) @@ -84,10 +84,6 @@ def parse_input_params(expected_input_params): if const.SIGNED in filters: if is_authenticated(): filters[const.OPENID] = get_user_id() - filters[const.USER_PUBKEYS] = [ - ' '.join((pk['format'], pk['pubkey'])) - for pk in get_user_public_keys() - ] else: raise api_exc.ParseInputsError( 'To see signed test results you need to authenticate') @@ -228,8 +224,8 @@ def get_user_role(test_id): def _check_user(test_id): """Check that user has access to shared test run.""" - test_pubkey = db.get_test_meta_key(test_id, const.PUBLIC_KEY) - if not test_pubkey: + test_owner = db.get_test_meta_key(test_id, const.USER) + if not test_owner: return True elif db.get_test_meta_key(test_id, const.SHARED_TEST_RUN): return True @@ -241,9 +237,8 @@ def _check_owner(test_id): """Check that user has access to specified test run as owner.""" if not is_authenticated(): return False - test_pubkey = db.get_test_meta_key(test_id, const.PUBLIC_KEY) - return test_pubkey in [' '.join((pk['format'], pk['pubkey'])) - for pk in get_user_public_keys()] + user = db.get_test_meta_key(test_id, const.USER) + return user and user == get_user_id() def check_permissions(level): diff --git a/refstack/db/api.py b/refstack/db/api.py index aaf9f04d..abf6da80 100644 --- a/refstack/db/api.py +++ b/refstack/db/api.py @@ -139,6 +139,14 @@ def user_save(user_info): return IMPL.user_save(user_info) +def get_pubkey(key): + """Get pubkey info for a given key. + + :param key: public key + """ + return IMPL.get_pubkey(key) + + def store_pubkey(pubkey_info): """Store public key in to DB.""" return IMPL.store_pubkey(pubkey_info) diff --git a/refstack/db/migrations/alembic/versions/428e5aef5534_associate_test_result.py b/refstack/db/migrations/alembic/versions/428e5aef5534_associate_test_result.py new file mode 100644 index 00000000..170ad0cb --- /dev/null +++ b/refstack/db/migrations/alembic/versions/428e5aef5534_associate_test_result.py @@ -0,0 +1,46 @@ +"""Associate test results to users. + +Revision ID: 428e5aef5534 +Revises: 534e20be9964 +Create Date: 2015-11-03 00:51:34.096598 + +""" + +# revision identifiers, used by Alembic. +revision = '428e5aef5534' +down_revision = '534e20be9964' +MYSQL_CHARSET = 'utf8' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + """Upgrade DB.""" + conn = op.get_bind() + res = conn.execute("select openid,format,pubkey from pubkeys") + results = res.fetchall() + + # Get public key to user mappings. + pubkeys = {} + for result in results: + pubkeys[result[1] + " " + result[2]] = result[0] + + res = conn.execute("select test_id,value from meta where " + "meta_key='public_key'") + results = res.fetchall() + + for result in results: + test_id = result[0] + if result[1] in pubkeys: + openid = pubkeys[result[1]] + conn.execute(sa.text("update meta set meta_key='user', " + "value=:value where " + "test_id=:testid and meta_key='public_key'" + ), + value=openid, testid=test_id) + + +def downgrade(): + """Downgrade DB.""" + pass diff --git a/refstack/db/sqlalchemy/api.py b/refstack/db/sqlalchemy/api.py index 6a90d7ef..2850f97c 100644 --- a/refstack/db/sqlalchemy/api.py +++ b/refstack/db/sqlalchemy/api.py @@ -218,17 +218,17 @@ def _apply_filters_for_query(query, filters): query = query.filter(models.Test.cpid == cpid) signed = api_const.SIGNED in filters + # If we only want to get the user's test results. if signed: query = (query .join(models.Test.meta) - .filter(models.TestMeta.meta_key == api_const.PUBLIC_KEY) - .filter(models.TestMeta.value.in_( - filters[api_const.USER_PUBKEYS])) + .filter(models.TestMeta.meta_key == api_const.USER) + .filter(models.TestMeta.value == filters[api_const.OPENID]) ) else: signed_results = (query.session .query(models.TestMeta.test_id) - .filter_by(meta_key=api_const.PUBLIC_KEY)) + .filter_by(meta_key=api_const.USER)) shared_results = (query.session .query(models.TestMeta.test_id) .filter_by(meta_key=api_const.SHARED_TEST_RUN)) @@ -280,6 +280,23 @@ def user_save(user_info): return user +def get_pubkey(key): + """Get the pubkey info corresponding to the given public key. + + The md5 hash of the key is used for the query for quicker lookups. + """ + session = get_session() + md5_hash = hashlib.md5(base64.b64decode(key.encode('ascii'))).hexdigest() + pubkeys = session.query(models.PubKey).filter_by(md5_hash=md5_hash).all() + if len(pubkeys) == 1: + return pubkeys[0] + elif len(pubkeys) > 1: + for pubkey in pubkeys: + if pubkey['pubkey'] == key: + return pubkey + return None + + def store_pubkey(pubkey_info): """Store public key in to DB.""" pubkey = models.PubKey() diff --git a/refstack/tests/unit/test_api.py b/refstack/tests/unit/test_api.py index 9e616a10..2caa890c 100644 --- a/refstack/tests/unit/test_api.py +++ b/refstack/tests/unit/test_api.py @@ -158,12 +158,16 @@ class ResultsControllerTestCase(BaseControllerTestCase): mock_store_results.assert_called_once_with({'answer': 42}) @mock.patch('refstack.db.store_results') - def test_post_with_sign(self, mock_store_results): + @mock.patch('refstack.db.get_pubkey') + def test_post_with_sign(self, mock_get_pubkey, mock_store_results): self.mock_request.body = '{"answer": 42}' self.mock_request.headers = { 'X-Signature': 'fake-sign', - 'X-Public-Key': 'fake-key' + 'X-Public-Key': 'ssh-rsa Zm9vIGJhcg==' } + + mock_get_pubkey.return_value.openid = 'fake_openid' + mock_store_results.return_value = 'fake_test_id' result = self.controller.post() self.assertEqual(result, @@ -171,7 +175,7 @@ class ResultsControllerTestCase(BaseControllerTestCase): 'url': self.test_results_url % 'fake_test_id'}) self.assertEqual(self.mock_response.status, 201) mock_store_results.assert_called_once_with( - {'answer': 42, 'meta': {const.PUBLIC_KEY: 'fake-key'}} + {'answer': 42, 'meta': {const.USER: 'fake_openid'}} ) @mock.patch('refstack.db.get_test') diff --git a/refstack/tests/unit/test_api_utils.py b/refstack/tests/unit/test_api_utils.py index ef39abd6..5d249977 100644 --- a/refstack/tests/unit/test_api_utils.py +++ b/refstack/tests/unit/test_api_utils.py @@ -142,8 +142,7 @@ class APIUtilsTestCase(base.BaseTestCase): @mock.patch.object(api_utils, '_get_input_params_from_request') @mock.patch.object(api_utils, 'is_authenticated', return_value=True) @mock.patch.object(api_utils, 'get_user_id', return_value='fake_id') - @mock.patch('refstack.db.get_user_pubkeys') - def test_parse_input_params_success(self, mock_get_user_pubkeys, + def test_parse_input_params_success(self, mock_get_user_id, mock_is_authenticated, mock_get_input): @@ -157,9 +156,7 @@ class APIUtilsTestCase(base.BaseTestCase): const.CPID: '12345', const.SIGNED: True } - fake_pubkeys = ({'format': 'fake', - 'pubkey': 'fake_pk'},) - mock_get_user_pubkeys.return_value = fake_pubkeys + expected_params = mock.Mock() mock_get_input.return_value = raw_filters @@ -179,11 +176,10 @@ class APIUtilsTestCase(base.BaseTestCase): const.CPID: '12345', const.SIGNED: True, const.OPENID: 'fake_id', - const.USER_PUBKEYS: ['fake fake_pk'], } result = api_utils.parse_input_params(expected_params) - self.assertEqual(result, expected_result) + self.assertEqual(expected_result, result) mock_get_input.assert_called_once_with(expected_params) @@ -333,8 +329,8 @@ class APIUtilsTestCase(base.BaseTestCase): @mock.patch('pecan.abort', side_effect=exc.HTTPError) @mock.patch('refstack.db.get_test_meta_key') @mock.patch.object(api_utils, 'is_authenticated') - @mock.patch.object(api_utils, 'get_user_public_keys') - def test_check_get_user_role(self, mock_get_user_public_keys, + @mock.patch.object(api_utils, 'get_user_id') + def test_check_get_user_role(self, mock_get_user_id, mock_is_authenticated, mock_get_test_meta_key, mock_pecan_abort): @@ -346,7 +342,7 @@ class APIUtilsTestCase(base.BaseTestCase): 'fake_test', const.ROLE_OWNER) mock_get_test_meta_key.side_effect = { - ('fake_test', const.PUBLIC_KEY): 'fake key', + ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): 'true', }.get self.assertEqual(const.ROLE_USER, api_utils.get_user_role('fake_test')) @@ -355,10 +351,9 @@ class APIUtilsTestCase(base.BaseTestCase): 'fake_test', const.ROLE_OWNER) mock_is_authenticated.return_value = True - mock_get_user_public_keys.return_value = [{'format': 'fake', - 'pubkey': 'key'}] + mock_get_user_id.return_value = 'fake_openid' mock_get_test_meta_key.side_effect = { - ('fake_test', const.PUBLIC_KEY): 'fake key', + ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): 'true', }.get self.assertEqual(const.ROLE_USER, api_utils.get_user_role('fake_test')) @@ -368,10 +363,9 @@ class APIUtilsTestCase(base.BaseTestCase): # Check owner level mock_is_authenticated.return_value = True - mock_get_user_public_keys.return_value = [{'format': 'fake', - 'pubkey': 'key'}] + mock_get_user_id.return_value = 'fake_openid' mock_get_test_meta_key.side_effect = lambda *args: { - ('fake_test', const.PUBLIC_KEY): 'fake key', + ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): None, }.get(args) self.assertEqual(const.ROLE_OWNER, @@ -382,7 +376,7 @@ class APIUtilsTestCase(base.BaseTestCase): # Check negative cases mock_is_authenticated.return_value = False mock_get_test_meta_key.side_effect = lambda *args: { - ('fake_test', const.PUBLIC_KEY): 'fake key', + ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): None, }.get(args) self.assertRaises(exc.HTTPError, api_utils.enforce_permissions, @@ -391,10 +385,9 @@ class APIUtilsTestCase(base.BaseTestCase): 'fake_test', const.ROLE_OWNER) mock_is_authenticated.return_value = True - mock_get_user_public_keys.return_value = [{'format': 'fake', - 'pubkey': 'key'}] + mock_get_user_id.return_value = 'fake_openid' mock_get_test_meta_key.side_effect = lambda *args: { - ('fake_test', const.PUBLIC_KEY): 'fake other_key', + ('fake_test', const.USER): 'some_other_user', ('fake_test', const.SHARED_TEST_RUN): None, }.get(args) self.assertEqual(None, api_utils.get_user_role('fake_test')) @@ -406,8 +399,8 @@ class APIUtilsTestCase(base.BaseTestCase): @mock.patch('pecan.abort', side_effect=exc.HTTPError) @mock.patch('refstack.db.get_test_meta_key') @mock.patch.object(api_utils, 'is_authenticated') - @mock.patch.object(api_utils, 'get_user_public_keys') - def test_check_permissions(self, mock_get_user_public_keys, + @mock.patch.object(api_utils, 'get_user_id') + def test_check_permissions(self, mock_get_user_id, mock_is_authenticated, mock_get_test_meta_key, mock_pecan_abort): @@ -431,11 +424,10 @@ class APIUtilsTestCase(base.BaseTestCase): public_test = 'fake_public_test' private_test = 'fake_test' - mock_get_user_public_keys.return_value = [{'format': 'fake', - 'pubkey': 'key'}] + mock_get_user_id.return_value = 'fake_openid' mock_get_test_meta_key.side_effect = lambda *args: { - (public_test, const.PUBLIC_KEY): None, - (private_test, const.PUBLIC_KEY): 'fake key', + (public_test, const.USER): None, + (private_test, const.USER): 'fake_openid', (private_test, const.SHARED_TEST_RUN): None, }.get(args) diff --git a/refstack/tests/unit/test_app.py b/refstack/tests/unit/test_app.py index af102d83..a661607f 100644 --- a/refstack/tests/unit/test_app.py +++ b/refstack/tests/unit/test_app.py @@ -58,11 +58,14 @@ class JSONErrorHookTestCase(base.BaseTestCase): self.CONF.set_override('app_dev_mode', False, 'api') exc = mock.Mock(spec=webob.exc.HTTPError, status=418, status_int=418, - title='fake_title') + title='fake_title', + detail='fake_detail') self._on_error( response, exc, expected_status_code=exc.status, - expected_body={'code': exc.status_int, 'title': exc.title} + expected_body={'code': exc.status_int, + 'title': exc.title, + 'detail': exc.detail} ) @mock.patch.object(webob, 'Response') diff --git a/refstack/tests/unit/test_db.py b/refstack/tests/unit/test_db.py index fbbfbe42..552de813 100644 --- a/refstack/tests/unit/test_db.py +++ b/refstack/tests/unit/test_db.py @@ -370,7 +370,7 @@ class DBBackendTestCase(base.BaseTestCase): unsigned_query.session.query.assert_has_calls(( mock.call(mock_meta.test_id), - mock.call().filter_by(meta_key='public_key'), + mock.call().filter_by(meta_key='user'), mock.call(mock_meta.test_id), mock.call().filter_by(meta_key='shared'), )) @@ -390,13 +390,16 @@ class DBBackendTestCase(base.BaseTestCase): query = mock.Mock() mock_test.created_at = six.text_type() mock_meta.test_id = six.text_type() + mock_meta.meta_key = 'user' + mock_meta.value = 'test-openid' filters = { api_const.START_DATE: 'fake1', api_const.END_DATE: 'fake2', api_const.CPID: 'fake3', api_const.USER_PUBKEYS: ['fake_pk'], - api_const.SIGNED: 'true' + api_const.SIGNED: 'true', + api_const.OPENID: 'test-openid' } signed_query = (query @@ -409,14 +412,12 @@ class DBBackendTestCase(base.BaseTestCase): signed_query.join.assert_called_once_with(mock_test.meta) signed_query = signed_query.join.return_value signed_query.filter.assert_called_once_with( - mock_meta.meta_key == api_const.PUBLIC_KEY + mock_meta.meta_key == api_const.USER ) signed_query = signed_query.filter.return_value - mock_meta.value.in_.assert_called_once_with( - filters[api_const.USER_PUBKEYS]) signed_query.filter.assert_called_once_with( - mock_meta.value.in_.return_value) - + mock_meta.value == filters[api_const.OPENID] + ) filtered_query = signed_query.filter.return_value self.assertEqual(result, filtered_query) @@ -518,6 +519,36 @@ class DBBackendTestCase(base.BaseTestCase): user.update.assert_called_once_with(user_info) session.begin.assert_called_once_with() + @mock.patch.object(api, 'get_session', + return_value=mock.Mock(name='session'),) + @mock.patch('refstack.db.sqlalchemy.models.PubKey') + def test_get_pubkey(self, mock_model, mock_get_session): + key = 'AAAAB3Nz' + khash = hashlib.md5(base64.b64decode(key.encode('ascii'))).hexdigest() + session = mock_get_session.return_value + query = session.query.return_value + filtered = query.filter_by.return_value + + # Test no key match. + filtered.all.return_value = [] + result = api.get_pubkey(key) + self.assertEqual(None, result) + + session.query.assert_called_once_with(mock_model) + query.filter_by.assert_called_once_with(md5_hash=khash) + filtered.all.assert_called_once_with() + + # Test only one key match. + filtered.all.return_value = [{'pubkey': key, 'md5_hash': khash}] + result = api.get_pubkey(key) + self.assertEqual({'pubkey': key, 'md5_hash': khash}, result) + + # Test multiple keys with same md5 hash. + filtered.all.return_value = [{'pubkey': 'key2', 'md5_hash': khash}, + {'pubkey': key, 'md5_hash': khash}] + result = api.get_pubkey(key) + self.assertEqual({'pubkey': key, 'md5_hash': khash}, result) + @mock.patch.object(api, 'get_session') @mock.patch('refstack.db.sqlalchemy.api.models') def test_store_pubkey(self, mock_models, mock_get_session):