search

abitdodgy / cfw-usermanager

A user manager demo app for ColdFusion on Wheels
23 stars 11 forks source link

Compare function won't give expected results in all cases #4

Open snaquaye opened 12 years ago

snaquaye commented 12 years ago

The compare function returns -1 if this.password is less than hashPassword(arguments.password, this.salt), 0 if they are equal and 1 if the first case is reversed. this url shows that

http://help.adobe.com/en_US/ColdFusion/9.0/Developing/WSc3ff6d0ea77859461172e0811cbec09af4-7fd0.html

In Boolean expressions, True, nonzero numbers, and the strings “Yes”, “1”, “True” are equivalent; and False, 0, and the strings “No”, “0”, and “False” are equivalent.

Based on this, using the compare won't be the best thing to do.

abitdodgy commented 12 years ago

This is intentional. It's a double negative. We're testing for "if not 0", meaning if the strings don't match, then the password is wrong. So 1, -1, or whatever, does not make a difference. The only time we get a positive match is when the result is 0. We're evaluating for false. -1 and 1 will return true, and we're testing for "not true" or false, hence the "if ! Compare()".

I'm open to merging this if you think it's that important, but I think it's fine the way it is.

snaquaye commented 12 years ago

The reason why i choose the fixed was that implementing that on a project I'm working on refuses authentication even if the password was typed in correctly. If it works fine with you, then the merge won't be necessary.

Thanks