Closed Bug 1490832 Opened 6 years ago Closed 6 years ago

Show "Trigger {missing jobs, all talos jobs}" to all users, not just is_staff

Categories

(Tree Management :: Treeherder, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: emorley)

References

Details

Attachments

(1 file)

There was some confusion earlier on IRC because one of the new sheriffs was unable to use the "Trigger missing jobs" feature. It turned out they did not have the `is_staff` bit set in their Django user, causing Treeherder to hide the UI elements: https://github.com/mozilla/treeherder/blob/829402cf45df3063b477a7472ef84edf7cf40d77/ui/job-view/pushes/PushActionMenu.jsx#L144-L155 However I don't think this is ideal, since: * having the feature be hidden completely (rather than say greyed out) causes a confusing user experience * the feature uses taskcluster actions to trigger the jobs, so (a) treeherder's is_staff check can be easily circumvented, (b) taskcluster scopes linked to LDAP groups seem like a much more appropriate place for this control (and would reduce the number of places permissions need updating for sheriff onboarding) As such, should we just always show those links, and leave it to taskcluster to control who can use the feature? Thoughts?
Component: Treeherder: Frontend → Treeherder: Job Triggering & Cancellation
And I'm presuming the reason access was limited was to prevent people using too many resources?
Always showing them seems a good solution to me.
Attachment #9009071 - Flags: review?(helfi92)
Oops forgot I hadn't finished that message before pressing submit. It was supposed to say something about the initial implementation of these features using pulse_actions via Treeherder's API, which (a) didn't have fine-grained permissions controls, (b) meant the requests weren't being made client side. Whereas now taskcluster actions does have fine-grained permissions via scopes, and the requests are made client side, so the Treeherder check is purely aesthetic, so less useful.
Commit pushed to master at https://github.com/mozilla/treeherder https://github.com/mozilla/treeherder/commit/6101c7f3f6ab39bcb7dd16d9fe0b8ffd91830999 Bug 1490832 - Show trigger missing/talos jobs UI to all users (#4029) ...rather than just to users with the `is_staff` Django permission (which mostly maps to sheriffs). This prevents the confusing UX of UI elements being unknowingly hidden to users who are missing the `is_staff` permission. The check existed since these features used to use pulse_actions via Treeherder's API (which had coarser permissions controls), whereas now they use Taskcluster's API client side - so this check is purely aesthetic, since people could just use Taskcluster's API/UIs directly. If certain groups of users should be prevented from accidentally scheduling too many jobs, access should instead be controlled via Taskcluster scopes at the task configuration level.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attachment #9009071 - Flags: review?(helfi92)
Component: Treeherder: Job Triggering & Cancellation → TreeHerder
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: