From 5f14ce4198b2cb0cecde1153a433587c18cce14e Mon Sep 17 00:00:00 2001
From: Jan Kaluza <jkaluza@redhat.com>
Date: Jan 05 2018 09:22:08 +0000
Subject: Require 'submit-build' openidc scope to trigger manual rebuild, return HTTP errors in case of exception in Flask handler.


---

diff --git a/freshmaker/errors.py b/freshmaker/errors.py
index b162a4c..eec3fb6 100644
--- a/freshmaker/errors.py
+++ b/freshmaker/errors.py
@@ -22,6 +22,9 @@
 # Written by Jan Kaluza <jkaluza@redhat.com>
 """ Defines custom exceptions and error handling functions """
 
+from flask import jsonify
+from freshmaker import app, log
+
 
 class ValidationError(ValueError):
     pass
@@ -49,3 +52,43 @@ class Unauthorized(ValueError):
 
 class Forbidden(ValueError):
     pass
+
+
+def json_error(status, error, message):
+    response = jsonify(
+        {'status': status,
+         'error': error,
+         'message': message})
+    response.status_code = status
+    return response
+
+
+@app.errorhandler(NotFound)
+def notfound_error(e):
+    """Flask error handler for NotFound exceptions"""
+    return json_error(404, 'Not Found', e.args[0])
+
+
+@app.errorhandler(Unauthorized)
+def unauthorized_error(e):
+    """Flask error handler for Unauthorized exceptions"""
+    return json_error(401, 'Unauthorized', e.args[0])
+
+
+@app.errorhandler(Forbidden)
+def forbidden_error(e):
+    """Flask error handler for Forbidden exceptions"""
+    return json_error(403, 'Forbidden', e.args[0])
+
+
+@app.errorhandler(ValueError)
+def validationerror_error(e):
+    """Flask error handler for ValueError exceptions"""
+    return json_error(400, 'Bad Request', str(e))
+
+
+@app.errorhandler(Exception)
+def internal_server_error(e):
+    """Flask error handler for RuntimeError exceptions"""
+    log.exception('Internal server error: %s', e)
+    return json_error(500, 'Internal Server Error', str(e))
diff --git a/freshmaker/views.py b/freshmaker/views.py
index e3c83f4..cac6a36 100644
--- a/freshmaker/views.py
+++ b/freshmaker/views.py
@@ -32,7 +32,7 @@ from freshmaker.api_utils import pagination_metadata
 from freshmaker.api_utils import filter_artifact_builds
 from freshmaker.api_utils import filter_events
 from freshmaker.api_utils import json_error
-from freshmaker.auth import login_required, requires_role
+from freshmaker.auth import login_required, requires_role, require_scopes
 from freshmaker import messaging
 
 api_v1 = {
@@ -223,6 +223,7 @@ class BuildAPI(MethodView):
                 return json_error(404, "Not Found", "No such build found.")
 
     @login_required
+    @require_scopes('submit-build')
     @requires_role('admins')
     def post(self):
         """Trigger image rebuild"""
diff --git a/tests/test_views.py b/tests/test_views.py
index 4a61415..6acda0f 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -22,11 +22,98 @@
 import unittest
 import json
 import six
+import contextlib
+import flask
 
 from mock import patch
 
-from freshmaker import app, db, events, models
+from freshmaker import app, db, events, models, login_manager
 from freshmaker.types import ArtifactType, ArtifactBuildState, EventState
+import freshmaker.auth
+
+
+@login_manager.user_loader
+def user_loader(username):
+    return models.User.find_user_by_name(username=username)
+
+
+class ViewBaseTest(unittest.TestCase):
+    def setUp(self):
+        db.session.remove()
+        db.drop_all()
+        db.create_all()
+        db.session.commit()
+
+        patched_allowed_clients = {'groups': ['freshmaker-clients'],
+                                   'users': ['dev']}
+        patched_admins = {'groups': ['admin'], 'users': ['root']}
+        self.patch_allowed_clients = patch.object(freshmaker.auth.conf,
+                                                  'allowed_clients',
+                                                  new=patched_allowed_clients)
+        self.patch_admins = patch.object(freshmaker.auth.conf,
+                                         'admins',
+                                         new=patched_admins)
+        self.patch_allowed_clients.start()
+        self.patch_admins.start()
+
+        self.patch_oidc_base_namespace = patch.object(
+            freshmaker.auth.conf, 'oidc_base_namespace',
+            new='http://example.com/')
+        self.patch_oidc_base_namespace.start()
+
+        self.client = app.test_client()
+
+        self.setup_test_data()
+
+    def tearDown(self):
+        db.session.remove()
+        db.drop_all()
+        db.session.commit()
+
+        self.patch_allowed_clients.stop()
+        self.patch_admins.stop()
+        self.patch_oidc_base_namespace.stop()
+
+    @contextlib.contextmanager
+    def test_request_context(self, user=None, groups=None, auth_backend=None,
+                             oidc_scopes=None, **kwargs):
+        with app.test_request_context(**kwargs):
+            patch_auth_backend = None
+            if user is not None:
+                # authentication is disabled with auth_backend=noauth
+                patch_auth_backend = patch.object(
+                    freshmaker.auth.conf, 'auth_backend',
+                    new=auth_backend if auth_backend else "kerberos")
+                patch_auth_backend.start()
+                if not models.User.find_user_by_name(user):
+                    models.User.create_user(username=user)
+                    db.session.commit()
+                flask.g.user = models.User.find_user_by_name(user)
+
+                if groups is not None:
+                    if isinstance(groups, list):
+                        flask.g.groups = groups
+                    else:
+                        flask.g.groups = [groups]
+                else:
+                    flask.g.groups = []
+                with self.client.session_transaction() as sess:
+                    sess['user_id'] = user
+                    sess['_fresh'] = True
+
+                oidc_scopes = oidc_scopes if oidc_scopes else []
+                oidc_namespace = freshmaker.auth.conf.oidc_base_namespace
+                flask.g.oidc_scopes = [
+                    '{0}{1}'.format(oidc_namespace, scope) for scope in
+                    oidc_scopes]
+            try:
+                yield
+            finally:
+                if patch_auth_backend is not None:
+                    patch_auth_backend.stop()
+
+    def setup_test_data(self):
+        """Set up data for running tests"""
 
 
 class TestViews(unittest.TestCase):
@@ -118,9 +205,11 @@ class TestViews(unittest.TestCase):
         self.assertEqual(len(builds), 3)
 
     def test_query_builds_by_invalid_type(self):
-        with self.assertRaises(ValueError) as ctx:
-            self.client.get('/api/1/builds/?type=100')
-        self.assertEqual(str(ctx.exception), 'An invalid artifact type was supplied')
+        resp = self.client.get('/api/1/builds/?type=100')
+        data = json.loads(resp.data.decode('utf8'))
+        self.assertEqual(data["status"], 400)
+        self.assertEqual(data["message"],
+                         "An invalid artifact type was supplied")
 
     def test_query_builds_by_state(self):
         resp = self.client.get('/api/1/builds/?state=0')
@@ -128,9 +217,11 @@ class TestViews(unittest.TestCase):
         self.assertEqual(len(builds), 3)
 
     def test_query_builds_by_invalid_state(self):
-        with self.assertRaises(ValueError) as ctx:
-            self.client.get('/api/1/builds/?state=100')
-        self.assertEqual(str(ctx.exception), 'An invalid state was supplied')
+        resp = self.client.get('/api/1/builds/?state=100')
+        data = json.loads(resp.data.decode('utf8'))
+        self.assertEqual(data["status"], 400)
+        self.assertEqual(data["message"],
+                         "An invalid state was supplied")
 
     def test_query_build_by_event_type_id(self):
         event1 = models.Event.create(db.session,
@@ -340,5 +431,33 @@ class TestManualTriggerRebuild(unittest.TestCase):
         publish.assert_called_once_with('manual.rebuild', {u'errata_id': 1})
 
 
+class TestOpenIDCLogin(ViewBaseTest):
+    """Test that OpenIDC login"""
+
+    def setUp(self):
+        super(TestOpenIDCLogin, self).setUp()
+        self.patch_auth_backend = patch.object(
+            freshmaker.auth.conf, 'auth_backend', new='openidc')
+        self.patch_auth_backend.start()
+
+    def tearDown(self):
+        super(TestOpenIDCLogin, self).tearDown()
+        self.patch_auth_backend.stop()
+
+    def test_openidc_manual_trigger_unauthorized(self):
+        rv = self.client.post('/api/1/builds/',
+                              data=json.dumps({'errata_id': 1}),
+                              content_type='application/json')
+        self.assertEqual(rv.status, '401 UNAUTHORIZED')
+
+    def test_openidc_manual_trigger_authorized(self):
+        with self.test_request_context(user='dev', auth_backend="openidc",
+                                       oidc_scopes=["submit-build"]):
+            rv = self.client.post('/api/1/builds/',
+                                  data=json.dumps({'errata_id': 1}),
+                                  content_type='application/json')
+            self.assertEqual(rv.status, '403 FORBIDDEN')
+
+
 if __name__ == '__main__':
     unittest.main()

