fix(ingest) Azure AD: support nested groups (#4367) #4368
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR @cccs-eric ! I'm assuming "nested groups" are groups inside other groups? Do we plan to deal with this by "flattening" the nested group to add all of the nested users to the root group on DataHub? |
|
That's exactly it @jjoyce0510 . Let's say we have the following:
When ingesting Group A, the source will now create a CorpGroup for Group A, and users 1,2,3,4,5 will be a member of Group A only. Group B and C will not be mapped to a CorpGroup unless they are also ingested as a root group. Does it makes sense? |
|
Yes - that makes sense. Thanks for the explanation! |
jjoyce0510
reviewed
Mar 11, 2022
jjoyce0510
reviewed
Mar 11, 2022
jjoyce0510
reviewed
Mar 11, 2022
metadata-ingestion/src/datahub/ingestion/source/identity/azure_ad.py
Outdated
Show resolved
Hide resolved
jjoyce0510
reviewed
Mar 11, 2022
metadata-ingestion/src/datahub/ingestion/source/identity/azure_ad.py
Outdated
Show resolved
Hide resolved
jjoyce0510
reviewed
Mar 11, 2022
jjoyce0510
reviewed
Mar 11, 2022
jjoyce0510
reviewed
Mar 11, 2022
|
Looks like lint is failing. You can run |
cccs-eric
force-pushed
the
feature/azure_ad_nested_groups
branch
from
1ddf934 to
ffb7fe2
Compare
shirshanka
approved these changes
Mar 14, 2022
aditya-radhakrishnan
pushed a commit
to aditya-radhakrishnan/datahub
that referenced
this pull request
Mar 14, 2022
aditya-radhakrishnan
pushed a commit
to aditya-radhakrishnan/datahub
that referenced
this pull request
Mar 14, 2022
maggiehays
pushed a commit
to maggiehays/datahub
that referenced
this pull request
Aug 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to issue #4367 .
The proposed PR adds support to nested groups in Azure AD, as well as adding a config option to disable logging of filtered out users/groups (see this Slack thread). When dealing with a large Azure AD, many users/groups can be filtered out and the resulting log is very long and useless. Having the possibility to disable that reporting makes the user experience better in my opinion. There might be a better way of doing this as discussed with @dexter-mh-lee . I'm open for feedback.
This PR also:
datahub-frontendREADME build instructionsAUTH_OIDC_SCOPEfor Azure AD authenticationconfigure-oidc-react.mdfile