mikker / passwordless

🗝 Authentication for your Rails app without the icky-ness of passwords
MIT License
1.26k stars 85 forks source link

enumerate passwordless_sessions #169

Closed yshmarov closed 9 months ago

yshmarov commented 10 months ago

Currently, after claiming a session it is still possible to navigate to the claimed session URL like http://localhost:3000/sign_in/3.

It is also possible to navigate to something like http://localhost:3000/sign_in/334534543abc and get an error like

ActiveRecord::RecordNotFound in Passwordless::SessionsController#show
Couldn't find Passwordless::Session with [WHERE "passwordless_sessions"."id" = $1 AND "passwordless_sessions"."authenticatable_type" = $2]

Thus, a user can find how many sessions there were.

I would suggest:


Also I thinksign_in/:id will be a good candidate for captcha / rate limiting in a production app 🤔

mikker commented 10 months ago

Hey! Of the three (four?) what would you suggest as the simplest yet most effective improvement?

yshmarov commented 10 months ago

I think the best thing to do is always "use slugged session ids out of the box"

- /sign_in/:id_slug
- /sign_in/:id

Maybe that would mean adding an additional attribute to the session model like :slugged_id and having Session.find_by(slugged_id: params[:id])

Maybe this could be added as an optional feature... 🤔

yshmarov commented 10 months ago

Separately, something like this would prevent being able to view screens for the "old" sessions:

-@session = Session.find(params[:id])
+@session = Session.not_expired.not_claimed.find(params[:id])
mikker commented 10 months ago

Could we just use uuids for the slugs? Alternatively: Should the table default to uuids for primary keys instead?

yshmarov commented 10 months ago

Alternatively: Should the table default to uuids for primary keys instead?

Yesss! I think uuids are an even better idea!

mikker commented 9 months ago

Thank you for your help @yshmarov! 💙💛