diff --git a/data/database.py b/data/database.py index b0cb60bb6..d6a67bd80 100644 --- a/data/database.py +++ b/data/database.py @@ -176,7 +176,7 @@ class RepositoryBuildTrigger(BaseModel): auth_token = CharField() config = TextField(default='{}') write_token = ForeignKeyField(AccessToken, null=True) - pull_user = ForeignKeyField(User, null=True, related_name='pulluser') + pull_robot = ForeignKeyField(User, null=True, related_name='triggerpullrobot') class EmailConfirmation(BaseModel): @@ -245,6 +245,7 @@ class RepositoryBuild(BaseModel): started = DateTimeField(default=datetime.now) display_name = CharField() trigger = ForeignKeyField(RepositoryBuildTrigger, null=True, index=True) + pull_robot = ForeignKeyField(User, null=True, related_name='buildpullrobot') class QueueItem(BaseModel): diff --git a/data/model/legacy.py b/data/model/legacy.py index a2cac1d26..3cfa6c27f 100644 --- a/data/model/legacy.py +++ b/data/model/legacy.py @@ -1453,27 +1453,42 @@ def get_recent_repository_build(namespace_name, repository_name): def create_repository_build(repo, access_token, job_config_obj, dockerfile_id, - display_name, trigger=None): + display_name, trigger=None, pull_robot_name=None): + pull_robot = None + if pull_robot_name: + pull_robot = lookup_robot(pull_robot_name) + return RepositoryBuild.create(repository=repo, access_token=access_token, job_config=json.dumps(job_config_obj), display_name=display_name, trigger=trigger, - resource_key=dockerfile_id) + resource_key=dockerfile_id, + pull_robot=pull_robot) -def get_pull_credentials(trigger): - if not trigger.pull_user: + +def get_pull_robot_name(trigger): + if not trigger.pull_robot: return None + return trigger.pull_robot.username + + +def get_pull_credentials(robotname): + robot = lookup_robot(robotname) + if not robot: + return None + try: - login_info = FederatedLogin.get(user=trigger.pull_user) + login_info = FederatedLogin.get(user=robot) except FederatedLogin.DoesNotExist: return None return { - 'username': trigger.pull_user.username, + 'username': robot.username, 'password': login_info.service_ident, 'registry': '%s://%s/v1/' % (app.config['URL_SCHEME'], app.config['URL_HOST']), } + def create_webhook(repo, params_obj): return Webhook.create(repository=repo, parameters=json.dumps(params_obj)) @@ -1530,12 +1545,12 @@ def log_action(kind_name, user_or_organization_name, performer=None, metadata_json=json.dumps(metadata), datetime=timestamp) -def create_build_trigger(repo, service_name, auth_token, user, pull_user=None): +def create_build_trigger(repo, service_name, auth_token, user, pull_robot=None): service = BuildTriggerService.get(name=service_name) trigger = RepositoryBuildTrigger.create(repository=repo, service=service, auth_token=auth_token, connected_user=user, - pull_user=None) + pull_robot=pull_robot) return trigger diff --git a/endpoints/api/build.py b/endpoints/api/build.py index 3ff6dfd9d..9fa130054 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -10,8 +10,10 @@ from endpoints.api import (RepositoryParamResource, parse_args, query_param, nic from endpoints.common import start_build from endpoints.trigger import BuildTrigger from data import model -from auth.permissions import ModifyRepositoryPermission +from auth.auth_context import get_authenticated_user +from auth.permissions import ModifyRepositoryPermission, AdministerOrganizationPermission from data.buildlogs import BuildStatusRetrievalError +from util.names import parse_robot_username logger = logging.getLogger(__name__) @@ -33,13 +35,14 @@ def get_job_config(build_obj): return None +def user_view(user): + return { + 'name': user.username, + 'kind': 'user', + 'is_robot': user.robot, + } + def trigger_view(trigger): - def user_view(user): - return { - 'name': user.username, - 'kind': 'user', - 'is_robot': user.robot, - } if trigger and trigger.uuid: config_dict = get_trigger_config(trigger) @@ -50,7 +53,7 @@ def trigger_view(trigger): 'id': trigger.uuid, 'connected_user': trigger.connected_user.username, 'is_active': build_trigger.is_active(config_dict), - 'pull_user': user_view(trigger.pull_user) if trigger.pull_user else None + 'pull_robot': user_view(trigger.pull_robot) if trigger.pull_robot else None } return None @@ -75,6 +78,7 @@ def build_status_view(build_obj, can_write=False): 'is_writer': can_write, 'trigger': trigger_view(build_obj.trigger), 'resource_key': build_obj.resource_key, + 'pull_robot': user_view(build_obj.pull_robot) if build_obj.pull_robot else None, } if can_write: @@ -103,28 +107,9 @@ class RepositoryBuildList(RepositoryParamResource): 'type': 'string', 'description': 'Subdirectory in which the Dockerfile can be found', }, - 'pull_credentials': { - 'type': 'object', - 'description': 'Credentials used by the builder when pulling images', - 'required': [ - 'username', - 'password', - 'registry' - ], - 'properties': { - 'username': { - 'type': 'string', - 'description': 'The username for the pull' - }, - 'password': { - 'type': 'string', - 'description': 'The password for the pull' - }, - 'registry': { - 'type': 'string', - 'description': 'The registry hostname for the pull' - }, - } + 'pull_robot': { + 'type': 'string', + 'description': 'Username of a Quay robot account to use as pull credentials', } }, }, @@ -154,7 +139,22 @@ class RepositoryBuildList(RepositoryParamResource): dockerfile_id = request_json['file_id'] subdir = request_json['subdirectory'] if 'subdirectory' in request_json else '' - pull_credentials = request_json.get('pull_credentials', None) + pull_robot_name = request_json.get('pull_robot', None) + + # Verify the security behind the pull robot. + if pull_robot_name: + result = parse_robot_username(pull_robot_name) + if result: + pull_robot = model.lookup_robot(pull_robot_name) + if not pull_robot: + raise NotFound() + + # Make sure the user has administer permissions for the robot's namespace. + (robot_namespace, shortname) = result + if not AdministerOrganizationPermission(robot_namespace).can(): + raise Unauthorized() + else: + raise Unauthorized() # Check if the dockerfile resource has already been used. If so, then it # can only be reused if the user has access to the repository for which it @@ -170,7 +170,7 @@ class RepositoryBuildList(RepositoryParamResource): display_name = user_files.get_file_checksum(dockerfile_id) build_request = start_build(repo, dockerfile_id, ['latest'], display_name, subdir, True, - pull_credentials=pull_credentials) + pull_robot_name=pull_robot_name) resp = build_status_view(build_request, True) repo_string = '%s/%s' % (namespace, repository) diff --git a/endpoints/api/trigger.py b/endpoints/api/trigger.py index fc734d3db..c62367f52 100644 --- a/endpoints/api/trigger.py +++ b/endpoints/api/trigger.py @@ -190,7 +190,7 @@ class BuildTriggerActivate(RepositoryParamResource): raise Unauthorized() # Set the pull robot. - trigger.pull_user = pull_robot + trigger.pull_robot = pull_robot # Update the config. new_config_dict = request.get_json()['config'] @@ -224,7 +224,7 @@ class BuildTriggerActivate(RepositoryParamResource): log_action('setup_repo_trigger', namespace, {'repo': repository, 'namespace': namespace, 'trigger_id': trigger.uuid, 'service': trigger.service.name, - 'pull_user': trigger.pull_user.username if trigger.pull_user else None, + 'pull_robot': trigger.pull_robot.username if trigger.pull_robot else None, 'config': final_config}, repo=repo) return trigger_view(trigger) @@ -254,10 +254,10 @@ class ActivateBuildTrigger(RepositoryParamResource): dockerfile_id, tags, name, subdir = specs repo = model.get_repository(namespace, repository) - pull_credentials = model.get_pull_credentials(trigger) + pull_robot_name = model.get_pull_robot_name(trigger) build_request = start_build(repo, dockerfile_id, tags, name, subdir, True, - pull_credentials=pull_credentials) + pull_robot_name=pull_robot_name) resp = build_status_view(build_request, True) repo_string = '%s/%s' % (namespace, repository) diff --git a/endpoints/common.py b/endpoints/common.py index e22df6226..e25d7c797 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -107,7 +107,7 @@ def check_repository_usage(user_or_org, plan_found): def start_build(repository, dockerfile_id, tags, build_name, subdir, manual, - trigger=None, pull_credentials=None): + trigger=None, pull_robot_name=None): host = urlparse.urlparse(request.url).netloc repo_path = '%s/%s/%s' % (host, repository.namespace, repository.name) @@ -118,18 +118,18 @@ def start_build(repository, dockerfile_id, tags, build_name, subdir, manual, job_config = { 'docker_tags': tags, 'repository': repo_path, - 'build_subdir': subdir, - 'pull_credentials': pull_credentials, + 'build_subdir': subdir } build_request = model.create_repository_build(repository, token, job_config, dockerfile_id, build_name, - trigger) + trigger, pull_robot_name = pull_robot_name) dockerfile_build_queue.put(json.dumps({ 'build_uuid': build_request.uuid, 'namespace': repository.namespace, 'repository': repository.name, + 'pull_credentials': model.get_pull_credentials(pull_robot_name) if pull_robot_name else None }), retries_remaining=1) metadata = { diff --git a/endpoints/webhooks.py b/endpoints/webhooks.py index 69ba97b58..93d5e413c 100644 --- a/endpoints/webhooks.py +++ b/endpoints/webhooks.py @@ -73,10 +73,10 @@ def build_trigger_webhook(namespace, repository, trigger_uuid): # This was just a validation request, we don't need to build anything return make_response('Okay') - pull_credentials = model.get_pull_credentials(trigger) + pull_robot_name = model.get_pull_robot_name(trigger) repo = model.get_repository(namespace, repository) start_build(repo, dockerfile_id, tags, name, subdir, False, trigger, - pull_credentials=pull_credentials) + pull_robot_name=pull_robot_name) return make_response('Okay') diff --git a/initdb.py b/initdb.py index edad2f2b1..854f83f8a 100644 --- a/initdb.py +++ b/initdb.py @@ -332,7 +332,7 @@ def populate_database(): token = model.create_access_token(building, 'write') trigger = model.create_build_trigger(building, 'github', '123authtoken', - new_user_1, pull_user = dtrobot) + new_user_1, pull_robot=dtrobot[0]) trigger.config = json.dumps({ 'build_source': 'jakedt/testconnect', 'subdir': '', diff --git a/static/js/controllers.js b/static/js/controllers.js index e907398eb..08bcca561 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -962,9 +962,13 @@ function RepoBuildCtrl($scope, Restangular, ApiService, $routeParams, $rootScope var data = { 'file_id': build['resource_key'], - 'subdirectory': subdirectory + 'subdirectory': subdirectory, }; + if (build['pull_robot']) { + data['pull_robot'] = build['pull_robot']['name']; + } + var params = { 'repository': namespace + '/' + name }; @@ -1486,7 +1490,8 @@ function RepoAdminCtrl($scope, Restangular, ApiService, KeyService, $routeParams } ApiService.activateBuildTrigger(data, params).then(function(resp) { - trigger['is_active'] = true; + trigger['is_active'] = true; + trigger['pull_robot'] = resp['pull_robot']; }, function(resp) { $scope.triggers.splice($scope.triggers.indexOf(trigger), 1); bootbox.dialog({ diff --git a/static/partials/repo-admin.html b/static/partials/repo-admin.html index 172b6d9f8..f1ee224f5 100644 --- a/static/partials/repo-admin.html +++ b/static/partials/repo-admin.html @@ -270,11 +270,11 @@ Setting up trigger
-
+
Pull Credentials: - +
diff --git a/test/data/test.db b/test/data/test.db index b52ec7fe1..a2d065948 100644 Binary files a/test/data/test.db and b/test/data/test.db differ diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 20d216df2..f03a48d87 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -964,7 +964,7 @@ class TestRequestRepoBuild(ApiTestCase): def test_requestrepobuild(self): self.login(ADMIN_ACCESS_USER) - # Ensure where not yet building. + # Ensure we are not yet building. json = self.getJsonResponse(RepositoryBuildList, params=dict(repository=ADMIN_ACCESS_USER + '/simple')) @@ -982,24 +982,20 @@ class TestRequestRepoBuild(ApiTestCase): assert len(json['builds']) > 0 - def test_requestrepobuild_with_credentials(self): + def test_requestrepobuild_with_robot(self): self.login(ADMIN_ACCESS_USER) - # Ensure where not yet building. + # Ensure we are not yet building. json = self.getJsonResponse(RepositoryBuildList, params=dict(repository=ADMIN_ACCESS_USER + '/simple')) assert len(json['builds']) == 0 # Request a (fake) build. - pull_creds = { - 'username': 'foo', - 'password': 'bar', - 'registry': 'baz' - } + pull_robot = ADMIN_ACCESS_USER + '+dtrobot' self.postResponse(RepositoryBuildList, params=dict(repository=ADMIN_ACCESS_USER + '/simple'), - data=dict(file_id='foobarbaz', pull_credentials=pull_creds), + data=dict(file_id='foobarbaz', pull_robot=pull_robot), expected_code=201) # Check for the build. @@ -1007,7 +1003,28 @@ class TestRequestRepoBuild(ApiTestCase): params=dict(repository=ADMIN_ACCESS_USER + '/building')) assert len(json['builds']) > 0 - + + + def test_requestrepobuild_with_invalid_robot(self): + self.login(ADMIN_ACCESS_USER) + + # Request a (fake) build. + pull_robot = ADMIN_ACCESS_USER + '+invalidrobot' + self.postResponse(RepositoryBuildList, + params=dict(repository=ADMIN_ACCESS_USER + '/simple'), + data=dict(file_id='foobarbaz', pull_robot=pull_robot), + expected_code=404) + + def test_requestrepobuild_with_unauthorized_robot(self): + self.login(ADMIN_ACCESS_USER) + + # Request a (fake) build. + pull_robot = 'freshuser+anotherrobot' + self.postResponse(RepositoryBuildList, + params=dict(repository=ADMIN_ACCESS_USER + '/simple'), + data=dict(file_id='foobarbaz', pull_robot=pull_robot), + expected_code=403) + class TestWebhooks(ApiTestCase): @@ -1746,7 +1763,7 @@ class TestBuildTriggers(ApiTestCase): # Verify that the robot was saved. self.assertEquals(True, activate_json['is_active']) - self.assertEquals(ADMIN_ACCESS_USER + '+dtrobot', activate_json['pull_user']['name']) + self.assertEquals(ADMIN_ACCESS_USER + '+dtrobot', activate_json['pull_robot']['name']) # Start a manual build. start_json = self.postJsonResponse(ActivateBuildTrigger, diff --git a/util/names.py b/util/names.py index b705bec36..57fafdd10 100644 --- a/util/names.py +++ b/util/names.py @@ -26,4 +26,7 @@ def format_robot_username(parent_username, robot_shortname): return '%s+%s' % (parent_username, robot_shortname) def parse_robot_username(robot_username): + if not '+' in robot_username: + return None + return robot_username.split('+', 2) diff --git a/workers/dockerfilebuild.py b/workers/dockerfilebuild.py index 6e8be1f4d..9d552a4ae 100644 --- a/workers/dockerfilebuild.py +++ b/workers/dockerfilebuild.py @@ -352,13 +352,14 @@ class DockerfileBuildWorker(Worker): job_details['repository'], job_details['build_uuid']) + pull_credentials = job_details.get('pull_credentials', None) + job_config = json.loads(repository_build.job_config) resource_url = user_files.get_file_url(repository_build.resource_key) tag_names = job_config['docker_tags'] build_subdir = job_config['build_subdir'] repo = job_config['repository'] - pull_credentials = job_config.get('pull_credentials', None) access_token = repository_build.access_token.code