[JENKINS-62054] Ensure the Support action is not displayed if the use… #220
Conversation
…r does not have the proper rights
There was a problem hiding this comment.
I agree that there is a problem to fix.
https://ci.jenkins.io/job/Plugins/job/shelve-project-plugin/job/master/29/support/ shouldn't propose these actions when anonymous (and without the CREATE_BUNDLE permission
Not sure if the approach you are using here is the right one. I never saw it and I am not sure why the actions permissions aren't hiding them:
| } | ||
|
|
||
| boolean shouldDisplay() { | ||
| return Jenkins.get().hasPermission(Jenkins.ADMINISTER); |
There was a problem hiding this comment.
Shouldn't you use SupportAction.CREATE_BUNDLE instead ?
There was a problem hiding this comment.
There was a problem hiding this comment.
Yeah sorry link was bad, I meant:
This is an interesting point, because SupportAction.CREATE_BUNDLE defines a permission (Download Bundle) but the action I linked checks for ADMINISTER :)
I managed to reproduce the issue:
- have a user with Overall/Read + Job/read + Download Permission
- go to a job page
- click on generate bundle (for the job)
- you won't be allowed to generate the bundle
Question is: is it by design, ie only an admin can retrieve a build directory? Or is it a bug?
If it's a bug, happy to fix it while fixing the action visibility issue.
There was a problem hiding this comment.
Maybe @Dohbedoh can better illustrate us.
| @@ -22,7 +22,7 @@ public SupportAbstractItemAction(AbstractItem target) { | |||
|
|
|||
| @Override | |||
| public String getDisplayName() { | |||
| return Messages.SupportItemAction_DisplayName(); | |||
| return shouldDisplay() ? Messages.SupportItemAction_DisplayName() : null; | |||
There was a problem hiding this comment.
Wouldn't it be possible to implement methods in the super class to avoid having to change every method in the subclasses?
There was a problem hiding this comment.
Functionally what you suggest is possible (all action display names are the same) but they are in fact referring to a different message, eg:
I suppose I could build the key to retrieve the message from the class name, but maybe it's worth considering removing the duplication of messages instead. AFAIU, the name will always be Support and the icon will always be the same too. Wdyt?
There was a problem hiding this comment.
If the user result is the same, +1 for removing the duplication.
There was a problem hiding this comment.
Messages are made unique by #239
|
@PierreBtz I proposed #252 |
|
yeah, my bad for not following up on this... |
…r does not have the proper rights