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

Bind user before fetching #9312

Merged
merged 2 commits into from Oct 11, 2018

Conversation

Projects
None yet
3 participants
@murrant
Member

murrant commented Oct 9, 2018

Fixes an issue when ldap is configured without a bind user and it cannot find groups resulting in a user level of 0.

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

murrant added some commits Oct 4, 2018

@murrant murrant changed the title from WIP Bind user before fetching to Bind user before fetching Oct 9, 2018

@murrant murrant added the Bug 🐞 label Oct 9, 2018

@laf

laf approved these changes Oct 9, 2018

I have no way to test this but I'm assuming it's a bug that should be fixed before 1.44, if so I think we need to push the release back again.

Code looks sane.

@murrant murrant added this to the 1.44 milestone Oct 9, 2018

@murrant

This comment has been minimized.

Member

murrant commented Oct 9, 2018

Yeah, I think so. Lets push it back. Bummer I forgot about this fix :/

I tested it on my test ldap setup and prod AD setup.

@laf

This comment has been minimized.

Member

laf commented Oct 9, 2018

Bummer I forgot about this fix :/

You've done loads for this release so it's no big deal :)

@jviersel

This comment has been minimized.

Contributor

jviersel commented Oct 9, 2018

I don't know if my problem is equal to the one you tried to fix, but this fix does not work for me.

All my LDAP-users are in the group 'members', which has level 1 in LibreNMS. Some of them are also in the group 'admin', which has level 10. Users are authorized to some devices and/or ports.

Admin-users can see everything (works as expected), normal users do not see any devices or ports (not even the ones they are authorized for).

@murrant

This comment has been minimized.

Member

murrant commented Oct 9, 2018

@jviersel This fixes users having the incorrect level (1,10). Please report your issue https://community.librenms.org/c/help

@laf laf merged commit 44747fd into librenms:master Oct 11, 2018

2 of 3 checks passed

codeclimate 4 issues to fix
Details
WIP ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@murrant murrant deleted the murrant:bind-before-use branch Oct 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment