LaunchPadLab / token-master

Minimal and Simple user management for Ruby and Rails applications.
MIT License
17 stars 1 forks source link

Add ability to resend instructions #52

Closed francirp closed 6 years ago

francirp commented 6 years ago

send_x_instructions! errs out (intentionally) when the instructions have already been sent. However, we commonly need to implement a "resend invitation" feature since recipients often miss the invitation the first time around.

I can think of two approaches to this "resending" functionality:

  1. Extend the existing token: set x_created_at to Time.now
  2. Reset the token: generate a new one and update x_created_at, etc.

Perhaps we actually implement the following two methods:

extend_x_token!
send_new_x_token!

Thoughts @inveterateliterate?

inveterateliterate commented 6 years ago

@francirp, I definitely agree with this feature request. I'm surprised we missed it (or maybe we talked about it and punted for a later version, I don't remember).

I don't have any questions about the send_new_x_token! method.

However, for the extend_x_token! are you suggesting it updates the time and just accepts a block (e.g., to send another email or whatever the user wants to do?) . Would this maybe be more like resend_x_token!? Or does that lose the context that this is the same token getting extended? What are the use cases for extending a token rather than sending a new one?

francirp commented 6 years ago

@inveterateliterate well, if you send a new token, it effectively disables prior instructions that are already in the user's inbox. The thought is that by extending the token, you could send another email which would just have the same token. Thus both instructions would be valid rather than just one.

I'm sure there is a reason why this is an anti-pattern that should be avoided, but just putting it out there.

inveterateliterate commented 6 years ago

It does feel like an anti-pattern to me, I feel like the standard is to send a new token and you have to use that email. Feels like a security thing? @dpikt any thoughts?

dpikt commented 6 years ago

I don't know enough about token auth to say whether it's a security concern. I do think generating a new token for each email is pretty standard though.

francirp commented 6 years ago

Sounds good. For now let's just focus on the "resend_x_instructions" concept and we can always circle back to the "extend token" idea in the future if it comes back up.

inveterateliterate commented 6 years ago

My thought process is that resend_x_instructions! generates a new token (calls set_token!) then sends instructions with a block (calls send_x_instructions!). The set_token! method actaully sets the x_sent_at column to nil, preventing an already_sent error.

For reference, the already_sent error is looking at the x_sent_at column, so other approaches include separate methods for simply resending the same token with an updated x_sent_at column, and for extending a token by updating the x_created_at. These two can also be combined. However, for now, neither of these are being pursued at this time.