From 4bd03cb8f60a0b597b9945a264505b855545106c Mon Sep 17 00:00:00 2001 From: Lars Karlitski Date: Fri, 31 May 2019 18:35:01 +0200 Subject: [PATCH] Don't send CORS headers These are meant for web applications that are accessed by browsers, not REST APIs. --- src/pylorax/api/__init__.py | 3 - src/pylorax/api/crossdomain.py | 64 ---------------- src/pylorax/api/server.py | 2 - src/pylorax/api/v0.py | 36 --------- test/check-api | 5 +- tests/pylorax/test_crossdomain.py | 120 ------------------------------ 6 files changed, 4 insertions(+), 226 deletions(-) delete mode 100644 src/pylorax/api/crossdomain.py delete mode 100644 tests/pylorax/test_crossdomain.py diff --git a/src/pylorax/api/__init__.py b/src/pylorax/api/__init__.py index 4ca8ccb8..60a620a0 100644 --- a/src/pylorax/api/__init__.py +++ b/src/pylorax/api/__init__.py @@ -15,6 +15,3 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from pylorax.api.crossdomain import crossdomain - -__all__ = ["crossdomain"] diff --git a/src/pylorax/api/crossdomain.py b/src/pylorax/api/crossdomain.py deleted file mode 100644 index 74bfcaaa..00000000 --- a/src/pylorax/api/crossdomain.py +++ /dev/null @@ -1,64 +0,0 @@ -# -# Copyright (C) 2017 Red Hat, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# - -# crossdomain decorator from - http://flask.pocoo.org/snippets/56/ -from datetime import timedelta -from flask import make_response, request, current_app -from functools import update_wrapper - - -def crossdomain(origin, methods=None, headers=None, - max_age=21600, attach_to_all=True, - automatic_options=True): - if methods is not None: - methods = ', '.join(sorted(x.upper() for x in methods)) - if headers is not None and not isinstance(headers, str): - headers = ', '.join(x.upper() for x in headers) - if not isinstance(origin, list): - origin = [origin] - if isinstance(max_age, timedelta): - max_age = int(max_age.total_seconds()) - - def get_methods(): - if methods is not None: - return methods - - options_resp = current_app.make_default_options_response() - return options_resp.headers['allow'] - - def decorator(f): - def wrapped_function(*args, **kwargs): - if automatic_options and request.method == 'OPTIONS': - resp = current_app.make_default_options_response() - else: - resp = make_response(f(*args, **kwargs)) - if not attach_to_all and request.method != 'OPTIONS': - return resp - - h = resp.headers - - h.extend([("Access-Control-Allow-Origin", orig) for orig in origin]) - h['Access-Control-Allow-Methods'] = get_methods() - h['Access-Control-Max-Age'] = str(max_age) - if headers is not None: - h['Access-Control-Allow-Headers'] = headers - return resp - - f.provide_automatic_options = False - f.required_methods = ['OPTIONS'] - return update_wrapper(wrapped_function, f) - return decorator diff --git a/src/pylorax/api/server.py b/src/pylorax/api/server.py index c3bbf257..0c0c81f9 100644 --- a/src/pylorax/api/server.py +++ b/src/pylorax/api/server.py @@ -24,7 +24,6 @@ import os import werkzeug from pylorax import vernum -from pylorax.api.crossdomain import crossdomain from pylorax.api.errors import HTTP_ERROR from pylorax.api.v0 import v0_api from pylorax.sysutils import joinpaths @@ -54,7 +53,6 @@ def api_docs(path=None): return send_from_directory(docs_path, path) @server.route("/api/status") -@crossdomain(origin="*") def v0_status(): """ `/api/v0/status` diff --git a/src/pylorax/api/v0.py b/src/pylorax/api/v0.py index 081ca14f..565f810e 100644 --- a/src/pylorax/api/v0.py +++ b/src/pylorax/api/v0.py @@ -998,7 +998,6 @@ import pytoml as toml from pylorax.sysutils import joinpaths from pylorax.api.checkparams import checkparams from pylorax.api.compose import start_build, compose_types -from pylorax.api.crossdomain import crossdomain from pylorax.api.errors import * # pylint: disable=wildcard-import from pylorax.api.projects import projects_list, projects_info, projects_depsolve from pylorax.api.projects import modules_list, modules_info, ProjectsError, repo_to_source @@ -1048,7 +1047,6 @@ def blueprint_exists(api, branch, blueprint_name): def v0_api(api): # Note that Sphinx will not generate documentations for any of these. @api.route("/api/v0/blueprints/list") - @crossdomain(origin="*") def v0_blueprints_list(): """List the available blueprints on a branch.""" branch = request.args.get("branch", "master") @@ -1068,7 +1066,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/info", defaults={'blueprint_names': ""}) @api.route("/api/v0/blueprints/info/") - @crossdomain(origin="*") @checkparams([("blueprint_names", "", "no blueprint names given")]) def v0_blueprints_info(blueprint_names): """Return the contents of the blueprint, or a list of blueprints""" @@ -1142,7 +1139,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/changes", defaults={'blueprint_names': ""}) @api.route("/api/v0/blueprints/changes/") - @crossdomain(origin="*") @checkparams([("blueprint_names", "", "no blueprint names given")]) def v0_blueprints_changes(blueprint_names): """Return the changes to a blueprint or list of blueprints""" @@ -1182,7 +1178,6 @@ def v0_api(api): return jsonify(blueprints=blueprints, errors=errors, offset=offset, limit=limit) @api.route("/api/v0/blueprints/new", methods=["POST"]) - @crossdomain(origin="*") def v0_blueprints_new(): """Commit a new blueprint""" branch = request.args.get("branch", "master") @@ -1212,7 +1207,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/delete", defaults={'blueprint_name': ""}, methods=["DELETE"]) @api.route("/api/v0/blueprints/delete/", methods=["DELETE"]) - @crossdomain(origin="*") @checkparams([("blueprint_name", "", "no blueprint name given")]) def v0_blueprints_delete(blueprint_name): """Delete a blueprint from git""" @@ -1234,7 +1228,6 @@ def v0_api(api): return jsonify(status=True) @api.route("/api/v0/blueprints/workspace", methods=["POST"]) - @crossdomain(origin="*") def v0_blueprints_workspace(): """Write a blueprint to the workspace""" branch = request.args.get("branch", "master") @@ -1260,7 +1253,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/workspace", defaults={'blueprint_name': ""}, methods=["DELETE"]) @api.route("/api/v0/blueprints/workspace/", methods=["DELETE"]) - @crossdomain(origin="*") @checkparams([("blueprint_name", "", "no blueprint name given")]) def v0_blueprints_delete_workspace(blueprint_name): """Delete a blueprint from the workspace""" @@ -1283,7 +1275,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/undo", defaults={'blueprint_name': "", 'commit': ""}, methods=["POST"]) @api.route("/api/v0/blueprints/undo/", defaults={'commit': ""}, methods=["POST"]) @api.route("/api/v0/blueprints/undo//", methods=["POST"]) - @crossdomain(origin="*") @checkparams([("blueprint_name", "", "no blueprint name given"), ("commit", "", "no commit ID given")]) def v0_blueprints_undo(blueprint_name, commit): @@ -1310,7 +1301,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/tag", defaults={'blueprint_name': ""}, methods=["POST"]) @api.route("/api/v0/blueprints/tag/", methods=["POST"]) - @crossdomain(origin="*") @checkparams([("blueprint_name", "", "no blueprint name given")]) def v0_blueprints_tag(blueprint_name): """Tag a blueprint's latest blueprint commit as a 'revision'""" @@ -1337,7 +1327,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/diff/", defaults={'from_commit': "", 'to_commit': ""}) @api.route("/api/v0/blueprints/diff//", defaults={'to_commit': ""}) @api.route("/api/v0/blueprints/diff///") - @crossdomain(origin="*") @checkparams([("blueprint_name", "", "no blueprint name given"), ("from_commit", "", "no from commit ID given"), ("to_commit", "", "no to commit ID given")]) @@ -1388,7 +1377,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/freeze", defaults={'blueprint_names': ""}) @api.route("/api/v0/blueprints/freeze/") - @crossdomain(origin="*") @checkparams([("blueprint_names", "", "no blueprint names given")]) def v0_blueprints_freeze(blueprint_names): """Return the blueprint with the exact modules and packages selected by depsolve""" @@ -1455,7 +1443,6 @@ def v0_api(api): @api.route("/api/v0/blueprints/depsolve", defaults={'blueprint_names': ""}) @api.route("/api/v0/blueprints/depsolve/") - @crossdomain(origin="*") @checkparams([("blueprint_names", "", "no blueprint names given")]) def v0_blueprints_depsolve(blueprint_names): """Return the dependencies for a blueprint""" @@ -1520,7 +1507,6 @@ def v0_api(api): return jsonify(blueprints=blueprints, errors=errors) @api.route("/api/v0/projects/list") - @crossdomain(origin="*") def v0_projects_list(): """List all of the available projects/packages""" try: @@ -1541,7 +1527,6 @@ def v0_api(api): @api.route("/api/v0/projects/info", defaults={'project_names': ""}) @api.route("/api/v0/projects/info/") - @crossdomain(origin="*") @checkparams([("project_names", "", "no project names given")]) def v0_projects_info(project_names): """Return detailed information about the listed projects""" @@ -1564,7 +1549,6 @@ def v0_api(api): @api.route("/api/v0/projects/depsolve", defaults={'project_names': ""}) @api.route("/api/v0/projects/depsolve/") - @crossdomain(origin="*") @checkparams([("project_names", "", "no project names given")]) def v0_projects_depsolve(project_names): """Return detailed information about the listed projects""" @@ -1586,7 +1570,6 @@ def v0_api(api): return jsonify(projects=deps) @api.route("/api/v0/projects/source/list") - @crossdomain(origin="*") def v0_projects_source_list(): """Return the list of source names""" with api.config["DNFLOCK"].lock: @@ -1596,7 +1579,6 @@ def v0_api(api): @api.route("/api/v0/projects/source/info", defaults={'source_names': ""}) @api.route("/api/v0/projects/source/info/") - @crossdomain(origin="*") @checkparams([("source_names", "", "no source names given")]) def v0_projects_source_info(source_names): """Return detailed info about the list of sources""" @@ -1633,7 +1615,6 @@ def v0_api(api): return jsonify(sources=sources, errors=errors) @api.route("/api/v0/projects/source/new", methods=["POST"]) - @crossdomain(origin="*") def v0_projects_source_new(): """Add a new package source. Or change an existing one""" if request.headers['Content-Type'] == "text/x-toml": @@ -1694,7 +1675,6 @@ def v0_api(api): @api.route("/api/v0/projects/source/delete", defaults={'source_name': ""}, methods=["DELETE"]) @api.route("/api/v0/projects/source/delete/", methods=["DELETE"]) - @crossdomain(origin="*") @checkparams([("source_name", "", "no source name given")]) def v0_projects_source_delete(source_name): """Delete the named source and return a status response""" @@ -1725,7 +1705,6 @@ def v0_api(api): @api.route("/api/v0/modules/list") @api.route("/api/v0/modules/list/") - @crossdomain(origin="*") def v0_modules_list(module_names=None): """List available modules, filtering by module_names""" if module_names and VALID_API_STRING.match(module_names) is None: @@ -1757,7 +1736,6 @@ def v0_api(api): @api.route("/api/v0/modules/info", defaults={'module_names': ""}) @api.route("/api/v0/modules/info/") - @crossdomain(origin="*") @checkparams([("module_names", "", "no module names given")]) def v0_modules_info(module_names): """Return detailed information about the listed modules""" @@ -1778,7 +1756,6 @@ def v0_api(api): return jsonify(modules=modules) @api.route("/api/v0/compose", methods=["POST"]) - @crossdomain(origin="*") def v0_compose_start(): """Start a compose @@ -1836,7 +1813,6 @@ def v0_api(api): return jsonify(status=True, build_id=build_id) @api.route("/api/v0/compose/types") - @crossdomain(origin="*") def v0_compose_types(): """Return the list of enabled output types @@ -1846,26 +1822,22 @@ def v0_api(api): return jsonify(types=[{"name": k, "enabled": True} for k in compose_types(share_dir)]) @api.route("/api/v0/compose/queue") - @crossdomain(origin="*") def v0_compose_queue(): """Return the status of the new and running queues""" return jsonify(queue_status(api.config["COMPOSER_CFG"])) @api.route("/api/v0/compose/finished") - @crossdomain(origin="*") def v0_compose_finished(): """Return the list of finished composes""" return jsonify(finished=build_status(api.config["COMPOSER_CFG"], "FINISHED")) @api.route("/api/v0/compose/failed") - @crossdomain(origin="*") def v0_compose_failed(): """Return the list of failed composes""" return jsonify(failed=build_status(api.config["COMPOSER_CFG"], "FAILED")) @api.route("/api/v0/compose/status", defaults={'uuids': ""}) @api.route("/api/v0/compose/status/") - @crossdomain(origin="*") @checkparams([("uuids", "", "no UUIDs given")]) def v0_compose_status(uuids): """Return the status of the listed uuids""" @@ -1909,7 +1881,6 @@ def v0_api(api): @api.route("/api/v0/compose/cancel", defaults={'uuid': ""}, methods=["DELETE"]) @api.route("/api/v0/compose/cancel/", methods=["DELETE"]) - @crossdomain(origin="*") @checkparams([("uuid", "", "no UUID given")]) def v0_compose_cancel(uuid): """Cancel a running compose and delete its results directory""" @@ -1932,7 +1903,6 @@ def v0_api(api): @api.route("/api/v0/compose/delete", defaults={'uuids': ""}, methods=["DELETE"]) @api.route("/api/v0/compose/delete/", methods=["DELETE"]) - @crossdomain(origin="*") @checkparams([("uuids", "", "no UUIDs given")]) def v0_compose_delete(uuids): """Delete the compose results for the listed uuids""" @@ -1958,7 +1928,6 @@ def v0_api(api): @api.route("/api/v0/compose/info", defaults={'uuid': ""}) @api.route("/api/v0/compose/info/") - @crossdomain(origin="*") @checkparams([("uuid", "", "no UUID given")]) def v0_compose_info(uuid): """Return detailed info about a compose""" @@ -1977,7 +1946,6 @@ def v0_api(api): @api.route("/api/v0/compose/metadata", defaults={'uuid': ""}) @api.route("/api/v0/compose/metadata/") - @crossdomain(origin="*") @checkparams([("uuid","", "no UUID given")]) def v0_compose_metadata(uuid): """Return a tar of the metadata for the build""" @@ -1997,7 +1965,6 @@ def v0_api(api): @api.route("/api/v0/compose/results", defaults={'uuid': ""}) @api.route("/api/v0/compose/results/") - @crossdomain(origin="*") @checkparams([("uuid","", "no UUID given")]) def v0_compose_results(uuid): """Return a tar of the metadata and the results for the build""" @@ -2017,7 +1984,6 @@ def v0_api(api): @api.route("/api/v0/compose/logs", defaults={'uuid': ""}) @api.route("/api/v0/compose/logs/") - @crossdomain(origin="*") @checkparams([("uuid","", "no UUID given")]) def v0_compose_logs(uuid): """Return a tar of the metadata for the build""" @@ -2037,7 +2003,6 @@ def v0_api(api): @api.route("/api/v0/compose/image", defaults={'uuid': ""}) @api.route("/api/v0/compose/image/") - @crossdomain(origin="*") @checkparams([("uuid","", "no UUID given")]) def v0_compose_image(uuid): """Return the output image for the build""" @@ -2063,7 +2028,6 @@ def v0_api(api): @api.route("/api/v0/compose/log", defaults={'uuid': ""}) @api.route("/api/v0/compose/log/") - @crossdomain(origin="*") @checkparams([("uuid","", "no UUID given")]) def v0_compose_log_tail(uuid): """Return the end of the main anaconda.log, defaults to 1Mbytes""" diff --git a/test/check-api b/test/check-api index 92a228fb..68b61e5e 100755 --- a/test/check-api +++ b/test/check-api @@ -43,9 +43,12 @@ class TestApi(composertest.ComposerTestCase): # # API status without depsolve errors # - status = self.request("GET", "/api/status").json() + r = self.request("GET", "/api/status") + self.assertEqual(r.status_code, 200) + status = r.json() self.assertEqual(status.keys(), { "build", "api", "db_version", "schema_version", "db_supported", "backend", "msgs" }) self.assertEqual(status["msgs"], []) + self.assertEqual(r.headers.keys(), { "Content-Type", "Content-Length", "Date" }) # # HTTP errors should return json responses diff --git a/tests/pylorax/test_crossdomain.py b/tests/pylorax/test_crossdomain.py deleted file mode 100644 index 74ed5201..00000000 --- a/tests/pylorax/test_crossdomain.py +++ /dev/null @@ -1,120 +0,0 @@ -# -# Copyright (C) 2018 Red Hat, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# -import unittest -from flask import Flask -from datetime import timedelta - -from pylorax.api.crossdomain import crossdomain - - -server = Flask(__name__) - -@server.route('/01') -@crossdomain(origin='*', methods=['GET']) -def hello_world_01(): - return 'Hello, World!' - - -@server.route('/02') -@crossdomain(origin='*', headers=['TESTING']) -def hello_world_02(): - return 'Hello, World!' - -@server.route('/03') -@crossdomain(origin='*', max_age=timedelta(days=7)) -def hello_world_03(): - return 'Hello, World!' - - -@server.route('/04') -@crossdomain(origin='*', attach_to_all=False) -def hello_world_04(): - return 'Hello, World!' - - -@server.route('/05') -@crossdomain(origin='*', automatic_options=False) -def hello_world_05(): - return 'Hello, World!' - - -@server.route('/06') -@crossdomain(origin=['https://redhat.com', 'http://weldr.io']) -def hello_world_06(): - return 'Hello, World!' - - -class CrossdomainTest(unittest.TestCase): - @classmethod - def setUpClass(self): - self.server = server.test_client() - - def test_01_with_methods_specified(self): - # first send a preflight request to check what methods are allowed - response = self.server.options("/01") - self.assertEqual(200, response.status_code) - self.assertIn('GET', response.headers['Access-Control-Allow-Methods']) - - # then try to issue a POST request which isn't allowed - response = self.server.post("/01") - self.assertEqual(405, response.status_code) - - def test_02_with_headers_specified(self): - response = self.server.get("/02") - self.assertEqual(200, response.status_code) - self.assertEqual(b'Hello, World!', response.data) - - self.assertEqual('TESTING', response.headers['Access-Control-Allow-Headers']) - - def test_03_with_max_age_as_timedelta(self): - response = self.server.get("/03") - self.assertEqual(200, response.status_code) - self.assertEqual(b'Hello, World!', response.data) - - expected_max_age = int(timedelta(days=7).total_seconds()) - actual_max_age = int(response.headers['Access-Control-Max-Age']) - self.assertEqual(expected_max_age, actual_max_age) - - def test_04_attach_to_all_false(self): - response = self.server.get("/04") - self.assertEqual(200, response.status_code) - self.assertEqual(b'Hello, World!', response.data) - - # when attach_to_all is False the decorator will not assign - # the Access-Control-* headers to the response - for header, _ in response.headers: - self.assertFalse(header.startswith('Access-Control-')) - - - def test_05_options_request(self): - response = self.server.options("/05") - self.assertEqual(200, response.status_code) - self.assertEqual(b'Hello, World!', response.data) - - # Not always in the same order, so test individually - for m in ["HEAD", "OPTIONS", "GET"]: - self.assertIn(m, response.headers['Access-Control-Allow-Methods']) - - - def test_06_with_origin_as_list(self): - response = self.server.get("/06") - self.assertEqual(200, response.status_code) - self.assertEqual(b'Hello, World!', response.data) - - for header, value in response.headers: - if header == 'Access-Control-Allow-Origin': - self.assertIn(value, ['https://redhat.com', 'http://weldr.io'])