Skip to content

Commit

Permalink
Fix Default Time & Update Documentation (#161)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
H0llyW00dzZ authored Jun 7, 2024
1 parent e45627e commit 5acecf7
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 24 deletions.
22 changes: 16 additions & 6 deletions 2fa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
//
// License: BSD 3-Clause License

// Note: The tests here are outdated and won't work. It will crash because the focus is still on improving the internal implementation.

package twofa_test

import (
Expand Down Expand Up @@ -243,6 +241,8 @@ func TestMiddleware_Handle(t *testing.T) {
middleware := twofa.New(twofa.Config{
Secret: secret,
Storage: store,
TimeSource: twofa.DefaultConfig.TimeSource,
Hash: twofa.DefaultConfig.Hash,
ContextKey: "user123",
RedirectURL: "/2fa",
CookieMaxAge: 86400,
Expand All @@ -268,7 +268,9 @@ func TestMiddleware_Handle(t *testing.T) {

// Generate a valid 2FA token
totp := otp.NewTOTPVerifier(otp.Config{
Secret: info.Secret,
Secret: info.Secret,
TimeSource: twofa.DefaultConfig.TimeSource,
Hash: twofa.DefaultConfig.Hash,
})

validToken := totp.GenerateToken()
Expand All @@ -277,7 +279,9 @@ func TestMiddleware_Handle(t *testing.T) {
// Create a separate instance of the Middleware struct for testing
testMiddleware := &twofa.Middleware{
Config: &twofa.Config{
Secret: secret,
Secret: secret,
TimeSource: twofa.DefaultConfig.TimeSource,
Hash: twofa.DefaultConfig.Hash,
},
}

Expand Down Expand Up @@ -1253,6 +1257,8 @@ func TestMiddlewareUUIDContextKey_Handle(t *testing.T) {
middleware := twofa.New(twofa.Config{
Secret: secret,
Storage: store,
TimeSource: twofa.DefaultConfig.TimeSource,
Hash: twofa.DefaultConfig.Hash,
ContextKey: contextKey,
RedirectURL: "/2fa",
CookieMaxAge: 86400,
Expand All @@ -1278,7 +1284,9 @@ func TestMiddlewareUUIDContextKey_Handle(t *testing.T) {

// Generate a valid 2FA token
totp := otp.NewTOTPVerifier(otp.Config{
Secret: info.Secret,
Secret: info.Secret,
TimeSource: twofa.DefaultConfig.TimeSource,
Hash: twofa.DefaultConfig.Hash,
})

validToken := totp.GenerateToken()
Expand All @@ -1287,7 +1295,9 @@ func TestMiddlewareUUIDContextKey_Handle(t *testing.T) {
// Create a separate instance of the Middleware struct for testing
testMiddleware := &twofa.Middleware{
Config: &twofa.Config{
Secret: secret,
Secret: secret,
TimeSource: twofa.DefaultConfig.TimeSource,
Hash: twofa.DefaultConfig.Hash,
},
}

Expand Down
2 changes: 1 addition & 1 deletion config.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ var DefaultConfig = Config{
DigitsCount: otp.DefaultConfig.Digits,
Period: otp.DefaultConfig.Period,
Hash: otp.SHA512,
TimeSource: otp.DefaultConfig.TOTPTime().UTC,
TimeSource: otp.DefaultConfig.TOTPTime,
SyncWindow: otp.DefaultConfig.SyncWindow,
SkipCookies: nil,
CookieName: "twofa_cookie",
Expand Down
4 changes: 2 additions & 2 deletions internal/otpverifier/otpverifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,11 +440,11 @@ func (v *Config) cryptopowpow10(exponent int) uint64 {
return result
}

// TOTPTime returns the current time in the https://en.wikipedia.org/wiki/South_Pole time zone.
// TOTPTime returns the current time in the South Pole (see https://en.wikipedia.org/wiki/Time_in_Antarctica) time zone.
// 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.
//
// Note: The returned time is always expressed in UTC (Coordinated Universal Time) to avoid any ambiguity caused by local time zone offsets.
func (v *Config) TOTPTime() time.Time {
location, _ := time.LoadLocation("Antarctica/South_Pole")
return time.Now().In(location)
return time.Now().In(location).UTC()
}
26 changes: 20 additions & 6 deletions internal/otpverifier/otpverifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1268,40 +1268,54 @@ func TestHOTPVerifier_VerifySyncWindowWithResync(t *testing.T) {
Secret: secret,
Counter: initialCounter,
Hasher: otpverifier.Hashers[hashFunc],
SyncWindow: otpverifier.HighStrict,
ResyncWindowDelay: 50 * time.Millisecond,
SyncWindow: otpverifier.MediumStrict,
ResyncWindowDelay: 1 * time.Second,
}

verifier := otpverifier.NewHOTPVerifier(config)

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

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

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

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

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

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

// Verify this token should fail
if verifier.Verify(outsideWindowToken) {
t.Errorf("Token outside sync window verified but should not have (hash function: %s)", hashFunc)
}

// Trigger the AdjustSyncWindow function
verifier.AdjustSyncWindow(config.CounterMismatch)

// Get the actual sync window size
actualSyncWindow := verifier.GetSyncWindow()

// Get the expected sync window range for MediumStrict
expectedRange := otpverifier.SyncWindowRanges[otpverifier.MediumStrict]

// Check if the actual sync window size falls within the expected range
if actualSyncWindow < expectedRange[0] || actualSyncWindow > expectedRange[1] {
t.Errorf("Expected sync window size to be within the range %v, but got %d", expectedRange, actualSyncWindow)
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/otpverifier/totp.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewTOTPVerifier(config ...Config) *TOTPVerifier {
// 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
// for real-world scenarios where perfect clock synchronization is not always possible (e.g, traveling (in airplane), living in antartica, ISS-NASA).
if c.TimeSource == nil && c.SyncWindow != 0 {
c.TimeSource = DefaultConfig.TOTPTime().UTC
c.TimeSource = DefaultConfig.TOTPTime
c.SyncWindow = DefaultConfig.SyncWindow
}

Expand Down
15 changes: 7 additions & 8 deletions middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,16 +301,15 @@ func (m *Middleware) extractToken(c *fiber.Ctx) string {
// verifyToken verifies the provided token against the user's secret and returns the updated Info struct.
func (m *Middleware) verifyToken(token string) bool {
totp := otp.NewTOTPVerifier(otp.Config{
Secret: m.Info.GetSecret(),
Digits: m.Config.DigitsCount,
Period: m.Config.Period,
SyncWindow: m.Config.SyncWindow,
UseSignature: otp.DefaultConfig.UseSignature,
Hash: m.Config.Hash,
TimeSource: m.Config.TimeSource,
Secret: m.Info.GetSecret(),
Digits: m.Config.DigitsCount,
Period: m.Config.Period,
SyncWindow: m.Config.SyncWindow,
Hash: m.Config.Hash,
TimeSource: m.Config.TimeSource,
})

if !totp.Verify(token, "") {
if !totp.Verify(token) {
return false
}

Expand Down

0 comments on commit 5acecf7

Please sign in to comment.