Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I've inherited a custom made authentication system that needs some refactoring. I'm not quite sure what the best way to split it up the password section and could use some help.

The password class(es) need the following functionality:

  • Check to see if a password conforms to restrictions. It needs to be able to provide feedback on why it is incorrect.
  • Generate three kinds of hashes. We have several file-systems with incompatible hashes, so unfortunately I have to maintain three hashes.
  • Give a different hash for a certain user type. If a user is web-only we do not keep a password for them as they cannot use the system. Their passwords are always *.
  • Be able to verify a password for a given user. Fortunately I only have to authenticate against one of the three hashes I have to maintain.
  • Be able to generate a random password, unencrypted.
  • Submit new password to the database.

I'm thinking about using four classes, Validator, Hasher, Generator, and Changer.

  • Validator
    • isCurrentPassword(int userid, string password): bool
    • getViolations(string password): array
  • Generator
    • generate(int length):string Creates a password.
  • Hasher
    • hash_*(string password): string where * is the hash type. I'll have four hashing functions.
  • Changer
    • change(int userid, string password):void; throws database exceptions

What would you recommend and why?

share|improve this question
add comment

3 Answers

Who else is supposed to use this authentication system? If it is only your application, use the simplest code possible that can be understood, maintained and tested. However, I don't see that you should use 2 separate classes as in method 1, because Validator is not a class it is strictly a method of some class. The same goes for Generator.

share|improve this answer
add comment
  1. Check to see if a password conforms to restrictions

    I think this belongs in its own class. In your method 1, you've got it as part of the Validator class. But it doesn't have the same dependencies as isCurrentPassword or share any implementation details. As a result, I'd put in a seperate PasswordAudit class.

  2. Generate three kinds of hashes

    You should have some sort of interface which provides uniform access to the different filesystems you are using. I'd suggest the hash() function should live there. Its an implementation detail of the filesystem, and belongs in your handling for that filesystem

  3. Give a different hash for a certain user type

    If they can't use the system, why do they have accounts? Or perhaps they can login into the web system, but don't have access to the filesystem backends? But in that case, they'd still have a password wouldn't they?

    I'd suggest implementing this by a NullFileSystem backend. The backend would support the interface of a filesystem, but not actually work. Overriding the hash is easily done.

  4. Be able to verify a password for a given user.

    I'd be inclined to use the user object for this purpose. It would have an authenticate method which determines whether the given password matches the user object.

Of course, I've only got a glimpse of what you are doing, so my suggestions may or may not make any sense.

share|improve this answer
    
Regarding #3: They don't have access to the file system but that doesn't mean they don't have accounts. The authentication for these users happens upstream from my website. I'm in a sub-organization. –  Levi Morrison Jan 5 '12 at 4:06
    
@LeviMorrison, then I wonder why you need to do any authentication. Can't you use the upstream authentication? –  Winston Ewert Jan 5 '12 at 5:01
    
I can't use upstream authentication for the file-system access because the hashes are invalid, and I don't have access to the upstream hash. –  Levi Morrison Jan 5 '12 at 14:44
    
Also, I don't do anything with the file system. The web interface simply provides a way to do things with passwords, such as password resetting. –  Levi Morrison Jan 5 '12 at 20:38
    
@LeviMorrison, so your system resets the passwords on backend filesystems for the users from the website? –  Winston Ewert Jan 5 '12 at 22:13
show 2 more comments

I think you need 2 interfaces:

  • IPasswordValidator : validates password / handles UI messaging about validation.
  • IAuthenticationScheme : handles the authentication / hashing / updating of passwords

IPasswordValidator probably only needs one concrete implementation but it is simple enough to put it behind an interface to document the contract at least. Testability is a bonus. IPasswordValidator covers your getViolations method.

IAuthenticationScheme is where the fun happens -- this is where you wire up your alternate hashing schemes and whatever data dependencies you have. In your original scheme this combines hasher and changer and your isCurrentPassword classes and methods.

Generation isn't really a concern covered in either, it would probably be best left to a utility library. If generation is implementation dependent -- such as if a certain provider had any particular requirements -- you could move generation into IAuthenticationScheme.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.