openinfra/refstack
Code Issues Proposed changes

Associate results to user instead of key

Browse Source 
Results will now be tied to openids instead of public keys. With this,
users won't lose access to their data if they delete associated public
keys from their profiles.

Addresses-Spec: https://review.openstack.org/#/c/228565
Change-Id: I916a8e588247c43e6d220c84b08610b10cc64933
This commit is contained in:
Paul Van Eck 2015-09-27 20:54:15 -07:00
parent 3f08865327
commit 93c8dda431
12 changed files with 165 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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>

3
refstack/api/app.py
View File

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

2
refstack/api/constants.py
View File

@ -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

11
refstack/api/controllers/results.py
View File

@ -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}

17
refstack/api/utils.py
View File

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

8
refstack/db/api.py
View File

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

46
refstack/db/migrations/alembic/versions/428e5aef5534_associate_test_result.py Normal file
View File

@ -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

25
refstack/db/sqlalchemy/api.py
View File

@ -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()

10
refstack/tests/unit/test_api.py
View File

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

44
refstack/tests/unit/test_api_utils.py
View File

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

7
refstack/tests/unit/test_app.py
View File

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

45
refstack/tests/unit/test_db.py
View File

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