From 30b532254cb687862893f616cefe58b1be20175d Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 22 Mar 2017 14:30:13 -0400 Subject: [PATCH] Disallow non-apps-supported APIs for application repositories --- data/model/_basequery.py | 12 ++- data/model/repository.py | 5 +- endpoints/api/__init__.py | 16 +++- endpoints/api/build.py | 8 +- endpoints/api/image.py | 4 +- endpoints/api/manifest.py | 7 +- endpoints/api/repository.py | 3 +- endpoints/api/repositorynotification.py | 7 +- endpoints/api/secscan.py | 4 +- endpoints/api/tag.py | 8 +- endpoints/api/test/__init__.py | 0 endpoints/api/test/shared.py | 58 ++++++++++++++ endpoints/api/test/test_disallow_for_apps.py | 80 ++++++++++++++++++++ endpoints/api/test/test_security.py | 53 +++++-------- endpoints/api/trigger.py | 14 +++- test/test_api_usage.py | 8 +- 16 files changed, 236 insertions(+), 51 deletions(-) create mode 100644 endpoints/api/test/__init__.py create mode 100644 endpoints/api/test/shared.py create mode 100644 endpoints/api/test/test_disallow_for_apps.py diff --git a/data/model/_basequery.py b/data/model/_basequery.py index 2ebefbc62..d53c14434 100644 --- a/data/model/_basequery.py +++ b/data/model/_basequery.py @@ -3,15 +3,23 @@ from cachetools import lru_cache from data.model import DataModelException from data.database import (Repository, User, Team, TeamMember, RepositoryPermission, TeamRole, - Namespace, Visibility, ImageStorage, Image, db_for_update) + Namespace, Visibility, ImageStorage, Image, RepositoryKind, + db_for_update) -def get_existing_repository(namespace_name, repository_name, for_update=False): +def get_existing_repository(namespace_name, repository_name, for_update=False, kind_filter=None): query = (Repository .select(Repository, Namespace) .join(Namespace, on=(Repository.namespace_user == Namespace.id)) .where(Namespace.username == namespace_name, Repository.name == repository_name)) + + if kind_filter: + query = (query + .switch(Repository) + .join(RepositoryKind) + .where(RepositoryKind.name == kind_filter)) + if for_update: query = db_for_update(query) diff --git a/data/model/repository.py b/data/model/repository.py index 765d60c9d..876029c73 100644 --- a/data/model/repository.py +++ b/data/model/repository.py @@ -50,9 +50,10 @@ def create_repository(namespace, name, creating_user, visibility='private', repo return repo -def get_repository(namespace_name, repository_name): +def get_repository(namespace_name, repository_name, kind_filter=None): try: - return _basequery.get_existing_repository(namespace_name, repository_name) + return _basequery.get_existing_repository(namespace_name, repository_name, + kind_filter=kind_filter) except Repository.DoesNotExist: return None diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 6a9369d8d..f7e23eba5 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -22,7 +22,7 @@ from auth.auth_context import get_authenticated_user, get_validated_oauth_token from auth.process import process_oauth from endpoints.csrf import csrf_protect from endpoints.exception import (ApiException, Unauthorized, InvalidRequest, InvalidResponse, - FreshLoginRequired) + FreshLoginRequired, NotFound) from endpoints.decorators import check_anon_protection from util.metrics.metricqueue import time_decorator from util.names import parse_namespace_repository @@ -200,6 +200,20 @@ class RepositoryParamResource(ApiResource): method_decorators = [check_anon_protection, parse_repository_name] +def disallow_for_app_repositories(func): + @wraps(func) + def wrapped(self, namespace, repository, *args, **kwargs): + # Lookup the repository with the given namespace and name and ensure it is not an application + # repository. + repo = model.repository.get_repository(namespace, repository, kind_filter='application') + if repo: + abort(501) + + return func(self, namespace, repository, *args, **kwargs) + + return wrapped + + def require_repo_permission(permission_class, scope, allow_public=False): def wrapper(func): @add_method_metadata('oauth2_scope', scope) diff --git a/endpoints/api/build.py b/endpoints/api/build.py index ca7866f85..1bb575a4f 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -14,7 +14,7 @@ from buildtrigger.basehandler import BuildTriggerHandler from endpoints.api import (RepositoryParamResource, parse_args, query_param, nickname, resource, require_repo_read, require_repo_write, validate_json_request, ApiResource, internal_only, format_date, api, path_param, - require_repo_admin, abort) + require_repo_admin, abort, disallow_for_app_repositories) from endpoints.exception import Unauthorized, NotFound, InvalidRequest from endpoints.building import start_build, PreparedBuild, MaximumBuildsQueuedException from data import database @@ -200,6 +200,7 @@ class RepositoryBuildList(RepositoryParamResource): @query_param('limit', 'The maximum number of builds to return', type=int, default=5) @query_param('since', 'Returns all builds since the given unix timecode', type=int, default=None) @nickname('getRepoBuilds') + @disallow_for_app_repositories def get(self, namespace, repository, parsed_args): """ Get the list of repository builds. """ limit = parsed_args.get('limit', 5) @@ -215,6 +216,7 @@ class RepositoryBuildList(RepositoryParamResource): @require_repo_write @nickname('requestRepoBuild') + @disallow_for_app_repositories @validate_json_request('RepositoryBuildRequest') def post(self, namespace, repository): """ Request that a repository be built and pushed from the specified input. """ @@ -315,6 +317,7 @@ class RepositoryBuildResource(RepositoryParamResource): """ Resource for dealing with repository builds. """ @require_repo_read @nickname('getRepoBuild') + @disallow_for_app_repositories def get(self, namespace, repository, build_uuid): """ Returns information about a build. """ try: @@ -329,6 +332,7 @@ class RepositoryBuildResource(RepositoryParamResource): @require_repo_admin @nickname('cancelRepoBuild') + @disallow_for_app_repositories def delete(self, namespace, repository, build_uuid): """ Cancels a repository build. """ try: @@ -352,6 +356,7 @@ class RepositoryBuildStatus(RepositoryParamResource): """ Resource for dealing with repository build status. """ @require_repo_read @nickname('getRepoBuildStatus') + @disallow_for_app_repositories def get(self, namespace, repository, build_uuid): """ Return the status for the builds specified by the build uuids. """ build = model.build.get_repository_build(build_uuid) @@ -392,6 +397,7 @@ class RepositoryBuildLogs(RepositoryParamResource): """ Resource for loading repository build logs. """ @require_repo_write @nickname('getRepoBuildLogs') + @disallow_for_app_repositories def get(self, namespace, repository, build_uuid): """ Return the build logs for the build specified by the build uuid. """ build = model.build.get_repository_build(build_uuid) diff --git a/endpoints/api/image.py b/endpoints/api/image.py index 0d6e59425..35c346ff1 100644 --- a/endpoints/api/image.py +++ b/endpoints/api/image.py @@ -4,7 +4,7 @@ import json from collections import defaultdict from endpoints.api import (resource, nickname, require_repo_read, RepositoryParamResource, - format_date, path_param) + format_date, path_param, disallow_for_app_repositories) from endpoints.exception import NotFound from data import model @@ -49,6 +49,7 @@ class RepositoryImageList(RepositoryParamResource): """ Resource for listing repository images. """ @require_repo_read @nickname('listRepositoryImages') + @disallow_for_app_repositories def get(self, namespace, repository): """ List the images for the specified repository. """ repo = model.repository.get_repository(namespace, repository) @@ -89,6 +90,7 @@ class RepositoryImage(RepositoryParamResource): """ Resource for handling repository images. """ @require_repo_read @nickname('getImage') + @disallow_for_app_repositories def get(self, namespace, repository, image_id): """ Get the information available for the specified image. """ image = model.image.get_repo_image_extended(namespace, repository, image_id) diff --git a/endpoints/api/manifest.py b/endpoints/api/manifest.py index aede35bda..e96283f7f 100644 --- a/endpoints/api/manifest.py +++ b/endpoints/api/manifest.py @@ -4,7 +4,8 @@ from app import label_validator from flask import request from endpoints.api import (resource, nickname, require_repo_read, require_repo_write, RepositoryParamResource, log_action, validate_json_request, - path_param, parse_args, query_param, truthy_bool, abort, api) + path_param, parse_args, query_param, truthy_bool, abort, api, + disallow_for_app_repositories) from endpoints.exception import NotFound from data import model @@ -59,6 +60,7 @@ class RepositoryManifestLabels(RepositoryParamResource): @require_repo_read @nickname('listManifestLabels') + @disallow_for_app_repositories @parse_args() @query_param('filter', 'If specified, only labels matching the given prefix will be returned', type=str, default=None) @@ -75,6 +77,7 @@ class RepositoryManifestLabels(RepositoryParamResource): @require_repo_write @nickname('addManifestLabel') + @disallow_for_app_repositories @validate_json_request('AddLabel') def post(self, namespace, repository, manifestref): """ Adds a new label into the tag manifest. """ @@ -121,6 +124,7 @@ class ManageRepositoryManifestLabel(RepositoryParamResource): """ Resource for managing the labels on a specific repository manifest. """ @require_repo_read @nickname('getManifestLabel') + @disallow_for_app_repositories def get(self, namespace, repository, manifestref, labelid): """ Retrieves the label with the specific ID under the manifest. """ try: @@ -137,6 +141,7 @@ class ManageRepositoryManifestLabel(RepositoryParamResource): @require_repo_write @nickname('deleteManifestLabel') + @disallow_for_app_repositories def delete(self, namespace, repository, manifestref, labelid): """ Deletes an existing label from a manifest. """ try: diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index d8b08483b..939be4e68 100644 --- a/endpoints/api/repository.py +++ b/endpoints/api/repository.py @@ -14,7 +14,7 @@ from endpoints.api import (truthy_bool, format_date, nickname, log_action, valid require_repo_read, require_repo_write, require_repo_admin, RepositoryParamResource, resource, query_param, parse_args, ApiResource, request_error, require_scope, path_param, page_support, parse_args, - query_param, truthy_bool) + query_param, truthy_bool, disallow_for_app_repositories) from endpoints.exception import Unauthorized, NotFound, InvalidRequest, ExceedsLicenseException from endpoints.api.billing import lookup_allowed_private_repos, get_namespace_plan from endpoints.api.subscribe import check_repository_usage @@ -354,6 +354,7 @@ class Repository(RepositoryParamResource): @require_repo_admin @nickname('deleteRepository') + @disallow_for_app_repositories def delete(self, namespace, repository): """ Delete a repository. """ model.repository.purge_repository(namespace, repository) diff --git a/endpoints/api/repositorynotification.py b/endpoints/api/repositorynotification.py index 1a68ec20d..ac14ec2e0 100644 --- a/endpoints/api/repositorynotification.py +++ b/endpoints/api/repositorynotification.py @@ -7,7 +7,7 @@ from flask import request from app import notification_queue from endpoints.api import (RepositoryParamResource, nickname, resource, require_repo_admin, log_action, validate_json_request, request_error, - path_param) + path_param, disallow_for_app_repositories) from endpoints.exception import NotFound from endpoints.notificationevent import NotificationEvent from endpoints.notificationmethod import (NotificationMethod, @@ -80,6 +80,7 @@ class RepositoryNotificationList(RepositoryParamResource): @require_repo_admin @nickname('createRepoNotification') + @disallow_for_app_repositories @validate_json_request('NotificationCreateRequest') def post(self, namespace, repository): """ Create a new notification for the specified repository. """ @@ -110,6 +111,7 @@ class RepositoryNotificationList(RepositoryParamResource): @require_repo_admin @nickname('listRepoNotifications') + @disallow_for_app_repositories def get(self, namespace, repository): """ List the notifications for the specified repository. """ notifications = model.notification.list_repo_notifications(namespace, repository) @@ -125,6 +127,7 @@ class RepositoryNotification(RepositoryParamResource): """ Resource for dealing with specific notifications. """ @require_repo_admin @nickname('getRepoNotification') + @disallow_for_app_repositories def get(self, namespace, repository, uuid): """ Get information for the specified notification. """ try: @@ -140,6 +143,7 @@ class RepositoryNotification(RepositoryParamResource): @require_repo_admin @nickname('deleteRepoNotification') + @disallow_for_app_repositories def delete(self, namespace, repository, uuid): """ Deletes the specified notification. """ deleted = model.notification.delete_repo_notification(namespace, repository, uuid) @@ -158,6 +162,7 @@ class TestRepositoryNotification(RepositoryParamResource): """ Resource for queuing a test of a notification. """ @require_repo_admin @nickname('testRepoNotification') + @disallow_for_app_repositories def post(self, namespace, repository, uuid): """ Queues a test notification for this repository. """ try: diff --git a/endpoints/api/secscan.py b/endpoints/api/secscan.py index 7295e6604..9ec9dafb2 100644 --- a/endpoints/api/secscan.py +++ b/endpoints/api/secscan.py @@ -7,7 +7,7 @@ from app import secscan_api from data import model from endpoints.api import (require_repo_read, path_param, RepositoryParamResource, resource, nickname, show_if, parse_args, - query_param, truthy_bool) + query_param, truthy_bool, disallow_for_app_repositories) from endpoints.exception import NotFound, DownstreamIssue from endpoints.api.manifest import MANIFEST_DIGEST_ROUTE from util.secscan.api import APIRequestFailure @@ -67,6 +67,7 @@ class RepositoryImageSecurity(RepositoryParamResource): @require_repo_read @nickname('getRepoImageSecurity') + @disallow_for_app_repositories @parse_args() @query_param('vulnerabilities', 'Include vulnerabilities informations', type=truthy_bool, default=False) @@ -88,6 +89,7 @@ class RepositoryManifestSecurity(RepositoryParamResource): @require_repo_read @nickname('getRepoManifestSecurity') + @disallow_for_app_repositories @parse_args() @query_param('vulnerabilities', 'Include vulnerabilities informations', type=truthy_bool, default=False) diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index bbc88ebcf..014052ad7 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -4,7 +4,8 @@ from flask import request, abort from endpoints.api import (resource, nickname, require_repo_read, require_repo_write, RepositoryParamResource, log_action, validate_json_request, - path_param, parse_args, query_param, truthy_bool) + path_param, parse_args, query_param, truthy_bool, + disallow_for_app_repositories) from endpoints.exception import NotFound from endpoints.api.image import image_view from data import model @@ -18,6 +19,7 @@ class ListRepositoryTags(RepositoryParamResource): """ Resource for listing full repository tag history, alive *and dead*. """ @require_repo_read + @disallow_for_app_repositories @parse_args() @query_param('specificTag', 'Filters the tags to the specific tag.', type=str, default='') @query_param('limit', 'Limit to the number of results to return per page. Max 100.', type=int, default=50) @@ -82,6 +84,7 @@ class RepositoryTag(RepositoryParamResource): } @require_repo_write + @disallow_for_app_repositories @nickname('changeTagImage') @validate_json_request('MoveTag') def put(self, namespace, repository, tag): @@ -116,6 +119,7 @@ class RepositoryTag(RepositoryParamResource): return 'Updated', 201 @require_repo_write + @disallow_for_app_repositories @nickname('deleteFullTag') def delete(self, namespace, repository, tag): """ Delete the specified repository tag. """ @@ -136,6 +140,7 @@ class RepositoryTagImages(RepositoryParamResource): """ Resource for listing the images in a specific repository tag. """ @require_repo_read @nickname('listTagImages') + @disallow_for_app_repositories @parse_args() @query_param('owned', 'If specified, only images wholely owned by this tag are returned.', type=truthy_bool, default=False) @@ -206,6 +211,7 @@ class RestoreTag(RepositoryParamResource): } @require_repo_write + @disallow_for_app_repositories @nickname('restoreTag') @validate_json_request('RestoreTag') def post(self, namespace, repository, tag): diff --git a/endpoints/api/test/__init__.py b/endpoints/api/test/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/endpoints/api/test/shared.py b/endpoints/api/test/shared.py new file mode 100644 index 000000000..110f1044c --- /dev/null +++ b/endpoints/api/test/shared.py @@ -0,0 +1,58 @@ +import datetime +import json + +from contextlib import contextmanager + +from data import model +from endpoints.api import api + +CSRF_TOKEN_KEY = '_csrf_token' +CSRF_TOKEN = '123csrfforme' + + +@contextmanager +def client_with_identity(auth_username, client): + with client.session_transaction() as sess: + if auth_username: + if auth_username is not None: + loaded = model.user.get_user(auth_username) + sess['user_id'] = loaded.uuid + sess['login_time'] = datetime.datetime.now() + sess[CSRF_TOKEN_KEY] = CSRF_TOKEN + + yield client + + with client.session_transaction() as sess: + sess['user_id'] = None + sess['login_time'] = None + sess[CSRF_TOKEN_KEY] = None + + +def add_csrf_param(params): + """ Returns a params dict with the CSRF parameter added. """ + params = params or {} + params[CSRF_TOKEN_KEY] = CSRF_TOKEN + return params + + +def conduct_api_call(client, resource, method, params, body=None, expected_code=200): + """ Conducts an API call to the given resource via the given client, and ensures its returned + status matches the code given. + + Returns the response. + """ + params = add_csrf_param(params) + + final_url = api.url_for(resource, **params) + + headers = {} + headers.update({"Content-Type": "application/json"}) + + if body is not None: + body = json.dumps(body) + + rv = client.open(final_url, method=method, data=body, headers=headers) + msg = '%s %s: got %s expected: %s | %s' % (method, final_url, rv.status_code, expected_code, + rv.data) + assert rv.status_code == expected_code, msg + return rv diff --git a/endpoints/api/test/test_disallow_for_apps.py b/endpoints/api/test/test_disallow_for_apps.py new file mode 100644 index 000000000..a9e3b4d00 --- /dev/null +++ b/endpoints/api/test/test_disallow_for_apps.py @@ -0,0 +1,80 @@ +import pytest + +from data import model +from endpoints.api.repository import Repository +from endpoints.api.build import (RepositoryBuildList, RepositoryBuildResource, + RepositoryBuildStatus, RepositoryBuildLogs) +from endpoints.api.image import RepositoryImageList, RepositoryImage +from endpoints.api.manifest import RepositoryManifestLabels, ManageRepositoryManifestLabel +from endpoints.api.repositorynotification import (RepositoryNotification, + RepositoryNotificationList, + TestRepositoryNotification) +from endpoints.api.secscan import RepositoryImageSecurity, RepositoryManifestSecurity +from endpoints.api.tag import ListRepositoryTags, RepositoryTag, RepositoryTagImages, RestoreTag +from endpoints.api.trigger import (BuildTriggerList, BuildTrigger, BuildTriggerSubdirs, + BuildTriggerActivate, BuildTriggerAnalyze, ActivateBuildTrigger, + TriggerBuildList, BuildTriggerFieldValues, BuildTriggerSources, + BuildTriggerSourceNamespaces) +from endpoints.api.test.shared import client_with_identity, conduct_api_call +from endpoints.test.fixtures import app, appconfig, database_uri, init_db_path, sqlitedb_file + +BUILD_ARGS = {'build_uuid': '1234'} +IMAGE_ARGS = {'imageid': '1234', 'image_id': 1234} +MANIFEST_ARGS = {'manifestref': 'sha256:abcd1234'} +LABEL_ARGS = {'manifestref': 'sha256:abcd1234', 'labelid': '1234'} +NOTIFICATION_ARGS = {'uuid': '1234'} +TAG_ARGS = {'tag': 'foobar'} +TRIGGER_ARGS = {'trigger_uuid': '1234'} +FIELD_ARGS = {'trigger_uuid': '1234', 'field_name': 'foobar'} + +@pytest.mark.parametrize('resource, method, params', [ + (Repository, 'delete', None), + (RepositoryBuildList, 'get', None), + (RepositoryBuildList, 'post', None), + (RepositoryBuildResource, 'get', BUILD_ARGS), + (RepositoryBuildResource, 'delete', BUILD_ARGS), + (RepositoryBuildStatus, 'get', BUILD_ARGS), + (RepositoryBuildLogs, 'get', BUILD_ARGS), + (RepositoryImageList, 'get', None), + (RepositoryImage, 'get', IMAGE_ARGS), + (RepositoryManifestLabels, 'get', MANIFEST_ARGS), + (RepositoryManifestLabels, 'post', MANIFEST_ARGS), + (ManageRepositoryManifestLabel, 'get', LABEL_ARGS), + (ManageRepositoryManifestLabel, 'delete', LABEL_ARGS), + (RepositoryNotificationList, 'get', None), + (RepositoryNotificationList, 'post', None), + (RepositoryNotification, 'get', NOTIFICATION_ARGS), + (RepositoryNotification, 'delete', NOTIFICATION_ARGS), + (TestRepositoryNotification, 'post', NOTIFICATION_ARGS), + (RepositoryImageSecurity, 'get', IMAGE_ARGS), + (RepositoryManifestSecurity, 'get', MANIFEST_ARGS), + (ListRepositoryTags, 'get', None), + (RepositoryTag, 'put', TAG_ARGS), + (RepositoryTag, 'delete', TAG_ARGS), + (RepositoryTagImages, 'get', TAG_ARGS), + (RestoreTag, 'post', TAG_ARGS), + (BuildTriggerList, 'get', None), + (BuildTrigger, 'get', TRIGGER_ARGS), + (BuildTrigger, 'delete', TRIGGER_ARGS), + (BuildTriggerSubdirs, 'post', TRIGGER_ARGS), + (BuildTriggerActivate, 'post', TRIGGER_ARGS), + (BuildTriggerAnalyze, 'post', TRIGGER_ARGS), + (ActivateBuildTrigger, 'post', TRIGGER_ARGS), + (TriggerBuildList, 'get', TRIGGER_ARGS), + (BuildTriggerFieldValues, 'post', FIELD_ARGS), + (BuildTriggerSources, 'post', TRIGGER_ARGS), + (BuildTriggerSourceNamespaces, 'get', TRIGGER_ARGS), +]) +def test_disallowed_for_apps(resource, method, params, client): + namespace = 'devtable' + repository = 'someapprepo' + + devtable = model.user.get_user('devtable') + model.repository.create_repository(namespace, repository, devtable, repo_kind='application') + + params = params or {} + params['repository'] = '%s/%s' % (namespace, repository) + + with client_with_identity('devtable', client) as cl: + conduct_api_call(cl, resource, method, params, None, 501) + diff --git a/endpoints/api/test/test_security.py b/endpoints/api/test/test_security.py index 2d2babbe7..e14051a23 100644 --- a/endpoints/api/test/test_security.py +++ b/endpoints/api/test/test_security.py @@ -1,44 +1,29 @@ -import datetime - import pytest -from data import model -from endpoints.api import api +from endpoints.api.test.shared import client_with_identity, conduct_api_call from endpoints.api.superuser import SuperUserRepositoryBuildLogs, SuperUserRepositoryBuildResource from endpoints.api.superuser import SuperUserRepositoryBuildStatus from endpoints.test.fixtures import app, appconfig, database_uri, init_db_path, sqlitedb_file +TEAM_PARAMS = {'orgname': 'buynlarge', 'teamname': 'owners'} +BUILD_PARAMS = {'build_uuid': 'test-1234'} -def client_with_identity(auth_username, client): - with client.session_transaction() as sess: - if auth_username: - if auth_username is not None: - loaded = model.user.get_user(auth_username) - sess['user_id'] = loaded.uuid - sess['login_time'] = datetime.datetime.now() - return client +@pytest.mark.parametrize('resource,method,params,body,identity,expected', [ + (SuperUserRepositoryBuildLogs, 'GET', BUILD_PARAMS, None, None, 401), + (SuperUserRepositoryBuildLogs, 'GET', BUILD_PARAMS, None, 'freshuser', 403), + (SuperUserRepositoryBuildLogs, 'GET', BUILD_PARAMS, None, 'reader', 403), + (SuperUserRepositoryBuildLogs, 'GET', BUILD_PARAMS, None, 'devtable', 400), + (SuperUserRepositoryBuildStatus, 'GET', BUILD_PARAMS, None, None, 401), + (SuperUserRepositoryBuildStatus, 'GET', BUILD_PARAMS, None, 'freshuser', 403), + (SuperUserRepositoryBuildStatus, 'GET', BUILD_PARAMS, None, 'reader', 403), + (SuperUserRepositoryBuildStatus, 'GET', BUILD_PARAMS, None, 'devtable', 400), -@pytest.mark.parametrize('resource,identity,expected', [ - (SuperUserRepositoryBuildLogs, None, 401), - (SuperUserRepositoryBuildLogs, 'freshuser', 403), - (SuperUserRepositoryBuildLogs, 'reader', 403), - (SuperUserRepositoryBuildLogs, 'devtable', 400), - - (SuperUserRepositoryBuildStatus, None, 401), - (SuperUserRepositoryBuildStatus, 'freshuser', 403), - (SuperUserRepositoryBuildStatus, 'reader', 403), - (SuperUserRepositoryBuildStatus, 'devtable', 400), - - (SuperUserRepositoryBuildResource, None, 401), - (SuperUserRepositoryBuildResource, 'freshuser', 403), - (SuperUserRepositoryBuildResource, 'reader', 403), - (SuperUserRepositoryBuildResource, 'devtable', 404), + (SuperUserRepositoryBuildResource, 'GET', BUILD_PARAMS, None, None, 401), + (SuperUserRepositoryBuildResource, 'GET', BUILD_PARAMS, None, 'freshuser', 403), + (SuperUserRepositoryBuildResource, 'GET', BUILD_PARAMS, None, 'reader', 403), + (SuperUserRepositoryBuildResource, 'GET', BUILD_PARAMS, None, 'devtable', 404), ]) -def test_super_user_build_endpoints(resource, identity, expected, client): - cl = client_with_identity(identity, client) - final_url = api.url_for(resource, build_uuid='1234') - rv = cl.open(final_url) - msg = '%s %s: %s expected: %s' % ('GET', final_url, rv.status_code, expected) - assert rv.status_code == expected, msg - +def test_api_security(resource, method, params, body, identity, expected, client): + with client_with_identity(identity, client) as cl: + conduct_api_call(cl, resource, method, params, body, expected) diff --git a/endpoints/api/trigger.py b/endpoints/api/trigger.py index 95328818d..102aca0e1 100644 --- a/endpoints/api/trigger.py +++ b/endpoints/api/trigger.py @@ -15,7 +15,8 @@ from buildtrigger.triggerutil import (TriggerDeactivationException, RepositoryReadException, TriggerStartException) from endpoints.api import (RepositoryParamResource, nickname, resource, require_repo_admin, log_action, request_error, query_param, parse_args, internal_only, - validate_json_request, api, path_param, abort) + validate_json_request, api, path_param, abort, + disallow_for_app_repositories) from endpoints.exception import NotFound, Unauthorized, InvalidRequest from endpoints.api.build import build_status_view, trigger_view, RepositoryBuildStatus from endpoints.building import start_build, MaximumBuildsQueuedException @@ -40,6 +41,7 @@ class BuildTriggerList(RepositoryParamResource): """ Resource for listing repository build triggers. """ @require_repo_admin + @disallow_for_app_repositories @nickname('listBuildTriggers') def get(self, namespace_name, repo_name): """ List the triggers for the specified repository. """ @@ -56,6 +58,7 @@ class BuildTrigger(RepositoryParamResource): """ Resource for managing specific build triggers. """ @require_repo_admin + @disallow_for_app_repositories @nickname('getBuildTrigger') def get(self, namespace_name, repo_name, trigger_uuid): """ Get information for the specified build trigger. """ @@ -67,6 +70,7 @@ class BuildTrigger(RepositoryParamResource): return trigger_view(trigger, can_admin=True) @require_repo_admin + @disallow_for_app_repositories @nickname('deleteBuildTrigger') def delete(self, namespace_name, repo_name, trigger_uuid): """ Delete the specified build trigger. """ @@ -110,6 +114,7 @@ class BuildTriggerSubdirs(RepositoryParamResource): } @require_repo_admin + @disallow_for_app_repositories @nickname('listBuildTriggerSubdirs') @validate_json_request('BuildTriggerSubdirRequest') def post(self, namespace_name, repo_name, trigger_uuid): @@ -170,6 +175,7 @@ class BuildTriggerActivate(RepositoryParamResource): } @require_repo_admin + @disallow_for_app_repositories @nickname('activateBuildTrigger') @validate_json_request('BuildTriggerActivateRequest') def post(self, namespace_name, repo_name, trigger_uuid): @@ -271,6 +277,7 @@ class BuildTriggerAnalyze(RepositoryParamResource): } @require_repo_admin + @disallow_for_app_repositories @nickname('analyzeBuildTrigger') @validate_json_request('BuildTriggerAnalyzeRequest') def post(self, namespace_name, repo_name, trigger_uuid): @@ -413,6 +420,7 @@ class ActivateBuildTrigger(RepositoryParamResource): } @require_repo_admin + @disallow_for_app_repositories @nickname('manuallyStartBuildTrigger') @validate_json_request('RunParameters') def post(self, namespace_name, repo_name, trigger_uuid): @@ -453,6 +461,7 @@ class ActivateBuildTrigger(RepositoryParamResource): class TriggerBuildList(RepositoryParamResource): """ Resource to represent builds that were activated from the specified trigger. """ @require_repo_admin + @disallow_for_app_repositories @parse_args() @query_param('limit', 'The maximum number of builds to return', type=int, default=5) @nickname('listTriggerRecentBuilds') @@ -472,6 +481,7 @@ FIELD_VALUE_LIMIT = 30 class BuildTriggerFieldValues(RepositoryParamResource): """ Custom verb to fetch a values list for a particular field name. """ @require_repo_admin + @disallow_for_app_repositories @nickname('listTriggerFieldValues') def post(self, namespace_name, repo_name, trigger_uuid, field_name): """ List the field values for a custom run field. """ @@ -515,6 +525,7 @@ class BuildTriggerSources(RepositoryParamResource): } @require_repo_admin + @disallow_for_app_repositories @nickname('listTriggerBuildSources') @validate_json_request('BuildTriggerSourcesRequest') def post(self, namespace_name, repo_name, trigger_uuid): @@ -548,6 +559,7 @@ class BuildTriggerSources(RepositoryParamResource): class BuildTriggerSourceNamespaces(RepositoryParamResource): """ Custom verb to fetch the list of namespaces (orgs, projects, etc) for the trigger config. """ @require_repo_admin + @disallow_for_app_repositories @nickname('listTriggerBuildSourceNamespaces') def get(self, namespace_name, repo_name, trigger_uuid): """ List the build sources for the trigger configuration thus far. """ diff --git a/test/test_api_usage.py b/test/test_api_usage.py index c29a1c622..81831f050 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -2546,8 +2546,8 @@ class TestRepoBuilds(ApiTestCase): def test_getrepo_nobuilds(self): self.login(ADMIN_ACCESS_USER) - # Queries: Permission + the list query - with assert_query_count(2): + # Queries: Permission + the list query + app check + with assert_query_count(3): json = self.getJsonResponse(RepositoryBuildList, params=dict(repository=ADMIN_ACCESS_USER + '/simple')) @@ -2556,8 +2556,8 @@ class TestRepoBuilds(ApiTestCase): def test_getrepobuilds(self): self.login(ADMIN_ACCESS_USER) - # Queries: Permission + the list query - with assert_query_count(2): + # Queries: Permission + the list query + app check + with assert_query_count(3): json = self.getJsonResponse(RepositoryBuildList, params=dict(repository=ADMIN_ACCESS_USER + '/building'))