From 4c2e243a72f254209a0986db63dee6df79607b9d Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Apr 19 2022 22:58:47 +0000
Subject: [PATCH 1/6] frontend: drop one unused route


---

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 68f3324..31062ae 100644
--- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -1037,20 +1037,6 @@ def render_generate_module_repo_file(copr, name_release, module_nsv):
 
 #########################################################
 
-@coprs_ns.route("/<username>/<coprname>/rpm/<name_release>/<rpmfile>")
-def copr_repo_rpm_file(username, coprname, name_release, rpmfile):
-    try:
-        packages_dir = os.path.join(app.config["DATA_DIR"], "repo-rpm-packages")
-        with open(os.path.join(packages_dir, rpmfile), "rb") as rpm:
-            response = flask.make_response(rpm.read())
-            response.mimetype = "application/x-rpm"
-            response.headers["Content-Disposition"] = \
-                "filename={0}".format(rpmfile)
-            return response
-    except IOError:
-        return flask.render_template("404.html")
-
-
 @coprs_ns.route("/<username>/<coprname>/monitor/")
 @coprs_ns.route("/<username>/<coprname>/monitor/<detailed>")
 @coprs_ns.route("/g/<group_name>/<coprname>/monitor/")

From 3dc89890063ce0f850027feda12757633bcae0fa Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Apr 19 2022 22:58:47 +0000
Subject: [PATCH 2/6] frontend: de-duplicate the html error templates


---

diff --git a/frontend/coprs_frontend/coprs/templates/403.html b/frontend/coprs_frontend/coprs/templates/403.html
deleted file mode 100644
index 785f90c..0000000
--- a/frontend/coprs_frontend/coprs/templates/403.html
+++ /dev/null
@@ -1,11 +0,0 @@
-{% extends "layout.html" %}
-{% block title %}Permission denied{% endblock %}
-{% block header %}Permission denied!{% endblock %}
-{% block body %}
-  <h1> Error 403: Access Denied/Forbidden </h1>
-  {% if message %}
-    {{ message }}
-  {% else %}
-    You are not allowed to see this page.
-  {% endif %}
-{% endblock %}
diff --git a/frontend/coprs_frontend/coprs/templates/404.html b/frontend/coprs_frontend/coprs/templates/404.html
deleted file mode 100644
index 70415a3..0000000
--- a/frontend/coprs_frontend/coprs/templates/404.html
+++ /dev/null
@@ -1,13 +0,0 @@
-{% extends "layout.html" %}
-{% block title %}Not Found{% endblock %}
-{% block header %}Not Found!{% endblock %}
-{% block body %}
-  <h1> Error 404: Not Found </h1>
-  {% if message %}
-    {% for line in message.split("\n") %}
-      {{ line }} {% if not loop.last %}<br />{% endif %}
-    {% endfor %}
-  {% else %}
-    The thing you're looking for does not exist.
-  {% endif %}
-{% endblock %}
diff --git a/frontend/coprs_frontend/coprs/templates/_error.html b/frontend/coprs_frontend/coprs/templates/_error.html
deleted file mode 100644
index 7938b34..0000000
--- a/frontend/coprs_frontend/coprs/templates/_error.html
+++ /dev/null
@@ -1,24 +0,0 @@
-{% extends "layout.html" %}
-{% block title %}Not Found{% endblock %}
-{% block header %}Not Found!{% endblock %}
-{% block body %}
-  <h1> Error {{ error_code }}
-  {% if error_title %}
-    : {{ error_title }}
-  {% endif %}
-
-  </h1>
-  {% if message %}
-    {{ message }}
-  {% else %}
-    Something went wrong
-  {% endif %}
-
-  {% if config.debug or config.env != "production" %}
-    <p>
-      Administrators should take a look at
-      <code>/var/log/copr-frontend/frontend.log</code>
-      to analyze the full error output.
-    </p>
-  {% endif %}
-{% endblock %}
diff --git a/frontend/coprs_frontend/coprs/templates/html-error.html b/frontend/coprs_frontend/coprs/templates/html-error.html
new file mode 100644
index 0000000..f442875
--- /dev/null
+++ b/frontend/coprs_frontend/coprs/templates/html-error.html
@@ -0,0 +1,20 @@
+{% extends "layout.html" %}
+{% set error_title = error_title or "Unknown error" %}
+{% block title %}Copr Build System – {{ error_title }}{% endblock %}
+{% block body %}
+<h1> Error {{ error_code }}: {{ error_title }}</h1>
+{% if message %}
+{% for line in message.split("\n") %}
+  {{ line }} {% if not loop.last %}<br />{% endif %}
+{% endfor %}
+{% else %}
+   <p>Something went wrong.  Contact admins.</p>
+   {% if config.debug or config.env != "production" %}
+     <p>
+       Administrators should take a look at
+       <code>/var/log/copr-frontend/frontend.log</code>
+       to analyze the full error output.
+     </p>
+   {% endif %}
+{% endif %}
+{% endblock %}
diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py
index 71c9390..1f0c6fd 100644
--- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py
+++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py
@@ -6,7 +6,7 @@ import flask
 from coprs import app, oid, db, models
 from coprs.views.apiv3_ns import apiv3_ns
 from coprs.exceptions import AccessRestricted
-from coprs.views.misc import api_login_required
+from coprs.views.misc import api_login_required, access_restricted
 from coprs.logic.users_logic import UsersLogic
 
 
@@ -62,7 +62,7 @@ def auth_403(message):
     """
     message = "Can't log-in using GSSAPI: " + message
     if "web-ui" in flask.request.full_path:
-        return flask.render_template("403.html", message=message), 403
+        return access_restricted(message)
     raise AccessRestricted(message)
 
 
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 31062ae..2ebe35e 100644
--- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -947,9 +947,12 @@ def render_generate_repo_file(copr_dir, name_release, arch=None):
                 available_chroots.append(available_chroot.name)
                 html_error_msg += available_chroot.name + "\n"
 
-        cmd_error_msg = {"available chroots": available_chroots}
-        return flask.render_template("404.html", message=html_error_msg), 404, {
-            'Copr-Error-Data': base64.b64encode(json.dumps(cmd_error_msg).encode("utf-8"))}
+        dnf_copr_error = {"available chroots": available_chroots}
+        dnf_copr_error_data = json.dumps(dnf_copr_error).encode("utf-8")
+        dnf_copr_plugin_headers = {
+            "Copr-Error-Data": base64.b64encode(dnf_copr_error_data),
+        }
+        return page_not_found(html_error_msg, headers=dnf_copr_plugin_headers)
 
     # append multilib counterpart repo only upon explicit request (ach != None),
     # and only if the chroot actually is multilib capable
diff --git a/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py b/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py
index c70ca58..8d0e4b9 100644
--- a/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py
+++ b/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py
@@ -12,7 +12,7 @@ from coprs.logic.users_logic import UsersLogic
 from coprs import app
 
 from ... import db
-from ..misc import login_required
+from ..misc import login_required, page_not_found
 from ..user_ns import user_general
 
 from . import groups_ns
@@ -86,7 +86,7 @@ def list_projects_by_group(group_name, page=1):
 @login_required
 def list_user_groups():
     if not app.config['FAS_LOGIN']:
-        return flask.render_template("404.html"), 404
+        return page_not_found("FAS Login not enabled")
 
     teams = flask.g.user.user_teams
     active_map = {
diff --git a/frontend/coprs_frontend/coprs/views/misc.py b/frontend/coprs_frontend/coprs/views/misc.py
index 20897b4..54eaeef 100644
--- a/frontend/coprs_frontend/coprs/views/misc.py
+++ b/frontend/coprs_frontend/coprs/views/misc.py
@@ -52,28 +52,28 @@ def before_request():
             models.User.username == username).first()
 
 
-def page_not_found(message):
-    return flask.render_template("404.html", message=message), 404
-
-
-def access_restricted(message):
-    return flask.render_template("403.html", message=message), 403
-
-
-def generic_error(message, code=500, title=None):
+def generic_error(message, code=500, title=None, headers=None):
     """
     :type message: str
     :type err: CoprHttpException
     """
-    return flask.render_template("_error.html",
-                                 message=message,
-                                 error_code=code,
-                                 error_title=title), code
+
+    template = flask.render_template("html-error.html",
+                                     message=message,
+                                     error_code=code,
+                                     error_title=title)
+    retval = [template, code]
+    if headers is not None:
+        # custom headers needed
+        retval.append(headers)
+    return tuple(retval)
 
 
 server_error_handler = partial(generic_error, code=500, title="Internal Server Error")
 bad_request_handler = partial(generic_error, code=400, title="Bad Request")
 conflict_request_handler = partial(generic_error, code=409, title="Conflict")
+access_restricted = partial(generic_error, code=403, title="Access Restricted")
+page_not_found = partial(generic_error, code=404, title="Page Not Found")
 
 misc = flask.Blueprint("misc", __name__)
 
diff --git a/frontend/coprs_frontend/tests/test_exceptions.py b/frontend/coprs_frontend/tests/test_exceptions.py
index 70fabc8..1187b56 100644
--- a/frontend/coprs_frontend/tests/test_exceptions.py
+++ b/frontend/coprs_frontend/tests/test_exceptions.py
@@ -25,7 +25,7 @@ class TestExceptionHandling(CoprsTestCase):
     def test_both_nonexisting_page_and_object(self):
         r1 = self.tc.get("/nonexisting/endpoint/")
         assert r1.status_code == 404
-        assert "<h1> Error 404: Not Found </h1>" in str(r1.data)
+        assert "<h1> Error 404: Page Not Found</h1>" in str(r1.data)
 
         r2 = self.tc.get("/coprs/nonexisting/nonexisting/")
         assert r2.status_code == 404

From 36ddb5ad574ae48a1f5410872a2afbf581dd9c4d Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Apr 19 2022 22:58:47 +0000
Subject: [PATCH 3/6] frontend: no need to special-case @group_ns


The normal @app.errorhandler should take care of this.

---

diff --git a/frontend/coprs_frontend/coprs/views/groups_ns/__init__.py b/frontend/coprs_frontend/coprs/views/groups_ns/__init__.py
index 64ba92e..88d5e20 100644
--- a/frontend/coprs_frontend/coprs/views/groups_ns/__init__.py
+++ b/frontend/coprs_frontend/coprs/views/groups_ns/__init__.py
@@ -2,17 +2,4 @@
 
 import flask
 
-from coprs.views.misc import page_not_found, access_restricted
-from coprs.exceptions import ObjectNotFound, AccessRestricted
-
 groups_ns = flask.Blueprint("groups_ns", __name__, url_prefix="/groups")
-
-
-@groups_ns.errorhandler(ObjectNotFound)
-def handle_404(error):
-    return page_not_found(error.message)
-
-
-@groups_ns.errorhandler(AccessRestricted)
-def handle_403(error):
-    return access_restricted(error.message)

From 7dc3c424bfba7778bd136c854b9691641de1a979 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Apr 19 2022 22:58:47 +0000
Subject: [PATCH 4/6] frontend: concentrate exception handling in one file


Make sure that all the error responses are generated in
error_handlers.py file.  The rest of the Frontend code is not
responsible for generating responses from now on, and just raises valid
exceptions.

---

diff --git a/frontend/coprs_frontend/coprs/error_handlers.py b/frontend/coprs_frontend/coprs/error_handlers.py
index 00df06b..c24388b 100644
--- a/frontend/coprs_frontend/coprs/error_handlers.py
+++ b/frontend/coprs_frontend/coprs/error_handlers.py
@@ -3,28 +3,47 @@ A place for exception-handling logic
 """
 
 import logging
+from functools import partial
 
 import flask
 from sqlalchemy.exc import SQLAlchemyError
 from werkzeug.exceptions import HTTPException, NotFound, GatewayTimeout
 from coprs.exceptions import CoprHttpException
-from coprs.views.misc import (
-    generic_error,
-    access_restricted,
-    bad_request_handler,
-    conflict_request_handler,
-    page_not_found,
-)
 
 LOG = logging.getLogger(__name__)
 
 
+def generic_error(message, code=500, title=None, headers=None):
+    """
+    :type message: str
+    :type err: CoprHttpException
+    """
+
+    template = flask.render_template("html-error.html",
+                                     message=message,
+                                     error_code=code,
+                                     error_title=title)
+    retval = [template, code]
+    if headers is not None:
+        # custom headers needed
+        retval.append(headers)
+    return tuple(retval)
+
+
+bad_request_handler = partial(generic_error, code=400, title="Bad Request")
+conflict_request_handler = partial(generic_error, code=409, title="Conflict")
+access_restricted = partial(generic_error, code=403, title="Access Restricted")
+page_not_found = partial(generic_error, code=404, title="Page Not Found")
+unprocessable_entity = partial(generic_error, code=422, title="Unprocessable Entity")
+
+
 def get_error_handler():
     """
     Determine what error handler should be used for this request
     See http://flask.pocoo.org/docs/1.0/blueprints/#error-handlers
     """
-    if flask.request.path.startswith('/api_3/'):
+    path = flask.request.path
+    if path.startswith('/api_3/') and "gssapi_login/web-ui" not in path:
         return APIErrorHandler()
     return UIErrorHandler()
 
@@ -77,6 +96,7 @@ class UIErrorHandler(BaseErrorHandler):
     def handle_error(self, error):
         code = self.code(error)
         message = self.message(error)
+        headers = getattr(error, "headers", None)
 
         # The most common error has their own custom error pages. When catching
         # a new exception, try to keep it simple and just the the generic one.
@@ -86,9 +106,10 @@ class UIErrorHandler(BaseErrorHandler):
             403: access_restricted,
             404: page_not_found,
             409: conflict_request_handler,
+            422: unprocessable_entity,
         }
         if code in error_views:
-            return error_views[code](message)
+            return error_views[code](message, headers=headers)
 
         message = "Server error, contact admin"
         if isinstance(error, SQLAlchemyError):
diff --git a/frontend/coprs_frontend/coprs/exceptions.py b/frontend/coprs_frontend/coprs/exceptions.py
index ec08419..123080c 100644
--- a/frontend/coprs_frontend/coprs/exceptions.py
+++ b/frontend/coprs_frontend/coprs/exceptions.py
@@ -3,9 +3,10 @@ class CoprHttpException(Exception):
     _default = "Generic copr exception"
     _code = 500
 
-    def __init__(self, message=None, code=None, **kwargs):
+    def __init__(self, message=None, code=None, headers=None, **kwargs):
         self.message = str(message) if message else None
         self.code = code or self._code
+        self.headers = headers
         self.kwargs = kwargs
 
     def __unicode__(self):
diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py
index 1f0c6fd..4e17768 100644
--- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py
+++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py
@@ -6,7 +6,7 @@ import flask
 from coprs import app, oid, db, models
 from coprs.views.apiv3_ns import apiv3_ns
 from coprs.exceptions import AccessRestricted
-from coprs.views.misc import api_login_required, access_restricted
+from coprs.views.misc import api_login_required
 from coprs.logic.users_logic import UsersLogic
 
 
@@ -61,8 +61,6 @@ def auth_403(message):
     Return appropriately formatted GSSAPI 403 error for both web-ui and API
     """
     message = "Can't log-in using GSSAPI: " + message
-    if "web-ui" in flask.request.full_path:
-        return access_restricted(message)
     raise AccessRestricted(message)
 
 
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 2ebe35e..fd09857 100644
--- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -27,7 +27,7 @@ from coprs import exceptions
 from coprs import forms
 from coprs import helpers
 from coprs import models
-from coprs.exceptions import ObjectNotFound
+from coprs.exceptions import ObjectNotFound, BadRequest, CoprHttpException
 from coprs.logic.coprs_logic import CoprsLogic, PinnedCoprsLogic, MockChrootsLogic
 from coprs.logic.stat_logic import CounterStatLogic
 from coprs.logic.modules_logic import ModulesLogic, ModulemdGenerator, ModuleBuildFacade
@@ -37,9 +37,7 @@ from coprs.logic.complex_logic import ComplexLogic, ReposLogic
 from coprs.logic.outdated_chroots_logic import OutdatedChrootsLogic
 
 from coprs.views.misc import (
-    generic_error,
     login_required,
-    page_not_found,
     req_with_copr,
     req_with_copr_dir,
     req_with_pagination,
@@ -114,8 +112,7 @@ def coprs_show(page=1):
 def coprs_by_user(username=None, page=1):
     user = users_logic.UsersLogic.get(username).first()
     if not user:
-        return page_not_found(
-            "User {0} does not exist.".format(username))
+        raise ObjectNotFound("User {0} does not exist.".format(username))
 
     pinned = [pin.copr for pin in PinnedCoprsLogic.get_by_user_id(user.id)] if page == 1 else []
     query = CoprsLogic.get_multiple_owned_by_username(username)
@@ -952,7 +949,7 @@ def render_generate_repo_file(copr_dir, name_release, arch=None):
         dnf_copr_plugin_headers = {
             "Copr-Error-Data": base64.b64encode(dnf_copr_error_data),
         }
-        return page_not_found(html_error_msg, headers=dnf_copr_plugin_headers)
+        raise ObjectNotFound(html_error_msg, headers=dnf_copr_plugin_headers)
 
     # append multilib counterpart repo only upon explicit request (ach != None),
     # and only if the chroot actually is multilib capable
@@ -1095,7 +1092,7 @@ def copr_fork_post(copr):
     if form.validate_on_submit():
         dstgroup = ([g for g in flask.g.user.user_groups if g.at_name == form.owner.data] or [None])[0]
         if flask.g.user.name != form.owner.data and not dstgroup:
-            return generic_error("There is no such group: {}".format(form.owner.data))
+            raise BadRequest("There is no such group: {}".format(form.owner.data))
 
         dst_copr = CoprsLogic.get(flask.g.user.name, form.name.data).all()
         if dst_copr and not form.confirm.data:
@@ -1251,7 +1248,7 @@ def copr_module(copr, id):
         msg = ("Unable to parse module YAML. The most probable reason is that "
                "the module YAML is in an old modulemd version and/or has some "
                "mistake in it. Hint: {}".format(ex))
-        return generic_error(msg, code=422, title="Unable to parse module YAML")
+        raise CoprHttpException(msg, code=422) from ex
 
     return flask.render_template("coprs/detail/module.html", copr=copr, module=module,
                                  yaml=pretty_yaml, unique_chroots=unique_chroots)
diff --git a/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py b/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py
index 8d0e4b9..bff13a8 100644
--- a/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py
+++ b/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py
@@ -2,7 +2,7 @@
 
 import flask
 from flask import render_template, url_for
-from coprs.exceptions import InsufficientRightsException
+from coprs.exceptions import InsufficientRightsException, ObjectNotFound
 from coprs.forms import ActivateFasGroupForm
 from coprs.helpers import Paginator
 from coprs.logic import builds_logic
@@ -12,7 +12,7 @@ from coprs.logic.users_logic import UsersLogic
 from coprs import app
 
 from ... import db
-from ..misc import login_required, page_not_found
+from ..misc import login_required
 from ..user_ns import user_general
 
 from . import groups_ns
@@ -86,7 +86,7 @@ def list_projects_by_group(group_name, page=1):
 @login_required
 def list_user_groups():
     if not app.config['FAS_LOGIN']:
-        return page_not_found("FAS Login not enabled")
+        raise ObjectNotFound("Logging via Fedora Accounts not enabled")
 
     teams = flask.g.user.user_teams
     active_map = {
diff --git a/frontend/coprs_frontend/coprs/views/misc.py b/frontend/coprs_frontend/coprs/views/misc.py
index 54eaeef..4074b5f 100644
--- a/frontend/coprs_frontend/coprs/views/misc.py
+++ b/frontend/coprs_frontend/coprs/views/misc.py
@@ -1,7 +1,7 @@
 import base64
 import datetime
 import functools
-from functools import wraps, partial
+from functools import wraps
 from urllib.parse import urlparse
 import flask
 
@@ -52,29 +52,6 @@ def before_request():
             models.User.username == username).first()
 
 
-def generic_error(message, code=500, title=None, headers=None):
-    """
-    :type message: str
-    :type err: CoprHttpException
-    """
-
-    template = flask.render_template("html-error.html",
-                                     message=message,
-                                     error_code=code,
-                                     error_title=title)
-    retval = [template, code]
-    if headers is not None:
-        # custom headers needed
-        retval.append(headers)
-    return tuple(retval)
-
-
-server_error_handler = partial(generic_error, code=500, title="Internal Server Error")
-bad_request_handler = partial(generic_error, code=400, title="Bad Request")
-conflict_request_handler = partial(generic_error, code=409, title="Conflict")
-access_restricted = partial(generic_error, code=403, title="Access Restricted")
-page_not_found = partial(generic_error, code=404, title="Page Not Found")
-
 misc = flask.Blueprint("misc", __name__)
 
 
diff --git a/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py b/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py
index 65ccdb1..7f1008b 100644
--- a/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py
+++ b/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py
@@ -7,10 +7,9 @@ from coprs.logic.builds_logic import BuildsLogic
 from coprs.logic.complex_logic import ComplexLogic
 from coprs.logic.packages_logic import PackagesLogic
 
-from coprs.exceptions import ObjectNotFound
+from coprs.exceptions import ObjectNotFound, AccessRestricted
 
 from coprs.views.webhooks_ns import webhooks_ns
-from coprs.views.misc import page_not_found, access_restricted
 
 import logging
 import os
@@ -77,13 +76,9 @@ def package_name_required(route):
 def webhooks_bitbucket_push(copr_id, uuid):
     # For the documentation of the data we receive see:
     # https://confluence.atlassian.com/bitbucket/event-payloads-740262817.html
-    try:
-        copr = ComplexLogic.get_copr_by_id_safe(copr_id)
-    except ObjectNotFound:
-        return page_not_found("Project does not exist")
-
+    copr = ComplexLogic.get_copr_by_id_safe(copr_id)
     if copr.webhook_secret != uuid:
-        return access_restricted("This webhook is not valid")
+        raise AccessRestricted("This webhook is not valid")
 
     try:
         payload = flask.request.json
@@ -123,13 +118,9 @@ def webhooks_git_push(copr_id, uuid):
         return "OK", 200
     # For the documentation of the data we receive see:
     # https://developer.github.com/v3/activity/events/types/#pushevent
-    try:
-        copr = ComplexLogic.get_copr_by_id_safe(copr_id)
-    except ObjectNotFound:
-        return page_not_found("Project does not exist")
-
+    copr = ComplexLogic.get_copr_by_id_safe(copr_id)
     if copr.webhook_secret != uuid:
-        return access_restricted("This webhook is not valid")
+        raise AccessRestricted("This webhook is not valid")
 
     try:
         payload = flask.request.json
@@ -171,13 +162,9 @@ def webhooks_git_push(copr_id, uuid):
 def webhooks_gitlab_push(copr_id, uuid):
     # For the documentation of the data we receive see:
     # https://gitlab.com/help/user/project/integrations/webhooks#events
-    try:
-        copr = ComplexLogic.get_copr_by_id_safe(copr_id)
-    except ObjectNotFound:
-        return page_not_found("Project does not exist")
-
+    copr = ComplexLogic.get_copr_by_id_safe(copr_id)
     if copr.webhook_secret != uuid:
-        return access_restricted("This webhook is not valid")
+        raise AccessRestricted("This webhook is not valid")
 
     try:
         payload = flask.request.json

From 0514941339dc8eaf35e74a24beabe674456bb008 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Apr 19 2022 22:58:47 +0000
Subject: [PATCH 5/6] frontend: unify the UI and API error handling


This way we can handle most of the exceptions on one place.

This also removes significant amount of traceback log entries from
frontend.log/error_log because we fix the issue from 314cab8a69a where
we started logging tracebacks for every HTTP (non-200) error.

Thus, this fixes #1600.  The traceback noticed there was just caused
by connection failure between client and server (error 400).  The
situation can be reproduced by 'kill -9'ing the client while SRC.RPM is
being uploaded.

Fixes: #1600

---

diff --git a/frontend/coprs_frontend/coprs/error_handlers.py b/frontend/coprs_frontend/coprs/error_handlers.py
index c24388b..27dfda7 100644
--- a/frontend/coprs_frontend/coprs/error_handlers.py
+++ b/frontend/coprs_frontend/coprs/error_handlers.py
@@ -3,7 +3,6 @@ A place for exception-handling logic
 """
 
 import logging
-from functools import partial
 
 import flask
 from sqlalchemy.exc import SQLAlchemyError
@@ -13,30 +12,6 @@ from coprs.exceptions import CoprHttpException
 LOG = logging.getLogger(__name__)
 
 
-def generic_error(message, code=500, title=None, headers=None):
-    """
-    :type message: str
-    :type err: CoprHttpException
-    """
-
-    template = flask.render_template("html-error.html",
-                                     message=message,
-                                     error_code=code,
-                                     error_title=title)
-    retval = [template, code]
-    if headers is not None:
-        # custom headers needed
-        retval.append(headers)
-    return tuple(retval)
-
-
-bad_request_handler = partial(generic_error, code=400, title="Bad Request")
-conflict_request_handler = partial(generic_error, code=409, title="Conflict")
-access_restricted = partial(generic_error, code=403, title="Access Restricted")
-page_not_found = partial(generic_error, code=404, title="Page Not Found")
-unprocessable_entity = partial(generic_error, code=422, title="Unprocessable Entity")
-
-
 def get_error_handler():
     """
     Determine what error handler should be used for this request
@@ -53,32 +28,68 @@ class BaseErrorHandler:
     Do not use this class for handling errors. It is only a parent class for
     the actual error-handler classes.
     """
-
     def handle_error(self, error):
         """
         Return a flask response suitable for the current situation (e.g. reder
         HTML page for UI failures, send JSON back to API client, etc).
+        """
+        code = self.code(error)
+        message = self.message(error)
+        headers = getattr(error, "headers", None)
+        message = self.override_message(message, error)
+        return self.render(message, code), code, headers
 
-        This method is expected to be implemented in descendants of this class.
+    @staticmethod
+    def render(message, code):
+        """
+        Using the message and code, generate the appropriate error page content.
         """
         raise NotImplementedError
 
-    def code(self, error):  # pylint: disable=no-self-use
+    def override_message(self, message, error):
+        """
+        For particular sub-class, it may be desired to override/reformat the
+        message for particular error.  This method should be overridden then.
+        """
+        _used_in_subclass = self, error
+        return message
+
+    @staticmethod
+    def code(error):
         """
         Return status code for a given exception
         """
         code = getattr(error, "code", 500)
         return code if code is not None else 500
 
-
-    def message(self, error):  # pylint: disable=no-self-use
+    def message(self, error):
         """
         Return an error message for a given exception. We want to obtain messages
         differently for `CoprHttpException`, `HTTPException`, or others.
         """
+        message = "Unknown problem"
+
+        # Every `CoprHttpException` and `HTTPException` failure has a valuable
+        # message for the end user.  It holds information that e.g. some value
+        # is missing or incorrect, something cannot be done, something doesn't
+        # exist.
         if isinstance(error, HTTPException):
             return error.description
-        return str(error)
+
+        if isinstance(error, CoprHttpException):
+            return str(error)
+
+        # Everything else would normally be an uncaught exception caused by
+        # either not properly running all frontend requirements (PostgreSQL,
+        # Redis), or having a bug in the code.  For such cases we try to do
+        # our best to identify the failure reason (but we stay with
+        # error_code=500).
+        message = ("Request wasn't successful, "
+                   "there is probably a bug in the Copr code.")
+        if isinstance(error, SQLAlchemyError):
+            message = "Database error, contact admin"
+        self._log_admin_only_exception()
+        return message
 
     def _log_admin_only_exception(self):
         # pylint: disable=no-self-use
@@ -92,71 +103,31 @@ class UIErrorHandler(BaseErrorHandler):
     """
     Handle exceptions raised from the web user interface
     """
-
-    def handle_error(self, error):
-        code = self.code(error)
-        message = self.message(error)
-        headers = getattr(error, "headers", None)
-
-        # The most common error has their own custom error pages. When catching
-        # a new exception, try to keep it simple and just the the generic one.
-        # Create it's own view only if necessary.
-        error_views = {
-            400: bad_request_handler,
-            403: access_restricted,
-            404: page_not_found,
-            409: conflict_request_handler,
-            422: unprocessable_entity,
-        }
-        if code in error_views:
-            return error_views[code](message, headers=headers)
-
-        message = "Server error, contact admin"
-        if isinstance(error, SQLAlchemyError):
-            code = 500
-            message = "Database error, contact admin"
-
-        self._log_admin_only_exception()
-        return generic_error(message, code)
+    @staticmethod
+    def render(message, code):
+        title = {
+            400: "Bad Request",
+            409: "Conflict",
+            403: "Access Restricted",
+            404: "Page Not Found",
+            422: "Unprocessable Entity",
+        }.get(code, "Unknown error")
+        return flask.render_template("html-error.html",
+                                     message=message,
+                                     error_code=code,
+                                     error_title=title)
 
 
 class APIErrorHandler(BaseErrorHandler):
     """
     Handle exceptions raised from API (v3)
     """
-
-    def handle_error(self, error):
-        code = self.code(error)
-        message = self.message(error)
-
-        # In the majority of cases, we want to return the message that was
-        # passed through an exception, but occasionally we want to redefine the
-        # message to some API-related one. Please try to keep it simple and
-        # do this only if necessary.
-        errors = {
+    def override_message(self, message, error):
+        return {
             NotFound: "Such API endpoint doesn't exist",
             GatewayTimeout: "The API request timeouted",
-        }
-        if error.__class__ in errors:
-            message = errors[error.__class__]
-
-        # Every `CoprHttpException` and `HTTPException` failure has valuable
-        # message for the end user. It holds information that e.g. some value is
-        # missing or incorrect, something cannot be done, something doesn't
-        # exist. Eveything else should really be an uncaught exception caused by
-        # either not properly running all frontend requirements (PostgreSQL,
-        # Redis), or having a bug in the code.
-        if not any([isinstance(error, CoprHttpException),
-                    isinstance(error, HTTPException)]):
-            message = ("Request wasn't successful, "
-                       "there is probably a bug in the API code.")
-        self._log_admin_only_exception()
-        return self.respond(message, code)
+        }.get(error.__class__, message)
 
-    def respond(self, message, code):  # pylint: disable=no-self-use
-        """
-        Return JSON response suitable for API clients
-        """
-        response = flask.jsonify(error=message)
-        response.status_code = code
-        return response
+    @staticmethod
+    def render(message, code):
+        return flask.jsonify(error=message)
diff --git a/frontend/coprs_frontend/tests/test_exceptions.py b/frontend/coprs_frontend/tests/test_exceptions.py
index 1187b56..1e8767f 100644
--- a/frontend/coprs_frontend/tests/test_exceptions.py
+++ b/frontend/coprs_frontend/tests/test_exceptions.py
@@ -113,7 +113,7 @@ class TestExceptionHandling(CoprsTestCase):
         assert r1.status_code == 500
         data = json.loads(r1.data)
         assert ("Request wasn't successful, there is probably "
-                "a bug in the API code.") in data["error"]
+                "a bug in the Copr code.") in data["error"]
 
     @new_app_context
     def test_api_500_storage(self):

From 35e88520567279b63e2073c36f28681df587c618 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Apr 19 2022 22:58:47 +0000
Subject: [PATCH 6/6] frontend: detect ClientDisconnected errors


The ClientDisconnected is just HttpException sub-class, so we already
handle that.  It is though good to special-case it and properly dump
what is happening to logs.

I realized that previous commit stopped logging the HTTP errors to logs
entirely (we only have error code in the access_log).  It is probably
better to say explicit, and still have a single-line error for each
non-200 error we send to users.

Relates: #1600

---

diff --git a/frontend/coprs_frontend/coprs/error_handlers.py b/frontend/coprs_frontend/coprs/error_handlers.py
index 27dfda7..ea4683d 100644
--- a/frontend/coprs_frontend/coprs/error_handlers.py
+++ b/frontend/coprs_frontend/coprs/error_handlers.py
@@ -6,7 +6,12 @@ import logging
 
 import flask
 from sqlalchemy.exc import SQLAlchemyError
-from werkzeug.exceptions import HTTPException, NotFound, GatewayTimeout
+from werkzeug.exceptions import (
+    ClientDisconnected,
+    GatewayTimeout,
+    HTTPException,
+    NotFound,
+)
 from coprs.exceptions import CoprHttpException
 
 LOG = logging.getLogger(__name__)
@@ -37,6 +42,7 @@ class BaseErrorHandler:
         message = self.message(error)
         headers = getattr(error, "headers", None)
         message = self.override_message(message, error)
+        LOG.error("Response error: %s %s", code, message)
         return self.render(message, code), code, headers
 
     @staticmethod
@@ -74,7 +80,10 @@ class BaseErrorHandler:
         # is missing or incorrect, something cannot be done, something doesn't
         # exist.
         if isinstance(error, HTTPException):
-            return error.description
+            message = error.description
+            if isinstance(error, ClientDisconnected):
+                message = "Client disconnected: " + message
+            return message
 
         if isinstance(error, CoprHttpException):
             return str(error)