vbatts/quay
Archived
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

Change superuser API errors to be more descriptive

Browse source 
Fixes https://jira.coreos.com/browse/QS-103
This commit is contained in:
Joseph Schorr 2017-12-22 17:26:49 -05:00
parent 13b738c43c
commit c887aa543b
2 changed files with 82 additions and 81 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

131
endpoints/api/superuser.py
View file

@ -20,13 +20,13 @@ from auth.permissions import SuperUserPermission
from data.database import ServiceKeyApprovalType from data.database import ServiceKeyApprovalType
from endpoints.api import (ApiResource, nickname, resource, validate_json_request, from endpoints.api import (ApiResource, nickname, resource, validate_json_request,
internal_only, require_scope, show_if, parse_args, internal_only, require_scope, show_if, parse_args,
query_param, abort, require_fresh_login, path_param, verify_not_prod, query_param, require_fresh_login, path_param, verify_not_prod,
page_support, log_action, InvalidRequest, format_date, truthy_bool) page_support, log_action, format_date, truthy_bool,
InvalidRequest, NotFound, Unauthorized, InvalidResponse)
from endpoints.api.build import get_logs_or_log_url from endpoints.api.build import get_logs_or_log_url
from endpoints.api.superuser_models_pre_oci import (pre_oci_model, ServiceKeyDoesNotExist, from endpoints.api.superuser_models_pre_oci import (pre_oci_model, ServiceKeyDoesNotExist,
ServiceKeyAlreadyApproved, ServiceKeyAlreadyApproved,
InvalidRepositoryBuildException) InvalidRepositoryBuildException)
from endpoints.exception import NotFound, InvalidResponse
from util.useremails import send_confirmation_email, send_recovery_email from util.useremails import send_confirmation_email, send_recovery_email
from util.license import decode_license, LicenseDecodeError from util.license import decode_license, LicenseDecodeError
from util.security.ssl import load_certificate, CertInvalidException from util.security.ssl import load_certificate, CertInvalidException
@ -83,23 +83,22 @@ class SuperUserGetLogsForService(ApiResource):
""" Returns the logs for the specific service. """ """ Returns the logs for the specific service. """
if SuperUserPermission().can(): if SuperUserPermission().can():
if service not in get_services(): if service not in get_services():
abort(404) raise NotFound()
logs = [] logs = []
try: try:
with open(app.config['SYSTEM_LOGS_FILE'], 'r') as f: with open(app.config['SYSTEM_LOGS_FILE'], 'r') as f:
logs = [line for line in f if line.find(service + '[') >= 0] logs = [line for line in f if line.find(service + '[') >= 0]
except Exception: except Exception:
logger.exception('Cannot read logs') logger.exception('Cannot read logs')
abort(400) raise InvalidRequest('Cannot read logs')
return { return {
'instance': socket.gethostname(), 'instance': socket.gethostname(),
'logs': '\n'.join(logs) 'logs': '\n'.join(logs)
} }
abort(403) raise Unauthorized()
@resource('/v1/superuser/systemlogs/') @resource('/v1/superuser/systemlogs/')
@ -120,7 +119,7 @@ class SuperUserSystemLogServices(ApiResource):
'services': list(get_services()) 'services': list(get_services())
} }
abort(403) raise Unauthorized()
@resource('/v1/superuser/aggregatelogs') @resource('/v1/superuser/aggregatelogs')
@ -145,7 +144,7 @@ class SuperUserAggregateLogs(ApiResource):
'aggregated': [log.to_dict() for log in aggregated_logs] 'aggregated': [log.to_dict() for log in aggregated_logs]
} }
abort(403) raise Unauthorized()
LOGS_PER_PAGE = 20 LOGS_PER_PAGE = 20
@ -178,7 +177,7 @@ class SuperUserLogs(ApiResource):
'logs': [log.to_dict() for log in log_page.logs], 'logs': [log.to_dict() for log in log_page.logs],
}, log_page.next_page_token }, log_page.next_page_token
abort(403) raise Unauthorized()
def org_view(org): def org_view(org):
@ -225,7 +224,7 @@ class ChangeLog(ApiResource):
'log': f.read() 'log': f.read()
} }
abort(403) raise Unauthorized()
@resource('/v1/superuser/organizations/') @resource('/v1/superuser/organizations/')
@ -245,7 +244,7 @@ class SuperUserOrganizationList(ApiResource):
'organizations': [org.to_dict() for org in pre_oci_model.get_organizations()] 'organizations': [org.to_dict() for org in pre_oci_model.get_organizations()]
} }
abort(403) raise Unauthorized()
@resource('/v1/superuser/users/') @resource('/v1/superuser/users/')
@ -287,7 +286,7 @@ class SuperUserList(ApiResource):
'users': [user.to_dict() for user in users] 'users': [user.to_dict() for user in users]
} }
abort(403) raise Unauthorized()
@require_fresh_login @require_fresh_login
@verify_not_prod @verify_not_prod
@ -298,7 +297,7 @@ class SuperUserList(ApiResource):
""" Creates a new user. """ """ Creates a new user. """
# Ensure that we are using database auth. # Ensure that we are using database auth.
if app.config['AUTHENTICATION_TYPE'] != 'Database': if app.config['AUTHENTICATION_TYPE'] != 'Database':
abort(400) raise InvalidRequest('Cannot create a user in a non-database auth system')
user_information = request.get_json() user_information = request.get_json()
if SuperUserPermission().can(): if SuperUserPermission().can():
@ -320,7 +319,7 @@ class SuperUserList(ApiResource):
'encrypted_password': authentication.encrypt_user_password(password), 'encrypted_password': authentication.encrypt_user_password(password),
} }
abort(403) raise Unauthorized()
@resource('/v1/superusers/users/<username>/sendrecovery') @resource('/v1/superusers/users/<username>/sendrecovery')
@ -337,15 +336,15 @@ class SuperUserSendRecoveryEmail(ApiResource):
def post(self, username): def post(self, username):
# Ensure that we are using database auth. # Ensure that we are using database auth.
if app.config['AUTHENTICATION_TYPE'] != 'Database': if app.config['AUTHENTICATION_TYPE'] != 'Database':
abort(400) raise InvalidRequest('Cannot send a recovery e-mail for non-database auth')
if SuperUserPermission().can(): if SuperUserPermission().can():
user = pre_oci_model.get_nonrobot_user(username) user = pre_oci_model.get_nonrobot_user(username)
if user is None: if user is None:
abort(404) raise NotFound()
if superusers.is_superuser(username): if superusers.is_superuser(username):
abort(403) raise InvalidRequest('Cannot send a recovery email for a superuser')
code = pre_oci_model.create_reset_password_email_code(user.email) code = pre_oci_model.create_reset_password_email_code(user.email)
send_recovery_email(user.email, code) send_recovery_email(user.email, code)
@ -353,7 +352,7 @@ class SuperUserSendRecoveryEmail(ApiResource):
'email': user.email 'email': user.email
} }
abort(403) raise Unauthorized()
@resource('/v1/superuser/users/<username>') @resource('/v1/superuser/users/<username>')
@ -393,11 +392,11 @@ class SuperUserManagement(ApiResource):
if SuperUserPermission().can(): if SuperUserPermission().can():
user = pre_oci_model.get_nonrobot_user(username) user = pre_oci_model.get_nonrobot_user(username)
if user is None: if user is None:
abort(404) raise NotFound()
return user.to_dict() return user.to_dict()
abort(403) raise Unauthorized()
@require_fresh_login @require_fresh_login
@verify_not_prod @verify_not_prod
@ -408,15 +407,15 @@ class SuperUserManagement(ApiResource):
if SuperUserPermission().can(): if SuperUserPermission().can():
user = pre_oci_model.get_nonrobot_user(username) user = pre_oci_model.get_nonrobot_user(username)
if user is None: if user is None:
abort(404) raise NotFound()
if superusers.is_superuser(username): if superusers.is_superuser(username):
abort(403) raise InvalidRequest('Cannot delete a superuser')
pre_oci_model.delete_user(username) pre_oci_model.delete_user(username)
return '', 204 return '', 204
abort(403) raise Unauthorized()
@require_fresh_login @require_fresh_login
@verify_not_prod @verify_not_prod
@ -428,23 +427,23 @@ class SuperUserManagement(ApiResource):
if SuperUserPermission().can(): if SuperUserPermission().can():
user = pre_oci_model.get_nonrobot_user(username) user = pre_oci_model.get_nonrobot_user(username)
if user is None: if user is None:
abort(404) raise NotFound()
if superusers.is_superuser(username): if superusers.is_superuser(username):
abort(403) raise InvalidRequest('Cannot update a superuser')
user_data = request.get_json() user_data = request.get_json()
if 'password' in user_data: if 'password' in user_data:
# Ensure that we are using database auth. # Ensure that we are using database auth.
if app.config['AUTHENTICATION_TYPE'] != 'Database': if app.config['AUTHENTICATION_TYPE'] != 'Database':
abort(400) raise InvalidRequest('Cannot change password in non-database auth')
pre_oci_model.change_password(username, user_data['password']) pre_oci_model.change_password(username, user_data['password'])
if 'email' in user_data: if 'email' in user_data:
# Ensure that we are using database auth. # Ensure that we are using database auth.
if app.config['AUTHENTICATION_TYPE'] != 'Database': if app.config['AUTHENTICATION_TYPE'] != 'Database':
abort(400) raise InvalidRequest('Cannot change e-mail in non-database auth')
pre_oci_model.update_email(username, user_data['email'], auto_verify=True) pre_oci_model.update_email(username, user_data['email'], auto_verify=True)
@ -471,7 +470,7 @@ class SuperUserManagement(ApiResource):
return return_value return return_value
abort(403) raise Unauthorized()
@resource('/v1/superuser/takeownership/<namespace>') @resource('/v1/superuser/takeownership/<namespace>')
@ -490,12 +489,12 @@ class SuperUserTakeOwnership(ApiResource):
if SuperUserPermission().can(): if SuperUserPermission().can():
# Disallow for superusers. # Disallow for superusers.
if superusers.is_superuser(namespace): if superusers.is_superuser(namespace):
abort(400) raise InvalidRequest('Cannot take ownership of a superuser')
authed_user = get_authenticated_user() authed_user = get_authenticated_user()
entity_id, was_user = pre_oci_model.take_ownership(namespace, authed_user) entity_id, was_user = pre_oci_model.take_ownership(namespace, authed_user)
if entity_id is None: if entity_id is None:
abort(404) raise NotFound()
# Log the change. # Log the change.
log_metadata = { log_metadata = {
@ -511,7 +510,7 @@ class SuperUserTakeOwnership(ApiResource):
'namespace': namespace 'namespace': namespace
}) })
abort(403) raise Unauthorized()
@resource('/v1/superuser/organizations/<name>') @resource('/v1/superuser/organizations/<name>')
@ -544,7 +543,7 @@ class SuperUserOrganizationManagement(ApiResource):
pre_oci_model.delete_organization(name) pre_oci_model.delete_organization(name)
return '', 204 return '', 204
abort(403) raise Unauthorized()
@require_fresh_login @require_fresh_login
@verify_not_prod @verify_not_prod
@ -559,7 +558,7 @@ class SuperUserOrganizationManagement(ApiResource):
org = pre_oci_model.change_organization_name(name, old_name) org = pre_oci_model.change_organization_name(name, old_name)
return org.to_dict() return org.to_dict()
abort(403) raise Unauthorized()
def key_view(key): def key_view(key):
@ -631,7 +630,7 @@ class SuperUserServiceKeyManagement(ApiResource):
'keys': [key.to_dict() for key in keys], 'keys': [key.to_dict() for key in keys],
}) })
abort(403) raise Unauthorized()
@require_fresh_login @require_fresh_login
@verify_not_prod @verify_not_prod
@ -647,11 +646,11 @@ class SuperUserServiceKeyManagement(ApiResource):
if expiration_date is not None: if expiration_date is not None:
try: try:
expiration_date = datetime.utcfromtimestamp(float(expiration_date)) expiration_date = datetime.utcfromtimestamp(float(expiration_date))
except ValueError: except ValueError as ve:
abort(400) raise InvalidRequest('Invalid expiration date: %s' % ve)
if expiration_date <= datetime.now(): if expiration_date <= datetime.now():
abort(400) raise InvalidRequest('Expiration date cannot be in the past')
# Create the metadata for the key. # Create the metadata for the key.
user = get_authenticated_user() user = get_authenticated_user()
@ -691,7 +690,7 @@ class SuperUserServiceKeyManagement(ApiResource):
'private_key': private_key.exportKey('PEM'), 'private_key': private_key.exportKey('PEM'),
}) })
abort(403) raise Unauthorized()
@resource('/v1/superuser/keys/<kid>') @resource('/v1/superuser/keys/<kid>')
@ -730,9 +729,9 @@ class SuperUserServiceKey(ApiResource):
key = pre_oci_model.get_service_key(kid, approved_only=False, alive_only=False) key = pre_oci_model.get_service_key(kid, approved_only=False, alive_only=False)
return jsonify(key.to_dict()) return jsonify(key.to_dict())
except ServiceKeyDoesNotExist: except ServiceKeyDoesNotExist:
abort(404) raise NotFound()
abort(403) raise Unauthorized()
@require_fresh_login @require_fresh_login
@verify_not_prod @verify_not_prod
@ -745,7 +744,7 @@ class SuperUserServiceKey(ApiResource):
try: try:
key = pre_oci_model.get_service_key(kid, approved_only=False, alive_only=False) key = pre_oci_model.get_service_key(kid, approved_only=False, alive_only=False)
except ServiceKeyDoesNotExist: except ServiceKeyDoesNotExist:
abort(404) raise NotFound()
key_log_metadata = { key_log_metadata = {
'kid': key.kid, 'kid': key.kid,
@ -759,11 +758,11 @@ class SuperUserServiceKey(ApiResource):
if expiration_date is not None and expiration_date != '': if expiration_date is not None and expiration_date != '':
try: try:
expiration_date = datetime.utcfromtimestamp(float(expiration_date)) expiration_date = datetime.utcfromtimestamp(float(expiration_date))
except ValueError: except ValueError as ve:
abort(400) raise InvalidRequest('Invalid expiration date: %s' % ve)
if expiration_date <= datetime.now(): if expiration_date <= datetime.now():
abort(400) raise InvalidRequest('Cannot have an expiration date in the past')
key_log_metadata.update({ key_log_metadata.update({
'old_expiration_date': key.expiration_date, 'old_expiration_date': key.expiration_date,
@ -780,7 +779,7 @@ class SuperUserServiceKey(ApiResource):
updated_key = pre_oci_model.get_service_key(kid, approved_only=False, alive_only=False) updated_key = pre_oci_model.get_service_key(kid, approved_only=False, alive_only=False)
return jsonify(updated_key.to_dict()) return jsonify(updated_key.to_dict())
abort(403) raise Unauthorized()
@require_fresh_login @require_fresh_login
@verify_not_prod @verify_not_prod
@ -791,7 +790,7 @@ class SuperUserServiceKey(ApiResource):
try: try:
key = pre_oci_model.delete_service_key(kid) key = pre_oci_model.delete_service_key(kid)
except ServiceKeyDoesNotExist: except ServiceKeyDoesNotExist:
abort(404) raise NotFound()
key_log_metadata = { key_log_metadata = {
'kid': kid, 'kid': kid,
@ -804,7 +803,7 @@ class SuperUserServiceKey(ApiResource):
log_action('service_key_delete', None, key_log_metadata) log_action('service_key_delete', None, key_log_metadata)
return make_response('', 204) return make_response('', 204)
abort(403) raise Unauthorized()
@resource('/v1/superuser/approvedkeys/<kid>') @resource('/v1/superuser/approvedkeys/<kid>')
@ -850,13 +849,13 @@ class SuperUserServiceKeyApproval(ApiResource):
log_action('service_key_approve', None, key_log_metadata) log_action('service_key_approve', None, key_log_metadata)
except ServiceKeyDoesNotExist: except ServiceKeyDoesNotExist:
abort(404) raise NotFound()
except ServiceKeyAlreadyApproved: except ServiceKeyAlreadyApproved:
pass pass
return make_response('', 201) return make_response('', 201)
abort(403) raise Unauthorized()
@resource('/v1/superuser/customcerts') @resource('/v1/superuser/customcerts')
@ -905,7 +904,7 @@ class SuperUserCustomCertificates(ApiResource):
'certs': cert_views, 'certs': cert_views,
} }
abort(403) raise Unauthorized()
@resource('/v1/superuser/customcerts/<certpath>') @resource('/v1/superuser/customcerts/<certpath>')
@ -922,12 +921,12 @@ class SuperUserCustomCertificate(ApiResource):
if SuperUserPermission().can(): if SuperUserPermission().can():
uploaded_file = request.files['file'] uploaded_file = request.files['file']
if not uploaded_file: if not uploaded_file:
abort(400) raise InvalidRequest('Missing certificate file')
# Save the certificate. # Save the certificate.
certpath = pathvalidate.sanitize_filename(certpath) certpath = pathvalidate.sanitize_filename(certpath)
if not certpath.endswith('.crt'): if not certpath.endswith('.crt'):
abort(400) raise InvalidRequest('Invalid certificate file: must have suffix `.crt`')
logger.debug('Saving custom certificate %s', certpath) logger.debug('Saving custom certificate %s', certpath)
cert_full_path = config_provider.get_volume_path(EXTRA_CA_DIRECTORY, certpath) cert_full_path = config_provider.get_volume_path(EXTRA_CA_DIRECTORY, certpath)
@ -950,13 +949,13 @@ class SuperUserCustomCertificate(ApiResource):
if not app.config['TESTING']: if not app.config['TESTING']:
logger.debug('Calling certs_install.sh') logger.debug('Calling certs_install.sh')
if os.system('/conf/init/certs_install.sh') != 0: if os.system('/conf/init/certs_install.sh') != 0:
abort(500) raise Exception('Could not install certificates')
logger.debug('certs_install.sh completed') logger.debug('certs_install.sh completed')
return '', 204 return '', 204
abort(403) raise Unauthorized()
@nickname('deleteCustomCertificate') @nickname('deleteCustomCertificate')
@require_fresh_login @require_fresh_login
@ -968,7 +967,7 @@ class SuperUserCustomCertificate(ApiResource):
config_provider.remove_volume_file(cert_full_path) config_provider.remove_volume_file(cert_full_path)
return '', 204 return '', 204
abort(403) raise Unauthorized()
@resource('/v1/superuser/license') @resource('/v1/superuser/license')
@ -1011,7 +1010,7 @@ class SuperUserLicense(ApiResource):
'success': all_met, 'success': all_met,
} }
abort(403) raise Unauthorized()
@nickname('updateLicense') @nickname('updateLicense')
@require_fresh_login @require_fresh_login
@ -1039,7 +1038,7 @@ class SuperUserLicense(ApiResource):
'success': all_met, 'success': all_met,
} }
abort(403) raise Unauthorized()
@resource('/v1/superuser/<build_uuid>/logs') @resource('/v1/superuser/<build_uuid>/logs')
@ -1054,14 +1053,15 @@ class SuperUserRepositoryBuildLogs(ApiResource):
@require_scope(scopes.SUPERUSER) @require_scope(scopes.SUPERUSER)
def get(self, build_uuid): def get(self, build_uuid):
""" Return the build logs for the build specified by the build uuid. """ """ Return the build logs for the build specified by the build uuid. """
if not SuperUserPermission().can(): if SuperUserPermission().can():
abort(403)
try: try:
repo_build = pre_oci_model.get_repository_build(build_uuid) repo_build = pre_oci_model.get_repository_build(build_uuid)
return get_logs_or_log_url(repo_build) return get_logs_or_log_url(repo_build)
except InvalidRepositoryBuildException as e: except InvalidRepositoryBuildException as e:
raise InvalidResponse(e.message) raise InvalidResponse(e.message)
raise Unauthorized()
@resource('/v1/superuser/<build_uuid>/status') @resource('/v1/superuser/<build_uuid>/status')
@path_param('repository', 'The full path of the repository. e.g. namespace/name') @path_param('repository', 'The full path of the repository. e.g. namespace/name')
@ -1076,14 +1076,15 @@ class SuperUserRepositoryBuildStatus(ApiResource):
@require_scope(scopes.SUPERUSER) @require_scope(scopes.SUPERUSER)
def get(self, build_uuid): def get(self, build_uuid):
""" Return the status for the builds specified by the build uuids. """ """ Return the status for the builds specified by the build uuids. """
if not SuperUserPermission().can(): if SuperUserPermission().can():
abort(403)
try: try:
build = pre_oci_model.get_repository_build(build_uuid) build = pre_oci_model.get_repository_build(build_uuid)
except InvalidRepositoryBuildException as e: except InvalidRepositoryBuildException as e:
raise InvalidResponse(e.message) raise InvalidResponse(e.message)
return build.to_dict() return build.to_dict()
raise Unauthorized()
@resource('/v1/superuser/<build_uuid>/build') @resource('/v1/superuser/<build_uuid>/build')
@path_param('repository', 'The full path of the repository. e.g. namespace/name') @path_param('repository', 'The full path of the repository. e.g. namespace/name')
@ -1098,12 +1099,12 @@ class SuperUserRepositoryBuildResource(ApiResource):
@require_scope(scopes.SUPERUSER) @require_scope(scopes.SUPERUSER)
def get(self, build_uuid): def get(self, build_uuid):
""" Returns information about a build. """ """ Returns information about a build. """
if not SuperUserPermission().can(): if SuperUserPermission().can():
abort(403)
try: try:
build = pre_oci_model.get_repository_build(build_uuid) build = pre_oci_model.get_repository_build(build_uuid)
except InvalidRepositoryBuildException: except InvalidRepositoryBuildException:
raise NotFound() raise NotFound()
return build.to_dict() return build.to_dict()
raise Unauthorized()

4
test/test_api_security.py
View file

@ -3957,7 +3957,7 @@ class TestSuperUserServiceKeyManagement(ApiTestCase):
self._set_url(SuperUserServiceKeyManagement) self._set_url(SuperUserServiceKeyManagement)
def test_get_anonymous(self): def test_get_anonymous(self):
self._run_test('GET', 403, None, None) self._run_test('GET', 401, None, None)
def test_get_freshuser(self): def test_get_freshuser(self):
self._run_test('GET', 403, 'freshuser', None) self._run_test('GET', 403, 'freshuser', None)
@ -3987,7 +3987,7 @@ class TestSuperUserServiceKey(ApiTestCase):
self._set_url(SuperUserServiceKey, kid=1234) self._set_url(SuperUserServiceKey, kid=1234)
def test_get_anonymous(self): def test_get_anonymous(self):
self._run_test('GET', 403, None, None) self._run_test('GET', 401, None, None)
def test_get_freshuser(self): def test_get_freshuser(self):
self._run_test('GET', 403, 'freshuser', None) self._run_test('GET', 403, 'freshuser', None)