mirror of
https://pagure.io/fm-orchestrator.git
synced 2026-04-05 11:48:33 +08:00
Merge #425 Use the 403 Forbidden result in case the user is unauthorized
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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('/')
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user