Getting to 1.0

[mikker]

This PR marks the beginnings of working towards a 1.0 of Passwordless.

There will be breaking changes.

Plans

1. Remove isolate_namespace
   • This prevents the weird behaviour of having to use users.sign_in_path and even weirder main_app.whatever_path.
2. Store hashed tokens.
   • Work has already gone into this but now that we are already introducing breaking changes, it will be somewhat easier.
3. Fix multiple resources
   • Due to how the default views are made, multiple resource (eg. passwordless_for :users and :admins) is broken
4. Add new input token manually screen when signing in.
   • Currently Passwordless doesn't give a clue whether a resource (User) record was found when logging in. This is so malicious users cannot check whether a given email exists in the system. However, it is very likely that user records have uniqueness constraints on the email field so the malicious can just go try signing up with the email instead of signing in and get the answer. This leaves us with a worse user experience and without any real "security" benefits.
   • Also, if users are logging in on one device but checking their email on another, they have to transfer the link somehow.
5. Make tokens shorter in length and in timeouts.
   • Shorter tokens are easier to type but also "easier" to brute force. However, a malicious user would have to know and be ready to brute force within the shorter token timeout. Adding the delay of BCrypt hashing tokens before checking, this is as good as impossible.
6. Remove deprecated and old upgrade code
7. More…
   • I'd love to hear any ideas or suggestions. Now is the time for the sweeping, possibly breaking changes.

[xdmx]

Thank you Mikkel for your continuous work on this gem, it's one of my favorite.

I love and really look forward having tokens hashed

I've always felt weird that the sign in would accept any email and say that if you're there you may receive an email. I understand the reasons behind it, but from a UX point of view it's not that great

Some food for thoughts:

• have you thought about having a token like what used nowadays? For example Notion uses a 4 words token that you have to enter to sign in, and Slack and others I think it's a numerical token to enter. I'd expect that they last only few minutes, but it may be enough for people to add them. That could be in addition to the current link, with the difference that they could click the link, or enter the words (useful if they need to sign in from another device). I guess this is for the "shorter tokens", my advice is to use words or numbers instead of random chars like dEW23dGRfsIRwef
• I think it could be beneficial having some kind of brute force in place, to only send one email every X minutes (or maybe allow just 2 and then add a delay/wait time). This would allow people to send an email, resend it if needed, but it'd block someone from sending/spamming someone with hundreds/thousands of emails
• another thing that could be useful is to support ajax calls on the sign in for those interested of not rendering a different page (but showing an inline message, especially if it allows to paste the token and continue like Notion), and/or the ability of "extending" it though callbacks
• I haven't tried/double check, but now the sign in confirmation is on a render on the create, it could be worth checking the behavior on page refreshes (I suppose it asks the user to submit the data again), not a big deals, but something I was thinking about the other day

[mikker]

Thank you @xdmx!

1. have you thought about having a token like what used nowadays […]:

   • Yes, this was what I was trying to describe in point number 4 (_ Add new input token manually screen when signing in._).
     This is the approach that I'm using in an actual app, gerating 6 chars, only upper case and numbers:

   class AuthToken
     CHARS = ('A'..'Z').to_a + (0..9).to_a.map(&:to_s)
   
     def self.generate
       Array.new(6).map { CHARS.sample }.join
     end
   end

2. Rate limiting code requests is a very good idea 👍

3. I don't think we should add more complexity to the views or the controller. It's not very hard to add your completely own, custom controller and view code if you have special needs, like AJAX responses and such. I'd rather provide examples of custom code than include actual implementations that we'd just have to then support going forward.

4. Yes, the render on create is sort of weird and messes with reloads. I'm not sure how big a problem it is outside development but it is sure a bit annoying when testing the create action.

[rickychilcott]

Having official support for system and controller/request specs (started in #71) would be really nice for 1.0

[rickychilcott]

I can’t think of major changes that would be needed. But 1.0 should be turbo compatible. I think that would only mean form errors may need to be handled differently.

[nickbayley]

I can’t think of major changes that would be needed. But 1.0 should be turbo compatible. I think that would only mean form errors may need to be handled differently.

I was trying out Turbo with my app the other day. I didn't spend a whole lot of time on it, but I did get the following error when trying to request a passwordless sign on:

Error: Form responses must redirect to another location

I do use custom templates for my session/new and session/create, but they don't fundamentally change anything. I can get round this by adding data: { turbo: false } to my form_with, but obviously then I don't make use of Turbo.

[duarme]

Protect from open redirect vulnerability

How can the open redirect vulnerability be avoided?

Probably, by making sure that passwordless_query_redirect_path cannot redirect to other websites.

In the rare instance of a multi-domain application, a passlist can be issued via a config param.

It seems to me that destination_path should accept only a path to the same domain, but here:

passwordless/app/controllers/passwordless/sessions_controller.rb

Line 77 in 484e05d

query_redirect_uri.to_s if query_redirect_uri.host.nil? || query_redirect_uri.host == URI(request.url).host

Are we sure that host can only be the application host?

[duarme]

Allow the user to update their email address

It's currently not possible for users to update their email addresses. For any long-lived application, this is a required feature. With @rickychilcott we're thinking of the best approach here: #106

[hibachrach]

Regarding the Turbo issue: I believe responding with a 422 (Unprocessable Entity) should address it (based on this comment hotwired/turbo-rails#12 (comment))

EDIT: ah sorry, this only applies for error scenarios

[xdmx]

@mikker what is missing to bring this to the finish line and release a v1 with these changes?

Would you consider releasing a v0.11 with these changes and then separately add the remaining ones (shorter tokens and sign in message) and then release a v1?

Following the semver a 0.x it would be acceptable to change something that could break (like the routes and hashed tokens) as it's still in development:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.