From a4763ee316e23d89eabbfa55ad4f3655128bcbb0 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Mon, 13 Mar 2017 07:26:30 +0000 Subject: [PATCH] Use the 403 Forbidden result in case the user is unauthorized The difference between 401 Unauthorized and 403 Forbidden is that 403 Forbidden is "permanent": it indicates that the user was authenticated correctly, but was not allowed to access this endpoint. In contrast, 401 Unauthorized means that the request as posted was not allowed, but if the user were to try again with (new) authorization tokens, it might actually succeed. Signed-off-by: Patrick Uiterwijk --- module_build_service/auth.py | 12 ++++++------ module_build_service/scm.py | 6 +++--- module_build_service/utils.py | 8 ++++---- module_build_service/views.py | 16 ++++++++-------- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/module_build_service/auth.py b/module_build_service/auth.py index 1a641a0c..9c9e2762 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,7 +103,7 @@ 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.") @@ -119,7 +119,7 @@ def get_user(request): ] for scope in required_scopes: if scope not in presented_scopes: - raise Unauthorized("Required OIDC scope %r not present: %r" % ( + raise Forbidden("Required OIDC scope %r not present: %r" % ( scope, presented_scopes)) try: @@ -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 37232309..850c0093 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 @@ -461,10 +461,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: @@ -475,7 +475,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 9ea9a31d..1d435b21 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') @@ -139,7 +139,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: @@ -154,7 +154,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() @@ -162,8 +162,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"))