Comments in-line ...
since the last addition of the PasswordPolicyService interceptor in
trunk, the server does not accept anymore passwords which are supposly
violating a policy : "Password violates policy: insufficient character
I was very surprised when I first got this message, and I had to dig
into the code to find the rationals.
What I found in the code didn't pleased me, and I think this code should
not have quit a branch to be injected to the trunk.
This was directly related to last Enrique merge.
Here are some samples of what I have found :
void check( String username, String password ) throws NamingException
int passwordLength = 6;
int categoryCount = 2;
int tokenSize = 3;
if ( !isValid( username, password, passwordLength,
categoryCount, tokenSize ) )
String explanation = buildErrorMessage( username, password,
passwordLength, categoryCount, tokenSize );
log.error( explanation );
throw new NamingException( explanation );
Such constants deep in the code is a sure way to get stuck when using
the server. I'm pretty sure that it was just intended for testing
purpose, but even then, it should not have been written this way.
else if ( attr instanceof byte )
String string = StringTools.utf8ToString( ( byte ) attr
StringBuffer sb = new StringBuffer();
sb.append( "'" + string + "' ( " );
sb.append( StringTools.dumpBytes( ( byte ) attr
log.debug( "Adding Attribute id : 'userPassword',
Values : [ " + sb.toString() + " ) ]" );
userPassword = string;
This is another example of what should not be done : there is no reason
for debug specific code to be found in the normal flow. Moreoever, I
have told Enrique the day before to not using such a pattern.
Now, beside those pieces of code, there is something much more important
: it changes the way the server works, and nobody has been warned about
it. Not only it has been committed as is, but the configuration has been
changed so that this code is now active.
I have checked the commit logs, and I found another explaination for a
problem I encoutered lately, but as I didn't had time to dig it, I just
commented some lines in the server.xml file :
<!-- limits searches by non-admin users to a max time of
<!-- milliseconds and has a default value of
<property name="maxTimeLimit" value="0" />
<!-- limits searches to max size of 1000 entries: default value is
<property name="maxSizeLimit" value="100000" />
Those two values was simply not accepted because the related members
have been removed from the StartupConfiguration class. I can't imagine
that it has been unnoticed before committing the code, or I guess that
*no* integration tests has been done, or that the server has never been
launch, because launching the server with such a configuration leads to
a direct crash in one second.
This is also a direct result of the merge.
This is simply not the way we should work. I have spent time to
understand what was happening, and time to write this mail with all the
necessary checks to be sure I was not misunderstanding what has been
done. I would have liked to spend this time on something else more usefull.