poshbotio / PoshBot

Powershell-based bot framework
MIT License
536 stars 108 forks source link

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

Open dreznicek opened 5 years ago

dreznicek commented 5 years ago

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

mgeorgebrown89 commented 5 years ago

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 commented 5 years ago

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

devblackops commented 5 years ago

Merged #154 and published 0.11.5 to the PSGallery.

dreznicek commented 5 years ago

@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 commented 5 years ago

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

dreznicek commented 5 years ago

@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 commented 4 years ago

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

devblackops commented 4 years ago

@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] :(
stale[bot] commented 4 years ago

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.