dev-xo / remix-auth-totp

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

[ Fix ] Cannot read properties of null (reading '0') #69

Closed kldeb closed 2 months ago

kldeb commented 2 months ago

This error only happens when deployed to AWS lambda but works fine locally in dev mode.

Seems to be happening in this call:

let result = await authenticator.authenticate("TOTP", request, {
    successRedirect: "/verify",
    failureRedirect: currentPath,
  });

Is it possible to debug somehow? I checked and the session and encryption secrets are deployed to the lambda.

dev-xo commented 2 months ago

Hello @kldeb. We haven't found that error before, although not sure if we deployed remix-auth-totp to AWS Lambda.

I assume it's a deployed site? Please let us know a few more details, as it's really hard to identify a possible issue. Also, we're not sure about debugging, as we will probably need to reproduce it first.

kldeb commented 2 months ago

I'm not seeing anything in the logs. I'll try to debug further.

kldeb commented 2 months ago

I think I tracked it down and it appears to be that thirty two package mentioned in the other open issue:

ERROR   generateSecret - error TypeError: Cannot read properties of null (reading '0')
    at isInsideNodeModules (node:internal/util:508:17)
    at showFlaggedDeprecation (node:buffer:178:8)
    at new Buffer (node:buffer:266:3)
    at exports.encode (file:///var/task/server.mjs:126959:21)
    at generateSecret (file:///var/task/server.mjs:147957:15)
    at TOTPStrategy._generateTOTP (file:///var/task/server.mjs:148184:53)
    at async TOTPStrategy.authenticate (file:///var/task/server.mjs:148110:49)
    at async action$6 (file:///var/task/server.mjs:149021:3)
    at async callRouteAction (file:///var/task/server.mjs:33110:16)
    at async file:///var/task/server.mjs:31001:22

this line

base32.encode(crypto.randomBytes(32)).toString()

specifically the base32.encode call.

alexfcosta commented 2 months ago

I'm having the same issue in my Remix app deployed using SST. It broke code that was working for months. It seems that a change on AWS is incompatible the TOPTStrategy.

INFO authError { message: "Cannot read properties of null (reading '0')" }

kldeb commented 2 months ago

PR here: https://github.com/dev-xo/remix-auth-totp/pull/70

kldeb commented 2 months ago

@alexfcosta - hello fellow remix/sst user!

I submitted a pr to this repo but I'm not sure about the other module involved (@epic-web/totp) because I broke the tests. In the meantime, you can use the code I'm attaching here in your project until this is sorted out. You'll have to add a couple of dependencies to make it work.

totp.zip

@dev-xo not sure how you want to handle this - I don't know if modifying the tests in @epic-web/totp will break other implementations.

alexfcosta commented 2 months ago

hi @kldeb I'll do some tests - I was very surprised as it stopped working on code I had running for months. Something changed on the AWS for the Lambda URL that is incompatible with the TOPTStrategy code.

I'll provide an update after I test

dev-xo commented 2 months ago

So, thirty-two was a common issue here, apparently. Happy you both got into this talk, @kldeb and @alexfcosta.

There is #70 opened by @kldeb that will probably solve this. Checking it currently, although we probably need some tests related to it, and I'm not sure if those are there yet.

kldeb commented 2 months ago

@dev-xo the issue won't be resolved until the @epic-web/totp package is also updated or some other solution. I prepared a change but it's breaking 2 of the tests. Not sure how you want to handle this but this issue won't be resolved until the change is made in both code bases.

dev-xo commented 2 months ago

Yeah I understand @kldeb. We will probably need to reach out to Kent by opening an issue in the @epic-web/totp package. We reached out before for a small update.

Currently, we are a bit busy, sadly. Not sure if you want to take the time to let him know about the possible fix. Otherwise, I can take some time and open an issue there later today.

kldeb commented 2 months ago

I created an issue @epic-web/totp. I was able to get all the tests working again except one.

markhker commented 2 months ago

Hi @kldeb Thanks for the fixes, and yes the changes in your PR will not be enough, I am running remix too in AWS with SST and it wasn't an issue before, but yeah the totp package by Kent needs to be fixed too for this to work.

dev-xo commented 2 months ago

Thanks for the efforts, @kldeb!

kldeb commented 2 months ago

Opened @epicweb-dev/totp PR: https://github.com/epicweb-dev/totp/pull/9

alexfcosta commented 2 months ago

I did some tests with the code from @kldeb and all worked fine for me now.