Skip to content
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

Clean up code around optional permissions #9275

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kevin-CB
Copy link
Contributor

@Kevin-CB Kevin-CB commented May 14, 2024

Some cleanup around optional permissions:

  • Deprecate helper methods and have them check real state, not system property
  • Remove undocumented artifactList attribute permission since only Run#ARTIFACTS is supported
  • On the artifacts-index view, the permission check was done outside of l:layout, which is what defines the h variable. So, this permission check was not effective (note that it's not a vulnerability as the Run#ARTIFACTS is not there to restrict the listing of artifacts but the access/download to them, see Clarify artifacts permission description #9276) .

Testing done

I only tested the artifacts-index view as other changes are minimal.

It can be reached by navigating to the endpoint /job/myJob/1/artifacts-index and require the Run#ARTIFACTS permission to be enabled (Run.ARTIFACTS.enabled = true on the script console)

Before:
image

After:
image

Proposed changelog entries

N/A

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@daniel-beck

Before the changes are marked as ready-for-merge:

Maintainer checklist

@NotMyFault NotMyFault added the skip-changelog Should not be shown in the changelog label May 18, 2024
@MarkEWaite MarkEWaite added developer Changes which impact plugin developers bug For changelog: Minor bug. Will be listed after features and removed developer Changes which impact plugin developers labels May 18, 2024
Copy link
Contributor

github-actions bot commented Sep 3, 2024

Please take a moment and address the merge conflicts of your pull request. Thanks!

@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Minor bug. Will be listed after features skip-changelog Should not be shown in the changelog unresolved-merge-conflict There is a merge conflict with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants