Add support for both OIDC and Kerberos auth (#26)

* Add support for both OIDC and Kerberos auth

Authentication would fall back to Kerberos/Negotiate if OpenID Bearer
token is not provided. This can be configured with the following
settings which overrides `AUTH_METHOD` option:

    AUTH_METHODS = ["OIDC", "Kerberos"]

JIRA: RHELWF-7346

* Add Create Waiver form with auto-login

MANIFEST.in

@@ -11,3 +11,5 @@ exclude conf/settings.py
 exclude conf/client.conf
 recursive-include docs *.py *.rst *.inv Makefile
 recursive-include waiverdb/migrations *.ini *mako README
+recursive-include waiverdb/static *
+recursive-include waiverdb/templates *.html

tests/conftest.py

@@ -44,6 +44,11 @@ def enable_kerberos(app, monkeypatch):
     monkeypatch.setitem(app.config, 'AUTH_METHOD', 'Kerberos')
 
 
+@pytest.fixture()
+def enable_kerberos_oidc_fallback(app, monkeypatch):
+    monkeypatch.setitem(app.config, 'AUTH_METHODS', ['Kerberos', 'OIDC'])
+
+
 @pytest.fixture()
 def enable_ssl(app, monkeypatch):
     monkeypatch.setitem(app.config, 'AUTH_METHOD', 'SSL')

tests/test_auth.py

@@ -12,6 +12,14 @@
 
 @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/', content_type='application/json')
@@ -27,31 +35,34 @@ def test_unauthorized(self, client, monkeypatch):
                          __new__=mock.Mock(return_value=None))
     def test_authorized(self, client, monkeypatch):
         monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')
-        data = {
-            'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
-            'testcase': 'testcase1',
-            'product_version': 'fool-1',
-            'waived': True,
-            'comment': 'it broke',
-        }
         headers = {'Authorization':
                    'Negotiate %s' % b64encode(b"CTOKEN").decode()}
-        r = client.post('/api/v1.0/waivers/', data=json.dumps(data),
+        r = client.post('/api/v1.0/waivers/', data=json.dumps(self.waiver_data),
                         content_type='application/json', headers=headers)
         assert r.status_code == 201
         assert r.headers.get('WWW-Authenticate') == \
             'negotiate %s' % b64encode(b"STOKEN").decode()
         res_data = json.loads(r.data.decode('utf-8'))
         assert res_data['username'] == 'foo'
 
+    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),
+                        content_type='application/json', headers=headers)
+        assert r.status_code == 401
+        assert r.json == {"message": "Invalid authentication token"}
+
 
 class TestOIDCAuthentication(object):
+    invalid_token_error = "Token required but invalid"
+    auth_missing_error = "No 'Authorization' header found"
 
     def test_get_user_without_token(self, session):
         with pytest.raises(Unauthorized) as excinfo:
             request = mock.MagicMock()
             waiverdb.auth.get_user(request)
-        assert "No 'Authorization' header found" in str(excinfo.value)
+        assert self.auth_missing_error in str(excinfo.value)
 
     @mock.patch.object(flask_oidc.OpenIDConnect, '_get_token_info')
     def test_get_user_with_invalid_token(self, mocked_get_token, session):
@@ -67,7 +78,7 @@ def test_get_user_with_invalid_token(self, mocked_get_token, session):
         request.headers.__contains__.side_effect = headers.__contains__
         with pytest.raises(Unauthorized) as excinfo:
             waiverdb.auth.get_user(request)
-        assert 'Token required but invalid' in excinfo.value.get_description()
+        assert self.invalid_token_error in excinfo.value.get_description()
 
     @mock.patch.object(flask_oidc.OpenIDConnect, '_get_token_info')
     def test_get_user_good(self, mocked_get_token, session):
@@ -110,3 +121,17 @@ def test_good_ssl_cert(self):
         request = mock.MagicMock(environ=ssl)
         user, header = waiverdb.auth.get_user(request)
         assert user == name
+
+
+@pytest.mark.usefixtures('enable_kerberos_oidc_fallback')
+class TestKerberosWithFallbackAuthentication(TestGSSAPIAuthentication):
+    def test_unauthorized(self, client, monkeypatch):
+        monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')
+        r = client.post('/api/v1.0/waivers/', content_type='application/json')
+        assert r.status_code == 401
+
+
+@pytest.mark.usefixtures('enable_kerberos_oidc_fallback')
+class TestOIDCWithFallbackAuthentication(TestOIDCAuthentication):
+    invalid_token_error = ""
+    auth_missing_error = ""

waiverdb/api_v1.py

@@ -4,7 +4,8 @@
 import logging
 
 import requests
-from flask import Blueprint, request, current_app
+from flask import Blueprint, render_template, request, current_app, Response
+from flask_oidc import OpenIDConnect
 from flask_restful import Resource, Api, reqparse, marshal_with, marshal
 from werkzeug.exceptions import (
     BadRequest,
@@ -25,6 +26,7 @@
 api = Api(api_v1)
 requests_session = requests.Session()
 log = logging.getLogger(__name__)
+oidc = OpenIDConnect()
 
 
 def valid_dict(value):
@@ -148,6 +150,14 @@ def _filter_out_obsolete_waivers(query):
 RP['create_waiver'].add_argument('username', type=str, default=None, location='json')
 RP['create_waiver'].add_argument('scenario', type=str, default=None, location='json')
 
+RP['create_waiver_form'] = reqparse.RequestParser()
+RP['create_waiver_form'].add_argument('subject_type', type=str, location='form')
+RP['create_waiver_form'].add_argument('subject_identifier', type=str, location='form')
+RP['create_waiver_form'].add_argument('testcase', type=str, location='form')
+RP['create_waiver_form'].add_argument('product_version', type=str, required=True, location='form')
+RP['create_waiver_form'].add_argument('comment', type=str, required=True, location='form')
+RP['create_waiver_form'].add_argument('scenario', type=str, default=None, location='form')
+
 RP['get_waivers'] = reqparse.RequestParser()
 RP['get_waivers'].add_argument('subject_type', location='args')
 RP['get_waivers'].add_argument('subject_identifier', location='args')
@@ -387,7 +397,7 @@ def _create_waiver(self, args, user):
             user = args['username']
 
         # WaiverDB < 0.6
-        if args['result_id']:
+        if args.get('result_id'):
             if args['subject'] or args['testcase'] or args['scenario']:
                 raise BadRequest('result_id argument should not be used together with arguments: '
                                  '"subject", "testcase" or "scenario"')
@@ -419,7 +429,7 @@ def _create_waiver(self, args, user):
                 args['scenario'] = result_data['scenario'][0]
 
         # WaiverDB < 0.11
-        if args['subject']:
+        if args.get('subject'):
             args['subject_type'], args['subject_identifier'] = \
                 subject_dict_to_type_identifier(args.pop('subject'))
 
@@ -446,13 +456,44 @@ def _create_waiver(self, args, user):
             args['testcase'],
             user,
             args['product_version'],
-            args['waived'],
+            args.get('waived', True),
             args['comment'],
             proxied_by,
             args['scenario']
         )
 
 
+class WaiversNewResource(WaiversResource):
+    def get(self):
+        """
+        HTML form to create a waiver.
+
+        Default form input field values can be passed as request query parameters.
+
+        :query string subject_type: The type of thing which this waiver is for.
+        :query string subject_identifier: The identifier of the thing this
+            waiver is for. The semantics of this identifier depend on the
+            "subject_type". For example, Koji builds are identified by their NVR.
+        :query string testcase: The result testcase for the waiver.
+        :query string scenario: The result scenario for the waiver.
+        :query string product_version: The product version string.
+        :query string comment: A comment explaining the waiver.
+
+        :statuscode 200: The HTML with the form is returned.
+        """
+        html = render_template('new_waiver.html', request_args=request.args)
+        return Response(html, mimetype='text/html')
+
+    @marshal_with(waiver_fields)
+    def post(self):
+        user, headers = waiverdb.auth.get_user(request)
+        args = RP['create_waiver_form'].parse_args()
+        result = self._create_waiver(args, user)
+        db.session.add(result)
+        db.session.commit()
+        return result, 201, headers
+
+
 class WaiverResource(Resource):
     @jsonp
     @marshal_with(waiver_fields)
@@ -766,6 +807,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(WaiverResource, '/waivers/<int:waiver_id>')
 api.add_resource(FilteredWaiversResource, '/waivers/+filtered')
 api.add_resource(GetWaiversBySubjectsAndTestcases, '/waivers/+by-subjects-and-testcases')

waiverdb/app.py

@@ -7,7 +7,7 @@
 except ImportError:
     from urlparse import urlparse, urlunsplit
 
-from flask import Flask, current_app
+from flask import Flask, current_app, send_from_directory
 from flask_cors import CORS
 from flask_migrate import Migrate
 from sqlalchemy import event
@@ -16,10 +16,9 @@
 
 from waiverdb.events import publish_new_waiver
 from waiverdb.logger import init_logging
-from waiverdb.api_v1 import api_v1
+from waiverdb.api_v1 import api_v1, oidc
 from waiverdb.models import db
-from waiverdb.utils import json_error
-from flask_oidc import OpenIDConnect
+from waiverdb.utils import auth_methods, json_error
 from werkzeug.exceptions import default_exceptions
 from waiverdb.monitor import db_hook_event_listeners
 
@@ -105,8 +104,9 @@ def create_app(config_obj=None):
     app.register_error_handler(requests.Timeout, json_error)
 
     populate_db_config(app)
-    if app.config['AUTH_METHOD'] == 'OIDC':
-        app.oidc = OpenIDConnect(app)
+    if 'OIDC' in auth_methods(app):
+        oidc.init_app(app)
+        app.oidc = oidc
     # initialize logging
     init_logging(app)
     # initialize db
@@ -118,6 +118,8 @@ def create_app(config_obj=None):
     # register blueprints
     app.register_blueprint(api_v1, url_prefix="/api/v1.0")
     app.add_url_rule('/healthcheck', view_func=healthcheck)
+    app.add_url_rule('/auth/oidclogin', view_func=login)
+    app.add_url_rule('/favicon.png', view_func=favicon)
     register_event_handlers(app)
 
     # initialize DB event listeners from the monitor module
@@ -145,6 +147,14 @@ def healthcheck():
     return ('Health check OK', 200, [('Content-Type', 'text/plain')])
 
 
+@oidc.require_login
+def login():
+    return {
+        'email': oidc.user_getfield('email'),
+        'token': oidc.get_access_token(),
+    }
+
+
 def register_event_handlers(app):
     """
     Register SQLAlchemy event handlers with the application's session factory.
@@ -158,3 +168,11 @@ def register_event_handlers(app):
         # can be removed after python-flask-sqlalchemy is upgraded to 2.2
         from flask_sqlalchemy import SignallingSession
         event.listen(SignallingSession, 'after_commit', publish_new_waiver)
+
+
+def favicon():
+    return send_from_directory(
+        os.path.join(current_app.root_path, 'static'),
+        'favicon.png',
+        mimetype='image/png',
+    )

waiverdb/auth.py

@@ -2,12 +2,17 @@
 
 
 import base64
+import binascii
 import os
 if not os.getenv('DOCS'):   # installing gssapi causing a problem for documentation building
     import gssapi
 from flask import current_app, Response, g
 from werkzeug.exceptions import Unauthorized, Forbidden
 
+from waiverdb.utils import auth_methods
+
+OIDC_AUTH_HEADER_PREFIX = "Bearer "
+
 
 # Inspired by https://github.com/mkomitee/flask-kerberos/blob/master/flask_kerberos.py
 # Later cleaned and ported to python-gssapi
@@ -40,16 +45,31 @@ def process_gssapi_request(token):
 
 
 def get_user(request):
+    methods = auth_methods(current_app)
+    if not methods:
+        raise Unauthorized("Authenticated user required")
+
+    if len(methods) > 1:
+        auth_header = request.headers.get("Authorization", "").strip()
+        if "OIDC" in methods and auth_header.startswith(OIDC_AUTH_HEADER_PREFIX):
+            return get_user_by_method(request, "OIDC")
+        if "Kerberos" in methods and not auth_header.startswith(OIDC_AUTH_HEADER_PREFIX):
+            return get_user_by_method(request, "Kerberos")
+
+    return get_user_by_method(request, methods[0])
+
+
+def get_user_by_method(request, auth_method):
     user = None
     headers = dict()
-    if current_app.config['AUTH_METHOD'] == 'OIDC':
+    if auth_method == 'OIDC':
         if 'Authorization' not in request.headers:
             raise Unauthorized("No 'Authorization' header found.")
         token = request.headers.get("Authorization").strip()
-        prefix = 'Bearer '
-        if not token.startswith(prefix):
-            raise Unauthorized('Authorization headers must start with %r' % prefix)
-        token = token[len(prefix):].strip()
+        if not token.startswith(OIDC_AUTH_HEADER_PREFIX):
+            raise Unauthorized(
+                f"Authorization headers must start with {OIDC_AUTH_HEADER_PREFIX}")
+        token = token[len(OIDC_AUTH_HEADER_PREFIX):].strip()
         required_scopes = [
             'openid',
             current_app.config['OIDC_REQUIRED_SCOPE'],
@@ -58,18 +78,21 @@ def get_user(request):
         if validity is not True:
             raise Unauthorized(validity)
         user = g.oidc_token_info['username']
-    elif current_app.config['AUTH_METHOD'] == 'Kerberos':
+    elif auth_method == 'Kerberos':
         if 'Authorization' not in request.headers:
             response = Response('Unauthorized', 401, {'WWW-Authenticate': 'Negotiate'})
             raise Unauthorized(response=response)
         header = request.headers.get("Authorization")
         token = ''.join(header.strip().split()[1:])
-        user, token = process_gssapi_request(base64.b64decode(token))
+        try:
+            user, token = process_gssapi_request(base64.b64decode(token))
+        except binascii.Error:
+            raise Unauthorized("Invalid authentication token")
         # remove realm
         user = user.split("@")[0]
         headers = {'WWW-Authenticate': ' '.join(
             ['negotiate', base64.b64encode(token).decode()])}
-    elif current_app.config['AUTH_METHOD'] == 'SSL':
+    elif auth_method == 'SSL':
         # Nginx sets SSL_CLIENT_VERIFY and SSL_CLIENT_S_DN in request.environ
         # when doing SSL authentication.
         ssl_client_verify = request.environ.get('SSL_CLIENT_VERIFY')
@@ -78,7 +101,7 @@ def get_user(request):
         if not request.environ.get('SSL_CLIENT_S_DN'):
             raise Unauthorized('Unable to get user information (DN) from the client certificate')
         user = request.environ.get('SSL_CLIENT_S_DN')
-    elif current_app.config['AUTH_METHOD'] == 'dummy':
+    elif auth_method == 'dummy':
         # Blindly accept any username. For testing purposes only of course!
         if not request.authorization:
             response = Response('Unauthorized', 401, {'WWW-Authenticate': 'Basic realm="dummy"'})

waiverdb/cli.py

@@ -271,10 +271,16 @@ def cli(username, comment, waived, product_version, testcase, scenario, subject,
         oidc_client_secret = None
         if config.has_option('waiverdb', 'oidc_client_secret'):
             oidc_client_secret = config.get('waiverdb', 'oidc_client_secret')
+        id_provider_mapping = {
+            'Token': config.get(
+                'waiverdb', 'oidc_token_endpoint', fallback='Token'),
+            'Authorization': config.get(
+                'waiverdb', 'oidc_auth_endpoint', fallback='Authorization'),
+        }
         oidc = openidc_client.OpenIDCClient(
             'waiverdb',
             config.get('waiverdb', 'oidc_id_provider'),
-            {'Token': 'Token', 'Authorization': 'Authorization'},
+            id_provider_mapping,
             config.get('waiverdb', 'oidc_client_id'),
             oidc_client_secret)
         scopes = config.get('waiverdb', 'oidc_scopes').strip().splitlines()