From 14b10015993587ee4dc485f050aabe178fcd50eb Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Apr 30 2019 08:09:54 +0000 Subject: [PATCH 1/2] Restrict waiver creation based on users/groups and testcase Introduce access control based on the users/groups and the testcase. Groups need to be defined in LDAP. New configuration is required to enable this feature: * PERMISSION_MAPPING: dictionary with keys regex applied to testcases and as values dictionaries with "users" and "groups" allowed to submit waivers for that matching testcase. If not specified, the feature is not enabled. * LDAP_HOST and LDAP_BASE: required to query the LDAP system. If PERMISSION_MAPPING is specified, but these are not, it throws an error. --- diff --git a/tests/conftest.py b/tests/conftest.py index 251fbc8..cf7eba5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -81,3 +81,23 @@ def enable_ssl(app, monkeypatch): @pytest.fixture() def enable_cors(app, monkeypatch): monkeypatch.setitem(app.config, 'CORS_URL', 'https://bodhi.fedoraproject.org') + + +@pytest.fixture() +def enable_permission_mapping(app, monkeypatch): + monkeypatch.setitem(app.config, 'PERMISSION_MAPPING', + { + "^testcase1.*": {"groups": ["factory-2-0"], "users": []}, # noqa + "^testcase2.*": {"groups": [], "users": ["foo"]}, # noqa + "^testcase4.*": {"groups": [], "users": []} # noqa + }) + + +@pytest.fixture() +def enable_ldap_host(app, monkeypatch): + monkeypatch.setitem(app.config, 'LDAP_HOST', 'ldap://ldap.something.com') + + +@pytest.fixture() +def enable_ldap_base(app, monkeypatch): + monkeypatch.setitem(app.config, 'LDAP_BASE', 'ou=Users,dc=something,dc=com') diff --git a/tests/test_access_control.py b/tests/test_access_control.py new file mode 100644 index 0000000..3e0a85d --- /dev/null +++ b/tests/test_access_control.py @@ -0,0 +1,119 @@ +# SPDX-License-Identifier: GPL-2.0+ + +import json +from base64 import b64encode + +import mock +import ldap +import pytest + + +@pytest.mark.usefixtures('enable_permission_mapping') +@pytest.mark.usefixtures('enable_kerberos') +@mock.patch.multiple("gssapi.SecurityContext", complete=True, + __init__=mock.Mock(return_value=None), + step=mock.Mock(return_value=b"STOKEN"), + initiator_name="foo@EXAMPLE.ORG") +@mock.patch.multiple("gssapi.Credentials", + __init__=mock.Mock(return_value=None), + __new__=mock.Mock(return_value=None)) +class TestAccessControl(object): + + data = { + 'subject_type': 'koji_build', + 'subject_identifier': 'glibc-2.26-27.fc27', + 'testcase': 'testcase1', + 'product_version': 'fool-1', + 'waived': True, + 'comment': 'it broke', + } + headers = {'Authorization': + 'Negotiate %s' % b64encode(b"CTOKEN").decode()} + + def test_ldap_host_base_not_defined(self, client, session): + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 500 + assert res_data['message'] == ("LDAP_HOST and LDAP_BASE also need to be " + "defined if PERMISSION_MAPPING is defined.") + + @pytest.mark.usefixtures('enable_ldap_host') + def test_ldap_host_defined_base_not(self, client, session): + self.test_ldap_host_base_not_defined(client, session) + + @pytest.mark.usefixtures('enable_ldap_base') + def test_ldap_base_defined_host_not(self, client, session): + self.test_ldap_host_base_not_defined(client, session) + + @pytest.mark.usefixtures('enable_ldap_host') + @pytest.mark.usefixtures('enable_ldap_base') + @mock.patch('ldap.initialize', side_effect=ldap.LDAPError()) + def test_initialization_ldap_connection(self, mocked, client, session): + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 401 + assert res_data['message'] == "Some error occured initializing the LDAP connection." + + @pytest.mark.usefixtures('enable_ldap_host') + @pytest.mark.usefixtures('enable_ldap_base') + @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', return_value=([])) + def test_user_not_found_in_ldap(self, mocked_conn, client, session, monkeypatch): + monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab') + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 401 + assert res_data['message'] == "Couldn't find user foo in LDAP" + + @pytest.mark.usefixtures('enable_ldap_host') + @pytest.mark.usefixtures('enable_ldap_base') + @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', + return_value=(['factory-2-0', 'something-else'])) + def test_group_has_permission(self, mocked_conn, client, session, monkeypatch): + monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab') + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 201 + assert res_data['username'] == 'foo' + assert res_data['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'} + assert res_data['subject_type'] == 'koji_build' + assert res_data['subject_identifier'] == 'glibc-2.26-27.fc27' + assert res_data['testcase'] == 'testcase1' + assert res_data['product_version'] == 'fool-1' + assert res_data['waived'] is True + assert res_data['comment'] == 'it broke' + + @pytest.mark.usefixtures('enable_ldap_host') + @pytest.mark.usefixtures('enable_ldap_base') + def test_user_has_permission(self, client, session, monkeypatch): + monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab') + self.data['testcase'] = 'testcase2' + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 201 + assert res_data['username'] == 'foo' + assert res_data['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'} + assert res_data['subject_type'] == 'koji_build' + assert res_data['subject_identifier'] == 'glibc-2.26-27.fc27' + assert res_data['testcase'] == 'testcase2' + assert res_data['product_version'] == 'fool-1' + assert res_data['waived'] is True + assert res_data['comment'] == 'it broke' + + @pytest.mark.usefixtures('enable_ldap_host') + @pytest.mark.usefixtures('enable_ldap_base') + @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', + return_value=(['factory-2-0', 'something-else'])) + def test_both_user_group_no_permission(self, mocked_conn, client, session, monkeypatch): + monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab') + self.data['testcase'] = 'testcase3' + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 401 + assert res_data['message'] == ("You are not authorized to submit a waiver " + "for the test case testcase3") diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py index ddf9cb9..420d2ce 100644 --- a/waiverdb/api_v1.py +++ b/waiverdb/api_v1.py @@ -1,11 +1,14 @@ # SPDX-License-Identifier: GPL-2.0+ import datetime +import re +import logging import requests from flask import Blueprint, request, current_app from flask_restful import Resource, Api, reqparse, marshal_with, marshal -from werkzeug.exceptions import BadRequest, Forbidden, ServiceUnavailable +from werkzeug.exceptions import (BadRequest, Forbidden, ServiceUnavailable, + InternalServerError, Unauthorized, BadGateway) from sqlalchemy.sql.expression import func, and_, or_ from waiverdb import __version__ @@ -18,6 +21,7 @@ import waiverdb.auth api_v1 = (Blueprint('api_v1', __name__)) api = Api(api_v1) requests_session = requests.Session() +log = logging.getLogger(__name__) def valid_dict(value): @@ -321,6 +325,46 @@ class WaiversResource(Resource): return result, 201, headers + def get_group_membership(self, user): + try: + import ldap + except ImportError: + raise InternalServerError(('If PERMISSION_MAPPING is defined, ' + 'python-ldap needs to be installed.')) + try: + con = ldap.initialize(current_app.config['LDAP_HOST']) + results = con.search_s(current_app.config['LDAP_BASE'], ldap.SCOPE_SUBTREE, + f'(memberUid={user})', ['cn']) + return [group[1]['cn'][0].decode('utf-8') for group in results] + except ldap.LDAPError: + log.exception('Some error occured initializing the LDAP connection.') + raise Unauthorized('Some error occured initializing the LDAP connection.') + except ldap.SERVER_DOWN: + log.exception('The LDAP server is not reachable.') + raise BadGateway('The LDAP server is not reachable.') + + def verify_authorization(self, user, testcase): + if not current_app.config['PERMISSION_MAPPING']: + return True + if not (current_app.config.get('LDAP_HOST') and current_app.config.get('LDAP_BASE')): + raise InternalServerError(('LDAP_HOST and LDAP_BASE also need to be defined ' + 'if PERMISSION_MAPPING is defined.')) + allowed_groups = [] + for testcase_pattern, permission in current_app.config['PERMISSION_MAPPING'].items(): + testcase_match = re.search(testcase_pattern, testcase) + if testcase_match: + # checking if the user is allowed + if user in permission['users']: + return True + allowed_groups += permission['groups'] + group_membership = self.get_group_membership(user) + if not group_membership: + raise Unauthorized(f'Couldn\'t find user {user} in LDAP') + if set(group_membership) & set(allowed_groups): + return True + raise Unauthorized(('You are not authorized to submit a waiver ' + f'for the test case {testcase}')) + def _create_waiver(self, args, user): proxied_by = None if args.get('username'): @@ -375,6 +419,8 @@ class WaiversResource(Resource): if not args['testcase']: raise BadRequest({'testcase': 'Missing required parameter in the JSON body'}) + self.verify_authorization(user, args['testcase']) + # brew-build is an alias for koji_build if args['subject_type'] == 'brew-build': args['subject_type'] = 'koji_build' diff --git a/waiverdb/cli.py b/waiverdb/cli.py index a233c25..7ac9b49 100644 --- a/waiverdb/cli.py +++ b/waiverdb/cli.py @@ -278,11 +278,12 @@ def cli(comment, waived, product_version, testcase, subject, subject_identifier, resp = requests.request( 'POST', url, auth=auth, **common_request_arguments) if resp.status_code == 401: - raise click.ClickException('WaiverDB authentication using GSSAPI failed. ' - 'Make sure you have a valid Kerberos ticket or ' - 'that you correctly configured your Kerberos ' - 'configuration file. Please check the doc for ' - 'troubleshooting information.') + msg = resp.json().get( + 'message', ('WaiverDB authentication using GSSAPI failed. Make sure you have a ' + 'valid Kerberos ticket or that you correctly configured your Kerberos ' + 'configuration file. Please check the doc for troubleshooting ' + 'information.')) + raise click.ClickException(msg) check_response(resp, result_ids) elif auth_method == 'dummy': resp = requests.request( diff --git a/waiverdb/config.py b/waiverdb/config.py index ac1fe14..3a1fe61 100644 --- a/waiverdb/config.py +++ b/waiverdb/config.py @@ -27,6 +27,7 @@ class Config(object): SQLALCHEMY_TRACK_MODIFICATIONS = True # A list of users are allowed to create waivers on behalf of other users. SUPERUSERS = [] + PERMISSION_MAPPING = {} class ProductionConfig(Config): From 4cb1f4d7d284c30a6b813e7520f77b1f0865cf54 Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Apr 30 2019 09:40:08 +0000 Subject: [PATCH 2/2] Test proxied_by with access control Let's make sure that proxied_by works correctly with the access control new feature. --- diff --git a/tests/test_access_control.py b/tests/test_access_control.py index 3e0a85d..67094df 100644 --- a/tests/test_access_control.py +++ b/tests/test_access_control.py @@ -117,3 +117,40 @@ class TestAccessControl(object): assert r.status_code == 401 assert res_data['message'] == ("You are not authorized to submit a waiver " "for the test case testcase3") + + @pytest.mark.usefixtures('enable_ldap_host') + @pytest.mark.usefixtures('enable_ldap_base') + @mock.patch('waiverdb.auth.get_user', return_value=('bodhi', {})) + @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', + return_value=(['factory-2-0', 'something-else'])) + def test_proxied_by_with_no_permission(self, mocked_conn, mock_get_user, client, session): + self.data['testcase'] = 'testcase3' + self.data['username'] = 'foo' + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 401 + assert res_data['message'] == ("You are not authorized to submit a waiver " + "for the test case testcase3") + + @pytest.mark.usefixtures('enable_ldap_host') + @pytest.mark.usefixtures('enable_ldap_base') + @mock.patch('waiverdb.auth.get_user', return_value=('bodhi', {})) + @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', + return_value=(['factory-2-0', 'something-else'])) + def test_proxied_by_has_permission(self, mocked_conn, mock_get_user, client, session): + self.data['testcase'] = 'testcase2' + self.data['username'] = 'foo' + r = client.post('/api/v1.0/waivers/', data=json.dumps(self.data), + content_type='application/json', headers=self.headers) + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 201 + assert res_data['username'] == 'foo' + assert res_data['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'} + assert res_data['subject_type'] == 'koji_build' + assert res_data['subject_identifier'] == 'glibc-2.26-27.fc27' + assert res_data['testcase'] == 'testcase2' + assert res_data['product_version'] == 'fool-1' + assert res_data['waived'] is True + assert res_data['comment'] == 'it broke' + assert res_data['proxied_by'] == 'bodhi'