-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
When either data_profiler_filter or superuser_filter aren't defined,… #930
Conversation
@bolkedebruin btw, without this patch, LDAP on master won't work unless dataprofiler/superuser filters are set (i.e. master is broken for default use case right now). |
Ok. Will have a look at it towards the end of this week |
Gentle nudge here. :) |
@criccomini In general the patch looks solid! I would like to see the tests also cover the use of data_profiler and superuser. This would require to add it to the ldap provisioning for a specific user. After that please squash your commits in some meaningful ones. |
@bolkedebruin can you elaborate a bit on what you mean by "tests covering the use of data_profiler and superuser"? |
@mtagle I think he means that he wants a test that adds several users to the LDAP test. One would be a user with just basic access (no data profiler/super user), one with just data profiler, and one with just super user (and potentially one with both). Then verify that the data_profiler() and superuser() methods return True/False as expected. |
Correct. The provisioning can be done by the files no need to encapsulate it in the test itself. |
…don't crash, and set user to have data_profiler or superuser access.
b2b64cf
to
0c5b390
Compare
1. make sure that if no superuser or dataprofiler filter is set, all logged in users get superuser or dataprofiler privileges. 2. make sure that, if filters are set, users get acess when the filters allow it. change ldif loading so that it always loads in a reasonable order.
0c5b390
to
3e99d58
Compare
@bolkedebruin could I get a further review/merge? |
ping @mistercrunch |
@mistercrunch I think this is ready to be committed. |
👍 |
When either data_profiler_filter or superuser_filter aren't defined,…
Signed-off-by: henneberger <git@danielhenneberger.com> Co-authored-by: Willy Lulciuc <willy@datakin.com>
… don't crash, and set user to have data_profiler or superuser access.
At present, if data_profiler_filter or superuser_filter aren't defined, dataprofiler and superuser aren't set in LDAPUser's init, resulting an an AttributeError when accessors are called.