[ Feat ] Add support for Remix Auth v4

[CyrusVorwald]

this commit replaces remix session with cookies

[dev-xo]

Thanks @CyrusVorwald, looks good, and I appreciate the efforts.

A question I have for you:

• Why have all the JSDoc comments been removed? Also, have the refactors been done with some AI? As I know, those usually remove the JSDoc comments sometimes.

Same thing for some renames:

• Does the implementation follow Sergio's naming conventions, or are those based on own criteria?

Asking all this as I haven't been able to check the latest Sergio updates related to remix-auth@v4, so probably everything is good. Just wondering a few things in case you can share some insights on the decisions made.

In any case, I need to give it a test, but overall, it looks good, especially if you can confirm that the tests successfully pass.

[CyrusVorwald]

I used ai to help me understand the code and it removed the js doc. I can add them back in.

All tests pass, but do note I removed a couple of them that were context related.

[dev-xo]

It's okay, don't worry. I suspected that, as AI usually tends to remove the JSDocs. Sadly, those were a preference we had decided on some time ago for this repository/module.

Don't worry; I will need to check the implementation and test it myself too, refactoring those back if required.

[diecodev]

IMO is good to keep the comments, trying the latest version of rr and remix-auth pkg noticed this pkg is outdated, but the comments helped me out to understand what I need to do. ALso, great to see this is gonna be working with the latest rr version soon. Great Job!

[dev-xo]

Checked the implementation @CyrusVorwald, and the tests seem to pass, which is good. I will probably add the JSDoc comments back and do a small clean-up, and that may be it.

Two questions for you, in case you are up to share some insights:

• Do you know if the Remix implementation of the Strategy changes?
• Also, do you know how the Strategy would be implemented into RR7?

I think we may also need to update the docs a bit, but overall, I think the PR and the changes may be the correct ones.

[diecodev]

@dev-xo I tested the pkg in local, using rr7 and the magic links seems to be broken. the sendEmail fn works, but the url and the verification is not working, always receiving error=Missing+email+to+verify.+Check+that+same+browser+used+for+verification. in the totp cookie.

---

Note

I'm using the same browser and implementing your basic example
May be u wonder how I tested, I just forked the @CyrusVorwald repo and add an extra gh workflow adding pkg.pr.new, you can check it out here

[CyrusVorwald]

I haven't had the chance to implement it in my application yet, but it might be because the success and failure redirects are set in the strategy directly in the latest version of remix-auth. For this reason, I instantiated two separate strategies in the unit tests in cases there were multiple success or failure redirects.

[diecodev]

IMO, the success or error handling function should be something that user should write by his own, like in the github auth helper. Of course an example of basic handler for this auth would be great.

[dev-xo]

Gonna start by saying that, I'm not a big fan of having the whole Strategy refactored with AI, either sure how much of the work has been handled by it, as there were no comments anywhere, so I guess it handled whole files.

With that said, checked the code myself, cleaned it up, added the JSDocs, and overall, it looks good, also, tests pass, and those looked good too.

---

About the possible implementation:

As far as I understand, session will not be a thing anymore, and the user storage (previously Remix session) may be handled by the client (storing it in a cookie, based on the data the authentication returns).

I haven't been able to test the current implementation on a real app, and I'm also unsure how that should be set up either, although I guess it will be more or less the same as what we had before.

Will look a bit more into it, do some tests, and share some feedback.

Please, if you have the time and want to do the same, sharing some insights and so on, I would appreciate it, as getting this out would not only be great for us but probably for some other folks.

[CyrusVorwald]

I marked this as draft because it has not been tested in a real app

[dev-xo]

Suggest leaving the implementation as it is, @CyrusVorwald, as it looks already good, and I want to continue it myself based on your feedback and @diecodev one.

So please, leave it as it is, do not add new commits, and allow me to continue from here, as you already did a good job!

[diecodev]

@dev-xo JIC, I was trying to implement something based on what you got here but only for rr7, may be that could help you in something. check it out here.

Note

I'm not trying to create a npm pkg or something, I just used the sergiodxa/remix-auth-strategy-template to create a example to you. :D

[dev-xo]

Looks good, @diecodev! Does it already work? I haven't had the chance to give it a try yet. Hopefully, you can help us get this out based on your implementation and the current one!

Also, if you want to create a package from your implementation, feel free to do so! 👍🏻

[diecodev]

@dev-xo The README.md has the example of usage. I'm currently using that implementation in my SaaS (WIP) just bc I need server-side verification.

[CyrusVorwald]

Ah sorry, I only wanted to push that last commit to my account's version for my own purposes

[dev-xo]

All good @CyrusVorwald.

I will use my last commit on here to continue the implementation (I'm a bit busy right now). If you get a working one, based on your own criteria or @diecodev implementation, feel free to let me know.

I will also share some feedback on here the moment I am able to look a bit more into it.

[CyrusVorwald]

The last commit I force pushed works. I just added a function to update passed in cookies so that I could update the session cookie. I think it could be improved by removing the success and failure redirects but this works fine for me.

[CyrusVorwald]

Hey @dev-xo , I re-opened this pull request because I think this may now be in a good place to merge. I made a few changes after working with this for the past couple weeks.

• I removed my force push changes.
• The verify callback function now properly gets called and additionally handles the case that the verify throws a redirect, since cookies may not propagate in that instance.
• I added a emailSentRedirect because previously successRedirect was thrown both when the email gets sent and when the verification passes.

I have created a minimal example at https://github.com/CyrusVorwald/react-router-playground. It still needs a bit of cleanup to be immediately useful to someone else, but the general functionality is that the user enters email on login page, then the system generates the TOTP code and sends a magic link to their email, then the user verifies via code entry or magic link. Upon verification, the user is redirected to dashboard where they can logout. The user can resend an email as well.

[dev-xo]

Thanks for the efforts, @CyrusVorwald! I will review and give it a test. I'm a bit busy currently, but I will try to look into this as soon as possible.

Once again, thanks for the effort put into this!

[CyrusVorwald]

Hi @dev-xo , one more update - I added the expiry to the url params and encrypted the url params so that applications can display when the magic link expired to the UI even if it's being accessed by a different browser. I also updated https://github.com/CyrusVorwald/react-router-playground if you or @diecodev want to check out a working example.

[dev-xo]

Thanks @CyrusVorwald! I will look into the implementation this weekend, add some comments here and there (if required), and give your example a test!

Feel free to add some other small goodies if you have any idea in mind in the meanwhile, otherwise, simply leave it as it is if it already works!

Will try to look into it in the next few days, and if all looks good, we can simply merge. Thanks!

[dev-xo]

Been a bit busy @CyrusVorwald, and hope that didn’t slow you down in using the package somehow, or your own version of it.

Will take some time in the following days to look into it, although as far as I can see, it will be more of a review than anything else, which is kinda amazing.

Once again, hope not to slow you down with the wait, will try my best to look into it and hopefully merge the v4, helping others with the migration to RR7 and so on. Thank you!

[CyrusVorwald]

No worries @dev-xo, I am early in developing my product and have been using my local version.

It seems to work well in chrome and firefox, but safari doesn't store the cookies. I've only tried localhost so far.

[dev-xo]

Hmm, that's interesting, as we may also need to tackle Safari somehow. I'll look into it. Let me know if you find something useful that could help us tackle and support Safari as well.

Thanks, @CyrusVorwald!

[mackermans]

@CyrusVorwald In case it helps your Safari issue: it sounds like you could have the secure attribute enabled on the cookie, but Safari only allows secure cookies to be stored on HTTPS connections (assuming you're accessing localhost over HTTP).

https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#block_access_to_your_cookies

[dev-xo]

All right, starting to look into the whole PR @CyrusVorwald. Will share some feedback here or simply merge after some clean-up.

Also, had a look at your Playground Repository, and if you are up for it, we can add it as part of the "examples" remix-auth-totp has, in this case with Postgres, Drizzle, etc.

---

Another thing would be to check if everything works as expected on Safari. Not sure if you have been able to look into it a bit, into what @mackermans suggested, although I will personally look into it myself too and share some feedback.

[dev-xo]

Lovely playground and Strategy implementation @CyrusVorwald, thank you so much!

I've noticed that in _auth.verify.tsx (loader), we also decrypt the token using the same method the strategy provides from jose Check it here - Loader.

Before, the magic link was simply returning the decrypted code, and by simply calling the authenticator (along with the request object), we were able to successfully complete the authentication, as the code was already decrypted in the URL.

• Do you know if not encrypting the magic link code in the URL could create any vulnerability?

We also seem to check for the URL token expiration and perform some validation in the loader, something that we could simply leverage the Strategy to handle (as it already does, if I'm not wrong).

• Is there a specific reason to also perform loader validation on the token, which also requires the token decryption? Is it to handle errors more effectively somehow?

---

As an alternative, in order to allow the developer handle decryption, it would be great to export a decrypt method directly from remix-auth-totp.

The intent of that is to avoid adding extra dependencies for the end user, rather than only remix-auth and remix-auth-totp. Gonna look into how we can handle this in a comfortable way, so any ideas or feedback would be much appreciated.

Again, the Strategy implementation looks really good, and the playground repository you showcased was a breeze to follow, so thank you so much!

[dev-xo]

An issue we seem to have is with Safari; it does not bypass the /login screen. The server is successfully being called, sadly, the session is either not properly set or not set at all.

The secure option is set for the cookie, in the same way as httpOnly, as @mackermans mentioned.

Any idea or feedback you guys can share, @CyrusVorwald, @mackermans? To test this, the Playground Repository from @CyrusVorwald can be a good starting point.

• React Router Playground from @CyrusVorwald

I also created the following minimal RR7 repository and invited you both in case you want to give it a test and share some feedback:

• Remix Auth TOTP V4 - Repository

[dev-xo]

Seems like removing the secure property from the cookie (in the same way Sergio does in OAuth Strategy) successfully allows setting the cookies on Safari too.

Secure: Indicates the cookie should only be sent over HTTPS (not the case for localhost).

As a implementation, we could pass a custom cookie, and depending on whether we are in production or not, handle secure properly (Will look into it).

For now, we are able to also handle Safari, and this should be more than ready to be released.

[dev-xo]

This should be ready for release. It just needs the docs updated a bit, but overall, it should be more than ready.

RR7 public example here: https://github.com/dev-xo/remix-auth-totp-v4

[mackermans]

@dev-xo Apologies, I couldn't find the time these past few days to look at the test repo you had set up, but really appreciate your work here! 🙌

[dev-xo]

It's already merged, @mackermans, so if you have the time to give it a test on your own apps or anything else, let me know!