@spichugi Please review the patch.

Is this baseDN correct? We shouldn't have the cn=encrypted attrs here, because the subtree search will find all the correctly classed attribute encryptino definitions. I think you only need the DN_CONFIG here (or ldbm plugin DN). I think this here will miss finding "non-userroot" attributes.

Hey there! This looks really good. There is one comment/concern inline. To help out could you extend this test also?

We need to test multiple backens as well where:

one backend is encrypted, one not
both backends encrypted.

This will highlight if you have correctly implemented the encrypted attribute classes :)

As well, we also need to test key rollover - but that can be another ticket because I think it relates to a bug I opened some time ago (key rollover is not a needed function ....).

I hope that helps, and @spichugi Can you review as well?

Leftovers, probably

I think we better name it 'test_basic' and the ''Test attribute encryption'' should also be changed.
We have the attribute encryption name in the filename of the module already

As William said, we can remove the line

Thanks for the valuable review.
Please check the latest patch. Thanks.

You don't need this line if you use 'with open()' word. It will close it for you.

If you delete the user here, it won't get created later (because your fixture has 'scope=module').

Besides the mentioned things, the rest looks good. I'll run the tests when I'll be around my pc.

Maybe @firstyear would like to check if the added test case is the one he has requested.

@spichigi Thanks for the review, please check the latest one. Thanks.

@spichugi sorry for the typo.

LGTM! Ack

Thanks @spichugi . Rebased the patch. Please merge. 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/2975

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.