Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-27134

Permission for input approval, or choice of Jenkins-specific group as submitter

    Details

    • Similar Issues:

      Description

      Currently the input step allows you to specify a submitter, which may be a user ID or an external group ("granted authority"). This does not work well with authorization strategies, especially those that allow you to group together users inside Jenkins, such as (but not limited to) nectar-rbac in Jenkins Enterprise by CloudBees.

      Arguably the more natural approach would be for InputStep to define a new permission, "approve flow", which would be checked on the WorkflowRun (can use PermissionScope.RUN). Thus rather than specifying a submitter for a particular step you would just ensure there was an RBAC group defined on the flow job (or its folder, etc.) which granted that permission and included the people you want. Then no reference need be made to any particular AuthorizationStrategy at all; this behavior would just follow from the Jenkins authorization model.

      If defined, submitter would still be consulted in case the approving user lacked the new permission. There is a potential compatibility issue in case submitter was not defined; today that means that any user (even anonymous) can approve the flow, whereas we would want the new permission to be enforced. Unfortunately Jenkins offers no way to mark a newly introduced Permission as "granted until explicitly denied", except via system property hacks; see comment in JENKINS-17200. Lacking that, the only option is to break compatibility and say that existing input steps without submitter will now reject random users from approving. The new permission could be implied by, for example, Item.CONFIGURE to minimize the impact—approval by the person who wrote the flow would still be guaranteed to work.

        Attachments

          Issue Links

            Activity

            jglick Jesse Glick created issue -
            jglick Jesse Glick made changes -
            Field Original Value New Value
            Link This issue is related to JENKINS-17200 [ JENKINS-17200 ]
            jglick Jesse Glick made changes -
            Link This issue is related to JENKINS-29346 [ JENKINS-29346 ]
            Hide
            jglick Jesse Glick added a comment -

            Would probably subsume JENKINS-29346.

            Show
            jglick Jesse Glick added a comment - Would probably subsume JENKINS-29346 .
            jglick Jesse Glick made changes -
            Link This issue is duplicated by JENKINS-31425 [ JENKINS-31425 ]
            jglick Jesse Glick made changes -
            Link This issue is related to JENKINS-31425 [ JENKINS-31425 ]
            jglick Jesse Glick made changes -
            Link This issue is duplicated by JENKINS-31425 [ JENKINS-31425 ]
            Hide
            jglick Jesse Glick added a comment -

            Arguably the more natural approach

            The counterargument has been made that you may have a single Pipeline job which needs to ask different groups of people for permission to proceed at different points (multiple input steps), so a per-job permission setting would not suffice. Instead we would need to have an API which an AuthorizationStrategy could use to instruct a given input step on who should be able to submit. TBD what such an API would look like; possibilities include

            • an ExtensionPoint for the strategy to specify whether a given user (typically, the current Jenkins.getAuthentication) is a member of a named virtual group (such as a CJE RBAC local group) in a given context (say, the Job); thus you could include a virtual group in the submitters attribute
            • an ExtensionPoint for the strategy to specify whether a given Authentication is eligible to approve a given InputStepExecution, allowing it to inspect id, submitters, etc. according to any policy it likes

            The first is actually general enough that it might make sense as a core API in AuthorizationStrategy.

            Show
            jglick Jesse Glick added a comment - Arguably the more natural approach The counterargument has been made that you may have a single Pipeline job which needs to ask different groups of people for permission to proceed at different points (multiple input steps), so a per-job permission setting would not suffice. Instead we would need to have an API which an AuthorizationStrategy could use to instruct a given input step on who should be able to submit. TBD what such an API would look like; possibilities include an ExtensionPoint for the strategy to specify whether a given user (typically, the current Jenkins.getAuthentication ) is a member of a named virtual group (such as a CJE RBAC local group) in a given context (say, the Job ); thus you could include a virtual group in the submitters attribute an ExtensionPoint for the strategy to specify whether a given Authentication is eligible to approve a given InputStepExecution , allowing it to inspect id , submitters , etc. according to any policy it likes The first is actually general enough that it might make sense as a core API in AuthorizationStrategy .
            jglick Jesse Glick made changes -
            Labels permissions api permissions
            jglick Jesse Glick made changes -
            Summary Permission for input approval Permission for input approval, or choice of Jenkins-specific group as submitter
            hrmpw Patrick Wolf made changes -
            Labels api permissions api followup permissions
            rtyler R. Tyler Croy made changes -
            Workflow JNJira [ 161330 ] JNJira + In-Review [ 180657 ]
            danielbeck Daniel Beck made changes -
            Labels api followup permissions api followup new-permission permissions
            abayer Andrew Bayer made changes -
            Component/s pipeline-general [ 21692 ]
            abayer Andrew Bayer made changes -
            Component/s workflow-plugin [ 18820 ]
            jglick Jesse Glick made changes -
            Component/s pipeline-input-step-plugin [ 21708 ]
            Component/s pipeline [ 21692 ]
            Hide
            uhp UHP added a comment -

            Is this also connected to the Role Strategy Plugin?
            Or is it already possible to use a role defined with the Role Strategy Plugin as submitter?

            Show
            uhp UHP added a comment - Is this also connected to the Role Strategy Plugin ? Or is it already possible to use a role defined with the Role Strategy Plugin as submitter ?
            Hide
            jglick Jesse Glick added a comment -

            If either of the extension points I proposed in my last comment were defined, then the Role Strategy plugin could in principle implement it. A CloudBees employee working on this would presumably focus on implementing it in the RBAC plugin that is included in CJP, though it would not be a bad idea to try two implementations to vet the API design for poor assumptions.

            I would tend to prefer the first one as it is more generic. The practical difficulty is that actually defining it in core would prevent uptake from plugins (both pipeline-input-step, the caller, and implementers such as nectar-rbac or role-strategy) for months after this was merged, unless we use some tricks such as commenting out @Override in the implementations and having the caller look it up reflectively pending a newer core dep.

            Show
            jglick Jesse Glick added a comment - If either of the extension points I proposed in my last comment were defined, then the Role Strategy plugin could in principle implement it. A CloudBees employee working on this would presumably focus on implementing it in the RBAC plugin that is included in CJP, though it would not be a bad idea to try two implementations to vet the API design for poor assumptions. I would tend to prefer the first one as it is more generic. The practical difficulty is that actually defining it in core would prevent uptake from plugins (both pipeline-input-step , the caller, and implementers such as nectar-rbac or role-strategy ) for months after this was merged, unless we use some tricks such as commenting out @Override in the implementations and having the caller look it up reflectively pending a newer core dep.
            Hide
            sag47 Sam Gleske added a comment - - edited

            -1 vote

            This proposal is not good for how my team works. We don't want one person or a group of people to approve all parts of the pipeline. There's a whole approval process at my place of work (like Dev deployer, QA teams deploying, and an entire separate permission for production deployments). The current behavior is good enough. I don't see any advantage as to why you'd want this to be a specific permission unless you plan on creating a multi-permission structure for each individual input step (which to me sounds like overkill compared to the current behavior).

            My team does not use RBAC in Jenkins Enterprise. How would this affect other authorization strategies?

            Show
            sag47 Sam Gleske added a comment - - edited -1 vote This proposal is not good for how my team works. We don't want one person or a group of people to approve all parts of the pipeline. There's a whole approval process at my place of work (like Dev deployer, QA teams deploying, and an entire separate permission for production deployments). The current behavior is good enough. I don't see any advantage as to why you'd want this to be a specific permission unless you plan on creating a multi-permission structure for each individual input step (which to me sounds like overkill compared to the current behavior). My team does not use RBAC in Jenkins Enterprise. How would this affect other authorization strategies?
            Hide
            jglick Jesse Glick added a comment -

            I don't see any advantage as to why you'd want this to be a specific permission

            Nor is that currently being proposed. The proposal is simply to extend the permitted values of “submitter” to include not just user IDs and external (e.g., LDAP) groups, but also “Jenkins-local” groups defined by any authorization strategy implementing a new SPI.

            The current behavior is good enough.

            Great, then you need not worry.

            Show
            jglick Jesse Glick added a comment - I don't see any advantage as to why you'd want this to be a specific permission Nor is that currently being proposed. The proposal is simply to extend the permitted values of “submitter” to include not just user IDs and external (e.g., LDAP) groups, but also “Jenkins-local” groups defined by any authorization strategy implementing a new SPI. The current behavior is good enough. Great, then you need not worry.
            Hide
            jglick Jesse Glick added a comment -

            actually defining it in core would prevent uptake from plugins

            Stephen Connolly proposes some mechanism TBD whereby the API could be defined in core for the long term, with a copy in some plugin permitting it to be used in the near term without a new core dependency. This has been done in the past for certain other APIs, though it can be tricky depending on the case.

            Show
            jglick Jesse Glick added a comment - actually defining it in core would prevent uptake from plugins Stephen Connolly proposes some mechanism TBD whereby the API could be defined in core for the long term, with a copy in some plugin permitting it to be used in the near term without a new core dependency. This has been done in the past for certain other APIs, though it can be tricky depending on the case.
            kshultz Karl Shultz made changes -
            Attachment screenshot-1.png [ 37563 ]
            kshultz Karl Shultz made changes -
            Attachment inputStepVideo.webm [ 37564 ]
            Hide
            kshultz Karl Shultz added a comment - - edited

            This can look like a pretty bad bug if you don't know about it. Because the input step appears to ignore Jenkins-level permissions, but still allows you to use them in the definition of the step, this can hang an entire build.

            inputStepVideo.webm

            Show
            kshultz Karl Shultz added a comment - - edited This can look like a pretty bad bug if you don't know about it. Because the input step appears to ignore Jenkins-level permissions, but still allows you to use them in the definition of the step, this can hang an entire build. inputStepVideo.webm
            Hide
            jglick Jesse Glick added a comment -

            Karl Shultz I am not sure what that video is showing. What is this “put your admin password please”? Not a message I recognize. Steps to reproduce from scratch?

            Show
            jglick Jesse Glick added a comment - Karl Shultz I am not sure what that video is showing. What is this “put your admin password please”? Not a message I recognize. Steps to reproduce from scratch?
            Hide
            kshultz Karl Shultz added a comment - - edited

            There are two ways to show this. the first is to demonstrate that the password provided for a valid Jenkins user seems to get ignored.

            1. Create a Pipeline job, using the following script:

            stage ('inputStage') {
                input id: 'CustomID', 
                message: 'Message Body', 
                ok: 'OK', 
                parameters: [password(defaultValue: 'admin', description: 'Password Parameter', name: 'admin')], 
                // Below in the submitter field I've got the correct 
                // username of admin. But I can provide any string 
                // as the password for admin, and the pipeline will 
                // move on to the next stage.
                submitter: 'admin', 
                submitterParameter: 'approvingSubmitter'
            }
            
            stage ('After Password') {
                echo 'I should only see this if I put in the right password'
            }

            2. Build this job.

            3. Enter an invalid password for the user supplied in 'submitter' which in this case is 'admin'

            4. That stage of the pipeline will complete, and you'll move on to the next stage, even though the password you provided is known to be wrong.

            The second problem to demonstrate is by setting the 'submitter' setting to something that's not a real Jenkins user, like "nonsenseThing" in the below:

            stage ('inputStage') {
                input id: 'CustomID', 
                message: 'Message Body', 
                ok: 'OK', 
                parameters: [password(defaultValue: 'admin', description: 'Password Parameter', name: 'admin')], 
                // Below in the submitter field I've got a bogus 
                // user name of nonsenseThing set. When I try it this 
                // way, the job will hang.
                submitter: 'nonsenseThing', 
                submitterParameter: 'approvingSubmitter'
            }
            
            stage ('After Password') {
                echo 'I should only see this if I put in the right password'
            }
            
            Show
            kshultz Karl Shultz added a comment - - edited There are two ways to show this. the first is to demonstrate that the password provided for a valid Jenkins user seems to get ignored. 1. Create a Pipeline job, using the following script: stage ('inputStage') { input id: 'CustomID', message: 'Message Body', ok: 'OK', parameters: [password(defaultValue: 'admin', description: 'Password Parameter', name: 'admin')], // Below in the submitter field I've got the correct // username of admin. But I can provide any string // as the password for admin, and the pipeline will // move on to the next stage. submitter: 'admin', submitterParameter: 'approvingSubmitter' } stage ('After Password') { echo 'I should only see this if I put in the right password' } 2. Build this job. 3. Enter an invalid password for the user supplied in 'submitter' which in this case is 'admin' 4. That stage of the pipeline will complete, and you'll move on to the next stage, even though the password you provided is known to be wrong. The second problem to demonstrate is by setting the 'submitter' setting to something that's not a real Jenkins user, like "nonsenseThing" in the below: stage ('inputStage') { input id: 'CustomID', message: 'Message Body', ok: 'OK', parameters: [password(defaultValue: 'admin', description: 'Password Parameter', name: 'admin')], // Below in the submitter field I've got a bogus // user name of nonsenseThing set. When I try it this // way, the job will hang. submitter: 'nonsenseThing', submitterParameter: 'approvingSubmitter' } stage ('After Password') { echo 'I should only see this if I put in the right password' }
            Hide
            jglick Jesse Glick added a comment -

            The first script makes no sense. You asked for some text from a password field, and you got it, and then threw away the return value. Nothing to do with Jenkins user accounts. There is no Pipeline feature to ask a user to log in with a password. Nor should there be: if they can get to the input request page, clearly they are already logged in.

            The second script does not demonstrate a bug that I can see, or perhaps I am not understanding your complaint. You requested that the input only be approved by a user named nonsenseThing or who is in a group by that name. Since the user attempting to submit is neither, Jenkins refuses to accept the submission.

            Again, this issue is about an RFE: being able to use something like a role or other Jenkins-defined group, rather than an external (e.g., LDAP) group, in the submitter parameter. Anything else you are trying to do is probably unrelated.

            Show
            jglick Jesse Glick added a comment - The first script makes no sense. You asked for some text from a password field, and you got it, and then threw away the return value. Nothing to do with Jenkins user accounts. There is no Pipeline feature to ask a user to log in with a password. Nor should there be: if they can get to the input request page, clearly they are already logged in. The second script does not demonstrate a bug that I can see, or perhaps I am not understanding your complaint. You requested that the input only be approved by a user named nonsenseThing or who is in a group by that name. Since the user attempting to submit is neither, Jenkins refuses to accept the submission. Again, this issue is about an RFE: being able to use something like a role or other Jenkins-defined group, rather than an external (e.g., LDAP) group, in the submitter parameter. Anything else you are trying to do is probably unrelated.
            Hide
            svanoort Sam Van Oort added a comment -

            Jesse Glick I explicitly asked Karl Shultz to leave a note here about some oddities he saw when testing my features – I think maybe there may be some confusion here, but the crux of the matter is that even admin permissions aren't enough to grant approval for an input step (even though you can rewrite the job totally with those permissions). 

            Feels like there was some miscommunication here, but in general this is just another facet of the feature request – adding integration of Jenkins permissions and roles into the input step approval process.

            Show
            svanoort Sam Van Oort added a comment - Jesse Glick I explicitly asked Karl Shultz to leave a note here about some oddities he saw when testing my features – I think maybe there may be some confusion here, but the crux of the matter is that even admin permissions aren't enough to grant approval for an input step (even though you can rewrite the job totally with those permissions).  Feels like there was some miscommunication here, but in general this is just another facet of the feature request – adding integration of Jenkins permissions and roles into the input step approval process.
            Hide
            jglick Jesse Glick added a comment -

            Better discussed in JENKINS-29346.

            Show
            jglick Jesse Glick added a comment - Better discussed in  JENKINS-29346 .
            Hide
            saikrishna sai krishna added a comment - - edited

            Karl Shultz I am looking for this one. Thank you for your information. But how can we validate the password of that particular user ? and what about the abort restrictions ?

            Show
            saikrishna sai krishna added a comment - - edited Karl Shultz I am looking for this one. Thank you for your information. But how can we validate the password of that particular user ? and what about the abort restrictions ?
            Hide
            jglick Jesse Glick added a comment -

            sai krishna sounds like questions for the Jenkins user list.

            Show
            jglick Jesse Glick added a comment - sai krishna sounds like questions for the Jenkins user list.
            jamesdumay James Dumay made changes -
            Labels api followup new-permission permissions api blueocean followup new-permission permissions
            michaelneale Michael Neale made changes -
            Priority Minor [ 4 ] Major [ 3 ]
            cloudbees CloudBees Inc. made changes -
            Remote Link This issue links to "CloudBees Internal CD-73 (Web Link)" [ 18976 ]
            Hide
            virasana Jean-Pierre Fouche added a comment - - edited

            Please would you be able to address the issue with RBAC stated in the description?

            i.e. 

            "Currently the input step allows you to specify a submitter, which may be a user ID or an external group ("granted authority"). This does not work well with authorization strategies, especially those that allow you to group together users inside Jenkins, such as (but not limited to) nectar-rbac in Jenkins Enterprise by CloudBees."

            I find that the 'submitter' attribute does not work on the input step.  We are using Keycloak role-based AuthorizationStrategy.  (Our code for the input step has not changed, but we recently changed Jenkins setup from a matrix based authorisation strategy to Keycloak).

             

            Code below.  Expected result is that if there is a user in  Keycloak, it should verify that the logged in user matches.  Similarly, if there is a group in Keycloak, the logged in user should be a member of the specified group.

            isApproved = input(
                    id: 'someId',
                    message: 'Approve?',
                    submitter: 'someuser', // <== 'does not query Keycloak; ignores this 
                     parameters: [choice(
                            choices: ['No', 'Yes'],
                            description: 'some description',
                            name: 'some name')]
            ) == 'Yes'
            

             

             

            Show
            virasana Jean-Pierre Fouche added a comment - - edited Please would you be able to address the issue with RBAC stated in the description? i.e.  "Currently the input step allows you to specify a submitter, which may be a user ID or an external group (" granted authority "). This does not work well with authorization strategies, especially those that allow you to group together users inside Jenkins, such as (but not limited to) nectar-rbac in Jenkins Enterprise by CloudBees." I find that the 'submitter' attribute does not work on the input step.  We are using Keycloak role-based AuthorizationStrategy.  (Our code for the input step has not changed, but we recently changed Jenkins setup from a matrix based authorisation strategy to Keycloak).   Code below.  Expected result is that if there is a user in  Keycloak, it should verify that the logged in user matches.  Similarly, if there is a group in Keycloak, the logged in user should be a member of the specified group. isApproved = input( id: 'someId' , message: 'Approve?' , submitter: 'someuser' , // <== 'does not query Keycloak; ignores this parameters: [choice( choices: [ 'No' , 'Yes' ], description: 'some description' , name: 'some name' )] ) == 'Yes'    

              People

              • Assignee:
                jglick Jesse Glick
                Reporter:
                jglick Jesse Glick
              • Votes:
                14 Vote for this issue
                Watchers:
                23 Start watching this issue

                Dates

                • Created:
                  Updated: