poshbotio / PoshBot

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

Fix approval group logging and approval group being bypassed #154

Closed dreznicek closed 5 years ago

dreznicek commented 5 years ago
  1. There is a LogDebug Message that is logging the wrong group in the message. It's currently logging the group that the user is a member of and not the approval groups that can approve the command.
  2. The approval logic is being bypassed for users in groups that are assigned a role that has permissions to run a command (BUT, it works if that user is a member of both the group that can execute the command and the approval group assigned)

Description

Fixed the LogDebug line that was showing the Groups that the user belonged in, and not the approval groups.

Next, changed the logic on the Approval Group check to say Hey, are you in the ApprovalGroup? Yeah? Ok, is Peerapproval enabled? Yeah? Then we better get a buddy to approve. Otherwise, you don't need approval, let's execute!

Conversely, Oh you aren't in the Approval Group? Then you definitly need to have approval to run this command.

The reason this logic works is because its already been determined that you have permission to execute this command before the ApprovalNeeded function is even called. Meaning, you wouldn't have gotten here if you didn't, so we're just trying to figure out if:

  1. There is an approval needed for this command?
  2. Are you in the approval group or not?
  3. Is peer approval required?

Related Issue

https://github.com/poshbotio/PoshBot/issues/153

Motivation and Context

This bug allows any user with permission to execute a command even when the command has an approval configuration setup.

How Has This Been Tested?

We tested by running through the scenarios layed out in the corresponding issue link.

Screenshots (if appropriate):

Types of changes

Checklist:

devblackops commented 5 years ago

Thanks for the fix @dreznicek!