From 0df3b68a8a2fea8e567e6d4b8aa5f29c232a84a2 Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Jul 02 2019 08:17:21 +0000 Subject: [PATCH 1/2] Refactoring: Move authorization to separate submodule Signed-off-by: Lukas Holecek --- diff --git a/tests/test_access_control.py b/tests/test_access_control.py index 5778e17..e12dd76 100644 --- a/tests/test_access_control.py +++ b/tests/test_access_control.py @@ -62,7 +62,7 @@ class TestAccessControl(object): @pytest.mark.usefixtures('enable_ldap_host') @pytest.mark.usefixtures('enable_ldap_base') - @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', return_value=([])) + @mock.patch('waiverdb.authorization.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), @@ -73,7 +73,7 @@ class TestAccessControl(object): @pytest.mark.usefixtures('enable_ldap_host') @pytest.mark.usefixtures('enable_ldap_base') - @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', + @mock.patch('waiverdb.authorization.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') @@ -110,7 +110,7 @@ class TestAccessControl(object): @pytest.mark.usefixtures('enable_ldap_host') @pytest.mark.usefixtures('enable_ldap_base') - @mock.patch('waiverdb.api_v1.WaiversResource.get_group_membership', + @mock.patch('waiverdb.authorization.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') @@ -125,7 +125,7 @@ class TestAccessControl(object): @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', + @mock.patch('waiverdb.authorization.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' @@ -140,7 +140,7 @@ class TestAccessControl(object): @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', + @mock.patch('waiverdb.authorization.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' diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py index 929933b..fdb25a2 100644 --- a/waiverdb/api_v1.py +++ b/waiverdb/api_v1.py @@ -1,17 +1,20 @@ # 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, - InternalServerError, Unauthorized, BadGateway) +from werkzeug.exceptions import ( + BadRequest, + Forbidden, + ServiceUnavailable, +) from sqlalchemy.sql.expression import func, and_, or_ from waiverdb import __version__ +from waiverdb.authorization import verify_authorization from waiverdb.models import db from waiverdb.models.waivers import Waiver, subject_dict_to_type_identifier from waiverdb.utils import json_collection, jsonp @@ -325,45 +328,14 @@ 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']: + def _verify_authorization(self, user, testcase): + permission_mapping = current_app.config.get('PERMISSION_MAPPING') + if not 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}')) + + ldap_host = current_app.config.get('LDAP_HOST') + ldap_base = current_app.config.get('LDAP_BASE') + return verify_authorization(user, testcase, permission_mapping, ldap_host, ldap_base) def _create_waiver(self, args, user): proxied_by = None @@ -419,7 +391,7 @@ class WaiversResource(Resource): if not args['testcase']: raise BadRequest({'testcase': 'Missing required parameter in the JSON body'}) - self.verify_authorization(user, args['testcase']) + self._verify_authorization(user, args['testcase']) # brew-build is an alias for koji_build if args['subject_type'] == 'brew-build': diff --git a/waiverdb/authorization.py b/waiverdb/authorization.py new file mode 100644 index 0000000..8720f76 --- /dev/null +++ b/waiverdb/authorization.py @@ -0,0 +1,56 @@ +# SPDX-License-Identifier: GPL-2.0+ + +import logging +import re + +from werkzeug.exceptions import ( + BadGateway, + InternalServerError, + Unauthorized, +) + +log = logging.getLogger(__name__) + + +def get_group_membership(user, ldap_host, ldap_base): + try: + import ldap + except ImportError: + raise InternalServerError(('If PERMISSION_MAPPING is defined, ' + 'python-ldap needs to be installed.')) + + try: + con = ldap.initialize(ldap_host) + results = con.search_s(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(user, testcase, permission_mapping, ldap_host, ldap_base): + if not (ldap_host and 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 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 = get_group_membership(user, ldap_host, ldap_base) + 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}')) From 4f93eeebe846ae0d25e510db2f2210ff963696c0 Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Jul 02 2019 08:17:49 +0000 Subject: [PATCH 2/2] Fix typo in error message Signed-off-by: Lukas Holecek --- diff --git a/tests/test_access_control.py b/tests/test_access_control.py index e12dd76..c6e7e95 100644 --- a/tests/test_access_control.py +++ b/tests/test_access_control.py @@ -58,7 +58,7 @@ class TestAccessControl(object): 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." + assert res_data['message'] == "Some error occurred initializing the LDAP connection." @pytest.mark.usefixtures('enable_ldap_host') @pytest.mark.usefixtures('enable_ldap_base') diff --git a/waiverdb/authorization.py b/waiverdb/authorization.py index 8720f76..f56d91d 100644 --- a/waiverdb/authorization.py +++ b/waiverdb/authorization.py @@ -24,8 +24,8 @@ def get_group_membership(user, ldap_host, ldap_base): results = con.search_s(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.') + log.exception('Some error occurred initializing the LDAP connection.') + raise Unauthorized('Some error occurred initializing the LDAP connection.') except ldap.SERVER_DOWN: log.exception('The LDAP server is not reachable.') raise BadGateway('The LDAP server is not reachable.')