From ab50e471b976d2820b8f2ff30de1534491e79f3a Mon Sep 17 00:00:00 2001 From: asmita singh Date: Thu, 3 Oct 2019 10:38:48 +0000 Subject: [PATCH] Auth parameters for uploading vnf package from URI Upload VNF Package from uri REST API accepts userName and password parameter but internally these parameters are not used while getting the csar zip file from the URI as specified in the addressInformation. If the server serving the CSAR zip requires authentication, it would return 401 error and the uploading vnf package will fail to set the VNF package to onboarded status. This patch uses userName and password parameters to set "Authorization" header as shown below if these parameters are passed in the request body. * The userName and password are combined with a single colon (:). This means that the username itself cannot contain a colon. * The resulting string is encoded using a variant of Base64. * The authorization method (Basic and a space (e.g. "Basic ") is then prepended to the encoded string. For example, if userName and password is "xyz" and "xyzpassword", then the field's value is the base64-encoding of xyz:xyzpassword, or eHl6Onh5enBhc3N3b3Jk. Then the Authorization header will appear as: Authorization: Basic eHl6Onh5enBhc3N3b3Jk Change-Id: Ie79d5e4659951f41db4a1003950c649acab8b439 --- tacker/api/vnfpkgm/v1/controller.py | 18 ++---- tacker/common/utils.py | 14 +++++ tacker/conductor/conductor_server.py | 8 ++- tacker/glance_store/store.py | 23 +++++++- .../functional/vnfpkgm/test_vnf_package.py | 55 ++++++++++++++++++ tacker/tests/unit/common/test_utils.py | 21 +++++++ .../unit/conductor/test_conductor_server.py | 20 +++++++ tacker/tests/unit/glance_store/__init__.py | 0 tacker/tests/unit/glance_store/test_store.py | 58 +++++++++++++++++++ tacker/tests/unit/vnfpkgm/test_controller.py | 17 +++--- tacker/tests/utils.py | 56 +++++++++++++++++- 11 files changed, 262 insertions(+), 28 deletions(-) create mode 100644 tacker/tests/unit/glance_store/__init__.py create mode 100644 tacker/tests/unit/glance_store/test_store.py diff --git a/tacker/api/vnfpkgm/v1/controller.py b/tacker/api/vnfpkgm/v1/controller.py index afec2fc34..8c25c7769 100644 --- a/tacker/api/vnfpkgm/v1/controller.py +++ b/tacker/api/vnfpkgm/v1/controller.py @@ -20,7 +20,6 @@ from oslo_utils import excutils from oslo_utils import uuidutils import six from six.moves import http_client -from six.moves import urllib import webob import zipfile from zipfile import ZipFile @@ -30,6 +29,7 @@ from tacker.api.schemas import vnf_packages from tacker.api import validation from tacker.api.views import vnf_packages as vnf_packages_view from tacker.common import exceptions +from tacker.common import utils from tacker.conductor.conductorrpc import vnf_pkgm_rpc from tacker.glance_store import store as glance_store from tacker.objects import fields @@ -339,18 +339,12 @@ class VnfPkgmController(wsgi.Controller): context = request.environ['tacker.context'] context.can(vnf_package_policies.VNFPKGM % 'upload_from_uri') - vnf_package = self._get_vnf_package(id, request) - url = body['addressInformation'] - try: - data_iter = urllib.request.urlopen(url) - except Exception: - data_iter = None - msg = _("Failed to open URL %s") - raise webob.exc.HTTPBadRequest(explanation=msg % url) - finally: - if hasattr(data_iter, 'close'): - data_iter.close() + if not utils.is_valid_url(url): + msg = _("Vnf package url '%s' is invalid") % url + raise webob.exc.HTTPBadRequest(explanation=msg) + + vnf_package = self._get_vnf_package(id, request) if vnf_package.onboarding_state != \ fields.PackageOnboardingStateType.CREATED: diff --git a/tacker/common/utils.py b/tacker/common/utils.py index 2f8eb07a9..803e347e3 100644 --- a/tacker/common/utils.py +++ b/tacker/common/utils.py @@ -39,6 +39,7 @@ from oslo_log import log as logging from oslo_utils import excutils from oslo_utils import importutils from six.moves import urllib +from six.moves.urllib import parse as urlparse from stevedore import driver try: from eventlet import sleep @@ -419,6 +420,19 @@ def deepgetattr(obj, attr): return reduce(getattr, attr.split('.'), obj) +def is_valid_url(url): + url_parts = urlparse.urlparse(url) + + if not (url_parts.scheme and url_parts.netloc and url_parts.path): + return False + + schemes = ['http', 'https', 'ftp'] + if url_parts.scheme not in schemes: + return False + + return True + + class CooperativeReader(object): """An eventlet thread friendly class for reading in image data. diff --git a/tacker/conductor/conductor_server.py b/tacker/conductor/conductor_server.py index 393edf93e..d5b76301e 100644 --- a/tacker/conductor/conductor_server.py +++ b/tacker/conductor/conductor_server.py @@ -112,7 +112,8 @@ def revert_upload_vnf_package(function): *args, **kwargs) context = keyed_args['context'] vnf_package = keyed_args['vnf_package'] - if not isinstance(exp, exceptions.UploadFailedToGlanceStore): + if not (isinstance(exp, exceptions.UploadFailedToGlanceStore) + or isinstance(exp, exceptions.VNFPackageURLInvalid)): # Delete the csar file from the glance store. glance_store.delete_csar(context, vnf_package.id, vnf_package.location_glance_store) @@ -265,7 +266,10 @@ class Conductor(manager.Manager): def upload_vnf_package_from_uri(self, context, vnf_package, address_information, user_name=None, password=None): - body = {"address_information": address_information} + + body = {"address_information": address_information, + "user_name": user_name, + "password": password} (location, size, checksum, multihash, loc_meta) = glance_store.store_csar(context, vnf_package.id, body) diff --git a/tacker/glance_store/store.py b/tacker/glance_store/store.py index 59a0283d4..4e8c12323 100644 --- a/tacker/glance_store/store.py +++ b/tacker/glance_store/store.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import base64 import os import glance_store @@ -41,13 +42,18 @@ def get_csar_data_iter(body): try: if isinstance(body, dict): url = body['address_information'] - data_iter = urllib.request.urlopen(url) + req = urllib.request.Request(url) + if body['user_name'] is not None or body['password'] is not None: + _add_basic_auth(req, body['user_name'], body['password']) + data_iter = urllib.request.urlopen(req) else: data_iter = body return data_iter - except Exception: - LOG.warn("Failed to open csar URL: %s", url) + except Exception as e: + error = encodeutils.exception_to_unicode(e) + LOG.warn("Failed to open csar URL: %(url)s due to error: %(error)s", + {"url": url, "error": error}) raise exceptions.VNFPackageURLInvalid(url=url) @@ -137,3 +143,14 @@ def _get_csar_chunks(package_uuid, location, offset, chunk_size): LOG.exception("Failed to get csar data from glance store %(location)s" "for package %(uuid)s", {"location": location, "uuid": package_uuid}) raise exceptions.VnfPackageLocationInvalid(location=location) + + +def _add_basic_auth(request, username, password): + """A helper function to add basic authentication. + + This function adds basic authentication information to a six.moves.urllib + request. + """ + auth_str = base64.encodestring(('%s:%s' % ( + username, password)).encode()).decode().strip() + request.add_header('Authorization', 'Basic %s' % auth_str) diff --git a/tacker/tests/functional/vnfpkgm/test_vnf_package.py b/tacker/tests/functional/vnfpkgm/test_vnf_package.py index 84be61d34..b0a963523 100644 --- a/tacker/tests/functional/vnfpkgm/test_vnf_package.py +++ b/tacker/tests/functional/vnfpkgm/test_vnf_package.py @@ -175,6 +175,61 @@ class VnfPackageTest(base.BaseTackerTest): return vnf_package['id'] + def test_upload_from_uri_without_auth_and_delete(self): + csar_dir = self._get_csar_dir_path("sample_vnfpkg_no_meta_single_vnfd") + file_path, vnfd_id = utils.create_csar_with_unique_vnfd_id(csar_dir) + self.addCleanup(os.remove, file_path) + + cls_obj = utils.StaticHttpFileHandler(os.path.dirname(file_path)) + self.addCleanup(cls_obj.stop) + + body = jsonutils.dumps({"userDefinedData": {"foo": "bar"}}) + vnf_package = self._create_vnf_package(body) + csar_file_uri = 'http://localhost:{port}/{filename}'.format( + port=cls_obj.port, filename=os.path.basename(file_path)) + + body = jsonutils.dumps({"addressInformation": csar_file_uri}) + resp, resp_body = self.http_client.do_request( + '{base_path}/{id}/package_content/upload_from_uri'.format( + id=vnf_package['id'], + base_path=self.base_url), + "POST", body=body) + self.assertEqual(202, resp.status_code) + + self._wait_for_onboard(vnf_package['id']) + + self._disable_operational_state(vnf_package['id']) + self._delete_vnf_package(vnf_package['id']) + self._wait_for_delete(vnf_package['id']) + + def test_upload_from_uri_with_auth_and_delete(self): + csar_dir = self._get_csar_dir_path("sample_vnfpkg_no_meta_single_vnfd") + file_path, vnfd_id = utils.create_csar_with_unique_vnfd_id(csar_dir) + self.addCleanup(os.remove, file_path) + + cls_obj = utils.StaticHttpFileHandler(os.path.dirname(file_path)) + self.addCleanup(cls_obj.stop) + + body = jsonutils.dumps({"userDefinedData": {"foo": "bar"}}) + vnf_package = self._create_vnf_package(body) + csar_file_uri = 'http://localhost:{port}/{filename}'.format( + port=cls_obj.port, filename=os.path.basename(file_path)) + body = jsonutils.dumps({"addressInformation": csar_file_uri, + "userName": "username", + "password": "password"}) + resp, resp_body = self.http_client.do_request( + '{base_path}/{id}/package_content/upload_from_uri'.format( + id=vnf_package['id'], + base_path=self.base_url), + "POST", body=body) + self.assertEqual(202, resp.status_code) + + self._wait_for_onboard(vnf_package['id']) + + self._disable_operational_state(vnf_package['id']) + self._delete_vnf_package(vnf_package['id']) + self._wait_for_delete(vnf_package['id']) + def test_patch_in_onboarded_state(self): user_data = jsonutils.dumps( {"userDefinedData": {"key1": "val1", "key2": "val2", diff --git a/tacker/tests/unit/common/test_utils.py b/tacker/tests/unit/common/test_utils.py index 0ba7e93f8..139a086d7 100644 --- a/tacker/tests/unit/common/test_utils.py +++ b/tacker/tests/unit/common/test_utils.py @@ -90,3 +90,24 @@ class TestSnakeToCamelCase(testtools.TestCase): actual_val = utils.convert_snakecase_to_camelcase(data) expected_val = ["snake_case_value1", "snake_case_value2"] self.assertEqual(expected_val, actual_val) + + +class TestValidateUrl(testtools.TestCase): + def test_valid_url(self): + result = utils.is_valid_url("https://10.10.10.10/test.zip") + self.assertTrue(result) + + def test_no_scheme(self): + result = utils.is_valid_url("//10.10.10.10/test.zip") + self.assertFalse(result) + + def test_invalid_scheme(self): + result = utils.is_valid_url("invalid://10.10.10.10/test.zip") + self.assertFalse(result) + + def test_no_path_in_url(self): + # The specified url `https://10.10.10.10` is valid but in context with + # the functionality implemented, expecting csar file in `path` as + # mandatory parameter. + result = utils.is_valid_url("https://10.10.10.10") + self.assertFalse(result) diff --git a/tacker/tests/unit/conductor/test_conductor_server.py b/tacker/tests/unit/conductor/test_conductor_server.py index 758679fc1..608784836 100644 --- a/tacker/tests/unit/conductor/test_conductor_server.py +++ b/tacker/tests/unit/conductor/test_conductor_server.py @@ -20,6 +20,8 @@ import sys from glance_store import exceptions as store_exceptions import mock +from six.moves import urllib +import six.moves.urllib.error as urlerr import yaml from tacker.common import coordination @@ -484,3 +486,21 @@ class TestConductor(SqlTestCase): self.assertIn("Config option 'vnf_package_csar_path' is not configured" " correctly. VNF package CSAR path directory %s doesn't" " exist", mock_log_error.call_args[0][0]) + + @mock.patch.object(urllib.request, 'urlopen') + def test_upload_vnf_package_from_uri_with_invalid_auth(self, + mock_url_open): + address_information = "http://localhost/test.zip" + user_name = "username" + password = "password" + mock_url_open.side_effect = urlerr.HTTPError( + url='', code=401, msg='HTTP Error 401 Unauthorized', hdrs={}, + fp=None) + self.assertRaises(exceptions.VNFPackageURLInvalid, + self.conductor.upload_vnf_package_from_uri, + self.context, + self.vnf_package, + address_information, + user_name=user_name, + password=password) + self.assertEqual('CREATED', self.vnf_package.onboarding_state) diff --git a/tacker/tests/unit/glance_store/__init__.py b/tacker/tests/unit/glance_store/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tacker/tests/unit/glance_store/test_store.py b/tacker/tests/unit/glance_store/test_store.py new file mode 100644 index 000000000..f0fac2e58 --- /dev/null +++ b/tacker/tests/unit/glance_store/test_store.py @@ -0,0 +1,58 @@ +# Copyright (C) 2019 NTT DATA +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock + +import glance_store +from six.moves import urllib +import six.moves.urllib.error as urlerr + +from tacker.common import exceptions +import tacker.conf +from tacker.glance_store import store +from tacker.tests.unit import base + + +CONF = tacker.conf.CONF + + +class StoreBaseTest(base.TestCase): + + def setUp(self): + super(StoreBaseTest, self).setUp() + self.conf = CONF + glance_store.create_stores(self.conf) + self.body = {"address_information": "http://welcome.com/test.zip", + "user_name": "user1", "password": "pass1"} + + @mock.patch.object(urllib.request, 'urlopen') + def test_get_csar_data_iter_with_username_password(self, mock_url_open): + store.get_csar_data_iter(self.body) + mock_url_open.assert_called_once() + + @mock.patch.object(urllib.request, 'urlopen') + def test_get_csar_data_iter_without_username_password(self, mock_url_open): + body = {"address_information": "http://welcome.com/test.zip", + "user_name": None, "password": None} + store.get_csar_data_iter(body) + mock_url_open.assert_called_once() + + @mock.patch.object(urllib.request, 'urlopen') + def test_get_csar_data_iter_unauthorised(self, mock_url_open): + mock_url_open.side_effect = urlerr.HTTPError( + url='', code=401, msg='HTTP Error 401 Unauthorized', hdrs={}, + fp=None) + self.assertRaises(exceptions.VNFPackageURLInvalid, + store.get_csar_data_iter, self.body) diff --git a/tacker/tests/unit/vnfpkgm/test_controller.py b/tacker/tests/unit/vnfpkgm/test_controller.py index d0bc5cb66..e4a0d4725 100644 --- a/tacker/tests/unit/vnfpkgm/test_controller.py +++ b/tacker/tests/unit/vnfpkgm/test_controller.py @@ -589,7 +589,7 @@ class TestController(base.TestCase): mock_vnf_by_id, mock_upload_vnf_package_from_uri, mock_url_open): - body = {"addressInformation": "http://test_data.zip"} + body = {"addressInformation": "http://localhost/test_data.zip"} updates = {'onboarding_state': 'CREATED', 'operational_state': 'DISABLED'} vnf_package_dict = fakes.fake_vnf_package(updates) @@ -606,7 +606,7 @@ class TestController(base.TestCase): self.assertEqual(http_client.ACCEPTED, resp.status_code) def test_upload_vnf_package_from_uri_with_invalid_uuid(self): - body = {"addressInformation": "http://test_data.zip"} + body = {"addressInformation": "http://localhost/test_data.zip"} req = fake_request.HTTPRequest.blank( '/vnf_packages/%s/package_content/upload_from_uri' % constants.INVALID_UUID) @@ -619,7 +619,7 @@ class TestController(base.TestCase): def test_upload_vnf_package_from_uri_without_vnf_pack(self, mock_vnf_by_id, mock_url_open): - body = {"addressInformation": "http://test_data.zip"} + body = {"addressInformation": "http://localhost/test_data.zip"} msg = _("Can not find requested vnf package: %s") % constants.UUID mock_vnf_by_id.side_effect = exc.HTTPNotFound(explanation=msg) req = fake_request.HTTPRequest.blank( @@ -634,7 +634,7 @@ class TestController(base.TestCase): def test_upload_vnf_package_from_uri_with_invalid_status(self, mock_vnf_by_id, mock_url_open): - body = {"addressInformation": "http://test.zip"} + body = {"addressInformation": "http://localhost/test_data.zip"} vnf_obj = fakes.return_vnfpkg_obj() vnf_obj.__setattr__('onboarding_state', 'ONBOARDED') mock_vnf_by_id.return_value = vnf_obj @@ -645,11 +645,10 @@ class TestController(base.TestCase): self.controller.upload_vnf_package_from_uri, req, constants.UUID, body=body) - @mock.patch.object(vnf_package.VnfPackage, "get_by_id") - def test_upload_vnf_package_from_uri_with_invalid_url( - self, mock_vnf_by_id): - mock_vnf_by_id.return_value = fakes.return_vnfpkg_obj() - body = {"addressInformation": "http://test_data.zip"} + @ddt.data("http://test_data.zip", "xyz://github.com/abc/xyz.git", + "xyz://github.com/abc/xyz") + def test_upload_vnf_package_from_uri_with_invalid_url(self, invalid_url): + body = {"addressInformation": invalid_url} req = fake_request.HTTPRequest.blank( '/vnf_packages/%s/package_content/upload_from_uri' % constants.UUID) diff --git a/tacker/tests/utils.py b/tacker/tests/utils.py index 92d5f5252..fc0e6588c 100644 --- a/tacker/tests/utils.py +++ b/tacker/tests/utils.py @@ -12,13 +12,17 @@ # License for the specific language governing permissions and limitations # under the License. +import base64 +import http.server import os +import threading + +from oslo_utils import uuidutils +import socketserver import tempfile import yaml import zipfile -from oslo_utils import uuidutils - def read_file(input_file): yaml_file = os.path.abspath(os.path.join(os.path.dirname(__file__), @@ -99,3 +103,51 @@ def create_csar_with_unique_vnfd_id(csar_dir): zcsar.close() return tempname, unique_id + + +class AuthHandler(http.server.SimpleHTTPRequestHandler): + '''Main class to present webpages and authentication.''' + + def do_AUTHHEAD(self): + self.send_response(401) + self.send_header('WWW-Authenticate', 'Basic realm=\"Test\"') + self.send_header('Content-type', 'text/plain') + self.end_headers() + + def do_GET(self): + '''Present frontpage with user authentication.''' + global key + if 'Authorization' not in self.headers: + http.server.SimpleHTTPRequestHandler.do_GET(self) + elif self.headers.get('Authorization') is None: + self.do_AUTHHEAD() + self.wfile.write(bytes('no auth header received')) + elif self.headers.get('Authorization') == 'Basic ' + base64.b64encode( + b"username:password").decode("utf-8"): + http.server.SimpleHTTPRequestHandler.do_GET(self) + else: + self.do_AUTHHEAD() + self.wfile.write(bytes(self.headers.get('Authorization'))) + self.wfile.write(bytes('not authenticated')) + + +class StaticHttpFileHandler(object): + + def __init__(self, static_files_path): + if os.path.isabs(static_files_path): + web_dir = static_files_path + else: + web_dir = os.path.join(os.path.dirname(__file__), + static_files_path) + os.chdir(web_dir) + server_address = ('127.0.0.1', 0) + self.httpd = socketserver.TCPServer(server_address, AuthHandler) + self.port = self.httpd.socket.getsockname()[1] + + thread = threading.Thread(target=self.httpd.serve_forever) + thread.daemon = True + thread.start() + + def stop(self): + self.httpd.shutdown() + self.httpd.server_close()