openstack
/
ironic-inspector
Code Issues Proposed changes

Properly implement authentication via keystonemiddleware 

This requires introducing new option 'identity_uri'.

(cherry picked from commit e7e9e0b6f9)

Conflicts:
	README.rst
	ironic_discoverd/main.py
	ironic_discoverd/test/test_main.py
	ironic_discoverd/utils.py

Closes-Bug: #1427125
Change-Id: Ibed9cf163357c7b38f6e9a980900bcc1d9d12804
This commit is contained in:
Dmitry Tantsur 2015-03-02 19:04:13 +01:00
parent aa653bede4
commit fbd8127f36
9 changed files with 96 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
README.rst
View File

@ -93,8 +93,9 @@ Configuration
Copy ``example.conf`` to some permanent place
(``/etc/ironic-discoverd/discoverd.conf`` is what is used in the RPM).
Fill in at least configuration values with names starting with *os_*.
They configure how **ironic-discoverd** authenticates with Keystone.
Fill in at least configuration values with names starting with ``os_`` and
``identity_uri``. They configure how **ironic-discoverd** authenticates
with Keystone and checks authentication of clients.
Also set *database* option to where you want **ironic-discoverd** SQLite
database to be placed.
@ -155,6 +156,7 @@ Here is *discoverd.conf* you may end up with::
[discoverd]
debug = false
identity_uri = http://127.0.0.1:35357
os_auth_url = http://127.0.0.1:5000/v2.0
os_username = admin
os_password = password
@ -327,6 +329,7 @@ Action required:
* Fill in ``database`` option in the configuration file before upgrading.
* Stop relying on **ironic-discoverd** setting maintenance mode itself.
* Stop relying on ``discovery_timestamp`` node extra field.
* Fill in ``identity_uri`` field in the configuration.
Action recommended:

12
example.conf
View File

@ -3,15 +3,15 @@
; Authentication options are mandatory and don't have reasonable defaults.
; Keystone authentication endpoint.
;os_auth_url =
; User name for accessing Ironic API.
;os_auth_url = http://127.0.0.1:5000/v2.0
; User name for accessing Keystone and Ironic API.
;os_username =
; Password for accessing Ironic API.
; Password for accessing Keystone and Ironic API.
;os_password =
; Tenant name for accessing Ironic API.
; Tenant name for accessing Keystone and Ironic API.
;os_tenant_name =
; Admin tenant name for checking if the keystone token has the admin role
;admin_tenant_name = 'admin'
; Keystone admin endpoint.
;identity_uri = http://127.0.0.1:35357
; Number of attempts to do when trying to connect to Ironic on start up.
;ironic_retry_attempts = 5

2
functest/run.py
View File

@ -125,7 +125,7 @@ class Test(base.NodeTest):
self.assertEqual({'finished': True, 'error': None}, status)
@mock.patch.object(utils, 'check_is_admin')
@mock.patch.object(utils, 'check_auth')
@mock.patch.object(utils, 'get_client')
def run(client_mock, keystone_mock):
d = tempfile.mkdtemp()

3
ironic_discoverd/conf.py
View File

@ -19,7 +19,8 @@ from six.moves import configparser
# TODO(dtantsur): switch to oslo.db
DEFAULTS = {
# Keystone credentials
'admin_tenant_name': 'admin',
'os_auth_url': 'http://127.0.0.1:5000/v2.0',
'identity_uri': 'http://127.0.0.1:35357',
# Ironic and Keystone connection settings
'ironic_retry_attempts': '5',
'ironic_retry_period': '5',

32
ironic_discoverd/main.py
View File

@ -21,7 +21,6 @@ import logging
import sys
import flask
from keystoneclient import exceptions
from ironic_discoverd import conf
from ironic_discoverd import firewall
@ -35,21 +34,6 @@ app = flask.Flask(__name__)
LOG = logging.getLogger('ironic_discoverd.main')
def check_auth():
"""Check whether request is properly authenticated."""
if not conf.getboolean('discoverd', 'authenticate'):
return
if not flask.request.headers.get('X-Auth-Token'):
LOG.error("No X-Auth-Token header, rejecting request")
raise utils.Error('Authentication required', code=401)
try:
utils.check_is_admin(token=flask.request.headers['X-Auth-Token'])
except exceptions.Unauthorized as exc:
LOG.error("Keystone denied access: %s, rejecting request", exc)
raise utils.Error('Access denied', code=403)
def convert_exceptions(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
@ -74,7 +58,7 @@ def api_continue():
@app.route('/v1/introspection/<uuid>', methods=['GET', 'POST'])
@convert_exceptions
def api_introspection(uuid):
check_auth()
utils.check_auth(flask.request)
if flask.request.method == 'POST':
setup_ipmi_credentials = flask.request.args.get(
@ -93,7 +77,8 @@ def api_introspection(uuid):
@app.route('/v1/discover', methods=['POST'])
@convert_exceptions
def api_discover():
check_auth()
utils.check_auth(flask.request)
data = flask.request.get_json(force=True)
LOG.debug("/v1/discover got JSON %s", data)
@ -155,7 +140,9 @@ def config_shim(args):
def init():
if not conf.getboolean('discoverd', 'authenticate'):
if conf.getboolean('discoverd', 'authenticate'):
utils.add_auth_middleware(app)
else:
LOG.warning('Starting unauthenticated, please check configuration')
node_cache.init()
@ -186,9 +173,10 @@ def main(): # pragma: no cover
debug = conf.getboolean('discoverd', 'debug')
logging.basicConfig(level=logging.DEBUG if debug else logging.INFO)
logging.getLogger('urllib3.connectionpool').setLevel(logging.WARNING)
logging.getLogger('requests.packages.urllib3.connectionpool').setLevel(
logging.WARNING)
for third_party in ('urllib3.connectionpool',
'keystonemiddleware.auth_token',
'requests.packages.urllib3.connectionpool'):
logging.getLogger(third_party).setLevel(logging.WARNING)
logging.getLogger('ironicclient.common.http').setLevel(
logging.INFO if debug else logging.ERROR)

17
ironic_discoverd/test/test_main.py
View File

@ -15,7 +15,6 @@ import json
import unittest
import eventlet
from keystoneclient import exceptions as keystone_exc
import mock
from ironic_discoverd import conf
@ -61,24 +60,16 @@ class TestApi(test_base.BaseTest):
introspect_mock.assert_called_once_with("uuid1",
setup_ipmi_credentials=False)
@mock.patch.object(introspect, 'introspect', autospec=True)
def test_introspect_missing_authentication(self, introspect_mock):
conf.CONF.set('discoverd', 'authenticate', 'true')
res = self.app.post('/v1/introspection/uuid1')
self.assertEqual(401, res.status_code)
self.assertFalse(introspect_mock.called)
@mock.patch.object(utils, 'check_is_admin', autospec=True)
@mock.patch.object(utils, 'check_auth', autospec=True)
@mock.patch.object(introspect, 'introspect', autospec=True)
def test_introspect_failed_authentication(self, introspect_mock,
keystone_mock):
auth_mock):
conf.CONF.set('discoverd', 'authenticate', 'true')
keystone_mock.side_effect = keystone_exc.Unauthorized()
res = self.app.post('/v1/introspection/uuid1',
auth_mock.side_effect = utils.Error('Boom', code=403)
res = self.app.post('/v1/introspection/uuid',
headers={'X-Auth-Token': 'token'})
self.assertEqual(403, res.status_code)
self.assertFalse(introspect_mock.called)
keystone_mock.assert_called_once_with(token='token')
@mock.patch.object(introspect, 'introspect', autospec=True)
def test_discover(self, discover_mock):

59
ironic_discoverd/test/test_utils.py
View File

@ -15,7 +15,7 @@ import unittest
import eventlet
from ironicclient import exceptions
from keystoneclient import exceptions as keystone_exc
from keystonemiddleware import auth_token
import mock
from ironic_discoverd import conf
@ -23,25 +23,46 @@ from ironic_discoverd.test import base
from ironic_discoverd import utils
class TestCheckIsAdmin(base.BaseTest):
@mock.patch('keystoneclient.v2_0.client.Client')
def test_admin_token(self, mock_ks):
conf.CONF.set('discoverd', 'os_auth_url', '127.0.0.1')
fake_client = mock_ks.return_value
mockAdmin = mock.Mock()
mockAdmin.name = 'admin'
fake_client.roles.roles_for_user.return_value = [mockAdmin]
utils.check_is_admin('token')
class TestCheckAuth(base.BaseTest):
def setUp(self):
super(TestCheckAuth, self).setUp()
conf.CONF.set('discoverd', 'authenticate', 'true')
@mock.patch('keystoneclient.v2_0.client.Client')
def test_non_admin_token(self, mock_ks):
conf.CONF.set('discoverd', 'os_auth_url', '127.0.0.1')
fake_client = mock_ks.return_value
mockMember = mock.Mock()
mockMember.name = 'member'
fake_client.roles.roles_for_user.return_value = [mockMember]
self.assertRaises(keystone_exc.Unauthorized,
utils.check_is_admin, 'token')
@mock.patch.object(auth_token, 'AuthProtocol')
def test_middleware(self, mock_auth):
conf.CONF.set('discoverd', 'os_username', 'admin')
conf.CONF.set('discoverd', 'os_tenant_name', 'admin')
conf.CONF.set('discoverd', 'os_password', 'password')
app = mock.Mock(wsgi_app=mock.sentinel.app)
utils.add_auth_middleware(app)
mock_auth.assert_called_once_with(
mock.sentinel.app,
{'admin_user': 'admin', 'admin_tenant_name': 'admin',
'admin_password': 'password', 'delay_auth_decision': True,
'auth_uri': 'http://127.0.0.1:5000/v2.0',
'identity_uri': 'http://127.0.0.1:35357'}
)
def test_ok(self):
request = mock.Mock(headers={'X-Identity-Status': 'Confirmed',
'X-Roles': 'admin,member'})
utils.check_auth(request)
def test_invalid(self):
request = mock.Mock(headers={'X-Identity-Status': 'Invalid'})
self.assertRaises(utils.Error, utils.check_auth, request)
def test_not_admin(self):
request = mock.Mock(headers={'X-Identity-Status': 'Confirmed',
'X-Roles': 'member'})
self.assertRaises(utils.Error, utils.check_auth, request)
def test_disabled(self):
conf.CONF.set('discoverd', 'authenticate', 'false')
request = mock.Mock(headers={'X-Identity-Status': 'Invalid'})
utils.check_auth(request)
@mock.patch.object(eventlet.greenthread, 'sleep', lambda _: None)

43
ironic_discoverd/utils.py
View File

@ -17,8 +17,7 @@ import re
import eventlet
from ironicclient import client
from ironicclient import exceptions
from keystoneclient import exceptions as keystone_exc
from keystoneclient.v2_0 import client as keystone
from keystonemiddleware import auth_token
import six
from ironic_discoverd import conf
@ -26,6 +25,8 @@ from ironic_discoverd import conf
LOG = logging.getLogger('ironic_discoverd.utils')
OS_ARGS = ('os_password', 'os_username', 'os_auth_url', 'os_tenant_name')
MIDDLEWARE_ARGS = ('admin_password', 'admin_user', 'auth_uri',
'admin_tenant_name')
RETRY_COUNT = 12
RETRY_DELAY = 5
@ -45,22 +46,32 @@ def get_client(): # pragma: no cover
return client.get_client(1, **args)
def check_is_admin(token):
"""Check whether the token is from a user with the admin role.
def add_auth_middleware(app):
"""Add authentication middleware to Flask application.
:param token: Keystone authentication token.
:raises: keystoneclient.exceptions.Unauthorized if the user does not have
the admin role in the tenant provided in the admin_tenant_name option.
:param app: application.
"""
kc = keystone.Client(token=token,
tenant_name=conf.get('discoverd',
'admin_tenant_name'),
auth_url=conf.get('discoverd', 'os_auth_url'))
if "admin" not in [role.name
for role in kc.roles.roles_for_user(
kc.user_id,
tenant=kc.tenant_id)]:
raise keystone_exc.Unauthorized()
auth_conf = {key: conf.get('discoverd', value)
for (key, value) in zip(MIDDLEWARE_ARGS, OS_ARGS)}
auth_conf['delay_auth_decision'] = True
auth_conf['identity_uri'] = conf.get('discoverd', 'identity_uri')
app.wsgi_app = auth_token.AuthProtocol(app.wsgi_app, auth_conf)
def check_auth(request):
"""Check authentication on request.
:param request: Flask request
:raises: utils.Error if access is denied
"""
if not conf.getboolean('discoverd', 'authenticate'):
return
if request.headers.get('X-Identity-Status').lower() == 'invalid':
raise Error('Authentication required', code=401)
roles = (request.headers.get('X-Roles') or '').split(',')
if 'admin' not in roles:
LOG.error('Role "admin" not in user role list %s', roles)
raise Error('Access denied', code=403)
def is_valid_mac(address):

1
requirements.txt
View File

@ -1,5 +1,6 @@
eventlet>=0.15.1,<0.16.0
Flask>=0.10,<1.0
keystonemiddleware>=1.0.0
python-ironicclient>=0.2.1
python-keystoneclient>=0.10.0
requests>=1.2.1,!=2.4.0