wstrange / GoogleAuth

Google Authenticator Server side code
BSD 3-Clause "New" or "Revised" License
1.04k stars 327 forks source link

Enhance the GoogleAuthenticator.authorise() to support scratchCodes? #14

Closed xhh11 closed 9 years ago

xhh11 commented 9 years ago

This will require that the interface ICredentialRepository to include a method: String getScratchCodes(String userName)

It is natural to have save and get in the same class/interface.

Alternatively, we can add a method GoogleAuthenticator.authoriseWithScratchCodes() and keep authorise() unchanged.

senthilkumarkj commented 9 years ago

Would it be safe to treat scratch codes like passwords? Instead of getScratchCodes(String userName), we can say, verifyScratchCode(userName, inputScratchCode). That way you can store the scratch codes in hashed form and need not do encryption that we need to do for secretKey.

senthilkumarkj commented 9 years ago

I had a requirement, where I got to list the scratch codes, later sometime. Had to store it encrypted. :|

emcrisostomo commented 9 years ago

Hi @xhh11, we'll think about it.

emcrisostomo commented 9 years ago

Hi @senthilkumar-kj, scratch codes come directly from random material: once you get them, it's up to you to safely store them somewhere, there's no way to "verify" them as you verify a TOTP password. TOTP passwords are completely different in nature: they are not random at all, they are the result of a calculation which takes the private key as one of the parameter.

In fact, scratch codes are better compared with the secret key: once you generate it, it's up to you to safely store it somewhere and use it when necessary.

If you need to store the scratch codes encrypted, then why don't you just encrypt them and store them in your repository of choice?

However: scratch codes are not a feature specified by the TOTP RFC: it's a provider-specific "safety net". I added it because Google's implementation does it, but in retrospect I think it's not a benefit, but a liability: a TOTP library such as this should not worry about scratch codes at all.

Scratch codes support, in fact, it's barely sufficient as is. For example: what happens when you run out of scratch codes? This library offers no way to generate new ones. Let alone application-specific policies such as scratch codes expiration, scratch codes maximum number of use, etc.

senthilkumarkj commented 9 years ago

Hi @emcrisostomo, I agree with you and I understand. 😊 The verify part will be part of the application. I initially thought it will be same as verifying password, so I had it stored hashed. Then there was a requirement to list the scratch codes for the user later. So I had to change my impl to save it encrypted.

I didn't want anyone to read my first comment and save it hashed. So I thought I would correct it. Hence the second comment.

yidongnan commented 9 years ago

I also want to enhance the GoogleAuthenticator.authorise() to support scratchCodes

emcrisostomo commented 9 years ago

Hi @yidongnan, I understand the interest in this feature but I sincerely believe it does not fit into this library. Besides scratch codes not being part of the TOTP RFC, the only way to `integrate' this feature that I devise would be providing a callback for library methods to call during the authentication phase. Here, the problems I see are many: