diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py index 212d5e69a..f5d0a9e81 100644 --- a/endpoints/api/superuser.py +++ b/endpoints/api/superuser.py @@ -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//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/') @@ -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/') @@ -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/') @@ -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/') @@ -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/') @@ -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/') @@ -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//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//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') @@ -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() diff --git a/test/test_api_security.py b/test/test_api_security.py index 085310c2c..655aa4252 100644 --- a/test/test_api_security.py +++ b/test/test_api_security.py @@ -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)