Merge pull request #2469 from coreos-inc/appr_403_vs_401

Use 401 for bad or missing credentials, 403 for forbidden access
This commit is contained in:
Jimmy Zelinskie 2017-03-27 11:39:23 -04:00 committed by GitHub
commit 8931609775
3 changed files with 26 additions and 26 deletions

View file

@ -2,7 +2,7 @@ import logging
from functools import wraps
from cnr.exception import UnauthorizedAccess
from cnr.exception import Forbidden
from flask import Blueprint
from app import metric_queue
@ -18,8 +18,8 @@ logger = logging.getLogger(__name__)
def _raise_method(repository, scopes):
raise UnauthorizedAccess("Unauthorized access for: %s" % repository,
{"package": repository, "scopes": scopes})
raise Forbidden("Unauthorized access for: %s" % repository,
{"package": repository, "scopes": scopes})
def _get_reponame_kwargs(*args, **kwargs):

View file

@ -7,7 +7,7 @@ import cnr
from cnr.api.impl import registry as cnr_registry
from cnr.api.registry import repo_name, _pull
from cnr.exception import (CnrException, InvalidUsage, InvalidParams, InvalidRelease,
UnableToLockResource, UnauthorizedAccess, Unsupported, ChannelNotFound,
UnableToLockResource, UnauthorizedAccess, Unsupported, ChannelNotFound, Forbidden,
PackageAlreadyExists, PackageNotFound, PackageReleaseNotFound)
from flask import request, jsonify
@ -28,6 +28,7 @@ logger = logging.getLogger(__name__)
@appr_bp.errorhandler(Unsupported)
@appr_bp.errorhandler(PackageAlreadyExists)
@appr_bp.errorhandler(InvalidRelease)
@appr_bp.errorhandler(Forbidden)
@appr_bp.errorhandler(UnableToLockResource)
@appr_bp.errorhandler(UnauthorizedAccess)
@appr_bp.errorhandler(PackageNotFound)
@ -192,13 +193,13 @@ def push(namespace, package_name):
owner = get_authenticated_user()
if not Package.exists(reponame):
if not CreateRepositoryPermission(namespace).can():
raise UnauthorizedAccess("Unauthorized access for: %s" % reponame,
{"package": reponame, "scopes": ['create']})
raise Forbidden("Unauthorized access for: %s" % reponame,
{"package": reponame, "scopes": ['create']})
Package.create_repository(reponame, private, owner)
if not ModifyRepositoryPermission(namespace, package_name).can():
raise UnauthorizedAccess("Unauthorized access for: %s" % reponame,
{"package": reponame, "scopes": ['push']})
raise Forbidden("Unauthorized access for: %s" % reponame,
{"package": reponame, "scopes": ['push']})
if not 'release' in values:
raise InvalidUsage('Missing release')

View file

@ -15,64 +15,64 @@ CHANNEL_ARGS = {'channel_name': 'c'}
CHANNEL_RELEASE_ARGS = {'channel_name': 'c', 'release': 'r'}
@pytest.mark.parametrize('resource,method,params,owned_by,is_public,identity,expected', [
('appr.blobs', 'GET', BLOB_ARGS, 'devtable', False, 'public', 401),
('appr.blobs', 'GET', BLOB_ARGS, 'devtable', False, 'public', 403),
('appr.blobs', 'GET', BLOB_ARGS, 'devtable', False, 'devtable', 404),
('appr.blobs', 'GET', BLOB_ARGS, 'devtable', True, 'public', 404),
('appr.blobs', 'GET', BLOB_ARGS, 'devtable', True, 'devtable', 404),
('appr.delete_package', 'DELETE', PACKAGE_ARGS, 'devtable', False, 'public', 401),
('appr.delete_package', 'DELETE', PACKAGE_ARGS, 'devtable', False, 'public', 403),
('appr.delete_package', 'DELETE', PACKAGE_ARGS, 'devtable', False, 'devtable', 404),
('appr.delete_package', 'DELETE', PACKAGE_ARGS, 'devtable', True, 'public', 401),
('appr.delete_package', 'DELETE', PACKAGE_ARGS, 'devtable', True, 'public', 403),
('appr.delete_package', 'DELETE', PACKAGE_ARGS, 'devtable', True, 'devtable', 404),
('appr.show_package', 'GET', PACKAGE_ARGS, 'devtable', False, 'public', 401),
('appr.show_package', 'GET', PACKAGE_ARGS, 'devtable', False, 'public', 403),
('appr.show_package', 'GET', PACKAGE_ARGS, 'devtable', False, 'devtable', 404),
('appr.show_package', 'GET', PACKAGE_ARGS, 'devtable', True, 'public', 404),
('appr.show_package', 'GET', PACKAGE_ARGS, 'devtable', True, 'devtable', 404),
('appr.show_package_releases', 'GET', {}, 'devtable', False, 'public', 401),
('appr.show_package_releases', 'GET', {}, 'devtable', False, 'public', 403),
('appr.show_package_releases', 'GET', {}, 'devtable', False, 'devtable', 200),
('appr.show_package_releases', 'GET', {}, 'devtable', True, 'public', 200),
('appr.show_package_releases', 'GET', {}, 'devtable', True, 'devtable', 200),
('appr.show_package_releasse_manifests', 'GET', RELEASE_ARGS, 'devtable', False, 'public', 401),
('appr.show_package_releasse_manifests', 'GET', RELEASE_ARGS, 'devtable', False, 'public', 403),
('appr.show_package_releasse_manifests', 'GET', RELEASE_ARGS, 'devtable', False, 'devtable', 200),
('appr.show_package_releasse_manifests', 'GET', RELEASE_ARGS, 'devtable', True, 'public', 200),
('appr.show_package_releasse_manifests', 'GET', RELEASE_ARGS, 'devtable', True, 'devtable', 200),
('appr.pull', 'GET', PACKAGE_ARGS, 'devtable', False, 'public', 401),
('appr.pull', 'GET', PACKAGE_ARGS, 'devtable', False, 'public', 403),
('appr.pull', 'GET', PACKAGE_ARGS, 'devtable', False, 'devtable', 404),
('appr.pull', 'GET', PACKAGE_ARGS, 'devtable', True, 'public', 404),
('appr.pull', 'GET', PACKAGE_ARGS, 'devtable', True, 'devtable', 404),
('appr.push', 'POST', {}, 'devtable', False, 'public', 401),
('appr.push', 'POST', {}, 'devtable', False, 'public', 403),
('appr.push', 'POST', {}, 'devtable', False, 'devtable', 400),
('appr.push', 'POST', {}, 'devtable', True, 'public', 401),
('appr.push', 'POST', {}, 'devtable', True, 'public', 403),
('appr.push', 'POST', {}, 'devtable', True, 'devtable', 400),
('appr.list_channels', 'GET', {}, 'devtable', False, 'public', 401),
('appr.list_channels', 'GET', {}, 'devtable', False, 'public', 403),
('appr.list_channels', 'GET', {}, 'devtable', False, 'devtable', 200),
('appr.list_channels', 'GET', {}, 'devtable', True, 'public', 200),
('appr.list_channels', 'GET', {}, 'devtable', True, 'devtable', 200),
('appr.show_channel', 'GET', CHANNEL_ARGS, 'devtable', False, 'public', 401),
('appr.show_channel', 'GET', CHANNEL_ARGS, 'devtable', False, 'public', 403),
('appr.show_channel', 'GET', CHANNEL_ARGS, 'devtable', False, 'devtable', 404),
('appr.show_channel', 'GET', CHANNEL_ARGS, 'devtable', True, 'public', 404),
('appr.show_channel', 'GET', CHANNEL_ARGS, 'devtable', True, 'devtable', 404),
('appr.delete_channel', 'DELETE', CHANNEL_ARGS, 'devtable', False, 'public', 401),
('appr.delete_channel', 'DELETE', CHANNEL_ARGS, 'devtable', False, 'public', 403),
('appr.delete_channel', 'DELETE', CHANNEL_ARGS, 'devtable', False, 'devtable', 404),
('appr.delete_channel', 'DELETE', CHANNEL_ARGS, 'devtable', True, 'public', 401),
('appr.delete_channel', 'DELETE', CHANNEL_ARGS, 'devtable', True, 'public', 403),
('appr.delete_channel', 'DELETE', CHANNEL_ARGS, 'devtable', True, 'devtable', 404),
('appr.add_channel_release', 'POST', CHANNEL_RELEASE_ARGS, 'devtable', False, 'public', 401),
('appr.add_channel_release', 'POST', CHANNEL_RELEASE_ARGS, 'devtable', False, 'public', 403),
('appr.add_channel_release', 'POST', CHANNEL_RELEASE_ARGS, 'devtable', False, 'devtable', 404),
('appr.add_channel_release', 'POST', CHANNEL_RELEASE_ARGS, 'devtable', True, 'public', 401),
('appr.add_channel_release', 'POST', CHANNEL_RELEASE_ARGS, 'devtable', True, 'public', 403),
('appr.add_channel_release', 'POST', CHANNEL_RELEASE_ARGS, 'devtable', True, 'devtable', 404),
('appr.delete_channel_release', 'DELETE', CHANNEL_RELEASE_ARGS, 'devtable', False, 'public', 401),
('appr.delete_channel_release', 'DELETE', CHANNEL_RELEASE_ARGS, 'devtable', False, 'public', 403),
('appr.delete_channel_release', 'DELETE', CHANNEL_RELEASE_ARGS, 'devtable', False, 'devtable', 404),
('appr.delete_channel_release', 'DELETE', CHANNEL_RELEASE_ARGS, 'devtable', True, 'public', 401),
('appr.delete_channel_release', 'DELETE', CHANNEL_RELEASE_ARGS, 'devtable', True, 'public', 403),
('appr.delete_channel_release', 'DELETE', CHANNEL_RELEASE_ARGS, 'devtable', True, 'devtable', 404),
])
def test_api_security(resource, method, params, owned_by, is_public, identity, expected, app, client):
@ -95,4 +95,3 @@ def test_api_security(resource, method, params, owned_by, is_public, identity, e
rv = cl.open(url, headers=headers, method=method)
assert rv.status_code == expected