Skip to content

Fix object permissions enforcement on Job Buttons #4993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 20, 2023

Conversation

glennmatthews
Copy link
Contributor

Closes: #4988 (will also be needed in 1.6.x)

What's Changed

  • The job_buttons template tag wasn't considering object-level permissions when determining whether to enable or disable each JobButton it rendered. I added the appropriate has_perms check.
    • There was a lot of duplicate code and logic in this function between the "ungrouped buttons" and "grouped buttons" cases, so I've extracted it into a helper function to reduce duplication.
  • Similarly, the JobButtonRunView implementation was also not considering object-level permissions. In this case this view is redundant as a JobButton's underlying Job can be invoked just as easily through the /extras/jobs/<id>/run/ view, which is already enforcing permissions correctly. I've therefore removed JobButtonRunView rather than attempt to fix it.
    • To keep the existing behavior where clicking a JobButton redirects back to the current page rather than redirecting to the JobResult page, I've added support for a _return_url parameter to the Job run view.
  • Enforcing extras.run_jobbutton permission on top of extras.run_job permission was redundant, especially since a user with extras.run_job permission alone could still run the underlying job without going through the JobButton. I've therefore removed extras.run_jobbutton as a required permission; extras.run_job suffices.
  • In manual testing, I found that the example plugin ExampleComplexJobButtonReceiver hadn't been completely/correctly updated to work under 2.0, so I fixed it.

TODO

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • n/a Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@gsnider2195 gsnider2195 self-requested a review December 18, 2023 18:19
Copy link
Contributor

@gsnider2195 gsnider2195 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@glennmatthews glennmatthews merged commit 3d964f9 into develop Dec 20, 2023
@glennmatthews glennmatthews deleted the u/glennmatthews-4988-jobbutton-security branch December 20, 2023 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Job and job-button constraints permissions have not been applied
3 participants