Skip to content

Commit

Permalink
Merge branch 'master' into STORM-2178/clear_error_message_for_passphr…
Browse files Browse the repository at this point in the history
…ase_protected_keys

* master:
  update changelog
  Test for show_secrets param
  cli support for unmasking secrets if user is an admin
  Fix imports
  change base class
  add missing import
  Use _get_mask_secrets in actionexecutions
  Ability to show_secrets for admin as applied to apikeys
  Use the right mask_secrets property

Conflicts:
	CHANGELOG.rst
  • Loading branch information
Lakshmi Kannan committed Jun 17, 2016
2 parents 1bf5009 + c220652 commit b762b3a
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 25 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ in development
client. (bug-fix) #2543 [Adam Mielke]
* Send out a clear error message when SSH private key is passphrase protected but user fails to
supply passphrase with private_key when running a remote SSH action. (improvement)
* Admins will now be able pass ``--show_secrets`` when listing apikeys to get the ``key_hash``
un-masked on the CLI. (new-feature)

1.4.0 - April 18, 2016
----------------------
Expand Down
4 changes: 2 additions & 2 deletions conf/st2.dev.conf
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
host = 0.0.0.0
port = 9101
logging = st2api/conf/logging.conf
mask_secrets = False
mask_secrets = True
# allow_origin is required for handling CORS in st2 web UI.
# allow_origin = http://myhost1.example.com:3000,http://myhost2.example.com:3000
# Development vagrant VM is setup to run on 172.168.50.50.
Expand Down Expand Up @@ -69,7 +69,7 @@ protocol = udp
[log]
excludes = requests,paramiko
redirect_stderr = False
mask_secrets = False
mask_secrets = True

[system_user]
user = stanley
Expand Down
30 changes: 29 additions & 1 deletion st2api/st2api/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,21 @@

import six
from pecan.rest import RestController
from oslo_config import cfg
from six.moves.urllib import parse as urlparse # pylint: disable=import-error

from st2api.controllers.controller_transforms import transform_to_bool
from st2common.rbac.utils import request_user_is_admin

__all__ = [
'BaseRestControllerMixin'
'BaseRestControllerMixin',
'SHOW_SECRETS_QUERY_PARAM'
]


SHOW_SECRETS_QUERY_PARAM = 'show_secrets'


class BaseRestControllerMixin(RestController):
"""
Base REST controller class which contains various utility functions.
Expand Down Expand Up @@ -64,3 +70,25 @@ def _get_query_param_value(self, request, param_name, param_type, default_value=
value = transform_to_bool(value)

return value

def _get_mask_secrets(self, request):
"""
Return a value for mask_secrets which can be used in masking secret properties
to be retruned by any API. The default value is as per the config however admin
users have the ability to override by passing in a special query parameter
?show_secrets=True.
:param request: Request object.
:rtype: ``bool``
"""
mask_secrets = cfg.CONF.api.mask_secrets
show_secrets = self._get_query_param_value(request=request,
param_name=SHOW_SECRETS_QUERY_PARAM,
param_type='bool',
default_value=False)

if show_secrets and request_user_is_admin(request=request):
mask_secrets = False

return mask_secrets
18 changes: 1 addition & 17 deletions st2api/st2api/controllers/v1/actionexecutions.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import traceback

import jsonschema
from oslo_config import cfg
import pecan
from pecan import abort
from six.moves import http_client
Expand All @@ -44,7 +43,6 @@
from st2common.services import action as action_service
from st2common.services import executions as execution_service
from st2common.services import trace as trace_service
from st2common.rbac.utils import request_user_is_admin
from st2common.util import jsonify
from st2common.util import isotime
from st2common.util import action_db as action_utils
Expand All @@ -69,10 +67,6 @@
'timestamp_lt': 'start_timestamp.lt'
})

# Name of the query parameter for toggling on the display of secrets to the admin users in the API
# responses
SHOW_SECRETS_QUERY_PARAM = 'show_secrets'

MONITOR_THREAD_EMPTY_Q_SLEEP_TIME = 5
MONITOR_THREAD_NO_WORKERS_SLEEP_TIME = 1

Expand All @@ -95,17 +89,7 @@ def _get_from_model_kwargs_for_request(self, request):
"""
Set mask_secrets=False if the user is an admin and provided ?show_secrets=True query param.
"""
from_model_kwargs = {'mask_secrets': cfg.CONF.api.mask_secrets}

show_secrets = self._get_query_param_value(request=request,
param_name=SHOW_SECRETS_QUERY_PARAM,
param_type='bool',
default_value=False)

if show_secrets and request_user_is_admin(request=request):
from_model_kwargs['mask_secrets'] = False

return from_model_kwargs
return {'mask_secrets': self._get_mask_secrets(request)}

def _handle_schedule_execution(self, liveaction_api):
"""
Expand Down
20 changes: 15 additions & 5 deletions st2api/st2api/controllers/v1/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@

from oslo_config import cfg
from pecan import abort
from pecan.rest import RestController
from mongoengine import ValidationError

from st2api.controllers.base import BaseRestControllerMixin, SHOW_SECRETS_QUERY_PARAM
from st2common import log as logging
from st2common.models.api.auth import ApiKeyAPI, ApiKeyCreateResponseAPI
from st2common.models.api.base import jsexpose
Expand All @@ -42,7 +42,7 @@
]


class ApiKeyController(RestController):
class ApiKeyController(BaseRestControllerMixin):
"""
Implements the REST endpoint for managing the key value store.
"""
Expand Down Expand Up @@ -77,7 +77,8 @@ def get_one(self, api_key_id_or_key):
abort(http_client.NOT_FOUND, msg)

try:
return ApiKeyAPI.from_model(api_key_db, mask_secrets=True)
mask_secrets = self._get_mask_secrets(pecan.request)
return ApiKeyAPI.from_model(api_key_db, mask_secrets=mask_secrets)
except (ValidationError, ValueError) as e:
LOG.exception('Failed to serialize API key.')
abort(http_client.INTERNAL_SERVER_ERROR, str(e))
Expand All @@ -91,9 +92,9 @@ def get_all(self, **kw):
Handles requests:
GET /keys/
"""

mask_secrets, kw = self._get_mask_secrets_ex(**kw)
api_key_dbs = ApiKey.get_all(**kw)
api_keys = [ApiKeyAPI.from_model(api_key_db, mask_secrets=True)
api_keys = [ApiKeyAPI.from_model(api_key_db, mask_secrets=mask_secrets)
for api_key_db in api_key_dbs]

return api_keys
Expand Down Expand Up @@ -192,3 +193,12 @@ def _get_user(self):

user_db = auth_context.get('user', None)
return user_db.name if user_db else cfg.CONF.system_user.user

def _get_mask_secrets_ex(self, **kw):
"""
Allowing SHOW_SECRETS_QUERY_PARAM to remain in the parameters causes downstream
lookup failures there removing. This is a pretty hackinsh way to manage query params.
"""
mask_secrets = self._get_mask_secrets(pecan.request)
kw.pop(SHOW_SECRETS_QUERY_PARAM, None)
return mask_secrets, kw
14 changes: 14 additions & 0 deletions st2api/tests/unit/controllers/v1/test_auth_api_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import string
import unittest

from oslo_config import cfg

from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE
from st2common.models.db.auth import UserDB
from st2common.persistence.auth import ApiKey
Expand Down Expand Up @@ -54,6 +56,10 @@ class TestApiKeyController(FunctionalTest):
@classmethod
def setUpClass(cls):
super(TestApiKeyController, cls).setUpClass()

cfg.CONF.set_override(name='mask_secrets', override=True, group='api')
cfg.CONF.set_override(name='mask_secrets', override=True, group='log')

models = FixturesLoader().save_fixtures_to_db(fixtures_pack=FIXTURES_PACK,
fixtures_dict=TEST_MODELS)
cls.apikey1 = models['apikeys']['apikey1.yaml']
Expand Down Expand Up @@ -102,6 +108,14 @@ def test_get_one_by_key(self):
self.assertEqual(resp.json['key_hash'], MASKED_ATTRIBUTE_VALUE,
'Key should be masked.')

def test_get_show_secrets(self):

resp = self.app.get('/v1/apikeys/?show_secrets=True')
self.assertEqual(resp.status_int, 200)
for key in resp.json:
self.assertNotEqual(key['key_hash'], MASKED_ATTRIBUTE_VALUE)
self.assertNotEqual(key['uid'], MASKED_ATTRIBUTE_VALUE)

def test_post_delete_key(self):
api_key = {
'user': 'herge'
Expand Down
8 changes: 8 additions & 0 deletions st2client/st2client/commands/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,20 @@ def __init__(self, resource, *args, **kwargs):
help='Only return ApiKeys belonging to the provided user')
self.parser.add_argument('-d', '--detail', action='store_true',
help='Full list of attributes.')
self.parser.add_argument('--show-secrets', action='store_true',
help='Full list of attributes.')

@resource.add_auth_token_to_kwargs_from_cli
def run(self, args, **kwargs):
filters = {}
filters['user'] = args.user
filters.update(**kwargs)
# show_secrets is not a filter but a query param. There is some special
# handling for filters in the get method which reuqires this odd hack.
if args.show_secrets:
params = filters.get('params', {})
params['show_secrets'] = True
filters['params'] = params
return self.manager.get_all(**filters)

def run_and_print(self, args, **kwargs):
Expand Down

0 comments on commit b762b3a

Please sign in to comment.