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

Issue 5184 - memberOf does not work correctly with multiple include scopes #5185

Merged
merged 1 commit into from Mar 4, 2022

Conversation

mreynolds389
Copy link
Contributor

Bug Description:

MemberOf Plugin only looks at the first include scope, and the rest are
ignored. So if multiple "memberOfEntryScope" attributes are set then the
plugin will not work as expected.

Fix Description:

The fix is to read all the memberOfEntryScope attributes and update the
group cache.

relates: #5184

}
if (!all_backends) {
goto done;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

in case 'all_backend'==True, is there a risk that it goes to the internal search below (line 836). In such case is base_sdn valid and is it processing the base_sdn twice ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is actually rather confusing. I've revised it again, and it passes CI tests, but please review...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tbordaz could you review this again? Remember there is second part to this that is waiting for this to be merged first.

…copes

Bug Description:

MemberOf Plugin only looks at the first include scope, and the rest are
ignored. So if multiple "memberOfEntryScope" attributes are set then the
plugin will not work as expected.

Fix Description:

The fix is to read all the memberOfEntryScope attributes and update the
group cache.

relates: 389ds#5184

Reviewed by: tbordaz(Thanks!)
@Firstyear
Copy link
Contributor

can you add a test for a "broken" case like dc=example,dc=com and ou=group,dc=example,dc=com?

Copy link
Contributor

@tbordaz tbordaz left a comment

Choose a reason for hiding this comment

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

The patch looks good to me. I admit that this part of code handling scoping/exclude/only one backend is a bit messy but not sure how to write in a better way.

Copy link
Contributor

@tbordaz tbordaz left a comment

Choose a reason for hiding this comment

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

ACK

@mreynolds389 mreynolds389 merged commit 17d1006 into 389ds:master Mar 4, 2022
@mreynolds389 mreynolds389 deleted the memberof_fix_include_scope branch March 4, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants