The point of this (what you removed), is you don't need to play DN shuffle to get the right one - you only ask the backend for it's related encrypted attrs part. So I think you should undo these lines unless there is a specific reason for this.

I don't like this change. I think that this will break and surprise users because the basedn suddenly has content prepended to it. Remember, this library is intended to be used by "not us", and this violates the least surprise principle. The "backend.get_encrypted_attrs()" is the right way to get the associated encrypted attrs object, and the basedn is set correctly in that process.

I don't see how this change is fixing a "breakage" in the tests? I think you have done two things. One is probably fixing the breakage, the other is a usability change to the api (that I don't think I'll approve).

What is the actual test failure you are seeing here? Why is it occuring? Is there a better way to fix it?

I don't like this change. I think that this will break and surprise users because the basedn suddenly has content prepended to it. Remember, this library is intended to be used by "not us", and this violates the least surprise principle. The "backend.get_encrypted_attrs()" is the right way to get the associated encrypted attrs object, and the basedn is set correctly in that process.

@firstyear backend.get_encrypted_attrs() doesn't have any attribute to create. We were seeing attribute error in for eg: emp_num_encrypt = encrypt_attrs.create(properties={'cn': 'employeeNumber', 'nsEncryptionAlgorithm': 'AES'}) seems like some changes in lib389.
While using I got to know it set the dn incorrectly, that is : cn=uid,cn=userRoot,cn=ldbm database,cn=plugins,cn=config whereas it should be cn=uid,cn=encrypted attributes,cn=userRoot,cn=ldbm database,cn=plugins,cn=config. But if it is what it is supposed to like we can fix it in the code, but if it is specific to attr_encrption then we should fix in lib389. if cn=encrypted attributes is not present we are seeing assertion errors.

It looks like the implemented api has a subtle incorrect behaviour. It should be:

list_encrypted_attrs()
list_encrypted_attrs_names()
get_encrypted_attr(name_of_attr)

Additionally, in backend.py, it's using the wrong dn:

EncryptedAttrs(self._instance, basedn=self._dn)
SHOULD BE
basedn="cn=encrypted attributes," + self._dn

That would cause the child EncryptedAttr to have the wrong dn, because it's pre-pending the rdn to the wrong location.

Thinking about it add_encrypted and del_encrypted are wrong to. You actually want:

ea = get_encrypted_attr(name)
ea.delete()
ea.create()
etc.

So to fix this.

remove add_encrypted_attr, del_encrypted_attr, get_encrypted_attrs, make it list_escrypted_attrs (return all of them), list_encrypted_attrs_name(return a list of strings of the cns) and get_encrypted_attr.

Thinking about it add_encrypted and del_encrypted are wrong to. You actually want:
ea = get_encrypted_attr(name)
ea.delete()
ea.create()

etc.

So to fix this.
remove add_encrypted_attr, del_encrypted_attr, get_encrypted_attrs, make it list_escrypted_attrs (return all of them), list_encrypted_attrs_name(return a list of strings of the cns) and get_encrypted_attr.

Thanks for looking into this, will try and do the changes :)

Thinking about it add_encrypted and del_encrypted are wrong to. You actually want:
ea = get_encrypted_attr(name)
ea.delete()
ea.create()

etc.

So to fix this.
remove add_encrypted_attr, del_encrypted_attr, get_encrypted_attrs, make it list_escrypted_attrs (return all of them), list_encrypted_attrs_name(return a list of strings of the cns) and get_encrypted_attr.

@firstyear Can't find any of the above-mentioned functions in the source, can you tell where are these, I have grepped it can't find anything like this. BTW rather than making changes to lib389 we can pass the correct DN, but I still think this should be handled in lib389.

These functions can be found in src/lib389/lib389/backend.py

But they are used in the CLI and UI and should not be removed.

These functions can be found in src/lib389/lib389/backend.py
But they are used in the CLI and UI and should not be removed.

Or, remove them and also fix/adjust the CLI and UI :-)

Thinking about it add_encrypted and del_encrypted are wrong to. You actually want:
ea = get_encrypted_attr(name)
ea.delete()
ea.create()
etc.
So to fix this.
remove add_encrypted_attr, del_encrypted_attr, get_encrypted_attrs, make it list_escrypted_attrs (return all of them), list_encrypted_attrs_name(return a list of strings of the cns) and get_encrypted_attr.

@firstyear Can't find any of the above-mentioned functions in the source, can you tell where are these, I have grepped it can't find anything like this. BTW rather than making changes to lib389 we can pass the correct DN, but I still think this should be handled in lib389.

You can't find them because I want you to make them ....

@aadhikari , please rebase it.
As for the refactoring encrypted attributes in lib389, let's open another ticket and fix it there. Refactoring is out of scope of this fix.

@vashirov I have rebased and ran the test again, we can merge this, I will open a new issue for refactoring encrypted attributes in lib389. Thanks!

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 pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3280

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.