openstack/nova
Code Issues Proposed changes

Reduce scope of 'path' query parameter to noVNC consoles

Browse Source 
This is a partial revert of commit
9606c80402 which added the 'path' query
parameter to work with noVNC v1.1.0. This broke all other console types
using websockify server (serial, spice) because the websockify server
itself doesn't know how to handle the 'path' query parameter. It is the
noVNC vnc_lite.html file which parses the 'path' variable and uses it
as the url to the websockify server. So, all other console types should
*not* be generating a console access url with a 'path' query parameter,
only noVNC.

Closes-Bug: #1845243

TODO(melwitt): Figure out how to test serial and/or spice console in
the gate

Change-Id: I9521f21a685edc44121d75bdf534c201fa87c2d7
This commit is contained in:
melanie witt 2019-09-27 02:10:22 +00:00
parent 207d2c2253
commit 54125a75fb
10 changed files with 53 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/api_samples/os-remote-consoles/get-rdp-console-post-resp.json
View File

@ -1,6 +1,6 @@
{
"console": {
"type": "rdp-html5",
"url": "http://127.0.0.1:6083/?path=%3Ftoken%3D21efbb20-b84c-4d1f-807d-4e14f6884b7f"
"url": "http://127.0.0.1:6083/?token=191996c3-7b0f-42f3-95a7-f1839f2da6ed"
}
}
}

4
doc/api_samples/os-remote-consoles/get-serial-console-post-resp.json
View File

@ -1,6 +1,6 @@
{
"console": {
"type": "serial",
"url": "ws://127.0.0.1:6083/?path=%3Ftoken%3D6ac46b4c-2705-4d8b-baa3-1b6f1b0c7dd3"
"url":"ws://127.0.0.1:6083/?token=f9906a48-b71e-4f18-baca-c987da3ebdb3"
}
}
}

2
doc/api_samples/os-remote-consoles/get-spice-console-post-resp.json
View File

@ -1,6 +1,6 @@
{
"console": {
"type": "spice-html5",
"url": "http://127.0.0.1:6082/spice_auto.html?path=%3Ftoken%3Da7bd9607-421c-44b9-8689-18e87ada2f78"
"url": "http://127.0.0.1:6082/spice_auto.html?token=a30e5d08-6a20-4043-958f-0852440c6af4"
}
}

14
nova/objects/console_auth_token.py
View File

@ -64,9 +64,17 @@ class ConsoleAuthToken(base.NovaTimestampObject, base.NovaObject):
specific to this authorization.
"""
if self.obj_attr_is_set('id'):
qparams = {'path': '?token=%s' % self.token}
return '%s?%s' % (self.access_url_base,
urlparse.urlencode(qparams))
if self.console_type == 'novnc':
# NOTE(melwitt): As of noVNC v1.1.0, we must use the 'path'
# query parameter to pass the auth token within, as the
# top-level 'token' query parameter was removed. The 'path'
# parameter is supported in older noVNC versions, so it is
# backward compatible.
qparams = {'path': '?token=%s' % self.token}
return '%s?%s' % (self.access_url_base,
urlparse.urlencode(qparams))
else:
return '%s?token=%s' % (self.access_url_base, self.token)
@staticmethod
def _from_db_object(context, obj, db_obj):

2
nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-rdp-console-post-resp.json.tpl
View File

@ -1,6 +1,6 @@
{
"console": {
"type": "rdp-html5",
"url": "http://127.0.0.1:6083/?path=%%3Ftoken%%3D%(uuid)s"
"url": "http://127.0.0.1:6083/?token=%(uuid)s"
}
}

2
nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-serial-console-post-resp.json.tpl
View File

@ -1,6 +1,6 @@
{
"console": {
"type": "serial",
"url": "ws://127.0.0.1:6083/?path=%%3Ftoken%%3D%(uuid)s"
"url": "ws://127.0.0.1:6083/?token=%(uuid)s"
}
}

2
nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-spice-console-post-resp.json.tpl
View File

@ -1,6 +1,6 @@
{
"console": {
"type": "spice-html5",
"url": "http://127.0.0.1:6082/spice_auto.html?path=%%3Ftoken%%3D%(uuid)s"
"url": "http://127.0.0.1:6082/spice_auto.html?token=%(uuid)s"
}
}

4
nova/tests/functional/api_sample_tests/test_console_auth_tokens.py
View File

@ -15,7 +15,6 @@
import re
from oslo_serialization import jsonutils
import six.moves.urllib.parse as urlparse
from nova.tests.functional.api_sample_tests import test_servers
@ -33,8 +32,7 @@ class ConsoleAuthTokensSampleJsonTests(test_servers.ServersSampleBase):
{'action': 'os-getRDPConsole'})
url = self._get_console_url(response.content)
path = urlparse.urlencode({'path': '?token='})
return re.match('.+?%s([^&]+)' % path, url).groups()[0]
return re.match('.+?token=([^&]+)', url).groups()[0]
def test_get_console_connect_info(self):
self.flags(enabled=True, group='rdp')

30
nova/tests/unit/console/test_websocketproxy.py
View File

@ -19,7 +19,6 @@ import socket
import mock
from oslo_utils.fixture import uuidsentinel as uuids
import six.moves.urllib.parse as urlparse
import nova.conf
from nova.console.securityproxy import base
@ -48,7 +47,6 @@ class NovaProxyRequestHandlerDBTestCase(test.TestCase):
self.wh.msg = mock.MagicMock()
self.wh.do_proxy = mock.MagicMock()
self.wh.headers = mock.MagicMock()
self.path = urlparse.urlencode({'path': '?token=123-456-789'})
def _fake_console_db(self, **updates):
console_db = copy.deepcopy(fake_ca.fake_token_dict)
@ -97,7 +95,7 @@ class NovaProxyRequestHandlerDBTestCase(test.TestCase):
tsock.recv.return_value = "HTTP/1.1 200 OK\r\n\r\n"
self.wh.socket.return_value = tsock
self.wh.path = "http://127.0.0.1/?%s" % self.path
self.wh.path = "http://127.0.0.1/?token=123-456-789"
self.wh.headers = self.fake_header
if instance_not_found:
@ -143,8 +141,6 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
self.wh.msg = mock.MagicMock()
self.wh.do_proxy = mock.MagicMock()
self.wh.headers = mock.MagicMock()
self.path = urlparse.urlencode({'path': '?token=123-456-789'})
self.path_invalid = urlparse.urlencode({'path': '?token=XXX'})
fake_header = {
'cookie': 'token="123-456-789"',
@ -215,7 +211,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
validate.return_value = objects.ConsoleAuthToken(**params)
self.wh.socket.return_value = '<socket>'
self.wh.path = "http://127.0.0.1/?%s" % self.path
self.wh.path = "http://127.0.0.1/?token=123-456-789"
self.wh.headers = self.fake_header
self.wh.new_websocket_client()
@ -240,7 +236,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
validate.return_value = objects.ConsoleAuthToken(**params)
self.wh.socket.return_value = '<socket>'
self.wh.path = "http://[2001:db8::1]/?%s" % self.path
self.wh.path = "http://[2001:db8::1]/?token=123-456-789"
self.wh.headers = self.fake_header_ipv6
self.wh.new_websocket_client()
@ -253,7 +249,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
def test_new_websocket_client_token_invalid(self, validate):
validate.side_effect = exception.InvalidToken(token='XXX')
self.wh.path = "http://127.0.0.1/?%s" % self.path_invalid
self.wh.path = "http://127.0.0.1/?token=XXX"
self.wh.headers = self.fake_header_bad_token
self.assertRaises(exception.InvalidToken,
@ -281,7 +277,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
tsock.recv.return_value = "HTTP/1.1 200 OK\r\n\r\n"
self.wh.socket.return_value = tsock
self.wh.path = "http://127.0.0.1/?%s" % self.path
self.wh.path = "http://127.0.0.1/?token=123-456-789"
self.wh.headers = self.fake_header
self.wh.new_websocket_client()
@ -314,7 +310,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
tsock.recv.return_value = "HTTP/1.1 500 Internal Server Error\r\n\r\n"
self.wh.socket.return_value = tsock
self.wh.path = "http://127.0.0.1/?%s" % self.path
self.wh.path = "http://127.0.0.1/?token=123-456-789"
self.wh.headers = self.fake_header
self.assertRaises(exception.InvalidConnectionInfo,
@ -346,7 +342,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
HTTP_RESP]
self.wh.socket.return_value = tsock
self.wh.path = "http://127.0.0.1/?%s" % self.path
self.wh.path = "http://127.0.0.1/?token=123-456-789"
self.wh.headers = self.fake_header
self.wh.new_websocket_client()
@ -376,7 +372,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
validate.return_value = objects.ConsoleAuthToken(**params)
self.wh.socket.return_value = '<socket>'
self.wh.path = "http://127.0.0.1/?%s" % self.path
self.wh.path = "http://127.0.0.1/?token=123-456-789"
self.wh.headers = self.fake_header
self.wh.new_websocket_client()
@ -388,9 +384,10 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
@mock.patch('socket.getfqdn')
def test_address_string_doesnt_do_reverse_dns_lookup(self, getfqdn):
request_mock = mock.MagicMock()
msg = 'GET /vnc.html?%s HTTP/1.1\r\n' % self.path
request_mock.makefile().readline.side_effect = [msg.encode('utf-8'),
b'']
request_mock.makefile().readline.side_effect = [
b'GET /vnc.html?token=123-456-789 HTTP/1.1\r\n',
b''
]
server_mock = mock.MagicMock()
client_address = ('8.8.8.8', 54321)
@ -644,8 +641,7 @@ class NovaWebsocketSecurityProxyTestCase(test.NoDBTestCase):
with mock.patch('websockify.ProxyRequestHandler'):
self.wh = websocketproxy.NovaProxyRequestHandler()
self.wh.server = self.server
path = urlparse.urlencode({'path': '?token=123-456-789'})
self.wh.path = "http://127.0.0.1/?%s" % path
self.wh.path = "http://127.0.0.1/?token=123-456-789"
self.wh.socket = mock.MagicMock()
self.wh.msg = mock.MagicMock()
self.wh.do_proxy = mock.MagicMock()

25
nova/tests/unit/objects/test_console_auth_token.py
View File

@ -30,7 +30,7 @@ from nova.tests.unit.objects import test_objects
class _TestConsoleAuthToken(object):
@mock.patch('nova.db.api.console_auth_token_create')
def test_authorize(self, mock_create):
def _test_authorize(self, console_type, mock_create):
# the expires time is calculated from the current time and
# a ttl value in the object. Fix the current time so we can
# test expires is calculated correctly as expected
@ -41,10 +41,12 @@ class _TestConsoleAuthToken(object):
db_dict = copy.deepcopy(fakes.fake_token_dict)
db_dict['expires'] = expires
db_dict['console_type'] = console_type
mock_create.return_value = db_dict
create_dict = copy.deepcopy(fakes.fake_token_dict)
create_dict['expires'] = expires
create_dict['console_type'] = console_type
del create_dict['id']
del create_dict['created_at']
del create_dict['updated_at']
@ -53,10 +55,11 @@ class _TestConsoleAuthToken(object):
del expected['token_hash']
del expected['expires']
expected['token'] = fakes.fake_token
expected['console_type'] = console_type
obj = token_obj.ConsoleAuthToken(
context=self.context,
console_type=fakes.fake_token_dict['console_type'],
console_type=console_type,
host=fakes.fake_token_dict['host'],
port=fakes.fake_token_dict['port'],
internal_access_path=fakes.fake_token_dict['internal_access_path'],
@ -71,11 +74,23 @@ class _TestConsoleAuthToken(object):
self.compare_obj(obj, expected)
url = obj.access_url
path = urlparse.urlencode({'path': '?token=%s' % fakes.fake_token})
expected_url = '%s?%s' % (
fakes.fake_token_dict['access_url_base'], path)
if console_type != 'novnc':
expected_url = '%s?token=%s' % (
fakes.fake_token_dict['access_url_base'],
fakes.fake_token)
else:
path = urlparse.urlencode({'path': '?token=%s' % fakes.fake_token})
expected_url = '%s?%s' % (
fakes.fake_token_dict['access_url_base'],
path)
self.assertEqual(expected_url, url)
def test_authorize(self):
self._test_authorize(fakes.fake_token_dict['console_type'])
def test_authorize_novnc(self):
self._test_authorize('novnc')
@mock.patch('nova.db.api.console_auth_token_create')
def test_authorize_duplicate_token(self, mock_create):
mock_create.side_effect = DBDuplicateEntry()