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