fix: totp code validation

[pozylon]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: b01244d

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]

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!