fix verifyTotp window asymmetry, add input validation

Author: nvms

packages/totp/README.md

@@ -20,10 +20,15 @@ const uri = Otp.createTotpKeyUriForQrCode("MyApp", "user@example.com", secret);
 
 ## Verification Window
 
+The third argument controls drift tolerance in both directions (behind and ahead):
+
 ```typescript
-// allow 1 step of drift (default)
+// 1 step of drift in each direction (default is 2)
 Otp.verifyTotp(secret, code, 1);
 
 // strict - no drift tolerance
 Otp.verifyTotp(secret, code, 0);
+
+// asymmetric - 2 steps behind, 0 ahead
+Otp.verifyTotp(secret, code, 2, 0);
 ```

packages/totp/src/index.test.ts

@@ -1,6 +1,6 @@
 import { base32 } from "@scure/base";
 import { describe, expect, it } from "vitest";
-import Otp, { InvalidHashFunctionError, InvalidOtpLengthError, InvalidSecretError } from "./index";
+import Otp, { InvalidHashFunctionError, InvalidIntervalError, InvalidOtpLengthError, InvalidSecretError } from "./index";
 
 describe("Otp", () => {
   it("should generate a secret of default strength", () => {
@@ -72,6 +72,13 @@ describe("Otp", () => {
     expect(uri).toContain("issuer=app.example.com");
   });
 
+  it("should encode issuer with special characters in URI", () => {
+    const secret = Otp.createSecret();
+    const uri = Otp.createTotpKeyUriForQrCode("My App/Service", "user@example.com", secret);
+    expect(uri).toContain("otpauth://totp/My%20App%2FService:");
+    expect(uri).toContain("issuer=My%20App%2FService");
+  });
+
   it("should throw an error for invalid secret length", () => {
     expect(() => {
       Otp.generateTotp("shortsecret");
@@ -229,6 +236,46 @@ describe("Otp - 6 Character Length", () => {
     expect(isValid).toBe(false);
   });
 
+  it("should throw for zero interval", () => {
+    const secret = Otp.createSecret();
+    expect(() => Otp.generateTotp(secret, undefined, 6, 0)).toThrow(InvalidIntervalError);
+  });
+
+  it("should throw for negative interval", () => {
+    const secret = Otp.createSecret();
+    expect(() => Otp.generateTotp(secret, undefined, 6, -30)).toThrow(InvalidIntervalError);
+  });
+
+  it("should use symmetric window when lookAheadSteps is not provided", () => {
+    const secret = Otp.createSecret();
+    const now = Math.floor(Date.now() / 1000);
+    const interval = 30;
+    const oneStepAhead = now + interval;
+    const totp = Otp.generateTotp(secret, oneStepAhead);
+    expect(Otp.verifyTotp(secret, totp, 0, undefined, now)).toBe(false);
+    expect(Otp.verifyTotp(secret, totp, 1, undefined, now)).toBe(true);
+  });
+
+  it("should allow asymmetric window when lookAheadSteps is explicit", () => {
+    const secret = Otp.createSecret();
+    const now = Math.floor(Date.now() / 1000);
+    const interval = 30;
+    const oneStepAhead = now + interval;
+    const totp = Otp.generateTotp(secret, oneStepAhead);
+    expect(Otp.verifyTotp(secret, totp, 2, 0, now)).toBe(false);
+    expect(Otp.verifyTotp(secret, totp, 0, 1, now)).toBe(true);
+  });
+
+  it("should be strict with window of 0", () => {
+    const secret = Otp.createSecret();
+    const now = Math.floor(Date.now() / 1000);
+    const totp = Otp.generateTotp(secret, now);
+    expect(Otp.verifyTotp(secret, totp, 0, undefined, now)).toBe(true);
+    const oneStepAgo = now - 30;
+    const oldTotp = Otp.generateTotp(secret, oneStepAgo);
+    expect(Otp.verifyTotp(secret, oldTotp, 0, undefined, now)).toBe(false);
+  });
+
   it("should verify RFC 6238 test vectors for SHA-1 with 6-character OTP", () => {
     // const rfc6238TestKeySha1 = "GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ"; // Base32 encoding of '12345678901234567890'
     const rfc6238TestKeySha1 = base32.encode(new TextEncoder().encode("12345678901234567890")).replace(/=+$/, "");

packages/totp/src/index.ts

@@ -5,6 +5,7 @@ export class InvalidOtpLengthError extends Error {}
 export class InvalidSecretError extends Error {}
 export class InvalidHashFunctionError extends Error {}
 export class InvalidSecretStrengthError extends Error {}
+export class InvalidIntervalError extends Error {}
 
 class Otp {
   static OTP_LENGTH_MIN = 6;
@@ -46,7 +47,7 @@ class Otp {
    * @throws {Error} - Throws an error if any of the parameters are invalid or missing.
    */
   static createTotpKeyUriForQrCode(issuer: string, accountName: string, secret: string): string {
-    return `otpauth://totp/${issuer}:${encodeURIComponent(accountName)}?secret=${secret}&issuer=${issuer}`;
+    return `otpauth://totp/${encodeURIComponent(issuer)}:${encodeURIComponent(accountName)}?secret=${secret}&issuer=${encodeURIComponent(issuer)}`;
   }
 
   /**
@@ -75,12 +76,14 @@ class Otp {
       throw new InvalidOtpLengthError();
     }
 
-    secret = secret ? secret : "";
-    t = t ? t : Math.floor(Date.now() / 1000);
-    t_x = t_x ? t_x : Otp.INTERVAL_LENGTH_DEFAULT;
-    t_0 = t_0 ? t_0 : Otp.EPOCH_DEFAULT;
+    if (t_x <= 0) {
+      throw new InvalidIntervalError();
+    }
+
+    secret = secret ?? "";
+    t = t ?? Math.floor(Date.now() / 1000);
 
-    const c_t = Math.max(0, Math.floor((t - t_0) / t_x)); // Ensure c_t is non-negative
+    const c_t = Math.max(0, Math.floor((t - t_0) / t_x));
 
     secret = secret.replace(/[^A-Za-z2-7]/g, "").toUpperCase();
 
@@ -144,13 +147,15 @@ class Otp {
     secret: string,
     otpValue: string,
     lookBehindSteps: number = 2,
-    lookAheadSteps: number = 2,
+    lookAheadSteps?: number,
     t: number = Math.floor(Date.now() / 1000),
     otpLength: number = Otp.OTP_LENGTH_DEFAULT,
     t_x: number = Otp.INTERVAL_LENGTH_DEFAULT,
     t_0: number = Otp.EPOCH_DEFAULT,
     hashFunction: number = Otp.HASH_FUNCTION_DEFAULT,
   ): boolean {
+    const ahead = lookAheadSteps ?? lookBehindSteps;
+
     otpValue = otpValue.replace(/[^0-9]/g, "");
 
     if (otpValue.length < Otp.OTP_LENGTH_MIN || otpValue.length > Otp.OTP_LENGTH_MAX) {
@@ -161,7 +166,7 @@ class Otp {
       return false;
     }
 
-    for (let s = -lookBehindSteps; s <= lookAheadSteps; s++) {
+    for (let s = -lookBehindSteps; s <= ahead; s++) {
       const expectedOtpValue = this.generateTotp(secret, t + t_x * s, otpLength, t_x, t_0, hashFunction);
       if (crypto.timingSafeEqual(Buffer.from(expectedOtpValue), Buffer.from(otpValue))) {
         return true;