SchedulesDirect / JSON-Service

Code related to download, slice-and-dice and generation of JSON into database.
36 stars 5 forks source link

Slim Application error on invalid password #46

Closed ronf closed 9 years ago

ronf commented 9 years ago

I'm trying to harden the error handling in my client code, so I tried intentionally sending an incorrect password hash when trying to retrieve a token. Instead of getting back a JSON error, I got back an HTTP 500 Internal Server Error, with the following output:

Slim Application Error The application could not run because of the following error:

Details

Type: ErrorException Code: 8 Message: Undefined index: userid File: /var/www/html/functions/func.20140530.php Line: 448 Trace

0 /var/www/html/functions/func.20140530.php(448): Slim\Slim::handleErrors(8, 'Undefined index...', '/var/www/html/f...', 448, Array)

1 /var/www/html/index.php(120): generateToken('ronf@timeheart....', '640df75d7165753...')

2 [internal function]: {closure}()

3 /var/www/html/vendor/slim/slim/Slim/Route.php(462): call_user_func_array(Object(Closure), Array)

4 /var/www/html/vendor/slim/slim/Slim/Slim.php(1326): Slim\Route->dispatch()

5 /var/www/html/vendor/slim/slim/Slim/Middleware/Flash.php(85): Slim\Slim->call()

6 /var/www/html/vendor/slim/slim/Slim/Middleware/MethodOverride.php(92): Slim\Middleware\Flash->call()

7 /var/www/html/vendor/slim/slim/Slim/Middleware/PrettyExceptions.php(67): Slim\Middleware\MethodOverride->call()

8 /var/www/html/vendor/slim/slim/Slim/Slim.php(1271): Slim\Middleware\PrettyExceptions->call()

9 /var/www/html/index.php(888): Slim\Slim->run()

10 {main}

rkulagowski commented 9 years ago

Can you try again please? You'll probably still get the same error, but I've got some logging turned up on the server that will let me see what's happening.

ronf commented 9 years ago

Sure - just tried it again now, and as you said I got pretty much the same error. In this case, I used 'xxx' instead of an actual hash of a bad password, but when I tried it earlier both produced the same error.

rkulagowski commented 9 years ago

I believe I found the bug; please give it another try.

ronf commented 9 years ago

Yes, thanks! I now get back an INVALID_USER (4003) error.

I actually tried 'xxx' instead of an incorrect but otherwise valid looking hash to see whether I'd get INVALID_HASH (4002) rather than INVALID_USER when I provided a value that was clearly not a hex SHA-1 hash string. Do you ever return that error today?

rkulagowski commented 9 years ago

We used to differentiate between invalid username vs. invalid hash, but that makes it to easy to find valid account names and then try to find the password. So now if the username or password is incorrect you'll get a 4003 error. "Invalid username or password."

ronf commented 9 years ago

I was thinking INVALID_HASH could be used in the case where the password string wasn't in the right format (40 hex digits), independent of whether the username is valid or not, so you wouldn't be giving that information away. The potential benefit here is someone who didn't read carefully that they need to send a SHA-1 hash encoded in hex might get a better idea what they did wrong.

That said, I'm fine with just having a single error here. You might want to update the error table at https://github.com/SchedulesDirect/JSON-Service/wiki/API-20140530 to reflect this.

rkulagowski commented 9 years ago

4002 will now indicate that the hash wasn't the correct length and will not be used to indicate the validity of the hash, only that the format wasn't correct.

4003 will indicate that either the username or password wasn't correct.

Error table in documentation has been updated.

ronf commented 9 years ago

Sounds great -- I just tested all three cases here (success, invalid hash, and valid but wrong hash) and they all worked as expected. Thanks!

rkulagowski commented 9 years ago

Great, then I will close this issue.