diff --git a/module_build_service/auth.py b/module_build_service/auth.py index 1a641a0c..43691e2c 100644 --- a/module_build_service/auth.py +++ b/module_build_service/auth.py @@ -23,7 +23,7 @@ """Auth system based on the client certificate and FAS account""" -from module_build_service.errors import Unauthorized +from module_build_service.errors import Unauthorized, Forbidden from module_build_service import app, log import requests @@ -43,7 +43,7 @@ def _load_secrets(): return if not "OIDC_CLIENT_SECRETS" in app.config: - raise Unauthorized("OIDC_CLIENT_SECRETS must be set in server config.") + raise Forbidden("OIDC_CLIENT_SECRETS must be set in server config.") secrets = _json_loads(open(app.config['OIDC_CLIENT_SECRETS'], 'r').read()) @@ -103,13 +103,13 @@ def get_user(request): except Exception as e: error = "Cannot verify OIDC token: %s" % str(e) log.exception(error) - raise Unauthorized(error) + raise Exception(error) if not data or not "active" in data or not data["active"]: raise Unauthorized("OIDC token invalid or expired.") if not "OIDC_REQUIRED_SCOPE" in app.config: - raise Unauthorized("OIDC_REQUIRED_SCOPE must be set in server config.") + raise Forbidden("OIDC_REQUIRED_SCOPE must be set in server config.") presented_scopes = data['scope'].split(' ') required_scopes = [ @@ -127,13 +127,13 @@ def get_user(request): except Exception as e: error = "Cannot verify determine user groups: %s" % str(e) log.exception(error) - raise Unauthorized(error) + raise Exception(error) try: groups = set(extended_data['groups']) except Exception as e: error = "Could not find groups in UserInfo from OIDC %s" % str(e) log.exception(extended_data) - raise Unauthorized(error) + raise Exception(error) return data["username"], groups diff --git a/module_build_service/scm.py b/module_build_service/scm.py index 4d25425e..9964256c 100644 --- a/module_build_service/scm.py +++ b/module_build_service/scm.py @@ -37,7 +37,7 @@ import shutil import datetime from module_build_service import log -from module_build_service.errors import Unauthorized, ValidationError +from module_build_service.errors import Forbidden, ValidationError import module_build_service.utils @@ -54,13 +54,13 @@ class SCM(object): :param str url: The unmodified scmurl :param list allowed_scm: The list of allowed SCMs, optional - :raises: Unauthorized or ValidationError + :raises: Forbidden or ValidationError """ if allowed_scm: if not (url.startswith(tuple(allowed_scm)) or (allow_local and url.startswith("file://"))): - raise Unauthorized( + raise Forbidden( '%s is not in the list of allowed SCMs' % url) url = url.rstrip('/') diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 0659fab1..ad9f6481 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -43,7 +43,7 @@ from module_build_service import log, models from module_build_service.errors import (ValidationError, UnprocessableEntity, ProgrammingError) from module_build_service import conf, db -from module_build_service.errors import (Unauthorized, Conflict) +from module_build_service.errors import (Forbidden, Conflict) import module_build_service.messaging from multiprocessing.dummy import Pool as ThreadPool import module_build_service.pdc @@ -465,10 +465,10 @@ def format_mmd(mmd, scmurl): # Add missing data in RPM components for pkgname, pkg in mmd.components.rpms.items(): if pkg.repository and not conf.rpms_allow_repository: - raise Unauthorized( + raise Forbidden( "Custom component repositories aren't allowed") if pkg.cache and not conf.rpms_allow_cache: - raise Unauthorized("Custom component caches aren't allowed") + raise Forbidden("Custom component caches aren't allowed") if not pkg.repository: pkg.repository = conf.rpms_default_repository + pkgname if not pkg.cache: @@ -479,7 +479,7 @@ def format_mmd(mmd, scmurl): # Add missing data in included modules components for modname, mod in mmd.components.modules.items(): if mod.repository and not conf.modules_allow_repository: - raise Unauthorized( + raise Forbidden( "Custom component repositories aren't allowed") if not mod.repository: mod.repository = conf.modules_default_repository + modname diff --git a/module_build_service/views.py b/module_build_service/views.py index 6bb9a47d..81fbf29e 100644 --- a/module_build_service/views.py +++ b/module_build_service/views.py @@ -37,7 +37,7 @@ from module_build_service.utils import ( pagination_metadata, filter_module_builds, submit_module_build_from_scm, submit_module_build_from_yaml, scm_url_schemes, get_scm_url_re, validate_optional_params) from module_build_service.errors import ( - ValidationError, Unauthorized, NotFound) + ValidationError, Forbidden, NotFound) api_v1 = { 'module_builds': { @@ -98,7 +98,7 @@ class ModuleBuildAPI(MethodView): username, groups = module_build_service.auth.get_user(request) if conf.allowed_groups and not (conf.allowed_groups & groups): - raise Unauthorized("%s is not in any of %r, only %r" % ( + raise Forbidden("%s is not in any of %r, only %r" % ( username, conf.allowed_groups, groups)) kwargs = {"username": username} @@ -121,11 +121,11 @@ class ModuleBuildAPI(MethodView): url = r["scmurl"] if not any(url.startswith(prefix) for prefix in conf.scmurls): log.error("The submitted scmurl %r is not allowed" % url) - raise Unauthorized("The submitted scmurl %s is not allowed" % url) + raise Forbidden("The submitted scmurl %s is not allowed" % url) if not get_scm_url_re().match(url): log.error("The submitted scmurl %r is not valid" % url) - raise Unauthorized("The submitted scmurl %s is not valid" % url) + raise Forbidden("The submitted scmurl %s is not valid" % url) if "branch" not in r: log.error('Missing branch') @@ -143,7 +143,7 @@ class ModuleBuildAPI(MethodView): def post_file(self, username): if not conf.yaml_submit_allowed: - raise Unauthorized("YAML submission is not enabled") + raise Forbidden("YAML submission is not enabled") validate_optional_params(request.form) try: @@ -158,7 +158,7 @@ class ModuleBuildAPI(MethodView): username, groups = module_build_service.auth.get_user(request) if conf.allowed_groups and not (conf.allowed_groups & groups): - raise Unauthorized("%s is not in any of %r, only %r" % ( + raise Forbidden("%s is not in any of %r, only %r" % ( username, conf.allowed_groups, groups)) module = models.ModuleBuild.query.filter_by(id=id).first() @@ -166,8 +166,8 @@ class ModuleBuildAPI(MethodView): raise NotFound('No such module found.') if module.owner != username: - raise Unauthorized('You are not owner of this build and ' - 'therefore cannot modify it.') + raise Forbidden('You are not owner of this build and ' + 'therefore cannot modify it.') try: r = json.loads(request.get_data().decode("utf-8")) diff --git a/tests/test_auth.py b/tests/test_auth.py index 12a9184f..2c562cba 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -109,7 +109,7 @@ class TestAuthModule(unittest.TestCase): @patch('module_build_service.auth.client_secrets', None) def test_misconfiguring_oidc_client_secrets_should_be_failed(self): request = mock.MagicMock() - with self.assertRaises(module_build_service.errors.Unauthorized) as cm: + with self.assertRaises(module_build_service.errors.Forbidden) as cm: module_build_service.auth.get_user(request) self.assertEquals(str(cm.exception), @@ -165,7 +165,7 @@ class TestAuthModule(unittest.TestCase): request.headers.__setitem__.side_effect = headers.__setitem__ request.headers.__contains__.side_effect = headers.__contains__ - with self.assertRaises(module_build_service.errors.Unauthorized) as cm: + with self.assertRaises(module_build_service.errors.Forbidden) as cm: result = module_build_service.auth.get_user(request) self.assertEquals(str(cm.exception), diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 64868cba..25928b3b 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -314,7 +314,7 @@ class TestBuild(unittest.TestCase): with patch("module_build_service.config.Config.yaml_submit_allowed", new_callable=PropertyMock, return_value = False): data = submit() - self.assertEqual(data['status'], 401) + self.assertEqual(data['status'], 403) self.assertEqual(data['message'], 'YAML submission is not enabled') @timed(30) diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 7d911005..2b01b991 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -389,8 +389,8 @@ class TestViews(unittest.TestCase): data = json.loads(rv.data) self.assertEquals(data['message'], 'The submitted scmurl ' 'git://badurl.com is not allowed') - self.assertEquals(data['status'], 401) - self.assertEquals(data['error'], 'Unauthorized') + self.assertEquals(data['status'], 403) + self.assertEquals(data['error'], 'Forbidden') @patch('module_build_service.auth.get_user', return_value=user) def test_submit_build_scm_url_without_hash(self, mocked_get_user): @@ -401,8 +401,8 @@ class TestViews(unittest.TestCase): self.assertEquals(data['message'], 'The submitted scmurl ' 'git://pkgs.stg.fedoraproject.org/modules/testmodule.git ' 'is not valid') - self.assertEquals(data['status'], 401) - self.assertEquals(data['error'], 'Unauthorized') + self.assertEquals(data['status'], 403) + self.assertEquals(data['error'], 'Forbidden') @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') @@ -521,8 +521,8 @@ class TestViews(unittest.TestCase): 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) - self.assertEquals(data['status'], 401) - self.assertEquals(data['error'], 'Unauthorized') + self.assertEquals(data['status'], 403) + self.assertEquals(data['error'], 'Forbidden') @patch('module_build_service.auth.get_user', return_value=other_user) def test_cancel_build(self, mocked_get_user): @@ -539,8 +539,8 @@ class TestViews(unittest.TestCase): data=json.dumps({'state': 'failed'})) data = json.loads(rv.data) - self.assertEquals(data['status'], 401) - self.assertEquals(data['error'], 'Unauthorized') + self.assertEquals(data['status'], 403) + self.assertEquals(data['error'], 'Forbidden') @patch('module_build_service.auth.get_user', return_value=other_user) def test_cancel_build_wrong_param(self, mocked_get_user): @@ -577,8 +577,8 @@ class TestViews(unittest.TestCase): "The submitted scmurl {} is not valid".format(scmurl), ) ) - self.assertEquals(data['status'], 401) - self.assertEquals(data['error'], 'Unauthorized') + self.assertEquals(data['status'], 403) + self.assertEquals(data['error'], 'Forbidden') @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM')