If you are going to move the LDAP error codes into separate classes instead of having a single LdapErrorDefinition class and allowing the deployer to configure the regex pattern and type, you should identify which LDAP server the error definition works for. All of the definitions in your pull request are for Active Directory, so they should probably be named ActiveDirectoryAccountDisabledLdapErrorDefinition, ActiveDirectoryAccountLockedLdapErrorDefinition, etc. Other LDAP servers could then have their own versions of those classes with the correct regex values, but that will probably lead to a lot of new error definition classes.
How about adding ldapPattern as an optional input to the LDAP error definition classes? That way, deployers using Active Directory can use the default, but those of us using something else (389, ODS, etc) could set our own regex. -Eric -- Eric Pierce Identity Management Architect Information Technology University of South Florida (813) 974-8868 -- [email protected] ________________________________ From: Misagh Moayyed [[email protected]] Sent: Monday, November 26, 2012 7:38 PM To: [email protected] Subject: RE:[cas-dev] Review of LPPE Changes I have put together a number of design notes as well as a component diagram that should facilitate better understanding of the proposed LPPE functionality. Please see this link here: https://wiki.jasig.org/pages/viewpage.action?pageId=55543468 -Misagh From: Misagh Moayyed [mailto:[email protected]] Sent: Monday, November 19, 2012 3:21 PM To: '[email protected]' Subject: RE: Review of LPPE Changes under Pull #171 Merged with master under a second pull: https://github.com/Jasig/cas/pull/171 Regards, -Misagh From: Misagh Moayyed [mailto:[email protected]] Sent: Tuesday, November 13, 2012 4:10 PM To: '[email protected]' Subject: Review of LPPE Changes under Pull #153 Team, I’d like to invite review of pull #153 that addresses a number of LPPE-related enhancements: https://github.com/Jasig/cas/pull/153 The changeset includes internalizing ldap error codes, taking advantage of existing ldap authN handlers, support for custom attribute that determine account state, fixes to the LPPE UI that were broken in style after the recent responsive UI design change and general cleanup and javadocs. I’d very much like to get the changeset in so that early adopters can review and provide feedback, especially those that are behind open ldap. Jérôme did a preliminary review and a number of comment were addressed and fixed in the pull. If you have time and do get a chance, please take a look and suggest improvements where needed. I am hoping that we might be able to merge towards end of the next week if not earlier. Regards, -Misagh -- You are currently subscribed to [email protected]<mailto:[email protected]> as: [email protected] To unsubscribe, change settings or access archives, see http://www.ja-sig.org/wiki/display/JSG/cas-dev -- You are currently subscribed to [email protected] as: [email protected] To unsubscribe, change settings or access archives, see http://www.ja-sig.org/wiki/display/JSG/cas-dev
