Skip to content

Commit 5acecf7

Browse files
authored
Fix Default Time & Update Documentation (#161)
* Fix Default Time - [+] refactor(tests): update tests to use default TimeSource and Hash from config - [+] fix(config): change default TimeSource to use UTC time - [+] fix(otpverifier): update TOTPTime to return UTC time - [+] test(otpverifier): add test for AdjustSyncWindow function - [+] refactor(middleware): remove unused options from TOTP verifier config * Docs [pkg.go.dev] Update Documentation - [+] docs(otpverifier): clarify TOTPTime function documentation - [+] The TOTPTime function documentation is updated to provide a clearer explanation of the time zone used and the format of the returned time. It now specifies that the South Pole time zone is used and links to the relevant Wikipedia article for more information. The note is also updated to clarify that the returned time is always expressed in UTC to avoid ambiguity.
1 parent e45627e commit 5acecf7

File tree

6 files changed

+47
-24
lines changed

6 files changed

+47
-24
lines changed

2fa_test.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
//
33
// License: BSD 3-Clause License
44

5-
// Note: The tests here are outdated and won't work. It will crash because the focus is still on improving the internal implementation.
6-
75
package twofa_test
86

97
import (
@@ -243,6 +241,8 @@ func TestMiddleware_Handle(t *testing.T) {
243241
middleware := twofa.New(twofa.Config{
244242
Secret: secret,
245243
Storage: store,
244+
TimeSource: twofa.DefaultConfig.TimeSource,
245+
Hash: twofa.DefaultConfig.Hash,
246246
ContextKey: "user123",
247247
RedirectURL: "/2fa",
248248
CookieMaxAge: 86400,
@@ -268,7 +268,9 @@ func TestMiddleware_Handle(t *testing.T) {
268268

269269
// Generate a valid 2FA token
270270
totp := otp.NewTOTPVerifier(otp.Config{
271-
Secret: info.Secret,
271+
Secret: info.Secret,
272+
TimeSource: twofa.DefaultConfig.TimeSource,
273+
Hash: twofa.DefaultConfig.Hash,
272274
})
273275

274276
validToken := totp.GenerateToken()
@@ -277,7 +279,9 @@ func TestMiddleware_Handle(t *testing.T) {
277279
// Create a separate instance of the Middleware struct for testing
278280
testMiddleware := &twofa.Middleware{
279281
Config: &twofa.Config{
280-
Secret: secret,
282+
Secret: secret,
283+
TimeSource: twofa.DefaultConfig.TimeSource,
284+
Hash: twofa.DefaultConfig.Hash,
281285
},
282286
}
283287

@@ -1253,6 +1257,8 @@ func TestMiddlewareUUIDContextKey_Handle(t *testing.T) {
12531257
middleware := twofa.New(twofa.Config{
12541258
Secret: secret,
12551259
Storage: store,
1260+
TimeSource: twofa.DefaultConfig.TimeSource,
1261+
Hash: twofa.DefaultConfig.Hash,
12561262
ContextKey: contextKey,
12571263
RedirectURL: "/2fa",
12581264
CookieMaxAge: 86400,
@@ -1278,7 +1284,9 @@ func TestMiddlewareUUIDContextKey_Handle(t *testing.T) {
12781284

12791285
// Generate a valid 2FA token
12801286
totp := otp.NewTOTPVerifier(otp.Config{
1281-
Secret: info.Secret,
1287+
Secret: info.Secret,
1288+
TimeSource: twofa.DefaultConfig.TimeSource,
1289+
Hash: twofa.DefaultConfig.Hash,
12821290
})
12831291

12841292
validToken := totp.GenerateToken()
@@ -1287,7 +1295,9 @@ func TestMiddlewareUUIDContextKey_Handle(t *testing.T) {
12871295
// Create a separate instance of the Middleware struct for testing
12881296
testMiddleware := &twofa.Middleware{
12891297
Config: &twofa.Config{
1290-
Secret: secret,
1298+
Secret: secret,
1299+
TimeSource: twofa.DefaultConfig.TimeSource,
1300+
Hash: twofa.DefaultConfig.Hash,
12911301
},
12921302
}
12931303

config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ var DefaultConfig = Config{
221221
DigitsCount: otp.DefaultConfig.Digits,
222222
Period: otp.DefaultConfig.Period,
223223
Hash: otp.SHA512,
224-
TimeSource: otp.DefaultConfig.TOTPTime().UTC,
224+
TimeSource: otp.DefaultConfig.TOTPTime,
225225
SyncWindow: otp.DefaultConfig.SyncWindow,
226226
SkipCookies: nil,
227227
CookieName: "twofa_cookie",

internal/otpverifier/otpverifier.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,11 +440,11 @@ func (v *Config) cryptopowpow10(exponent int) uint64 {
440440
return result
441441
}
442442

443-
// TOTPTime returns the current time in the https://en.wikipedia.org/wiki/South_Pole time zone.
443+
// TOTPTime returns the current time in the South Pole (see https://en.wikipedia.org/wiki/Time_in_Antarctica) time zone.
444444
// It is used as the default time source for TOTP if no custom time source is provided (nil) and the sync window is set to -1.
445445
//
446446
// Note: The returned time is always expressed in UTC (Coordinated Universal Time) to avoid any ambiguity caused by local time zone offsets.
447447
func (v *Config) TOTPTime() time.Time {
448448
location, _ := time.LoadLocation("Antarctica/South_Pole")
449-
return time.Now().In(location)
449+
return time.Now().In(location).UTC()
450450
}

internal/otpverifier/otpverifier_test.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,40 +1268,54 @@ func TestHOTPVerifier_VerifySyncWindowWithResync(t *testing.T) {
12681268
Secret: secret,
12691269
Counter: initialCounter,
12701270
Hasher: otpverifier.Hashers[hashFunc],
1271-
SyncWindow: otpverifier.HighStrict,
1272-
ResyncWindowDelay: 50 * time.Millisecond,
1271+
SyncWindow: otpverifier.MediumStrict,
1272+
ResyncWindowDelay: 1 * time.Second,
12731273
}
12741274

12751275
verifier := otpverifier.NewHOTPVerifier(config)
12761276

12771277
// Generate a token for the current counter value
1278-
verifier.Hotp.At(int(initialCounter))
1278+
verifier.Hotp.At(int(initialCounter + 5))
12791279

12801280
// Verify this token should fail
12811281
if verifier.Verify("invalid") {
12821282
t.Errorf("Token should be invalid since the user entering invalid token (hash function: %s)", hashFunc)
12831283
}
12841284

12851285
// Generate a token for a counter value within the sync window
1286-
withinWindowToken := verifier.Hotp.At(int(initialCounter) + otpverifier.HighStrict)
1286+
withinWindowToken := verifier.Hotp.At(int(initialCounter) + otpverifier.MediumStrict)
12871287

12881288
// Verify this token should also pass
12891289
if !verifier.Verify(withinWindowToken) {
12901290
t.Errorf("Token within sync window did not verify but should have (hash function: %s)", hashFunc)
12911291
}
12921292

12931293
// Verify that the counter has been updated to the last verified counter + 1
1294-
if verifier.GetCounter() != initialCounter+uint64(otpverifier.HighStrict)+1 {
1294+
if verifier.GetCounter() != initialCounter+uint64(otpverifier.MediumStrict)+1 {
12951295
t.Errorf("Counter was not updated correctly after sync window verification (hash function: %s)", hashFunc)
12961296
}
12971297

12981298
// Generate a token for a counter value outside the sync window
1299-
outsideWindowToken := verifier.Hotp.At(int(initialCounter) + otpverifier.HighStrict + 3)
1299+
outsideWindowToken := verifier.Hotp.At(int(initialCounter) + otpverifier.MediumStrict + 6)
13001300

13011301
// Verify this token should fail
13021302
if verifier.Verify(outsideWindowToken) {
13031303
t.Errorf("Token outside sync window verified but should not have (hash function: %s)", hashFunc)
13041304
}
1305+
1306+
// Trigger the AdjustSyncWindow function
1307+
verifier.AdjustSyncWindow(config.CounterMismatch)
1308+
1309+
// Get the actual sync window size
1310+
actualSyncWindow := verifier.GetSyncWindow()
1311+
1312+
// Get the expected sync window range for MediumStrict
1313+
expectedRange := otpverifier.SyncWindowRanges[otpverifier.MediumStrict]
1314+
1315+
// Check if the actual sync window size falls within the expected range
1316+
if actualSyncWindow < expectedRange[0] || actualSyncWindow > expectedRange[1] {
1317+
t.Errorf("Expected sync window size to be within the range %v, but got %d", expectedRange, actualSyncWindow)
1318+
}
13051319
}
13061320
}
13071321

internal/otpverifier/totp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func NewTOTPVerifier(config ...Config) *TOTPVerifier {
4949
// The RFC 6238 (TOTP specification) does not define a specific synchronization window, but it is a common practice to implement it in TOTP systems to accommodate
5050
// for real-world scenarios where perfect clock synchronization is not always possible (e.g, traveling (in airplane), living in antartica, ISS-NASA).
5151
if c.TimeSource == nil && c.SyncWindow != 0 {
52-
c.TimeSource = DefaultConfig.TOTPTime().UTC
52+
c.TimeSource = DefaultConfig.TOTPTime
5353
c.SyncWindow = DefaultConfig.SyncWindow
5454
}
5555

middleware.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -301,16 +301,15 @@ func (m *Middleware) extractToken(c *fiber.Ctx) string {
301301
// verifyToken verifies the provided token against the user's secret and returns the updated Info struct.
302302
func (m *Middleware) verifyToken(token string) bool {
303303
totp := otp.NewTOTPVerifier(otp.Config{
304-
Secret: m.Info.GetSecret(),
305-
Digits: m.Config.DigitsCount,
306-
Period: m.Config.Period,
307-
SyncWindow: m.Config.SyncWindow,
308-
UseSignature: otp.DefaultConfig.UseSignature,
309-
Hash: m.Config.Hash,
310-
TimeSource: m.Config.TimeSource,
304+
Secret: m.Info.GetSecret(),
305+
Digits: m.Config.DigitsCount,
306+
Period: m.Config.Period,
307+
SyncWindow: m.Config.SyncWindow,
308+
Hash: m.Config.Hash,
309+
TimeSource: m.Config.TimeSource,
311310
})
312311

313-
if !totp.Verify(token, "") {
312+
if !totp.Verify(token) {
314313
return false
315314
}
316315

0 commit comments

Comments
 (0)