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

Approvals are being bypassed for group members but not if group members are approvers #153

Open
dreznicek opened this issue Feb 25, 2019 · 9 comments
Assignees
Labels
bug pinned Prevent closing if stale

Comments

@dreznicek
Copy link
Contributor

Expected Behavior

The expected behavior is:

If user1 is in a group (groupA) and a command requires an approval from user2 in a different group (groupB), then the command when executed by user1 should prompt for approval.

Current Behavior

If user1 in groupA, tries to run a command that requires approval from user2 in groupB, the command is exectued without any approval process. In the log it shows that the user does not require approval to run the command.

But, If you put user1 and user2 in both groupA and groupB and PeerApproval for the command is set to $true, the process works as intended. The user (ie, user1) get's prompted that approval is needed, a different member of groupB (ie, user2) can then approve and the command is executed.

Possible Solution

It appears that the current code is only running logic to determine if the executing user is in the approval group (groupB) and if they are not, then the else statement kicks which is that approval is not needed.

I have a PR coming that addresses this issue.

Steps to Reproduce (for bugs)

To Reproduce groupA issue:

  1. Create a Permission (_permissionA) in your module
  2. Create a command (commandA) and give it permissionA
  3. Create roleA and give it permission to commandA's permission
  4. Create groupA (command execution group) and assign it to roleA
  5. Add user1 to groupA
  6. Create groupB (approval group)
  7. Add user2 to groupB
  8. In bot config, create Approval Configuration with an Expression for executing commandA, set PeerApproval to $true
  9. Have user1 execute commandA. The command will execute without approval.

NOTE:
If you put user1 and user2 in both groupA and groupB, approval process does kick in.

Context

We are working on an approval process for user commands that a product team can execute on remote servers, but need to have them approved before executing so we can have a business group approval in the process.

Your Environment

  • Module version used: 11.4
  • Operating System and PowerShell version: Windows10/PS 5.1
@mgeorgebrown89
Copy link

I just discovered this same issue. I was hoping to have our intern be able to start jobs but require approval, but it looks like the current work around would enable him to approve jobs from other people in the group, which somewhat defeats the purpose.

@devblackops
Copy link
Member

@dreznicek Thank you so much for catching this logic bug!

@devblackops
Copy link
Member

Merged #154 and published 0.11.5 to the PSGallery.

@dreznicek
Copy link
Contributor Author

@devblackops I don't know if its something I'm doing or if I was still using my branch, but I can't get approvals to work at all in 0.11.6. No matter how I put users in groups to do approvals, I get this...

 {"DataTime":"2019-08-20 16:54:42Z","Class":"Bot","Method":"Start","Severity":"Error","LogLevel":"Info","Message":"Cannot bind argument to parameter \u0027ReferenceObject\u0027 because it is
 null.","Data":{"CommandName":"Compare-Object","Message":"Cannot bind argument to parameter \u0027ReferenceObject\u0027 because it is null.","TargetObject":null,"Position":"At C:\\Program
 Files\\WindowsPowerShell\\Modules\\PoshBot\\0.11.6\\PoshBot.psm1:2346 char:56\r\n+ ...             $inApprovalGroup = (Compare-Object @compareParams).Count  ...\r\n+
 ~~~~~~~~~~~~~~","CategoryInfo":"InvalidData: (:) [Compare-Object],ParameterBindingValidationException","FullyQualifiedErrorId":"ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.CompareObjectCommand"}}

The $approvalGroup is always null and that blows up the compare.

@devblackops devblackops self-assigned this Aug 21, 2019
@devblackops
Copy link
Member

Hey @dreznicek, can you share the ApprovalConfiguration you're using so I can try and repro?

@dreznicek
Copy link
Contributor Author

@devblackops Thanks for looking, sir! Here's the Approval Config...

 ApprovalConfiguration = @{
  ExpireMinutes = 5
  Commands = @(
    @{
      Expression = 'modulename:my-command:*'
      Groups = @('JobApprovers')
      PeerApproval = $false   # $true doesn't work either
    }
  )
}

I put any users in the JobApprovers group and try to run the command, I get the above error.

If I do take out the Approval Config, then the behavior for permissions works correctly. If I give the user the right permissions via group and role, it works as expected...they can either execute the command or they cannot.

Lemme know if you need me to make a slimmed down module to replicate.

@dreznicek
Copy link
Contributor Author

@devblackops don't know if you've had a chance to look into this or if I need to create a new issue?

@devblackops
Copy link
Member

@dreznicek Have you tried the latest version v0.11.8?

This is what I just tested with success:

Can you try the following procedure (with PoshBot v0.11.8 and let us know if you get the same behavior?


  1. Install PoshBot.Networking from PSGallery

  2. Create approval configuration:

ApprovalConfiguration = @{
    ExpireMinutes = 30
    Commands = @(
        @{
            Expression = 'PoshBot.Networking:*'
            Groups = @('network-jockeys')
            PeerApproval = $false
        }
    )
}
  1. user1 tries to run the following but can't because they don't have the require permission (poshbot.networking:test-network)
!dig www.google.com
> You do not have authorization to run command [dig] :(
  1. Since commands from PoshBot.Networking require permissions to execute, _user1 creates a new role with the necessary permissions and adds it to the admin group (which they are a part of so they can do RBAC management).
!new-role -name network-troubleshooters 
!add-rolepermission -role network-troubleshooters -permission poshbot.networking:test-network
!add-grouprole -group admin -role network-troubleshooters
  1. _user1 tries to run the command again. They have permission now but since it requires approval, they get the approval prompt.
!dig www.google.com
> Approval is needed to run [dig www.google.com] from someone in the approval group(s) [network-jockeys].
To approve, say '!approve 9b7d13e6'.
To deny, say '!deny 9b7d13e6'.
To list pending approvals, say '!pending'.
  1. user1 attempts to self-approve with the following but can't (self-approving isn't allowed)
!approve 9b7d13e6
> Nice try. Self-approving is not allowed.
  1. _user1 creates the approval group network-jockeys and adds user2 to it
!new-group network-jockeys
!add-groupuser -group network-jockeys -user user2
  1. user2 now approves the command
!approve 9b7d13e6
  1. user2 also tries to run the command but they can't, they are just an approver, they don't have the correct role to actually run the command
!dig www.microsoft.com
> You do not have authorization to run command [dig] :(

@devblackops devblackops reopened this Oct 30, 2019
@stale
Copy link

stale bot commented Dec 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 29, 2019
@stale stale bot closed this as completed Jan 12, 2020
@devblackops devblackops added the pinned Prevent closing if stale label Jan 13, 2020
@devblackops devblackops reopened this Jan 13, 2020
@stale stale bot removed the stale label Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug pinned Prevent closing if stale
Projects
None yet
Development

No branches or pull requests

3 participants