dev-xo / remix-auth-totp

A Time-Based One-Time Password (TOTP) Authentication Strategy for Remix-Auth.
https://totp.fly.dev
MIT License
418 stars 28 forks source link

[ Feat ]: Pass email to storeCode function. #16

Closed CanRau closed 1 year ago

CanRau commented 1 year ago

Hey thanks a lot for this fantastic strategy 🙏🏼

As I'm migrating my existing system back over to Remix and improving auth with remix-auth on the way I'd like to have access to the provided email address when storing the code in my db, I have 2 tables, login_session and login_token, and I store the email address in the token table as well, I guess I could update the token in the sendCode step, tho I think it'd be nicer to just do it in storeCode and the code looked straight forward to pass it along as a second argument, this way, instead of providing both in one or sthg, it doesn't break existing implementations.

Let me know what you think and/or if I missed anything.

Update: Also got a question, how/where could I best store additional information to redirect the user dynamically back to where they came from? form in the sendCode and verify functions seems to be always an empty object {}

dev-xo commented 1 year ago

Hello @CanRau. First of all thank you for your contribution. Would appreciate if the next time you can open an issue and discuss this with me.

About the request, the Strategy has to be as simple as possible to not introduce leaks into our system. If you need the user email, you can get it from the verify method, that returns email, code, magicLink, form and request.

An example of it:

authenticator.use(
  new OTPStrategy(
    {
      // We've already set up this options.
      // ...
    },
    async ({ email, code, magicLink, form, request }) => {
      // Do something with the user email.
      // ...

      // Get user from database.
      let user = await db.user.findFirst({
        where: {
          email: email,
        },
      })

      if (!user) {
        // ...
      }

      // Return user as Session.
      return user
    },
  ),
)

Let me know if this is what you was looking for. Closing this for now, I'm here in case you need some help with it.

CanRau commented 1 year ago

Yea sorry for opening the PR like this though it seemed so simple that I thought why not, which could show you directly what I mean and you can just close it if you don't like it, could have mentioned that before sorry 🙏

Hmm maybe I don't yet fully get the complete flow, the idea was to store the email along the token in my db, but maybe it's unnecessary, also what I have right now kinda logs the user in just by entering the email address without actually clicking the link or entering the code and also I'm kinda confused why the code input is not showing even though I copied basically your login form. I guess I need to review my strategy config though I'm really tired so better tomorrow with fresh mind 🥴😅

Have a good one and again sorry for not discussing beforehand 😬

dev-xo commented 1 year ago

Nothing to worry about @CanRau! I'm okey with you opening a pull request directly!

And yeah, the behavior your app has does not looks like the correct one, user should not be logged instantly without entering the otp code, or clicking the magic-link url. Feel free to check the demo behavior!

About your request to store the email into database. You can still do that in the verify method from before, instead of storing it, you can update your user model, with the email returned from it.

Not sure if you need the email in the early stages of your authentication, but getting it at some point later could help you, and will not compromise any Strategy defined internal functionality.

Feel free to share with me your repo / code, maybe I could help you to successfully implement it! Take some rest for now, and look into it tomorrow! 🙂