Skip to content
This repository was archived by the owner on Jun 27, 2022. It is now read-only.

Conversation

tatsuyaueda
Copy link
Contributor

fix #10

Adding AD_GROUP_FILTER

Adding AD_GROUP_FILTER
Copy link
Owner

@marcus-crane marcus-crane left a comment

Choose a reason for hiding this comment

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

Requested a check for the group filter being defined

@@ -99,6 +99,9 @@ def _configure_access_groups(self, user, access_token):
azure_group_names = [entry.get('displayName') for entry in azure_groups]
AD_GROUP_MAP = PLUGIN_SETTINGS.get('AD_GROUP_MAP', {})
for group_name in azure_group_names:
if group_name not in PLUGIN_SETTINGS['AD_GROUP_FILTER']:
Copy link
Owner

Choose a reason for hiding this comment

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

I would first check if PLUGIN_SETTINGS['AD_GROUP_FILTER'] is defined. I can imagine AD_GROUP_FILTER being initialised as an empty list and then for users who upgrade but don't define a filter, suddenly all groups are filtered out because nothing is in the filter list!

@marcus-crane
Copy link
Owner

Hey @tatsuyaueda, thanks for taking the time to submit a pull request 🙂 It looks good! There's a small change I would make just to ensure that users who upgrade to a new version aren't accidentally impacted

@marcus-crane
Copy link
Owner

Ahh, I'll need to write some documentation for this anyway so I'll just merge it in and add the check. Thanks again

@marcus-crane marcus-crane merged commit e032579 into marcus-crane:main Nov 9, 2021
@tatsuyaueda
Copy link
Contributor Author

Thanks.

I didn't care enough about the code.
Thank you for merging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to filter which AD groups are registered in Netbox
2 participants