Skip to content

Catch situations where attributes cannot be read due to required password change #25

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

Merged
merged 2 commits into from
Feb 11, 2021

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