Merge 4962343 into d50068b

tests/test_auth.py

@@ -10,12 +10,30 @@
 import flask_oidc
 from flask import g
 
+WAIVER_DATA = {
+    'subject_type': 'koji_build',
+    'subject_identifier': 'glibc-2.26-27.fc27',
+    'testcase': 'testcase1',
+    'product_version': 'fool-1',
+    'waived': True,
+    'comment': 'it broke',
+}
+WAIVER_PARAMS = '&'.join(
+    f'{k}={v}'
+    for k, v in WAIVER_DATA.items()
+    if isinstance(v, str)
+)
+
 
 @pytest.fixture
 def oidc_token(app):
     with mock.patch.object(flask_oidc.OpenIDConnect, '_get_token_info') as mocked:
         mocked.return_value = {
-            'active': True, 'username': 'testuser', 'scope': 'openid waiverdb_scope'}
+            'active': True,
+            'username': 'testuser',
+            'preferred_username': 'testuser',
+            'scope': 'openid waiverdb_scope',
+        }
         with app.app_context():
             g.oidc_id_token = mocked()
             yield g.oidc_id_token
@@ -35,17 +53,9 @@ def permissions():
 
 @pytest.mark.usefixtures('enable_kerberos')
 class TestGSSAPIAuthentication(object):
-    waiver_data = {
-        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
-        'testcase': 'testcase1',
-        'product_version': 'fool-1',
-        'waived': True,
-        'comment': 'it broke',
-    }
-
     def test_unauthorized(self, client, monkeypatch):
         monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')
-        r = client.post('/api/v1.0/waivers/', data=json.dumps(self.waiver_data),
+        r = client.post('/api/v1.0/waivers/', data=json.dumps(WAIVER_DATA),
                         content_type='application/json')
         assert r.status_code == 401
         assert r.headers.get('www-authenticate') == 'Negotiate'
@@ -61,7 +71,7 @@ def test_authorized(self, client, monkeypatch):
         monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')
         headers = {'Authorization':
                    'Negotiate %s' % b64encode(b"CTOKEN").decode()}
-        r = client.post('/api/v1.0/waivers/', data=json.dumps(self.waiver_data),
+        r = client.post('/api/v1.0/waivers/', data=json.dumps(WAIVER_DATA),
                         content_type='application/json', headers=headers)
         assert r.status_code == 201
         assert r.headers.get('WWW-Authenticate') == \
@@ -72,7 +82,7 @@ def test_authorized(self, client, monkeypatch):
     def test_invalid_token(self, client, monkeypatch):
         monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')
         headers = {'Authorization': 'Negotiate INVALID'}
-        r = client.post('/api/v1.0/waivers/', data=json.dumps(self.waiver_data),
+        r = client.post('/api/v1.0/waivers/', data=json.dumps(WAIVER_DATA),
                         content_type='application/json', headers=headers)
         assert r.status_code == 401
         assert r.json == {"message": "Invalid authentication token"}
@@ -146,6 +156,79 @@ def test_no_warning_banner(
         assert "alert" not in r.text
         assert r.status_code == 200
 
+    def test_new_waiver_banner(
+        self,
+        oidc_token,
+        session,
+        client,
+    ):
+        headers = {'Authorization': 'Bearer foobar'}
+        r = client.get('/api/v1.0/waivers/new?new_waiver_id=123', headers=headers)
+        banner = (
+            '<div class="alert alert-success" role="alert">'
+            '<a href="/api/v1.0/waivers/123">New waiver created.</a>'
+            '</div>'
+        )
+        assert banner in r.text
+        assert r.status_code == 200
+
+    def test_create_new_waiver(
+        self,
+        verify_authorization,
+        permissions,
+        oidc_token,
+        session,
+        client,
+    ):
+        verify_authorization.return_value = True
+        permissions.return_value = [{"testcases": ["a.b.c"], "groups": []}]
+        headers = {'Authorization': 'Bearer foobar'}
+        url = f'/api/v1.0/waivers/create?{WAIVER_PARAMS}'
+        r = client.get(
+            url,
+            headers=headers,
+            follow_redirects=True,
+        )
+        assert 'New waiver created.' in r.text
+        assert r.status_code == 200
+        assert r.request.base_url.endswith('/api/v1.0/waivers/new')
+        expected_args = {
+            k: v
+            for k, v in WAIVER_DATA.items()
+            if isinstance(v, str)
+        }
+        expected_args['new_waiver_id'] = '1'
+        assert dict(r.request.args) == expected_args
+
+    def test_create_new_waiver_unauthorized(
+        self,
+        verify_authorization,
+        permissions,
+        oidc_token,
+        session,
+        client,
+    ):
+        verify_authorization.side_effect = Unauthorized("Unauthorized")
+        permissions.return_value = [{"testcases": ["a.b.c"], "groups": []}]
+        headers = {'Authorization': 'Bearer foobar'}
+        url = f'/api/v1.0/waivers/create?{WAIVER_PARAMS}'
+        r = client.get(
+            url,
+            headers=headers,
+            follow_redirects=True,
+        )
+        assert '401 Unauthorized: Unauthorized' in r.text
+        assert 'New waiver created.' not in r.text
+        assert r.status_code == 200
+        assert r.request.base_url.endswith('/api/v1.0/waivers/new')
+        expected_args = {
+            k: v
+            for k, v in WAIVER_DATA.items()
+            if isinstance(v, str)
+        }
+        expected_args['error'] = mock.ANY
+        assert dict(r.request.args) == expected_args
+
 
 @pytest.mark.usefixtures('enable_ssl')
 class TestSSLAuthentication(object):
@@ -176,7 +259,7 @@ def test_good_ssl_cert(self):
 class TestKerberosWithFallbackAuthentication(TestGSSAPIAuthentication):
     def test_unauthorized(self, client, monkeypatch):
         monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')
-        r = client.post('/api/v1.0/waivers/', data=json.dumps(self.waiver_data),
+        r = client.post('/api/v1.0/waivers/', data=json.dumps(WAIVER_DATA),
                         content_type='application/json')
         assert r.status_code == 401
 

waiverdb/api_v1.py

@@ -8,6 +8,7 @@
     Response,
     current_app,
     escape,
+    redirect,
     render_template,
     request,
     url_for,
@@ -107,20 +108,24 @@ def _verify_authorization(user, testcase):
     return verify_authorization(user, testcase, permissions(), ldap_host, ldap_searches)
 
 
+def _authorization_warning_from_exception(e: Unauthorized, testcase: str):
+    permissions_url = url_for('api_v1.permissionsresource', testcase=testcase)
+    return (
+        f"{escape(str(e))}<br>"
+        f"<a href={permissions_url}>See who has permission to"
+        f" waive {escape(testcase)} test case."
+        "</a>"
+    )
+
+
 def _authorization_warning(request):
     testcase = request.args.get("testcase")
     if testcase:
         user, _headers = waiverdb.auth.get_user(request)
         try:
             _verify_authorization(user, testcase)
         except Unauthorized as e:
-            permissions_url = url_for('api_v1.permissionsresource', testcase=testcase)
-            return (
-                f"{escape(str(e))}<br>"
-                f"<a href={permissions_url}>See who has permission to"
-                f" waive {escape(testcase)} test case."
-                "</a>"
-            )
+            return _authorization_warning_from_exception(e, testcase)
     return None
 
 
@@ -378,19 +383,46 @@ def get(self):
 
         :statuscode 200: The HTML with the form is returned.
         """
-        warning = _authorization_warning(request)
-        html = render_template('new_waiver.html', warning=warning, request_args=request.args)
+        warning = request.args.get("error") or _authorization_warning(request)
+        new_waiver_id = request.args.get("new_waiver_id")
+        new_waiver_url = None
+        if new_waiver_id is not None:
+            new_waiver_url = url_for('api_v1.waiverresource', waiver_id=new_waiver_id)
+        html = render_template(
+            'new_waiver.html',
+            warning=warning,
+            error=request.args.get("error"),
+            new_waiver_url=new_waiver_url,
+            request_args=request.args,
+        )
         return Response(html, mimetype='text/html')
 
+
+class WaiversCreateResource(WaiversResource):
     @oidc.require_login
-    @validate(form=CreateWaiver)
-    @marshal_with(waiver_fields)
-    def post(self):
+    @validate()
+    def get(self, query: CreateWaiver):
         user = oidc.user_getfield(current_app.config["OIDC_USERNAME_FIELD"])
-        result = self._create_waiver(CreateWaiver.parse_obj(request.form), user)
+        try:
+            result = self._create_waiver(query, user)
+        except Unauthorized as e:
+            error = _authorization_warning_from_exception(e, query.testcase)
+            url = url_for(
+                "api_v1.waiversnewresource",
+                error=error,
+                **request.args,
+            )
+            return redirect(url)
+
         db.session.add(result)
         db.session.commit()
-        return result, 201
+
+        url = url_for(
+            "api_v1.waiversnewresource",
+            new_waiver_id=result.id,
+            **request.args,
+        )
+        return redirect(url)
 
 
 class WaiverResource(Resource):
@@ -708,6 +740,7 @@ def get(self):
 # set up the Api resource routing here
 api.add_resource(WaiversResource, '/waivers/')
 api.add_resource(WaiversNewResource, '/waivers/new')
+api.add_resource(WaiversCreateResource, '/waivers/create')
 api.add_resource(WaiverResource, '/waivers/<int:waiver_id>')
 api.add_resource(FilteredWaiversResource, '/waivers/+filtered')
 api.add_resource(GetWaiversBySubjectsAndTestcases, '/waivers/+by-subjects-and-testcases')

waiverdb/templates/new_waiver.html

@@ -8,7 +8,10 @@
     {% if warning %}
     <div class="alert alert-danger" role="alert">{{ warning | safe }}</div>
     {% endif %}
-    <form method="post">
+    {% if new_waiver_url %}
+    <div class="alert alert-success" role="alert"><a href="{{ new_waiver_url | safe }}">New waiver created.</a></div>
+    {% endif %}
+    <form method="get" action="{{ url_for('api_v1.waiverscreateresource') }}">
         <div class="mb-3">
             <label for="subject_type" class="form-label">Subject Type</label>
             <input type="text" class="form-control" name="subject_type"