dev-xo / remix-auth-totp

A Time-Based One-Time Password (TOTP) Authentication Strategy for Remix-Auth.

Hey, I'm Daniel 👋🏻!

A Senior Software Engineer aiming to improve your Developer Experience.

You can find me on Twitter & Bento.

📍 Remote 𝕏 Spain
✨ Developing Experiences

remix-auth-totp's Issues

[ Feat ] 2FA Authentication Support

A work in progress is already underway to introduce support for 2FA authentication, leveraging the capabilities of @epic-web/totp.

Suggestions and PRs are welcome! 🙏

Wrapping authenticate in try/catch prevents redirect

Hi, I am seeing an issue that I think is specific to the TOTP strategy. but please let me know if that is incorrect.

I am getting intermittent Unhandled Promise Rejection errors for the deployed version of the app specifically during a login POST request. In order to be able to catch these errors, I would like to be able to wrap the authenticate call in a try/catch, as follows:

  try {
    await authenticator.authenticate(TOTP_STRATEGY, request, {
      successRedirect: route("/verify"),
      failureRedirect: currentPath,
      throwOnError: true,
    });
  } catch (error) {
    console.error("error: ", error);
    return json({
      authError: "Sorry, something went wrong.  Please try again.",
    });
  }

This is based on the recommended way to handle errors in the main auth docs.

However, if wrapping an authenticate call in try/catch there is no redirect to the verify route.

Any suggestions re. how to enable catching errors, while still also supporting a redirect to verify? Thanks for any tips or insights!

[ Refactor ] Decomplect handleTOTP() API

storeTOTP() and handleTOTP() seem to be CRUD operations.

handleTOTP() seems to combine Read and Update and doesn't do any "processing" as implied by "handle" in its name. It will need a bit of logic to determine if a Read will suffice or an Update is also needed. If handleTOTP() where split into getTOTP() and updateTOTP(), the understanding and implementation is dead simple and one does not have to pause to understand what a handleTOTP() does and how it should be implemented.

Removing the setting of expiresAt in handleTOTP() may simplify while making more robust.

• expiresAt is needed at Create time.
  • On Cloudflare, storing the TOTP in Cloudflare KV is an attractive option and one naturally wants to create the key with an expiration time. If the expiration is not known at Create time and set later, there is the potential to create garbage if handleTOTP() not subsequently called after creation due to unpredictable server failure.
  • With a database that has a column for expiresAt, the column would need to allow null's if expiresAt is not known at Create time. There is the potential to create garbage if handleTOTP() is not subsequently called after creation due to unpredictable server failure. The null handling also adds a little complexity to any batch script that cleans up expired totp's.
• remix-auth-totp does not need expiresAt from TOTP storage. expiresAt appears to be more like metadata at Create time. The application can chose what to do with it at Create time and need never return it to remix-auth-totp.

The proposal is to replace storeTOTP() and handleTOTP() with the following.

interface TOTP {
  hash: string;
  active: boolan;
  attempts: number;
}

createTOTP(totp: TOTP, expiresAt: Date): void;
getTOTP(hash: string): TOTP;
updateTOTP(hash: string, data: Partial<Omit<TOTP, "hash">>): void;

@dev-xo: please share your thoughts.

[ Feat ] Add `expiresAt` field to clean-up unused / expired OTPs from database.

The expiersAt database field in Legacy Remix Auth OTP package helped remove old, unused codes from database.

To invalidate a code, users had to enter it, but if at some point they quits the authentication flow, the code might stay active. If they later enters it, the system would deactivate it, but this didn't often happen when our users quits the authentication flow.

This database field will be reintroduced along with usage instructions for invalidating and cleaning up expired and unused codes in our database.

Can `maxAttempts` be optional in `TOTPGenerationOptions` interface?

First of all, thanks for your work on this project!

I am setting a different period for my TOTP generation, and noticed that doing so also required me to set the maxAttempts field. Is there a reason we have to manually set this field when overriding other fields in the TOTPGenerationOptions?

If there's a good reason that's fine! I will just continue to set to the default maxAttempts in that case.

remix-auth-totp/src/index.ts

Lines 19 to 57 in 1011fea

	export interface TOTPGenerationOptions {
	/**
	* The secret used to generate the OTP.
	* It should be Base32 encoded (Feel free to use: https://npm.im/thirty-two).
	*
	* @default random Base32 secret.
	*/
	secret?: string
	/**
	* The algorithm used to generate the OTP.
	* @default 'SHA1'
	*/
	algorithm?: string
	/**
	* The character set used to generate the OTP.
	* @default 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'
	*/
	charSet?: string
	/**
	* The number of digits used to generate the OTP.
	* @default 6
	*/
	digits?: number
	/**
	* The number of seconds the OTP will be valid.
	* @default 60
	*/
	period?: number
	/**
	* The max number of attempts the user can try to verify the OTP.
	* @default 3
	*/
	maxAttempts: number
	}

[ Request ] Allow the flow to continue when emails are invalid.

This is essentially a security feature where I don't want to reveal that the email is, in fact, invalid. In my case, I am currently checking the database prior to sending out the auth codes and I am doing so discreetly without informing the end user. However, I'd like to check the database prior to storing the auth codes as well so as to keep junk requests out of my system. There is no self sign-up in my flow so I'm not creating users via this process. By the time a user is authenticating, they have already been invited by an administrator and their email is in the system.

Because of this, attempting to get an auth code for a given email potentially reveals that the email is or is not in the system (I'm currently hiding this by pretending all emails are "valid") and populates the database with auth codes no matter what, even if emails are never sent. I'd like to discreetly stop the auth code population in the same way I'm stopping the email.

As validateEmail seems to be for throwing errors, exposing email to sendTOTP would satisfy my requirements as I could perform the db check before storing the auth code and continue the flow normally. Thanks for your consideration.

[ Feat ] Add support for Session ID rotation.

Hello!
First of all, great work done and I'm happy to join the community of people using this package! :)

I've been playing around with the package and I believe there could be a functionality for Session ID Rotation.

For example, when using Cloudflare KV, we store encrypted cookie in the user browser and the session in the KV. The session can have a maxAge let's say of 7 days. During these 7 days, I believe there should be a rotation of the session key in the cookie to prevent possible issues in case any person with bad intentions get access to the cookie and try to impersonate the user. For example, if the user login in a public computer and never open the page back. Then when the session is rotated, that token is no longer valid in that computer.

I don't know if is my paranoid mindset for security impacting me. I believe this could improve greatly the security.

What are your thoughts @dev-xo?

Thank you and hope to collaborate in the package in the near future!

[ Chore ] Simplify Tests

With PR #38, the tests have become longer and more complicated.

• Use typescript predicates and assertions instead of !
• Don't swallow errors eg .catch((error) => error)
• Test helper functions to pull out some of the boilerplate.
• Revisit block scoped tests within tests since they increase difficulty in reading

[ Feat ] Add support for different OTP sending methods.

hey @dev-xo !

Thanks for the amazing work!

my app requires to support both email and whatsapp for sending the otp. I see that to send the otp I can customize sendTOTP.

Is there a way to pass the function different parameters so I know which method to send the otp?

Thanks,
Chaim

[ Solved - Help ] How to reset verify state?

Hi, I've integrated your TOTP strategy and everything works great when following a normal verify flow.

However, once a verify code has been sent, it's not clear how to reset the login view to the default state, eg for entering a different email, or if there was an exception of some sort.

I tried both expiring the 'auth:email' cookie as well as destroying the current session, but neither appears to work. I am thinking it might be necessary to delete the corresponding totp record in the db but not clear where to do that.

Another alternative would be have verify be a separate route, which you alluded to in the docs. However, I wasn't clear on how one would split up login and verify into different routes.

Any tips or insights would be appreciated!

[ Feat ] Lack of Remix v1.0 Support Due to ESM-only Dependency

Currently, Remix Auth TOTP only supports Remix v2.0 and above.

The reason behind this is that one of the dependencies used is ESM-only, making it incompatible with Remix v1.0 by default. There's already a work in progress to address this, which will likely involve using dynamic imports.

Suggestions and PRs are welcome! 🙏

[ Refactor ] Store TOTP Data in Session Instead of Database

TOTPData consists of {hash, active, attempts}. With database storage, expiresAt is also stored in the database so that a clean up routine can delete expired totps.

Currently in session data, we store hash and expiresAt. To move the storage of TOTPData from the database to the session, we need only bring over attempts since active could be handled with expiresAt in session.

Impact of this change

• Slight increase in size of session data due to attempts
• Removes createTOTP, readTOTP, and updateTOTP from the API
• Eases application integration since a database is no longer needed and no need to figure out how to delete expired totps.
• The database seems to add little benefit from a security perspective since the TOTP hash is already stored in the session.

The implementation should consolidate all the TOTP data session keys under one session key to reduce top-level session vars.

@dev-xo: Please share your thoughts.

[ Feat ] Remove hostUrl from MagicLinkGenerationOptions

Remove hostUrl from MagicLinkGenerationOptions since it seems unnecessary and may even be a foot gun.

I am struggling to come up with a case where you would want to specify hostUrl explicitly because it would be different from the origin of the request url. If you do specify hostUrl and it is different from the origin of the request url, then the browser will not provide the right cookies to the magic link and you've shot yourself in the foot.

@dev-xo: Please share your thoughts and if you want to proceed, I can move it forward.

[ Feat ]: Support Cloudflare Runtime

Cloudflare wrangler reports an error when an Authenticator uses TOTPStrategy. The error seems to be reported during load time so the authenticator does not execute.

https://github.com/mw10013/remix-auth-totp-sandbox demonstrates the error.

• Created using pnpm create remix@latest --template remix-run/remix/templates/cloudflare-page
• Uses the latest versions of remix and wrangler
• remix.config.js needed serverNodeBuiltinsPolyfill to compile

serverNodeBuiltinsPolyfill: {
    modules: { crypto: true, stream: true },
    globals: {
      process: true,
    },
  },

• In package.json, added compatibility-flags option to start script for node:process

"start": "wrangler pages dev ./public --compatibility-date=2023-12-13 --compatibility-flags=\"nodejs_compat\""

• app/auth.server.ts has createAuthenticator() which returns an Authenticator with skeleton TOTPStrategy. It's non functional and contains just enough implementation to compile to check if any load issues. If the authenticator. use() statement is commented out, there will be no load error so it seems that loading the code for TOTPStrategy may be causing the error.

[ Feat ] Make TOTP_NOT_FOUND error message customizable

The error message for TOTP_NOT_FOUND is hard-coded with no option to customize.

TOTP_NOT_FOUND: 'Database TOTP not found.'

If the database record of the TOTP has been deleted, perhaps by a batch that deletes expired TOTP's, the error message may surface to the user and while technically correct, may not be so helpful to the user.

Allowing the application to customize the error message provides a simple way to surface a more user friendly error message. For example, 'Code is expired'. While not technically correct, it may be more useful to the user and the application gets to make that decision.

@dev-xo: Please share your thoughts and if you would like to proceed, I can get started on it.

[ Fix ]: Broke link on totp.fly.dev

Docs link is not working on https://totp.fly.dev/ when we click on docs it just opens https://totp.fly.dev/totp.fly.dev

[ Discussion ] Add support for different OTP sending methods.

To decouple the discussion within issue #47 i would like to start a discussion her.

Questions:

• How much code is email-related in the existing code base?
• What are possible methods to send a TOTP?

First: imho the only email relevant code is in the simple _validateEmailDefaults-Function. Beside that you could to a s/Email/Contact/g replace.

Second:

• Email needs an Email-Address
• WhatsApp needs a Phone Number
• SMS needs a Phone Number
• ...

What about removing the validation of the email in the above mentioned function and leave it up to the client as the sending of the totp is already left up to the client?

So you would just have to find a common field name (contact, address, contactInfo, destination, ...) and decouple the code completely from relaying on an email-Address for this field?

What do you think?

[ Feat ]: Add session flash message for new code requests.

Currently, it appears there is no feedback in the UI when a new OTP is requested, ie something like a "New Code Sent" flash message.

One way to enable this would be to set a session flash message once when a the TOTP is re-sent.

Thoughts?