summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDan Callaghan <dcallagh@redhat.com>2017-10-05 17:01:34 +1000
committerDan Callaghan <dcallagh@redhat.com>2017-10-06 07:08:46 +0000
commit1bd7e775bd34ad774278d803d0db050d44eaa449 (patch)
tree443f25e6a560e77368ab5476b4ce15c96e4004a8
parenta6a5fa71bf51cb3766694c09c6cb7c595e2634d5 (diff)
fix allowed_priorities() to use proper permission check
This method was not aware of the newer "group job" permissions, since it wasn't calling through can_change_priority(). And it was also missing a base case so it was falling through to return None, causing a confusing exception in the caller. Bug: 1497021 Change-Id: I8fea805e9d83d3debec5a4b74f7ad9ad2864f511
-rw-r--r--IntegrationTests/src/bkr/inttest/server/selenium/test_jobs.py18
-rw-r--r--Server/bkr/server/model/scheduler.py5
-rw-r--r--Server/bkr/server/recipesets.py11
3 files changed, 29 insertions, 5 deletions
diff --git a/IntegrationTests/src/bkr/inttest/server/selenium/test_jobs.py b/IntegrationTests/src/bkr/inttest/server/selenium/test_jobs.py
index c2400ce..299913d 100644
--- a/IntegrationTests/src/bkr/inttest/server/selenium/test_jobs.py
+++ b/IntegrationTests/src/bkr/inttest/server/selenium/test_jobs.py
@@ -2010,6 +2010,24 @@ class RecipeSetHTTPTest(DatabaseTestCase):
session.expire_all()
self.check_changed_recipeset()
+ # https://bugzilla.redhat.com/show_bug.cgi?id=1497021
+ def test_group_member_can_reduce_group_job_priority_by_tid(self):
+ with session.begin():
+ group = data_setup.create_group()
+ group_member = data_setup.create_user(password=u'member')
+ group.add_member(group_member)
+ self.job.group = group
+ s = requests.Session()
+ requests_login(s, user=group_member, password=u'member')
+ response = patch_json(get_server_base() +
+ 'recipesets/by-taskspec/%s' % self.job.t_id,
+ session=s, data={'priority': u'Low'})
+ response.raise_for_status()
+ with session.begin():
+ session.expire_all()
+ recipeset = self.job.recipesets[0]
+ self.assertEquals(recipeset.priority, TaskPriority.low)
+
def test_update_containing_no_changes_should_silently_do_nothing(self):
# PATCH request containing attributes with their existing values
# should succeed and do nothing, including adding no activity records.
diff --git a/Server/bkr/server/model/scheduler.py b/Server/bkr/server/model/scheduler.py
index 3577abb..cd06534 100644
--- a/Server/bkr/server/model/scheduler.py
+++ b/Server/bkr/server/model/scheduler.py
@@ -1916,9 +1916,12 @@ class RecipeSet(TaskBase, DeclarativeMappedObject, ActivityMixin):
return []
if user.in_group(['admin','queue_admin']):
return [pri for pri in TaskPriority]
- elif user == self.job.owner:
+ elif self.can_change_priority(user):
+ # Normal users are only allowed to *reduce* the priority,
+ # to prevent unfair queue jumping.
return [pri for pri in TaskPriority
if TaskPriority.index(pri) <= TaskPriority.index(self.priority)]
+ return []
def cancel_link(self):
""" return link to cancel this recipe
diff --git a/Server/bkr/server/recipesets.py b/Server/bkr/server/recipesets.py
index 392ada3..deb3ba1 100644
--- a/Server/bkr/server/recipesets.py
+++ b/Server/bkr/server/recipesets.py
@@ -49,10 +49,13 @@ def _update_recipeset(recipeset, data=None):
if 'priority' in data:
priority = TaskPriority.from_string(data['priority'])
if priority != recipeset.priority:
- if (not recipeset.can_change_priority(identity.current.user) or
- priority not in recipeset.allowed_priorities(identity.current.user)):
- raise Forbidden403('Cannot set recipe set %s priority to %s'
- % (recipeset.id, priority))
+ if not recipeset.can_change_priority(identity.current.user):
+ raise Forbidden403('Cannot change recipe set %s priority' % recipeset.id)
+ allowed = recipeset.allowed_priorities(identity.current.user)
+ if priority not in allowed:
+ raise Forbidden403('Cannot set recipe set %s priority to %s, '
+ 'permitted priorities are: %s'
+ % (recipeset.id, priority, ' '.join(unicode(pri) for pri in allowed)))
record_activity(u'Priority', old=recipeset.priority.value, new=priority.value)
recipeset.priority = priority
if 'waived' in data: