[geeklog-devel] Code feedback on Re: A&A AEPasswordGenerator.class.php

Tony Bibbs tony at tonybibbs.com
Tue Jun 22 14:16:47 EDT 2004


I meant to include everybody else in on this as well.

--Tony

-------- Original Message --------
Subject: 	Re: A&A AEPasswordGenerator.class.php
Date: 	Tue, 22 Jun 2004 13:15:21 -0500
From: 	
To: 	
References: 	<8319e2d6040622083174ac3d6b at mail.gmail.com>



Vinny, a couple of notes.

1) This is more of a personal thing.  I hate appreviate IF..THEN so 
stuff like this:

$len = ($gConf['randompasswordlength'] >= 4) ? 
$gConf['randompasswordlength'] : 4;

Should be replace with the traditional IF..THEN.  This is more of a 
readability thing really.  This sort of stuff will get ironed out more 
when we get formal coding standards agreed on.  We should do that in the 
next week or so.

2) Most of your config variables have some sort of Auth_Enterprise 
equivalent in AEServerConfig.php.  Not a huge deal but we should either 
replace what I have or modify your code to use mine.

3) Your code assumes pspell and crack are in the path.  We should have 
config variables for them.  In fact we may want to include a user 
executable path:
$gConf['user_executables'] = '/usr/bin/';
$gConf['path_pspell'] = $gConf['user_executables'] . 'pspell';
$gConf['path_crack'] = $gConf['user_executables'] . 'crack';

We should also add an option for the pspell dictionary as we don't want 
to assume english.

4) Use of die() in Auth_Enterprise should not be allowed.  Instead we 
should throw an exception and let the calling application figure out 
what to do more gracefully.  You can either use one of the ones I have 
in AEExceptions.php or add a new one for that case to that file.

Finally, in general I like the concept of my code in that they can 
configure if they require lowercase, require upper case, require special 
chars.  To that end, we should have regular expressions for all those 
permutations.  Also the minimum and maximum password length should 
probably be configurable.

The regex's are nice.  It's probably a better way to go.  Good work. If 
you have questions let me know.  When we get this all ironed out, do you 
want to take a stab at adding these updates?

FWIW, this is a great example of how the OO-nature of Auth_Enterprise 
has.  SHould someone not like our implementation fo AEPasswordGenerator, 
they can write their own, drop it in their filesystem and start using it.

--Tony


Vincent Furia wrote:

>Tony,
>
>I rewrote the password generator/validator using regex to give much
>more flexibility when it comes to creating rules (see attached). 
>Since the generator is creating a random password (can really only be
>broken brute force) from a decent sized set of characters I didn't
>think enforcing any rules is really necessary.  Especially since A&A
>should probably force a password change after a password reset upon
>first login...
>
>Also, I thought I'd suggest password reset support like Dirk has place
>into the most recent version of Geeklog.  i.e. you send a link that
>includes a big random value (hash of some kind) and make the user
>change their password when following the link.  The is easy to
>implement with a built in Auth system, not sure how you would do this
>(or force a password change) with A&A...  Catch me on IRC sometime and
>we can talk more about it...
>
>Anyway, hope the like the changes I made.  The rest of A&A looks
>pretty good, if a bit of overkill for most people running Geeklog. :)
>
>I think it'd be cool if we implemented single sign on.  I wasn't sure
>if you had any ideas how to implement that, again I'd be happy to chat
>(IRC or voice) with you about it sometime.
>
>-Vinny
>  
>




More information about the geeklog-devel mailing list