From 3f2447d831c9a056f2b167ef9e5e6ba5f63a5fec Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 9 Sep 2016 16:54:46 -0400 Subject: [PATCH 1/2] Make the frontend agnostic to why a trigger can be run manually --- endpoints/api/build.py | 2 +- static/directives/dockerfile-build-dialog.html | 4 ++-- static/directives/repo-view/repo-panel-builds.html | 2 +- static/js/directives/repo-view/repo-panel-builds.js | 5 ++--- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/endpoints/api/build.py b/endpoints/api/build.py index 75be9ffcc..665701f52 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -76,7 +76,7 @@ def trigger_view(trigger, can_read=False, can_admin=False, for_build=False): 'repository_url': repo_url if can_read else None, 'config': build_trigger.config if can_admin else {}, - 'is_connected_user': is_connected_user, + 'can_invoke': is_connected_user, } if not for_build and can_admin and trigger.pull_robot: diff --git a/static/directives/dockerfile-build-dialog.html b/static/directives/dockerfile-build-dialog.html index 6ec834cfd..6a8396f55 100644 --- a/static/directives/dockerfile-build-dialog.html +++ b/static/directives/dockerfile-build-dialog.html @@ -40,8 +40,8 @@ - - You cannot start triggers created by another user + + You do not have permission to run this trigger diff --git a/static/directives/repo-view/repo-panel-builds.html b/static/directives/repo-view/repo-panel-builds.html index 6ae3e68d7..6dbc8876d 100644 --- a/static/directives/repo-view/repo-panel-builds.html +++ b/static/directives/repo-view/repo-panel-builds.html @@ -147,7 +147,7 @@ View Credentials + ng-class="trigger.can_invoke ? '' : 'disabled'"> Run Trigger Now diff --git a/static/js/directives/repo-view/repo-panel-builds.js b/static/js/directives/repo-view/repo-panel-builds.js index e403e177a..9bc017631 100644 --- a/static/js/directives/repo-view/repo-panel-builds.js +++ b/static/js/directives/repo-view/repo-panel-builds.js @@ -199,9 +199,8 @@ angular.module('quay').directive('repoPanelBuilds', function () { }; $scope.askRunTrigger = function(trigger) { - if (!trigger.is_connected_user) { - bootbox.alert('For security reasons, only the user that created this trigger can ' + - 'manually invoke this trigger'); + if (!trigger.can_invoke) { + bootbox.alert('You do not have permission to manually invoke this trigger'); return; } From bda0311dbec489c30e594630318c405a18ff580b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 9 Sep 2016 17:21:14 -0400 Subject: [PATCH 2/2] Allow build triggers to be invoked by any repo admin Fixes #1079 --- endpoints/api/build.py | 12 ++---------- endpoints/api/trigger.py | 7 +++---- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/endpoints/api/build.py b/endpoints/api/build.py index 665701f52..cac1fdea1 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -19,7 +19,6 @@ from endpoints.exception import Unauthorized, NotFound, InvalidRequest from endpoints.building import start_build, PreparedBuild from data import database from data import model -from auth.auth_context import get_authenticated_user from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermission, AdministerRepositoryPermission, AdministerOrganizationPermission, SuperUserPermission) @@ -58,14 +57,7 @@ def trigger_view(trigger, can_read=False, can_admin=False, for_build=False): build_source = build_trigger.config.get('build_source') repo_url = build_trigger.get_repository_url() if build_source else None - - if can_admin: - can_read = True - - is_connected_user = False - if (can_admin and get_authenticated_user() and - trigger.connected_user_id == get_authenticated_user().id): - is_connected_user = True + can_read = can_read or can_admin trigger_data = { 'id': trigger.uuid, @@ -76,7 +68,7 @@ def trigger_view(trigger, can_read=False, can_admin=False, for_build=False): 'repository_url': repo_url if can_read else None, 'config': build_trigger.config if can_admin else {}, - 'can_invoke': is_connected_user, + 'can_invoke': can_admin, } if not for_build and can_admin and trigger.pull_robot: diff --git a/endpoints/api/trigger.py b/endpoints/api/trigger.py index d9d6a4b02..da9c98087 100644 --- a/endpoints/api/trigger.py +++ b/endpoints/api/trigger.py @@ -21,7 +21,7 @@ from endpoints.api.build import build_status_view, trigger_view, RepositoryBuild from endpoints.building import start_build from data import model from auth.permissions import (UserAdminPermission, AdministerOrganizationPermission, - ReadRepositoryPermission) + ReadRepositoryPermission, AdministerRepositoryPermission) from util.names import parse_robot_username from util.dockerfileparse import parse_dockerfile @@ -194,7 +194,7 @@ class BuildTriggerActivate(RepositoryParamResource): raise NotFound() # Make sure the user has administer permissions for the robot's namespace. - (robot_namespace, shortname) = parse_robot_username(pull_robot_name) + (robot_namespace, _) = parse_robot_username(pull_robot_name) if not AdministerOrganizationPermission(robot_namespace).can(): raise Unauthorized() @@ -480,8 +480,7 @@ class BuildTriggerFieldValues(RepositoryParamResource): raise NotFound() config = request.get_json() or None - user_permission = UserAdminPermission(trigger.connected_user.username) - if user_permission.can(): + if AdministerRepositoryPermission(namespace_name, repo_name).can(): handler = BuildTriggerHandler.get_handler(trigger, config) values = handler.list_field_values(field_name, limit=FIELD_VALUE_LIMIT)