DSLdapObject _create() method checks if the entry exists before creating it: https://pagure.io/389-ds-base/blob/master/f/src/lib389/lib389/_mapped_object.py#_728
try: self._instance.search_ext_s(dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1, serverctrls=self._server_controls, clientctrls=self._client_controls) exists = True except ldap.NO_SUCH_OBJECT: pass
But it does the search using the current bind, which can return different result errors depending on the permissions. For example, under DM if entry doesn't exist, result will be err=32, but under non-privileged user results will be err=0 (to prevent information disclosure?).
389-ds-base-1.4.0.20-1.fc29.x86_64
See attached reproducer <img alt="reproducer.py" src="/389-ds-base/issue/raw/files/21d22467c62f4a00bec0e86b03428d886e3f65a7ea73f3a2b7c5d993fb35880e-reproducer.py" />
I'm not quite sure I understand the problem you have in this scenario. It sounds like everything is working as intended but maybe I'm missing something? Is the issue that the non-priv user with err=0 then still tries to create the object and fails later in the script?
Metadata Update from @firstyear: - Custom field component adjusted to None - Custom field origin adjusted to None - Custom field reviewstatus adjusted to None - Custom field type adjusted to None - Custom field version adjusted to None
I'm not quite sure I understand the problem you have in this scenario.
Have you tried the reproducer?
It sounds like everything is working as intended but maybe I'm missing something?
The logic is flawed for users with different permissions. For non-privleged users we hide the existence of the entry in case it is not found. And based on that script decides if it exist or not. And for non-privelged users the result will be always "exist".
Is the issue that the non-priv user with err=0 then still tries to create the object and fails later in the script?
No, it doesn't try to create it, because it thinks that user is already there, because ldapsearch returned success (err=0) in try block, when under DM it would return error (err=32, already exist). Instead of relying on catching error in try block perhaps we should count number of entries returned? In this case if the entry doesn't exist it will be always 0 (since the search is done with basedn of the entry), and in case it is exist, it will be 1.
I can't try any reproducers as some of my patches are still in review to make installs work on my machine ... :( I've been doing some weird rebase-hacks to even submit the ones I have done .... :(
So does this mean we hide the existance of the entry, so create thinks "it does not exist" and then tries to create anyway?
Or that it reports there are no conflicting entries, then we try to create and it errors then?
It think it exists already and doesn't proceed to create.
I linked the problematic code in the description. Here's with some more explanations:
try: self._instance.search_ext_s(dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1, serverctrls=self._server_controls, clientctrls=self._client_controls)
Under DM we get err=32:
[root@server-f29 ds]# ldapsearch -D "cn=Directory Manager" -w password -h localhost -p 38901 -b "cn=user2,ou=People,dc=example,dc=com" # extended LDIF # # LDAPv3 # base <cn=user2,ou=People,dc=example,dc=com> with scope subtree # filter: (objectclass=*) # requesting: ALL # # search result search: 2 result: 32 No such object matchedDN: ou=People,dc=example,dc=com # numResponses: 1
But under non-priveleged user (that has an aci allowing to create entries):
[root@server-f29 ds]# ldapsearch -D "cn=user1,ou=People,dc=example,dc=com" -w password -h localhost -p 38901 -b "cn=user2,ou=People,dc=example,dc=com" # extended LDIF # # LDAPv3 # base <cn=user2,ou=People,dc=example,dc=com> with scope subtree # filter: (objectclass=*) # requesting: ALL # # search result search: 2 result: 0 Success # numResponses: 1
So it succeeds and ldap.NO_SUCH_OBJECT is not caught:
exists = True except ldap.NO_SUCH_OBJECT: pass
And because ensure is False by default and was not passed:
elif exists and not ensure: # raise "already exists." raise ldap.ALREADY_EXISTS("Entry %s already exists" % dn)
HTH
I have a different view on this, when reading this I just realized that "create" tries to be smart and behave differently if the entry exists or not. I do not like this for different reasons:
if in a test I want to create an entry I want a convenient wrapper around add_s but nothing more, doing an additional search already adds another operation and changes the load profile. if I do an add I want the result of the add, not the interpreted result of a search, if I try to add an entry which already exists I get an return code to handle, same if I do not have the permissions to add.
so in my opinion, the search part in create should not be there (and will not cause problems)
Metadata Update from @mreynolds: - Issue set to the milestone: 1.4.0
@lkrispen I think that the ensure() function relies on the exist check, but I'll draft a patch that addresses this today
https://pagure.io/389-ds-base/pull-request/50245
Looks like its working for me now .
https://pagure.io/389-ds-base/pull-request/50319
Metadata Update from @mreynolds: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/3196
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: fixed)
Login to comment on this ticket.