SAP / macOS-enterprise-privileges

For Mac users in enterprise environments, this application gives users control over the administration of their machine by elevating their level of access to administrator privileges on macOS. Users can set a timeframe in the application's settings to perform specific tasks, such as installing or removing an application.
Apache License 2.0
1.4k stars 149 forks source link

[Privileges 2] Content of the ‘Reason’ field is gone in some cases #108

Closed aduffner closed 1 month ago

aduffner commented 1 month ago

Impact

If you play around with the GUI, especially with the dropdown menu in combination with the reason text field you will get some unexpected behavior, at least from my pov.

Tested on

Config applied to testing system via mdm on computer level:

Same as in the last issue.

Steps of reproduction

Step 1: Click on the Privileges Beta Icon from your Dock, enter the reason (min. 20 chars):

Bildschirmfoto 2024-10-04 um 01 46 58

Step 2: Choose an actual option from the dropdown:

Bildschirmfoto 2024-10-04 um 01 47 15

Step 3: As you can see the content of the ‘Reason’ field is gone in this case:

Bildschirmfoto 2024-10-04 um 01 47 21

Step 4: Even choosing the standard value of the dropdown does not bring our texted reason back:

Bildschirmfoto 2024-10-04 um 01 47 31

It remains greyed out, I would expect to keep the content until you decided which reason you want to "finally submit".

A little bonus, may be connected to this issue: My webhook only receives JSON like this:

{
  "admin": false,
  "expires": "",
  "machine": "XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",
  "reason": "",
  "user": "XXXXXXXX"
}

Somehow reason and expires always seem to be empty.

mthielemann commented 1 month ago

The text field will only have a value if you select "Other…". In all other cases, the text field is empty. That's expected behavior. It's only there to specify the "Other" reason. Regarding the JSON. "expires" only has a value if a user gets admin rights (admin = true). Could you please check this again on your end? Thanks.

mthielemann commented 1 month ago

Just tested the web hook and here it looks like this if I get admin rights:

{
   "admin":true,
   "expires":"2024-10-04T05:52:44Z",
   "machine":"2DD7E58B-E77D-4433-B799-DC21F4AE9A60",
   "reason":"Just for fun",
   "user":"testuser"
}

and like this, if the admin rights are removed:

{
   "admin":false,
   "expires":"",
   "machine":"2DD7E58B-E77D-4433-B799-DC21F4AE9A60",
   "reason":"",
   "user":"testuser"
}

Seems to work as expected. Do you get different results?

aduffner commented 1 month ago

The text field will only have a value if you select "Other…". In all other cases, the text field is empty. That's expected behavior. It's only there to specify the "Other" reason.

Okay, let's say you're not sure what reason you want to provide and start typing in the texfield and then see oh there's a dropdown too, look at the options and end up deciding on ‘Other’ after all, then you have to re-enter your reason, which can be annoying from a UX perspective. Even if you just look at the dropdown (without selecting one of the actual options) your text disappears.

aduffner commented 1 month ago

Just tested the web hook and here it looks like this if I get admin rights:

Tested it again and I added previously a key I missed to report, sorry for this. If you configure PostChangeExecutablePath as empty string I expected to just fail "silently", but not to silent the webhooks also:

<key>PostChangeExecutablePath</key>
<string></string>

This way I will not receive a json at all, when activating the admin rights. Is this intended behavior?

In the comments of the provided .mobileconfig from the Resources folder you wrote this:

If the application or script does not exist or is not executable, the launch operation fails silently.

From my pov it would be cool that if you configure the string to be empty it "completely switches off" the option and also does not start an operation that could fail, but simply does not execute an additional script.

Some screenshots, with commented in key:

<key>PostChangeExecutablePath</key>
<string></string>
Bildschirmfoto 2024-10-04 um 09 30 15

Now, if I am commenting it out:

<!--<key>PostChangeExecutablePath</key>
<string></string>-->
Bildschirmfoto 2024-10-04 um 09 29 48
mthielemann commented 1 month ago

Please check the next build. I'll provide it beginning of next week. Thanks.

mthielemann commented 1 month ago

Build 1775 is now available here. Could you please check if it now works for you? Thanks.

aduffner commented 1 month ago

@mthielemann Wow, cool. I can confirm that it now works as expected:

Tested with:

Keep up the great work! Did you already made the source code of the beta version 2 public? I would love it to suggest such patches by myself.

mthielemann commented 1 month ago

@aduffner The source code of version 2 is not yet ready for release but as soon as we have a release version, the source code will be available.