RobThree / TwoFactorAuth

PHP library for Two Factor Authentication (TFA / 2FA)
MIT License
1.08k stars 128 forks source link

Slimming down the lib further #121

Closed NicolasCARPi closed 6 months ago

NicolasCARPi commented 6 months ago

So I was looking at: https://github.com/RobThree/TwoFactorAuth/blob/086a3758ec947da9a8680fe72cd5aba0b762bfa3/lib/TwoFactorAuth.php#L175-L189

And I wondered: in which case would anyone use something other than PHP's own random_bytes() function?

Before hacking away at the code, I wanted to open an issue to discuss it. IMHO, the openssl_random_pseudo_bytes can go away, along with the hash one, which should definitely go as it's not cryptographically secure.

Let's rely on the good work done by PHP devs to give us this CSPRNG function, while still leaving the door open for anyone to provide their own, of course, no need to lock it down. But no need to support anything else than the gold standard, don't you think?

TwoFactorAuth being a security related library, making the code smaller leaves less chances for bugs, and alleviates the maintenance, too!

RobThree commented 6 months ago

This is a leftover from the PHP5 era ~I guess~. I don't mind if the openssl & hash ones get trashed. We stopped supporting PHP5 long ago. I would like to keep the interface because some people may be using their own implementations (I can see people using a hardware RNG for example).

NicolasCARPi commented 6 months ago

@RobThree See #122 then. And yes, users can still swap out the rngprovider for their own, if they need to.

RobThree commented 6 months ago

Dangit! I was removing them too, you beat me to it (and did a better job).

Screenshot 2024-04-15 at 23 37 08

👊

(Editted because I drag & dropped the wrong screenshot 🤪 I'm an idiot 🤦 )

willpower232 commented 6 months ago

while you're at it, might be worth cleaning up all the unreachable code in codeEquals as hash_equals is PHP >= 5.6 so definitely always exists

NicolasCARPi commented 6 months ago

@willpower232 done, good spot!

When I'm done with this lib it'll be a 3 lines function :p

willpower232 commented 6 months ago

wait till they announce it as a native library or something haha