Skip to content

Conversation

@hsartoris-bard
Copy link
Contributor

When slapo-ppolicy is active on an OpenLDAP server, the relevant password policy specifies passwordMustChange: TRUE, and a given account has pwdReset: TRUE, then, although it is possible to bind as the user, the only operation of substance allowed once bound is password reset. This change catches the resulting error code when another action is attempted, such as querying the user's own attributes.

More details at https://linux.die.net/man/5/slapo-ppolicy

@jrivard
Copy link
Contributor

jrivard commented Jan 15, 2021

the file changed is specific to eDir, although the OpenLDAP factory class references EdirErrors here:

private static final ErrorMap ERROR_MAP = new EdirErrorMap();

so it's kinda correct, but we definitly shouldn't be putting OpenLDAP error codes in the edir error map. Instead what we need is a new error map file specific for OpenLDAP and change the reference in the factory to the new map. Are you able to do this and add basic list of OpenLDAP error codes?

@hsartoris-bard
Copy link
Contributor Author

I see your point; unfortunately, I don't know the operational error codes well enough to feel I can wholesale replace the current setup.

Would a wrapper approach be acceptable? i.e., a ChainingErrorMap that could be given a (very small) initial OpenLDAPErrorMap as the primary source, but with an EdirErrorMap to fall back on? Happy to provide an implementation if so.

@jrivard
Copy link
Contributor

jrivard commented Jan 20, 2021

Theres not much point in falling back on edir error, those codes don't exist in OpenLDAP. Even if it has only a handful of codes as long as the error catch mechanism is working we can expand the list later. I think this might be the list of OpenLDAP error codes here: https://www.openldap.org/doc/admin24/appendix-ldap-result-codes.html

@hsartoris-bard
Copy link
Contributor Author

Fair point! I did my best; let me know if there's anything obviously awry.

@hsartoris-bard
Copy link
Contributor Author

@jrivard anything else that you'd like to see happen with this?

@jrivard jrivard merged commit 2201f41 into ldapchai:master Feb 11, 2021
@jrivard
Copy link
Contributor

jrivard commented Feb 11, 2021

@hsartoris-bard looks great, I was hoping to get a chance to test this against a real open-ldap system, but I just havent had a chance, so I'll go ahead and merge based on its appearence. If u find any further bugs please send another pull request.

Thanks for contributing!

@hsartoris-bard
Copy link
Contributor Author

Welcome; will do!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants