fix: totp code validation

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: b01244d7b06909b27f58267d921a89a3a4f058f5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

| Name | Type | | -------------------- | ----- | | @accounts/password | Patch | | @accounts/two-factor | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pozylon]

Fixes #1227

[pradel]

@pozylon thanks for the pr!

A couple of changes are needed for the ci to pass so I can merge and release:

• do not change the version field, it will be changed automatically by the ci when we release the next version
• revert the change made to packages/password/package.json, same the ci will take care of it
• can you please add a changeset explaining the change and which packages needs to be bumped
• run pnpm install to update the lockfile of the repo and commit it

[pozylon]

@pradel Done.

[pradel]

@pozylon looks like with the upgrade, tests are failing

[pozylon]

@pradel The new version of speakeasy started to throw a custom exception when the token length did not match, I guarded it now so tests of two-factor passed locally but let's see what the workflow says.

[codecov[bot]]

Codecov Report

Merging #1228 (b01244d) into master (60d88ed) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1228   +/-   ##
=======================================
  Coverage   95.31%   95.32%           
=======================================
  Files         106      106           
  Lines        2391     2396    +5     
  Branches      511      503    -8     
=======================================
+ Hits         2279     2284    +5     
  Misses        103      103           
  Partials        9        9           

Impacted Files	Coverage Δ
packages/two-factor/src/two-factor.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 60d88ed...b01244d. Read the comment docs.

[pradel]

@pozylon this looks like a breaking change to me, some people may be catching some errors to return specific error messages to users, could we rather fix the tests or make this specific error silent?

[pozylon]

@pradel It's not breaking because version 1.3.1 of speakeasy did not throw any exception during token validation, https://github.com/Levminer/speakeasy/blob/1.3.1/main.js (before) vs https://github.com/Levminer/speakeasy/blob/1.3.3/main.js (now)

[pozylon]

@pradel So to outline the changes in speakeasy, it's 3 things:

1. throws now if token length mismatch, did not throw before
2. throws now if token is not a number, did not throw before
3. only returns the delta in verifyDelta when there is a code match, did also return a delta in "else" thus validating almost all codes as true

exports.hotp.verifyDelta = hotpVerifyDelta = (options) => {
    let i

    // shadow options
    options = Object.create(options)

    // unpack options
    let token = String(options.token)
    const digits = parseInt(options.digits, 10) || 6
    const window = parseInt(options.window, 10) || 0
    const counter = parseInt(options.counter, 10) || 0

    // fail if token is not of correct length
    if (token.length !== digits) {
        return
    }

    // parse token to integer
    token = parseInt(token, 10)

    // fail if token is NA
    if (isNaN(token)) {
        return
    }

    // loop from C to C + W inclusive

    // this is a temporary fix becaouse wrong codes return a value too
    // FIX THIS LATER
    // eslint-disable-next-line no-unreachable-loop
    for (i = counter; i <= counter + window; ++i) {
        options.counter = i
        // domain-specific constant-time comparison for integer codes
        if (parseInt(exports.hotp(options), 10) === token) {
            // found a matching code, return delta
            return { delta: i - counter }
        } else {
            return { delta: i - counter }
        }
    }

    // no codes have matched
}

exports.hotp.verifyDelta = hotpVerifyDelta = (options) => {
    let i

    // shadow options
    options = Object.create(options)

    // unpack options
    let token = String(options.token)
    const digits = parseInt(options.digits, 10) || 6
    const window = parseInt(options.window, 10) || 0
    const counter = parseInt(options.counter, 10) || 0

    // fail if token is not of correct length
    if (token.length !== digits) {
        throw new Error("@levminer/speakeasy - hotpVerifyDelta - Wrong toke length")
    }

    // parse token to integer
    token = parseInt(token, 10)

    // fail if token is NA
    if (isNaN(token)) {
        throw new Error("@levminer/speakeasy - hotpVerifyDelta - Token is not a number")
    }

    // loop from C to C + W inclusive
    for (i = counter; i <= counter + window; ++i) {
        options.counter = i
        // domain-specific constant-time comparison for integer codes
        if (parseInt(exports.hotp(options), 10) === token) {
            // found a matching code, return delta
            return { delta: i - counter }
        }
    }

    // no codes have matched
}

[pradel]

Okay thanks for the explanation, let's go with it then!