Add docs about why not to use sessions for storing the secret

[samwilson]

The docs say to generate a secret and save it to the user's account, and then show the QR code to the user. The example does this by saving the secret to the session, but also says to not do so in production. I'm just wondering why not?

I guess the recommended flow is to

1. generate the secret
2. save it to the user's account (e.g. in a database)
3. show the QR code and request a verification key
4. check that the verification key matches, and then mark the secret as verified

Is that about right? I guess I'm confused about the data storage requirements, because it seems like it'd be neater to save the secret to the session, and then only after getting the verification key save it to the database, because at that point we know the user has set up the TOTP app with the new secret.

[ChristianRiesen]

The major problem is that session storage (even if you use a DB below) is by design simple and can be "leaky". So in short, it's not very secure and should not be used to store critical things, such as a password, and the secret is kind of a password. Sessions are also ephemeral so technically it can vanish between loading of pages. Relying on them for critical functionality is not an ideal design :)

It would make more sense to store it with the user (in the database) but have a flag if it was verified or not. Then on verification you update that.

In the applications I use this, it's a seperated table, and users can technically have multiple MFA entries, so you can switch from one to another, while not deleting the old one until the new one is not yet confirmed.

But this library just makes suggestions, you can use it however you want. My aim is to to solve the complicated bits in code and give some inputs on best practices on how to integrate it from there. I see some other libraries that do already integrations using this library, check the packagist list of packages that depend on this perhaps if you want an easier time with it. I have not checked any of them for best practice usages though, so YMMV.

I'll see if I can come up with some better text there to explain it in more detail.

[samwilson]

Okay, great, that makes it clearer, thanks! Yeah, I think maybe just bit more of this detail in the main docs would be great (although I totally understand if it's just me being a bit slow on the uptake!). :)

Also, talking of leaky things: isn't the QR method here sending the secret to Google? I think I'm going to do this in-app with a QR code generator.

Thanks for maintaining this package by the way! :-)

[ChristianRiesen]

Absolutely, the QR code example is bad. I'll be removing that thing entirely in the future and suggest using other QR code libraries of which there are now a myriad of good ones. Back then that wasn't so much the case :)

And no worries about "slow uptake", this whole field of security is an opinionated mine field where what I said is going too far for some and not far enough for others. I'd always error on the side of the more paranoid, then worst case it's more secure than needed :)

I'm still having a large refactoring open for OTP to bring it up to speed with PHP 7 etc, so there will be some changes hopefully soonish.