Make AuthenticationFailed more specific for bad headers/expired tokens

[edmorley]

Whilst many of the possible mohawk exception types should not be revealed directly in the AuthenticationFailed exception message (since they would give clues to attackers - eg let them determine valid client ids), there are some that are useful (and safe) to surface in the response to the client.

Fixes #14.

The existing tests for the "dangerous to reveal" cases have also been updated to ensure they exact-match against the string, since previously they were only checking for the substring (since assertRaisesRegexp() uses re.search()).

[kumar303]

good idea!

[kumar303]

Could you add some tests for the BadHeaderValue and TokenExpire cases that check the exception message? They don't have to be exact, something like

self.assertRaisesRegexp(
    AuthenticationFailed, 
    '^Hawk authentication failed: The token has expired .*')

These will serve two purposes: 1) they'll make sure we don't regress showing extra output in special cases and 2) they'll cover the else blocks in these cases so that no exceptions get raised by future refactoring.

[kumar303]

There are no existing tests for the bad header and bad timestamp cases or I would have updated those too

They should be easy to add. I'd suggest copying test_hawk_post_wrong_sig but use mock to patch Receiver() and give it a side_effect for each exception. Let me know if you get stuck. Because of how mock patching works you may need to edit the code to do receiver = mohawk.Receiver so you can patch 'mohawk.Receiver' (i.e. a direct reference).

[edmorley]

PR updated with tests :-)

I ended up not mocking Receiver in the end, since it was easy to do without, and this way we ensure that mohawk still raises those exceptions for the specific cases we expect.

[kumar303]

Awesome, thanks! The added tests look great.

[edmorley]

Thank you :-)

[edmorley]

@kumar303 - would it be possible to make a new release for this at some point in the next week or two? :-)