Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master commit 0a96985ee73c163753d7ec5bd9367fd44345a383 Author: Valentin Gologuzov <vgologuz(a)redhat.com&gt; Date: Tue Sep 23 19:10:20 2014 +0200 [frontend] added helper function and flask filter which allows to ensure that url starts with either http or https, see config frontend/copr-frontend.spec | 4 + frontend/coprs_frontend/config/copr.conf | 6 + frontend/coprs_frontend/config/copr_devel.conf | 6 + frontend/coprs_frontend/config/copr_unit_test.conf | 3 + frontend/coprs_frontend/coprs/config.py | 10 ++ frontend/coprs_frontend/coprs/filters.py | 10 ++ frontend/coprs_frontend/coprs/helpers.py | 35 ++++++- .../coprs_frontend/coprs/templates/coprs/copr.repo | 2 +- .../coprs/templates/coprs/detail/overview.html | 3 +- .../coprs/views/api_ns/api_general.py | 9 +- .../coprs/views/coprs_ns/coprs_general.py | 12 ++- frontend/coprs_frontend/manage.py | 4 +- frontend/coprs_frontend/tests/test_helpers.py | 99 +++++++++++++++++++- .../test_views/test_coprs_ns/test_coprs_general.py | 20 +++- 14 files changed, 199 insertions(+), 24 deletions(-) diff --git a/frontend/copr-frontend.spec b/frontend/copr-frontend.spec index e30d70b..c72ad51 100644 --- a/frontend/copr-frontend.spec +++ b/frontend/copr-frontend.spec @@ -50,15 +50,18 @@ Requires: python-psycopg2 Requires: python-pylibravatar Requires: python-whoosh >= 2.5.3 Requires: pytz +Requires: python-six # for tests: Requires: pytest Requires: python-flexmock +Requires: python-mock Requires: python-decorator Requires: yum %if 0%{?rhel} < 7 && 0%{?rhel} > 0 BuildRequires: python-argparse %endif # check +BuildRequires: python-six BuildRequires: python-flask BuildRequires: python-flask-script BuildRequires: python-flask-sqlalchemy @@ -69,6 +72,7 @@ BuildRequires: python-flask-wtf BuildRequires: pytest BuildRequires: yum BuildRequires: python-flexmock +BuildRequires: python-mock BuildRequires: python-decorator BuildRequires: python-markdown BuildRequires: pytz diff --git a/frontend/coprs_frontend/config/copr.conf b/frontend/coprs_frontend/config/copr.conf index 4f70fe7..ffb0eac 100644 --- a/frontend/coprs_frontend/config/copr.conf +++ b/frontend/coprs_frontend/config/copr.conf @@ -38,6 +38,12 @@ SQLALCHEMY_ECHO = False # send emails when user's perms change in project? SEND_EMAILS = True +## post-process url leading to backend +## possible options: None, "http", "https" +# ENFORCE_PROTOCOL_FOR_BACKEND_URL = "https" +## post-process url leading to frontend +# ENFORCE_PROTOCOL_FOR_FRONTEND_URL = "https" + # Kerberos authentication configuration. Example configuration abbreviated # like 'RH' (should not changed once used, reflects what is stored in database). # This requires mod_auth_kerb package (Fedora/RHEL) installed on your frontend diff --git a/frontend/coprs_frontend/config/copr_devel.conf b/frontend/coprs_frontend/config/copr_devel.conf index eafc0b1..f161ca1 100644 --- a/frontend/coprs_frontend/config/copr_devel.conf +++ b/frontend/coprs_frontend/config/copr_devel.conf @@ -34,3 +34,9 @@ SQLALCHEMY_ECHO = True # send emails when user's perms change in project? SEND_EMAILS = True + +## post-process url leading to backend +## possible options: None, "http", "https" +ENFORCE_PROTOCOL_FOR_BACKEND_URL = "http" +## post-process url leading to frontend +ENFORCE_PROTOCOL_FOR_FRONTEND_URL = "http" diff --git a/frontend/coprs_frontend/config/copr_unit_test.conf b/frontend/coprs_frontend/config/copr_unit_test.conf index b8de251..85e12ae 100644 --- a/frontend/coprs_frontend/config/copr_unit_test.conf +++ b/frontend/coprs_frontend/config/copr_unit_test.conf @@ -34,3 +34,6 @@ WTF_CSRF_ENABLED = False # send emails when user's perms change in project? SEND_EMAILS = False + +ENFORCE_PROTOCOL_FOR_BACKEND_URL = "https" +ENFORCE_PROTOCOL_FOR_FRONTEND_URL = "https" diff --git a/frontend/coprs_frontend/coprs/config.py b/frontend/coprs_frontend/coprs/config.py index 14be052..15ef753 100644 --- a/frontend/coprs_frontend/coprs/config.py +++ b/frontend/coprs_frontend/coprs/config.py @@ -32,6 +32,12 @@ class Config(object): SEND_LEGAL_TO = ["root@localhost"] + # prefer http or https, default setting preserve old behaviour + USE_HTTPS_FOR_RESULTS = True + + ENFORCE_PROTOCOL_FOR_BACKEND_URL = "https" + ENFORCE_PROTOCOL_FOR_FRONTEND_URL = "https" + class ProductionConfig(Config): DEBUG = False @@ -44,6 +50,10 @@ class DevelopmentConfig(Config): DEBUG = True SQLALCHEMY_ECHO = True + ENFORCE_PROTOCOL_FOR_BACKEND_URL = "http" + ENFORCE_PROTOCOL_FOR_FRONTEND_URL = "http" + + class UnitTestConfig(Config): CSRF_ENABLED = False diff --git a/frontend/coprs_frontend/coprs/filters.py b/frontend/coprs_frontend/coprs/filters.py index 03e80cf..a27ffa4 100644 --- a/frontend/coprs_frontend/coprs/filters.py +++ b/frontend/coprs_frontend/coprs/filters.py @@ -130,3 +130,13 @@ def build_state_decoration(state): description = "This package has already been built previously" return description + + +(a)app.template_filter(&quot;fix_url_https_backend&quot;) +def fix_url_https_backend(url): + return helpers.fix_protocol_for_backend(url) + + +(a)app.template_filter(&quot;fix_url_https_frontend&quot;) +def fix_url_https_fronend(url): + return helpers.fix_protocol_for_frontend(url) diff --git a/frontend/coprs_frontend/coprs/helpers.py b/frontend/coprs_frontend/coprs/helpers.py index 653f4c5..1740da3 100644 --- a/frontend/coprs_frontend/coprs/helpers.py +++ b/frontend/coprs_frontend/coprs/helpers.py @@ -5,6 +5,7 @@ import urlparse import flask from coprs import constants +from coprs import app from rpmUtils.miscutils import splitFilename @@ -145,8 +146,10 @@ def parse_package_name(pkg): return pkg -def render_repo(copr, mock_chroot, url): - """ Render .repo file. No checks if copr or mock_chroot exists. """ +def generate_repo_url(mock_chroot, url): + """ Generates url with build results for .repo file. + No checks if copr or mock_chroot exists. + """ if mock_chroot.os_release == "fedora": if mock_chroot.os_version != "rawhide": mock_chroot.os_version = "$releasever" @@ -155,9 +158,33 @@ def render_repo(copr, mock_chroot, url): url, "{0}-{1}-{2}/".format(mock_chroot.os_release, mock_chroot.os_version, "$basearch")) - url = url.replace("http://", "https://") - return flask.render_template("coprs/copr.repo", copr=copr, url=url) + return url + + +def fix_protocol_for_backend(url): + """ + Ensure that url either has http or https protocol according to the + option in app config "ENFORCE_PROTOCOL_FOR_BACKEND_URL" + """ + if app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] == "https": + return url.replace("http://", "https://") + elif app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] == "http": + return url.replace("https://", "http://") + else: + return url + +def fix_protocol_for_frontend(url): + """ + Ensure that url either has http or https protocol according to the + option in app config "ENFORCE_PROTOCOL_FOR_FRONTEND_URL" + """ + if app.config["ENFORCE_PROTOCOL_FOR_FRONTEND_URL"] == "https": + return url.replace("http://", "https://") + elif app.config["ENFORCE_PROTOCOL_FOR_FRONTEND_URL"] == "http": + return url.replace("https://", "http://") + else: + return url class Serializer(object): diff --git a/frontend/coprs_frontend/coprs/templates/coprs/copr.repo b/frontend/coprs_frontend/coprs/templates/coprs/copr.repo index a2707fe..7f9f63a 100644 --- a/frontend/coprs_frontend/coprs/templates/coprs/copr.repo +++ b/frontend/coprs_frontend/coprs/templates/coprs/copr.repo @@ -1,6 +1,6 @@ [{{ copr.owner.name }}-{{ copr.name }}] name=Copr repo for {{ copr.name }} owned by {{ copr.owner.name }} -baseurl={{ url }} +baseurl={{ url | fix_url_https_backend }} skip_if_unavailable=True gpgcheck=0 enabled=1 diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/overview.html b/frontend/coprs_frontend/coprs/templates/coprs/detail/overview.html index 8c1f789..64afb47 100644 --- a/frontend/coprs_frontend/coprs/templates/coprs/detail/overview.html +++ b/frontend/coprs_frontend/coprs/templates/coprs/detail/overview.html @@ -62,9 +62,8 @@ coprname=copr.name, chroot=mock_chroot.os_release+"-"+mock_chroot.os_version, repofile=copr.owner.name+'-'+copr.name+'-'+mock_chroot.os_release+"-"+mock_chroot.os_version+'.repo', - _scheme='https', _external=True - )}}"> + )|fix_url_https_frontend}}"> {{ copr.owner.name }}-{{ copr.name }}-{{mock_chroot.os_release+"-"+mock_chroot.os_version}}.repo </a> </td> diff --git a/frontend/coprs_frontend/coprs/views/api_ns/api_general.py b/frontend/coprs_frontend/coprs/views/api_ns/api_general.py index 48f4d0b..3bb0351 100644 --- a/frontend/coprs_frontend/coprs/views/api_ns/api_general.py +++ b/frontend/coprs_frontend/coprs/views/api_ns/api_general.py @@ -8,6 +8,7 @@ from coprs import db from coprs import exceptions from coprs import forms from coprs import helpers +from coprs.helpers import fix_protocol_for_backend from coprs.logic.api_logic import BuildWrapper, MonitorWrapper from coprs.views.misc import login_required, api_login_required @@ -184,8 +185,8 @@ def api_coprs_by_owner(username=None): if build.results: for chroot in repo.active_chroots: release = release_tmpl.format(chroot=chroot) - yum_repos[release] = urlparse.urljoin( - build.results, release + '/') + yum_repos[release] = fix_protocol_for_backend( + urlparse.urljoin(build.results, release + '/')) break output["repos"].append({"name": repo.name, @@ -221,8 +222,8 @@ def api_coprs_by_owner_detail(username, coprname): if build.results: for chroot in copr.active_chroots: release = release_tmpl.format(chroot=chroot) - yum_repos[release] = urlparse.urljoin( - build.results, release + '/') + yum_repos[release] = fix_protocol_for_backend( + urlparse.urljoin(build.results, release + '/')) break output["detail"] = {"name": copr.name, "additional_repos": copr.repos, diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py index ed7d9ca..aba25c5 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py @@ -22,7 +22,7 @@ from coprs.views.coprs_ns import coprs_ns from coprs.logic import builds_logic from coprs.logic import coprs_logic -from coprs.helpers import parse_package_name, render_repo +from coprs.helpers import parse_package_name, generate_repo_url @coprs_ns.route("/", defaults={"page": 1}) @@ -525,13 +525,17 @@ def generate_repo_file(username, coprname, chroot, repofile): "Repository not initialized: No finished builds in {0}/{1}." .format(username, coprname)) - response = flask.make_response(render_repo(copr, mock_chroot, url)) + repo_url = generate_repo_url(mock_chroot, url) + response = flask.make_response( + flask.render_template("coprs/copr.repo", copr=copr, url=repo_url)) + response.mimetype = "text/plain" - response.headers["Content-Disposition"] = "filename={0}.repo".format( - reponame) + response.headers["Content-Disposition"] = \ + "filename={0}.repo".format(reponame) return response + @coprs_ns.route("/<username>/<coprname>/monitor/") def copr_build_monitor(username, coprname): try: diff --git a/frontend/coprs_frontend/manage.py b/frontend/coprs_frontend/manage.py index b8826c7..e23468d 100755 --- a/frontend/coprs_frontend/manage.py +++ b/frontend/coprs_frontend/manage.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/python import argparse import os @@ -22,7 +22,7 @@ class TestCommand(Command): if not (("COPR_CONFIG" in os.environ) and os.environ["COPR_CONFIG"]): os.environ["COPR_CONFIG"] = "/etc/copr/copr_unit_test.conf" os.environ["PYTHONPATH"] = "." - return subprocess.call(["py.test"] + (test_args or [])) + return subprocess.call(["/usr/bin/python", "-m", "pytest"] + (test_args or [])) option_list = ( Option("-a", diff --git a/frontend/coprs_frontend/tests/test_helpers.py b/frontend/coprs_frontend/tests/test_helpers.py index 6851bf6..df019da 100644 --- a/frontend/coprs_frontend/tests/test_helpers.py +++ b/frontend/coprs_frontend/tests/test_helpers.py @@ -1,4 +1,14 @@ -from coprs.helpers import parse_package_name +from copy import deepcopy +import six + +if six.PY3: + from unittest import mock +else: + import mock + +from coprs import app +from coprs.helpers import parse_package_name, generate_repo_url, \ + fix_protocol_for_frontend, fix_protocol_for_backend from tests.coprs_test_case import CoprsTestCase @@ -25,3 +35,90 @@ class TestHelpers(CoprsTestCase): for pkg, expected in EXP.iteritems(): assert parse_package_name(pkg) == expected + + def test_generate_repo_url(self): + test_sets = [] + http_url = "http://example.com/repo" + https_url = "https://example.com/repo" + + mock_chroot = mock.MagicMock() + mock_chroot.os_release = "fedora" + mock_chroot.os_version = "20" + + test_sets.extend([ + dict(args=(mock_chroot, http_url), + expected="http://example.com/fedora-$releasever-$basearch/"), + dict(args=(mock_chroot, https_url), + expected="https://example.com/fedora-$releasever-$basearch/")]) + + m2 = deepcopy(mock_chroot) + m2.os_version = "rawhide" + + test_sets.extend([ + dict(args=(m2, http_url), + expected="http://example.com/fedora-rawhide-$basearch/"), + dict(args=(m2, https_url), + expected="https://example.com/fedora-rawhide-$basearch/")]) + + m3 = deepcopy(mock_chroot) + m3.os_release = "rhel7" + m3.os_version = "7.1" + + test_sets.extend([ + dict(args=(m3, http_url), + expected="http://example.com/rhel7-7.1-$basearch/"), + dict(args=(m3, https_url), + expected="https://example.com/rhel7-7.1-$basearch/")]) + + app.config["USE_HTTPS_FOR_RESULTS"] = True + for test_set in test_sets: + result = generate_repo_url(*test_set["args"]) + assert result == test_set["expected"] + + def test_fix_protocol_for_backend(self): + http_url = "http://example.com/repo" + https_url = "https://example.com/repo" + + orig = app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] + print("Orig: {}".format(orig)) + try: + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = "https" + assert fix_protocol_for_backend(https_url) == https_url + assert fix_protocol_for_backend(http_url) == https_url + + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = "http" + assert fix_protocol_for_backend(https_url) == http_url + assert fix_protocol_for_backend(http_url) == http_url + + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = None + assert fix_protocol_for_backend(https_url) == https_url + assert fix_protocol_for_backend(http_url) == http_url + + except Exception as e: + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = orig + raise e + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = orig + + + def test_fix_protocol_for_frontend(self): + http_url = "http://example.com/repo" + https_url = "https://example.com/repo" + + orig = app.config["ENFORCE_PROTOCOL_FOR_FRONTEND_URL"] + try: + app.config["ENFORCE_PROTOCOL_FOR_FRONTEND_URL"] = "https" + assert fix_protocol_for_frontend(https_url) == https_url + assert fix_protocol_for_frontend(http_url) == https_url + + app.config["ENFORCE_PROTOCOL_FOR_FRONTEND_URL"] = "http" + assert fix_protocol_for_frontend(https_url) == http_url + assert fix_protocol_for_frontend(http_url) == http_url + + app.config["ENFORCE_PROTOCOL_FOR_FRONTEND_URL"] = None + assert fix_protocol_for_frontend(https_url) == https_url + assert fix_protocol_for_frontend(http_url) == http_url + + except Exception as e: + app.config["ENFORCE_PROTOCOL_FOR_FRONTEND_URL"] = orig + raise e + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = orig diff --git a/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py b/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py index a180c7d..07808ed 100644 --- a/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py +++ b/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py @@ -618,9 +618,17 @@ class TestCoprRepoGeneration(CoprsTestCase): def test_works_on_older_builds(self, f_users, f_coprs, f_custom_builds, f_db): - r = self.tc.get( - "/coprs/{0}/{1}/repo/fedora-18-x86_64/" - .format(self.u1.name, self.c1.name)) - - assert r.status_code == 200 - assert "baseurl=https://bar.baz" in r.data + from coprs import app + orig = app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] + try: + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = "https" + r = self.tc.get( + "/coprs/{0}/{1}/repo/fedora-18-x86_64/" + .format(self.u1.name, self.c1.name)) + + assert r.status_code == 200 + assert "baseurl=https://bar.baz" in r.data + except Exception as e: + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = orig + raise e + app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = orig