Change to store the pull robot on the repository build and only add the credentials to the queue item. This prevents the credentials from being exposed to the end user. Also fixes the restart build option

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):

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
 
 

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,14 +35,15 @@ def get_job_config(build_obj):
     return None
 
 
-def trigger_view(trigger):
+def user_view(user):
-  def user_view(user):
   return {
     'name': user.username,
     'kind': 'user',
     'is_robot': user.robot,
   }
 
+def trigger_view(trigger):
+
   if trigger and trigger.uuid:  
     config_dict = get_trigger_config(trigger)
     build_trigger = BuildTrigger.get_trigger_for_service(trigger.service.name)
@@ -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': {
+        'pull_robot': {
-          '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'
+          'description': 'Username of a Quay robot account to use as pull credentials',
-            },
-            'password': {
-              'type': 'string',
-              'description': 'The password for the pull'
-            },
-            'registry': {
-              'type': 'string',
-              'description': 'The registry hostname for the pull'
-            },
-          }
         }
       },
     },
@@ -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)

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)

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,
+    'build_subdir': subdir
-    'pull_credentials': pull_credentials,
   }
 
   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 = {

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')
 

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': '',

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
     };
@@ -1487,6 +1491,7 @@ function RepoAdminCtrl($scope, Restangular, ApiService, KeyService, $routeParams
 
     ApiService.activateBuildTrigger(data, params).then(function(resp) {
       trigger['is_active'] = true;
+      trigger['pull_robot'] = resp['pull_robot'];
     }, function(resp) {
       $scope.triggers.splice($scope.triggers.indexOf(trigger), 1);
       bootbox.dialog({

static/partials/repo-admin.html

@@ -270,11 +270,11 @@
                           Setting up trigger
                         </div>
                         <div ng-show="trigger.is_active" class="trigger-description" trigger="trigger"></div>
-                        <div class="trigger-pull-credentials" ng-if="trigger.is_active && trigger.pull_user">
+                        <div class="trigger-pull-credentials" ng-if="trigger.is_active && trigger.pull_robot">
                           <span class="context-tooltip" title="The credentials used by the builder when pulling images" bs-tooltip>
                             Pull Credentials:
                           </span>
-                          <span class="entity-reference" entity="trigger.pull_user"></span>
+                          <span class="entity-reference" entity="trigger.pull_robot"></span>
                         </div>
                       </td>
                       <td style="white-space: nowrap;">

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 = {
+    pull_robot = ADMIN_ACCESS_USER + '+dtrobot'
-      'username': 'foo',
-      'password': 'bar',
-      'registry': 'baz'
-    }
     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.
@@ -1009,6 +1005,27 @@ class TestRequestRepoBuild(ApiTestCase):
     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):
   def test_webhooks(self):
@@ -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,

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)

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