From b614a81f5b477edf1c9c72a3948b85915dcbb7a0 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Fri, 3 Aug 2018 15:46:19 -0400
Subject: [PATCH] Return a JSON error instead of a 404 on certain malformed
 URLs.

This handles the case where a route is requested, but without a required
parameter.  So, /blueprints/info is requested instead of
/blueprints/info/http-server.  It accomplishes this via a decorator, so
a lot of these route-related functions now have quite a few decorators
attached to them.

Typo'd URLs (/blueprints/nfo for instance) will still return a 404.  I
think this is a reasonable thing to do.

(cherry picked from commit 5daf2d416aa883de3664e7795239dccd7c332340)
---
 src/pylorax/api/checkparams.py | 44 ++++++++++++++++++++++++++++
 src/pylorax/api/v0.py          | 53 ++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)
 create mode 100644 src/pylorax/api/checkparams.py

diff --git a/src/pylorax/api/checkparams.py b/src/pylorax/api/checkparams.py
new file mode 100644
index 00000000..f15d95d8
--- /dev/null
+++ b/src/pylorax/api/checkparams.py
@@ -0,0 +1,44 @@
+#
+# 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 <http://www.gnu.org/licenses/>.
+#
+
+import logging
+log = logging.getLogger("lorax-composer")
+
+from flask import jsonify
+from functools import update_wrapper
+
+# A decorator for checking the parameters provided to the API route implementing
+# functions.  The tuples parameter is a list of tuples.  Each tuple is the string
+# name of a parameter ("blueprint_name", not blueprint_name), the value it's set
+# to by flask if the caller did not provide it, and a message to be returned to
+# the user.
+#
+# If the parameter is set to its default, the error message is returned.  Otherwise,
+# the decorated function is called and its return value is returned.
+def checkparams(tuples):
+    def decorator(f):
+        def wrapped_function(*args, **kwargs):
+            for tup in tuples:
+                if kwargs[tup[0]] == tup[1]:
+                    log.error("(%s) %s", f.__name__, tup[2])
+                    return jsonify(status=False, errors=[tup[2]]), 400
+
+            return f(*args, **kwargs)
+
+        return update_wrapper(wrapped_function, f)
+
+    return decorator
diff --git a/src/pylorax/api/v0.py b/src/pylorax/api/v0.py
index b91ac297..7a4645c7 100644
--- a/src/pylorax/api/v0.py
+++ b/src/pylorax/api/v0.py
@@ -980,6 +980,7 @@ from flask import jsonify, request, Response, send_file
 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.projects import projects_list, projects_info, projects_depsolve
@@ -1025,8 +1026,10 @@ def v0_api(api):
             blueprints = take_limits([f[:-5] for f in list_branch_files(api.config["GITLOCK"].repo, branch)], offset, limit)
         return jsonify(blueprints=blueprints, limit=limit, offset=offset, total=len(blueprints))
 
+    @api.route("/api/v0/blueprints/info", defaults={'blueprint_names': ""})
     @api.route("/api/v0/blueprints/info/<blueprint_names>")
     @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"""
         branch = request.args.get("branch", "master")
@@ -1081,8 +1084,10 @@ def v0_api(api):
         else:
             return jsonify(changes=changes, blueprints=blueprints, errors=errors)
 
+    @api.route("/api/v0/blueprints/changes", defaults={'blueprint_names': ""})
     @api.route("/api/v0/blueprints/changes/<blueprint_names>")
     @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"""
         branch = request.args.get("branch", "master")
@@ -1133,8 +1138,10 @@ def v0_api(api):
         else:
             return jsonify(status=True)
 
+    @api.route("/api/v0/blueprints/delete", defaults={'blueprint_name': ""}, methods=["DELETE"])
     @api.route("/api/v0/blueprints/delete/<blueprint_name>", methods=["DELETE"])
     @crossdomain(origin="*")
+    @checkparams([("blueprint_name", "", "no blueprint name given")])
     def v0_blueprints_delete(blueprint_name):
         """Delete a blueprint from git"""
         branch = request.args.get("branch", "master")
@@ -1166,8 +1173,10 @@ def v0_api(api):
         else:
             return jsonify(status=True)
 
+    @api.route("/api/v0/blueprints/workspace", defaults={'blueprint_name': ""}, methods=["DELETE"])
     @api.route("/api/v0/blueprints/workspace/<blueprint_name>", methods=["DELETE"])
     @crossdomain(origin="*")
+    @checkparams([("blueprint_name", "", "no blueprint name given")])
     def v0_blueprints_delete_workspace(blueprint_name):
         """Delete a blueprint from the workspace"""
         branch = request.args.get("branch", "master")
@@ -1180,8 +1189,12 @@ def v0_api(api):
         else:
             return jsonify(status=True)
 
+    @api.route("/api/v0/blueprints/undo", defaults={'blueprint_name': "", 'commit': ""}, methods=["POST"])
+    @api.route("/api/v0/blueprints/undo/<blueprint_name>", defaults={'commit': ""}, methods=["POST"])
     @api.route("/api/v0/blueprints/undo/<blueprint_name>/<commit>", methods=["POST"])
     @crossdomain(origin="*")
+    @checkparams([("blueprint_name", "", "no blueprint name given"),
+                  ("commit", "", "no commit ID given")])
     def v0_blueprints_undo(blueprint_name, commit):
         """Undo changes to a blueprint by reverting to a previous commit."""
         branch = request.args.get("branch", "master")
@@ -1198,8 +1211,10 @@ def v0_api(api):
         else:
             return jsonify(status=True)
 
+    @api.route("/api/v0/blueprints/tag", defaults={'blueprint_name': ""}, methods=["POST"])
     @api.route("/api/v0/blueprints/tag/<blueprint_name>", 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'"""
         branch = request.args.get("branch", "master")
@@ -1212,8 +1227,14 @@ def v0_api(api):
         else:
             return jsonify(status=True)
 
+    @api.route("/api/v0/blueprints/diff", defaults={'blueprint_name': "", 'from_commit': "", 'to_commit': ""})
+    @api.route("/api/v0/blueprints/diff/<blueprint_name>", defaults={'from_commit': "", 'to_commit': ""})
+    @api.route("/api/v0/blueprints/diff/<blueprint_name>/<from_commit>", defaults={'to_commit': ""})
     @api.route("/api/v0/blueprints/diff/<blueprint_name>/<from_commit>/<to_commit>")
     @crossdomain(origin="*")
+    @checkparams([("blueprint_name", "", "no blueprint name given"),
+                  ("from_commit", "", "no from commit ID given"),
+                  ("to_commit", "", "no to commit ID given")])
     def v0_blueprints_diff(blueprint_name, from_commit, to_commit):
         """Return the differences between two commits of a blueprint"""
         branch = request.args.get("branch", "master")
@@ -1249,8 +1270,10 @@ def v0_api(api):
         diff = recipe_diff(old_blueprint, new_blueprint)
         return jsonify(diff=diff)
 
+    @api.route("/api/v0/blueprints/freeze", defaults={'blueprint_names': ""})
     @api.route("/api/v0/blueprints/freeze/<blueprint_names>")
     @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"""
         branch = request.args.get("branch", "master")
@@ -1302,8 +1325,10 @@ def v0_api(api):
         else:
             return jsonify(blueprints=blueprints, errors=errors)
 
+    @api.route("/api/v0/blueprints/depsolve", defaults={'blueprint_names': ""})
     @api.route("/api/v0/blueprints/depsolve/<blueprint_names>")
     @crossdomain(origin="*")
+    @checkparams([("blueprint_names", "", "no blueprint names given")])
     def v0_blueprints_depsolve(blueprint_names):
         """Return the dependencies for a blueprint"""
         branch = request.args.get("branch", "master")
@@ -1377,8 +1402,10 @@ def v0_api(api):
         projects = take_limits(available, offset, limit)
         return jsonify(projects=projects, offset=offset, limit=limit, total=len(available))
 
+    @api.route("/api/v0/projects/info", defaults={'project_names': ""})
     @api.route("/api/v0/projects/info/<project_names>")
     @crossdomain(origin="*")
+    @checkparams([("project_names", "", "no project names given")])
     def v0_projects_info(project_names):
         """Return detailed information about the listed projects"""
         try:
@@ -1390,8 +1417,10 @@ def v0_api(api):
 
         return jsonify(projects=projects)
 
+    @api.route("/api/v0/projects/depsolve", defaults={'project_names': ""})
     @api.route("/api/v0/projects/depsolve/<project_names>")
     @crossdomain(origin="*")
+    @checkparams([("project_names", "", "no project names given")])
     def v0_projects_depsolve(project_names):
         """Return detailed information about the listed projects"""
         try:
@@ -1412,8 +1441,10 @@ def v0_api(api):
         sources = sorted([r.id for r in repos])
         return jsonify(sources=sources)
 
+    @api.route("/api/v0/projects/source/info", defaults={'source_names': ""})
     @api.route("/api/v0/projects/source/info/<source_names>")
     @crossdomain(origin="*")
+    @checkparams([("source_names", "", "no source names given")])
     def v0_projects_source_info(source_names):
         """Return detailed info about the list of sources"""
         out_fmt = request.args.get("format", "json")
@@ -1503,8 +1534,10 @@ def v0_api(api):
 
         return jsonify(status=True)
 
+    @api.route("/api/v0/projects/source/delete", defaults={'source_name': ""}, methods=["DELETE"])
     @api.route("/api/v0/projects/source/delete/<source_name>", 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"""
         system_sources = get_repo_sources("/etc/yum.repos.d/*.repo")
@@ -1553,8 +1586,10 @@ def v0_api(api):
         modules = take_limits(available, offset, limit)
         return jsonify(modules=modules, offset=offset, limit=limit, total=len(available))
 
+    @api.route("/api/v0/modules/info", defaults={'module_names': ""})
     @api.route("/api/v0/modules/info/<module_names>")
     @crossdomain(origin="*")
+    @checkparams([("module_names", "", "no module names given")])
     def v0_modules_info(module_names):
         """Return detailed information about the listed modules"""
         try:
@@ -1648,8 +1683,10 @@ def v0_api(api):
         """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/<uuids>")
     @crossdomain(origin="*")
+    @checkparams([("uuids", "", "no UUIDs given")])
     def v0_compose_status(uuids):
         """Return the status of the listed uuids"""
         results = []
@@ -1660,8 +1697,10 @@ def v0_api(api):
 
         return jsonify(uuids=results)
 
+    @api.route("/api/v0/compose/cancel", defaults={'uuid': ""}, methods=["DELETE"])
     @api.route("/api/v0/compose/cancel/<uuid>", methods=["DELETE"])
     @crossdomain(origin="*")
+    @checkparams([("uuid", "", "no UUID given")])
     def v0_compose_cancel(uuid):
         """Cancel a running compose and delete its results directory"""
         status = uuid_status(api.config["COMPOSER_CFG"], uuid)
@@ -1678,8 +1717,10 @@ def v0_api(api):
         else:
             return jsonify(status=True, uuid=uuid)
 
+    @api.route("/api/v0/compose/delete", defaults={'uuids': ""}, methods=["DELETE"])
     @api.route("/api/v0/compose/delete/<uuids>", methods=["DELETE"])
     @crossdomain(origin="*")
+    @checkparams([("uuids", "", "no UUIDs given")])
     def v0_compose_delete(uuids):
         """Delete the compose results for the listed uuids"""
         results = []
@@ -1699,8 +1740,10 @@ def v0_api(api):
                     results.append({"uuid":uuid, "status":True})
         return jsonify(uuids=results, errors=errors)
 
+    @api.route("/api/v0/compose/info", defaults={'uuid': ""})
     @api.route("/api/v0/compose/info/<uuid>")
     @crossdomain(origin="*")
+    @checkparams([("uuid", "", "no UUID given")])
     def v0_compose_info(uuid):
         """Return detailed info about a compose"""
         try:
@@ -1710,8 +1753,10 @@ def v0_api(api):
 
         return jsonify(**info)
 
+    @api.route("/api/v0/compose/metadata", defaults={'uuid': ""})
     @api.route("/api/v0/compose/metadata/<uuid>")
     @crossdomain(origin="*")
+    @checkparams([("uuid","", "no UUID given")])
     def v0_compose_metadata(uuid):
         """Return a tar of the metadata for the build"""
         status = uuid_status(api.config["COMPOSER_CFG"], uuid)
@@ -1725,8 +1770,10 @@ def v0_api(api):
                             headers=[("Content-Disposition", "attachment; filename=%s-metadata.tar;" % uuid)],
                             direct_passthrough=True)
 
+    @api.route("/api/v0/compose/results", defaults={'uuid': ""})
     @api.route("/api/v0/compose/results/<uuid>")
     @crossdomain(origin="*")
+    @checkparams([("uuid","", "no UUID given")])
     def v0_compose_results(uuid):
         """Return a tar of the metadata and the results for the build"""
         status = uuid_status(api.config["COMPOSER_CFG"], uuid)
@@ -1740,8 +1787,10 @@ def v0_api(api):
                             headers=[("Content-Disposition", "attachment; filename=%s.tar;" % uuid)],
                             direct_passthrough=True)
 
+    @api.route("/api/v0/compose/logs", defaults={'uuid': ""})
     @api.route("/api/v0/compose/logs/<uuid>")
     @crossdomain(origin="*")
+    @checkparams([("uuid","", "no UUID given")])
     def v0_compose_logs(uuid):
         """Return a tar of the metadata for the build"""
         status = uuid_status(api.config["COMPOSER_CFG"], uuid)
@@ -1755,8 +1804,10 @@ def v0_api(api):
                             headers=[("Content-Disposition", "attachment; filename=%s-logs.tar;" % uuid)],
                             direct_passthrough=True)
 
+    @api.route("/api/v0/compose/image", defaults={'uuid': ""})
     @api.route("/api/v0/compose/image/<uuid>")
     @crossdomain(origin="*")
+    @checkparams([("uuid","", "no UUID given")])
     def v0_compose_image(uuid):
         """Return the output image for the build"""
         status = uuid_status(api.config["COMPOSER_CFG"], uuid)
@@ -1776,8 +1827,10 @@ def v0_api(api):
             # XXX - Will mime type guessing work for all our output?
             return send_file(image_path, as_attachment=True, attachment_filename=image_name, add_etags=False)
 
+    @api.route("/api/v0/compose/log", defaults={'uuid': ""})
     @api.route("/api/v0/compose/log/<uuid>")
     @crossdomain(origin="*")
+    @checkparams([("uuid","", "no UUID given")])
     def v0_compose_log_tail(uuid):
         """Return the end of the main anaconda.log, defaults to 1Mbytes"""
         try: