Merge "Associate results to user instead of key"

refstack-ui/app/components/results-report/partials/reportDetails.html

@@ -42,6 +42,7 @@ report page.
             </span>
 
             <ul class="list-unstyled" collapse="!showTests">
+                <!-- Start passed test list -->
                 <li ng-repeat="test in capability.passedTests | orderBy:'toString()'"
                     ng-if="ctrl.isTestShown(test, capability)">
 
@@ -54,6 +55,9 @@ report page.
                     </span>
                     {{test}}
                 </li>
+                <!-- End passed test list -->
+
+                <!-- Start not passed test list -->
                 <li ng-repeat="test in capability.notPassedTests | orderBy:'toString()'"
                     ng-if="ctrl.isTestShown(test, capability)">
 
@@ -64,6 +68,7 @@ report page.
                     </span>
                     {{test}}
                 </li>
+                <!-- End not passed test list -->
             </ul>
         </li>
     </ol>

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'
             )

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

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] = \
+            pubkey = db.get_pubkey(key)
-                pecan.request.headers.get('X-Public-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}

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
+    :param expected_input_params: (array) Expected input
-                            params specified in constants.
+                                  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)
+    test_owner = db.get_test_meta_key(test_id, const.USER)
-    if not test_pubkey:
+    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)
+    user = db.get_test_meta_key(test_id, const.USER)
-    return test_pubkey in [' '.join((pk['format'], pk['pubkey']))
+    return user and user == get_user_id()
-                           for pk in get_user_public_keys()]
 
 
 def check_permissions(level):

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)

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

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.meta_key == api_const.USER)
-                 .filter(models.TestMeta.value.in_(
+                 .filter(models.TestMeta.value == filters[api_const.OPENID])
-                     filters[api_const.USER_PUBKEYS]))
                  )
     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()

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')

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,
-    def test_parse_input_params_success(self, mock_get_user_pubkeys,
                                         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')
+    @mock.patch.object(api_utils, 'get_user_id')
-    def test_check_get_user_role(self, mock_get_user_public_keys,
+    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',
+        mock_get_user_id.return_value = 'fake_openid'
-                                                   'pubkey': 'key'}]
         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',
+        mock_get_user_id.return_value = 'fake_openid'
-                                                   'pubkey': 'key'}]
         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',
+        mock_get_user_id.return_value = 'fake_openid'
-                                                   'pubkey': 'key'}]
         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')
+    @mock.patch.object(api_utils, 'get_user_id')
-    def test_check_permissions(self, mock_get_user_public_keys,
+    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',
+        mock_get_user_id.return_value = 'fake_openid'
-                                                   'pubkey': 'key'}]
         mock_get_test_meta_key.side_effect = lambda *args: {
-            (public_test, const.PUBLIC_KEY): None,
+            (public_test, const.USER): None,
-            (private_test, const.PUBLIC_KEY): 'fake key',
+            (private_test, const.USER): 'fake_openid',
             (private_test, const.SHARED_TEST_RUN): None,
         }.get(args)
 

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')

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):