daniel-de-wit / lighthouse-sanctum

Laravel Sanctum support for Laravel Lighthouse
MIT License
56 stars 9 forks source link

Improve email verification token security? #53

Closed wimski closed 3 years ago

wimski commented 3 years ago

Is this secure enough? https://github.com/daniel-de-wit/lighthouse-sanctum/blob/c1d5e7a6497c68fb33f8f75723a45f2c29b98070/src/Services/EmailVerificationService.php#L41

daniel-de-wit commented 3 years ago

It's the same as Laravel uses. Although we currently don't have an expiration in place..

image

https://github.com/laravel/framework/blob/9edd46fc6dcd550e4fd5d081bea37b0a43162165/src/Illuminate/Auth/Notifications/VerifyEmail.php#L77-L91

wimski commented 3 years ago

Maybe we can do something similar with expiration? Or at least use sha256 in combination with the app key instead of sha1. That is what is used for the signed route signature.

dennis-koster commented 3 years ago

Seeing as this is a Laravel specific package I would personally just inject Illuminate\Contracts\Hashing\Hasher and use that.

dennis-koster commented 3 years ago

You may want to consider adding some random token though, as in its current form the url is "guessable" and I could in theory verify someone else's email just by generating the URL myself. This is probably not a really big issue, but it could potentially lead to people starting to use fake emails, as verifying can be done without actually having entered an email address you own or even exists.