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

[microsoft.ad.user] Add parameter to fail, ignore or warn if the account performing the action does not have the permissions required to modify the AD Group #166

Merged
merged 8 commits into from
Nov 19, 2024

Conversation

tarmael
Copy link
Contributor

@tarmael tarmael commented Nov 15, 2024

SUMMARY

Fixes #140

Adds sub-parameter to groups
permissions_failure_action

Options:

  • fail
  • ignore
  • warn

Default: fail

This feature prevents microsoft.ad.user from failing when attempting to add or remove a user from a group that the domain_username user does not have permissions to modify if ignore or warn is specified.

Similar to the lookup_failure_action feature that prevents failures when attempting to add a group that does not exist,

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

groups:
permissions_failure_action:

ADDITIONAL INFORMATION
Before:
"exception": "Insufficient access rights to perform the operation\r\nAt line:458 char:25
+                         Set-ADObject -Identity $member -Add @{
+                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (CN=Restricted_Group...mydomain,DC=com:ADObject) [Set-ADObject], ADException

After:
  "warnings": [
    "Cannot add group 'CN=Restricted_Group,OU=Groups,DC=mydomain,DC=com'. You do not have the required permissions, skipping: Insufficient access rights to perform the operation"
  ],

This feature adds a new sub-parameter to the Groups section similar to lookup_failure_action to aid the scenario when the account used to add or remove the user from the specified AD Groups does not have appropriate permissions to perform the action.

This is achieved through wrapping the add or remove attempts around a try/catch and handling the try/catch based on the parameters specified.

Parameters accepted are fail, ignore, and warn
Default action is: fail
Minor update to the wording of lookup_failure_action as well
Copy link

Copy link

Copy link
Collaborator

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fantastic PR. The only thing missing is the changelog fragment. Are you able to add one under changelogs/fragments https://docs.ansible.com/ansible/latest/community/development_process.html#creating-changelog-fragments. Thist particular change would be under the minor_changes key.

Something like

minor_changes:
  - >-
    microsoft.ad.user - Added ``groups.permissions_failure_action`` to control the behaviour when failing to modify the user's groups -
    https://github.com/ansible-collections/microsoft.ad/issues/140

Feel free to modify the message in any way you prefer.

plugins/modules/user.yml Show resolved Hide resolved
Copy link

@tarmael tarmael requested a review from jborean93 November 19, 2024 04:57
Conforming changelog fragment to Ansible Fragment standards
@tarmael
Copy link
Contributor Author

tarmael commented Nov 19, 2024

Thanks for this, I've added the changelogs fragment

That official doc you linked doesn't feel very intuitive to read for newcomers, so I hope this is correct :)

Copy link

Copy link
Collaborator

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

@jborean93
Copy link
Collaborator

That official doc you linked doesn't feel very intuitive to read for newcomers, so I hope this is correct :

What you have looks good. I’ll try and pass along the feedback to the docs team.

@jborean93 jborean93 merged commit c950082 into ansible-collections:main Nov 19, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants