From 7cb34df4fadad6960abf3ca7d1831cdf4d7eba89 Mon Sep 17 00:00:00 2001 From: Kyle Robbertze Date: Tue, 19 Jan 2021 16:23:50 +0200 Subject: [PATCH 1/3] Use force_ssl in python apps Fixes: #957 --- .../api_clients/api_clients/api_client.py | 27 ++++++++++++----- python_apps/pypo/pypo/pypofile.py | 30 +++++++++---------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/python_apps/api_clients/api_clients/api_client.py b/python_apps/api_clients/api_clients/api_client.py index d983c5c6c..b2296e607 100644 --- a/python_apps/api_clients/api_clients/api_client.py +++ b/python_apps/api_clients/api_clients/api_client.py @@ -10,7 +10,7 @@ import sys import time import urllib.request, urllib.error, urllib.parse import requests -import socket +import socket import logging import json import base64 @@ -67,6 +67,16 @@ api_config['api_base'] = 'api' api_config['bin_dir'] = '/usr/lib/airtime/api_clients/' api_config['update_metadata_on_tunein'] = 'update-metadata-on-tunein/api_key/%%api_key%%' +def get_protocol(config): + port = config.get('general', 'base_port', fallback=80) + if config.getboolean('general', 'force_ssl', fallback=False): + protocol = 'https' + else: + try: + protocol = config.get(CONFIG_SECTION, 'protocol') + except NoOptionError as e: + protocol = str(("http", "https")[int(port) == 443]) + return protocol ################################################################################ @@ -169,9 +179,11 @@ class RequestProvider(object): self.requests = {} if self.config["general"]["base_dir"].startswith("/"): self.config["general"]["base_dir"] = self.config["general"]["base_dir"][1:] + protocol = get_protocol(self.config) + self.url = ApcUrl("%s://%s:%s/%s%s/%s" \ - % (str(("http", "https")[int(self.config["general"]["base_port"]) == 443]), - self.config["general"]["base_url"], str(self.config["general"]["base_port"]), + % (protocol, self.config["general"]["base_url"], + str(self.config["general"]["base_port"]), self.config["general"]["base_dir"], self.config["api_base"], '%%action%%')) # Now we must discover the possible actions @@ -208,7 +220,7 @@ class AirtimeApiClient(object): def __get_airtime_version(self): try: return self.services.version_url()['airtime_version'] except Exception: return -1 - + def __get_api_version(self): try: return self.services.version_url()['api_version'] except Exception: return -1 @@ -331,8 +343,9 @@ class AirtimeApiClient(object): # TODO : Make other methods in this class use this this method. if self.config["general"]["base_dir"].startswith("/"): self.config["general"]["base_dir"] = self.config["general"]["base_dir"][1:] + protocol = get_protocol(self.config) url = "%s://%s:%s/%s%s/%s" % \ - (str(("http", "https")[int(self.config["general"]["base_port"]) == 443]), + (protocol, self.config["general"]["base_url"], str(self.config["general"]["base_port"]), self.config["general"]["base_dir"], self.config["api_base"], self.config[config_action_key]) @@ -343,9 +356,9 @@ class AirtimeApiClient(object): """Constructs the base url for RESTful requests""" if self.config["general"]["base_dir"].startswith("/"): self.config["general"]["base_dir"] = self.config["general"]["base_dir"][1:] + protocol = get_protocol(self.config) url = "%s://%s:@%s:%s/%s/%s" % \ - (str(("http", "https")[int(self.config["general"]["base_port"]) == 443]), - self.config["general"]["api_key"], + (protocol, self.config["general"]["api_key"], self.config["general"]["base_url"], str(self.config["general"]["base_port"]), self.config["general"]["base_dir"], self.config[config_action_key]) diff --git a/python_apps/pypo/pypo/pypofile.py b/python_apps/pypo/pypo/pypofile.py index 3edf2c2d5..6b12e67bc 100644 --- a/python_apps/pypo/pypo/pypofile.py +++ b/python_apps/pypo/pypo/pypofile.py @@ -67,14 +67,14 @@ class PypoFile(Thread): CONFIG_SECTION = "general" username = self._config.get(CONFIG_SECTION, 'api_key') baseurl = self._config.get(CONFIG_SECTION, 'base_url') - try: - port = self._config.get(CONFIG_SECTION, 'base_port') - except NoOptionError as e: - port = 80 - try: - protocol = self._config.get(CONFIG_SECTION, 'protocol') - except NoOptionError as e: - protocol = str(("http", "https")[int(port) == 443]) + port = self._config.get(CONFIG_SECTION, 'base_port', 80) + if self._config.getboolean(CONFIG_SECTION, 'force_ssl', fallback=False): + protocol = 'https' + else: + try: + protocol = self._config.get(CONFIG_SECTION, 'protocol') + except NoOptionError as e: + protocol = str(("http", "https")[int(port) == 443]) try: host = [protocol, baseurl, port] @@ -84,15 +84,15 @@ class PypoFile(Thread): media_item["id"]) with open(dst, "wb") as handle: response = requests.get(url, auth=requests.auth.HTTPBasicAuth(username, ''), stream=True, verify=False) - + if not response.ok: self.logger.error(response) raise Exception("%s - Error occurred downloading file" % response.status_code) - + for chunk in response.iter_content(1024): if not chunk: break - + handle.write(chunk) #make file world readable and owner writable @@ -160,11 +160,11 @@ class PypoFile(Thread): """ Remove this media_item from the dictionary. On the next iteration - (from the main function) we won't consider it for prioritization + (from the main function) we won't consider it for prioritization anymore. If on the next iteration we have received a new schedule, - it is very possible we will have to deal with the same media_items + it is very possible we will have to deal with the same media_items again. In this situation, the worst possible case is that we try to - copy the file again and realize we already have it (thus aborting the copy). + copy the file again and realize we already have it (thus aborting the copy). """ del schedule[highest_priority] @@ -179,7 +179,7 @@ class PypoFile(Thread): logging.debug("Failed to open config file at %s: %s" % (config_path, e.strerror)) sys.exit() except Exception as e: - logging.debug(e.strerror) + logging.debug(e.strerror) sys.exit() return config From 0f9b6ffab68f9e7376afe5e78cad312e68a994fe Mon Sep 17 00:00:00 2001 From: Kyle Robbertze Date: Thu, 21 Jan 2021 09:49:15 +0200 Subject: [PATCH 2/3] remove known bug note on reverse proxy set up --- docs/_docs/reverse-proxy.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/_docs/reverse-proxy.md b/docs/_docs/reverse-proxy.md index 30ea5bae4..0104efb13 100644 --- a/docs/_docs/reverse-proxy.md +++ b/docs/_docs/reverse-proxy.md @@ -31,10 +31,8 @@ containers. ### Setup -There are known bugs when using LibreTime behind a reverse proxy ([#957](https://github.com/LibreTime/libretime/issues/957) -tracks the issue and contains a temporary workaround). For SSL redirection to work, you -need two domains: one for LibreTime and one for Icecast. Here, these will be -`libretime.example.com` and `icecast.example.com`. +For SSL redirection to work, you need two domains: one for LibreTime and one for Icecast. +Here, these will be `libretime.example.com` and `icecast.example.com`. You will also require two VMs, servers or containers. Alternatively the reverse proxy can be located on the server, proxying connections to containers also on the host. Setting up From 6b37b3dbab32fea631fbb25edd5ffd17d567f898 Mon Sep 17 00:00:00 2001 From: Kyle Robbertze Date: Fri, 22 Jan 2021 22:13:31 +0200 Subject: [PATCH 3/3] fully test set protocol --- .../api_clients/api_clients/api_client.py | 11 +-- python_apps/api_clients/tests/test_apcurl.py | 2 +- .../api_clients/tests/test_get_protocol.py | 75 +++++++++++++++++++ .../api_clients/tests/test_requestprovider.py | 3 + 4 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 python_apps/api_clients/tests/test_get_protocol.py diff --git a/python_apps/api_clients/api_clients/api_client.py b/python_apps/api_clients/api_clients/api_client.py index b2296e607..f9e72e4cd 100644 --- a/python_apps/api_clients/api_clients/api_client.py +++ b/python_apps/api_clients/api_clients/api_client.py @@ -68,13 +68,14 @@ api_config['bin_dir'] = '/usr/lib/airtime/api_clients/' api_config['update_metadata_on_tunein'] = 'update-metadata-on-tunein/api_key/%%api_key%%' def get_protocol(config): - port = config.get('general', 'base_port', fallback=80) - if config.getboolean('general', 'force_ssl', fallback=False): + positive_values = ['Yes', 'yes', 'True', 'true', True] + port = config['general'].get('base_port', 80) + force_ssl = config['general'].get('force_ssl', False) + if force_ssl in positive_values: protocol = 'https' else: - try: - protocol = config.get(CONFIG_SECTION, 'protocol') - except NoOptionError as e: + protocol = config['general'].get('protocol') + if not protocol: protocol = str(("http", "https")[int(port) == 443]) return protocol diff --git a/python_apps/api_clients/tests/test_apcurl.py b/python_apps/api_clients/tests/test_apcurl.py index e3c8b29d5..78e9fef56 100644 --- a/python_apps/api_clients/tests/test_apcurl.py +++ b/python_apps/api_clients/tests/test_apcurl.py @@ -5,7 +5,7 @@ class TestApcUrl(unittest.TestCase): def test_init(self): url = "/testing" u = ApcUrl(url) - self.assertEqual( u.base_url, url) + self.assertEqual(u.base_url, url) def test_params_1(self): u = ApcUrl("/testing/%%key%%") diff --git a/python_apps/api_clients/tests/test_get_protocol.py b/python_apps/api_clients/tests/test_get_protocol.py new file mode 100644 index 000000000..c19e7a9b5 --- /dev/null +++ b/python_apps/api_clients/tests/test_get_protocol.py @@ -0,0 +1,75 @@ +import unittest +import configparser +from api_clients.api_client import get_protocol + +def get_force_ssl(value, useConfigParser): + config = {} + if useConfigParser: + config = configparser.ConfigParser() + config['general'] = { + 'base_port': 80, + 'force_ssl': value, + } + return get_protocol(config) + +class TestGetProtocol(unittest.TestCase): + def test_dict_config_empty_http(self): + config = {'general': {}} + protocol = get_protocol(config) + self.assertEqual(protocol, 'http') + + def test_dict_config_http(self): + config = { + 'general': { + 'base_port': 80, + }, + } + protocol = get_protocol(config) + self.assertEqual(protocol, 'http') + + def test_dict_config_https(self): + config = { + 'general': { + 'base_port': 443, + }, + } + protocol = get_protocol(config) + self.assertEqual(protocol, 'https') + + def test_dict_config_force_https(self): + postive_values = ['yes', 'Yes', 'True', 'true', True] + negative_values = ['no', 'No', 'False', 'false', False] + for value in postive_values: + self.assertEqual(get_force_ssl(value, False), 'https') + for value in negative_values: + self.assertEqual(get_force_ssl(value, False), 'http') + + def test_configparser_config_empty_http(self): + config = configparser.ConfigParser() + config['general'] = {} + protocol = get_protocol(config) + self.assertEqual(protocol, 'http') + + def test_configparser_config_http(self): + config = configparser.ConfigParser() + config['general'] = { + 'base_port': 80, + } + protocol = get_protocol(config) + self.assertEqual(protocol, 'http') + + def test_configparser_config_https(self): + config = configparser.ConfigParser() + config['general'] = { + 'base_port': 443, + } + protocol = get_protocol(config) + self.assertEqual(protocol, 'https') + + def test_configparser_config_force_https(self): + postive_values = ['yes', 'Yes', 'True', 'true', True] + negative_values = ['no', 'No', 'False', 'false', False] + for value in postive_values: + self.assertEqual(get_force_ssl(value, True), 'https') + for value in negative_values: + self.assertEqual(get_force_ssl(value, True), 'http') diff --git a/python_apps/api_clients/tests/test_requestprovider.py b/python_apps/api_clients/tests/test_requestprovider.py index c210aad85..6c91a1947 100644 --- a/python_apps/api_clients/tests/test_requestprovider.py +++ b/python_apps/api_clients/tests/test_requestprovider.py @@ -13,11 +13,14 @@ class TestRequestProvider(unittest.TestCase): self.cfg['general']['base_url'] = 'localhost' self.cfg['general']['api_key'] = 'TEST_KEY' self.cfg['api_base'] = 'api' + def test_test(self): self.assertTrue('general' in self.cfg) + def test_init(self): rp = RequestProvider(self.cfg) self.assertTrue( len( rp.available_requests() ) > 0 ) + def test_contains(self): rp = RequestProvider(self.cfg) methods = ['upload_recorded', 'update_media_url', 'list_all_db_files']