Merge pull request #20 from nalundgaard/fix_window

Fix valid_totp to support upper bound on check_candidate

CONTRIBUTORS

@@ -5,3 +5,4 @@ Pedro Vieira
 Julius Beckmann
 Zbigniew Pekala
 Girish Ramnani
+Nicholas Lundgaard

src/pot.erl

@@ -111,10 +111,12 @@ valid_totp(Token, Secret, Opts) ->
                         false ->
                             false;
                         _ ->
-                            true end end;
+                            true
+                    end
+            end;
         _ ->
-            false end.
-
+            false
+    end.
 
 -spec time_interval(proplist()) -> time().
 time_interval(Opts) ->
@@ -123,17 +125,16 @@ time_interval(Opts) ->
   {MegaSecs, Secs, _} = proplists:get_value(timestamp, Opts, os:timestamp()),
   trunc((MegaSecs * 1000000 + (Secs + AddSeconds)) / IntervalLength).
 
+-spec check_candidate(token(), secret(), time(), time(), proplist()) -> time() | false.
 check_candidate(Token, Secret, Current, Last, Opts) when Current =< Last ->
-    case Current of
-        Last ->
-            false;
+    case hotp(Secret, Current, Opts) of
+        Token ->
+            Current;
         _ ->
-            Candidate = hotp(Secret, Current, Opts),
-            case Candidate of
-                Token ->
-                    Current;
-                _ ->
-                    check_candidate(Token, Secret, Current + 1, Last, Opts) end end.
+            check_candidate(Token, Secret, Current + 1, Last, Opts)
+    end;
+check_candidate(_Token, _Secret, _Current, _Last, _Opts) ->
+    false.
 
 -spec prepend_zeros(token(), non_neg_integer()) -> token().
 prepend_zeros(Token, N) ->

test/hotp_validity_tests.erl

@@ -5,67 +5,64 @@
 
 
 checking_hotp_validity_without_range_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun checking_hotp_validity_without_range/1}.
+    {foreach,
+     fun start/0,
+     fun stop/1,
+     [fun checking_hotp_validity_without_range/1,
+      fun checking_hotp_validity_max_default_range/1,
+      fun checking_hotp_validity_past_max_default_range/1,
+      fun validating_correct_hotp_after_exhaustion/1,
+      fun validating_correct_totp_as_hotp/1,
+      fun retrieving_proper_interval_from_validator/1,
+      fun hotp_for_range_exact_match/1,
+      fun hotp_for_range_preceding_match/1]}.
 
 
-validating_correct_hotp_after_exhaustion_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun validating_correct_hotp_after_exhaustion/1}.
-
-
-validating_correct_totp_as_hotp_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun validating_correct_totp_as_hotp/1}.
-
-
-retrieving_proper_interval_from_validator_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun retrieving_proper_interval_from_validator/1}.
+start() ->
+    _Secret = <<"MFRGGZDFMZTWQ2LK">>.
 
 
-hotp_for_range_preceding_match_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun hotp_for_range_preceding_match/1}.
+stop(_) ->
+    ok.
 
 
-start() ->
-    ok.
+checking_hotp_validity_without_range(Secret) ->
+    [?_assert(pot:valid_hotp(pot:hotp(Secret, 123), Secret))].
 
 
-stop(_) ->
-    ok.
+checking_hotp_validity_max_default_range(Secret) ->
+    [{"hotp at the max # of trials (1000) past the start (1) is valid",
+      ?_assert(pot:valid_hotp(pot:hotp(Secret, 1001), Secret))}].
 
 
-checking_hotp_validity_without_range(_) ->
-    Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    [?_assertEqual(pot:valid_hotp(pot:hotp(Secret, 123), Secret), true)].
+checking_hotp_validity_past_max_default_range(Secret) ->
+    [{"hotp beyond the max # of trials (1000) past the start (1) is invalid",
+      ?_assertNot(pot:valid_hotp(pot:hotp(Secret, 1002), Secret))}].
 
 
-validating_correct_hotp_after_exhaustion(_) ->
-    Secret = <<"MFRGGZDFMZTWQ2LK">>,
-     [?_assertEqual(pot:valid_hotp(
-            pot:hotp(Secret, 123), Secret, [{last, 123}]), false)].
+validating_correct_hotp_after_exhaustion(Secret) ->
+     [?_assertNot(pot:valid_hotp(pot:hotp(Secret, 123), Secret, [{last, 123}]))].
 
 
-validating_correct_totp_as_hotp(_) ->
-    Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    [?_assertEqual(pot:valid_hotp(pot:totp(Secret), Secret), false)].
+validating_correct_totp_as_hotp(Secret) ->
+    [?_assertNot(pot:valid_hotp(pot:totp(Secret), Secret))].
 
 
-retrieving_proper_interval_from_validator(_) ->
-    Secret = <<"MFRGGZDFMZTWQ2LK">>,
+retrieving_proper_interval_from_validator(Secret) ->
     Totp = <<"713385">>,
     Result = pot:valid_hotp(Totp, Secret, [{last, 1}, {trials, 5}]),
-    [?_assertEqual(Result, true),
+    [?_assert(Result),
      ?_assertEqual(pot:hotp(Secret, 4), Totp)].
 
 
+hotp_for_range_exact_match(_) ->
+    Secret = <<"MFRGGZDFMZTWQ2LK">>,
+    Totp = <<"816065">>,
+    Result = pot:valid_hotp(Totp, Secret, [{last, 1}, {trials, 1}]),
+    [?_assert(Result),
+     ?_assertEqual(pot:hotp(Secret, 2), Totp)].
+
+
 hotp_for_range_preceding_match(_) ->
     Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    [?_assertEqual(pot:valid_hotp(<<"713385">>, Secret, [{last, 1}, {trials, 2}]), false)].
+    [?_assertNot(pot:valid_hotp(<<"713385">>, Secret, [{last, 1}, {trials, 2}]))].

test/totp_generation_tests.erl

@@ -23,16 +23,13 @@ stop(_) ->
 
 generating_current_totp_and_validating(_) ->
     Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    IntervalsNo = trunc(time_now() / 30),
+    IntervalOpts = [{timestamp, os:timestamp()}],
+    IntervalsNo = pot:time_interval(IntervalOpts),
     Hotp = pot:hotp(Secret, IntervalsNo),
-    Totp = pot:totp(Secret),
+    Totp = pot:totp(Secret, IntervalOpts),
     [?_assertEqual(Hotp, Totp)].
 
 generating_totp_for_given_timestamp_and_compare(_) ->
     Secret = <<"MFRGGZDFMZTWQ2LK">>,
     Totp = pot:totp(Secret, [{timestamp, {1518, 179058, 919315}}]),
     [?_assertEqual(Totp, <<"151469">>)].
-
-time_now() ->
-    {MegaSecs, Secs, _} = os:timestamp(),
-    MegaSecs * 1000000 + Secs.

test/totp_validity_tests.erl

@@ -3,22 +3,15 @@
 -include_lib("eunit/include/eunit.hrl").
 
 
-validating_totp_for_same_secret_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun validating_totp_for_same_secret/1}.
-
-
-validating_invalid_totp_for_same_secret_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun validating_invalid_totp_for_same_secret/1}.
-
-
-validating_correct_hotp_as_totp_test_() ->
-    {setup, fun start/0,
-            fun stop/1,
-            fun validating_correct_hotp_as_totp/1}.
+totp_validity_test_() ->
+    {foreach,
+     fun start/0,
+     fun stop/1,
+     [fun validating_totp_for_same_secret/1,
+      fun validating_invalid_totp_for_same_secret/1,
+      fun validating_correct_hotp_as_totp/1,
+      fun validating_past_future_totp_too_small_window/1,
+      fun validating_past_future_totp_with_window/1]}.
 
 
 start() ->
@@ -31,14 +24,34 @@ stop(_) ->
 
 validating_totp_for_same_secret(_) ->
     Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    [?_assertEqual(pot:valid_totp(pot:totp(Secret), Secret), true)].
+    [?_assert(pot:valid_totp(pot:totp(Secret), Secret))].
 
 
 validating_invalid_totp_for_same_secret(_) ->
     Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    [?_assertEqual(pot:valid_totp(<<"123456">>, Secret), false)].
+    [?_assertNot(pot:valid_totp(<<"123456">>, Secret))].
 
 
 validating_correct_hotp_as_totp(_) ->
     Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    [?_assertEqual(pot:valid_totp(pot:hotp(Secret, 1), Secret), false)].
+    [?_assertNot(pot:valid_totp(pot:hotp(Secret, 1), Secret))].
+
+
+validating_past_future_totp_too_small_window(_) ->
+    Secret = <<"MFRGGZDFMZTWQ2LK">>,
+    IntervalOpts = [{timestamp, os:timestamp()}],
+    N = 5,
+    [?_assertNot(pot:valid_totp(pot:totp(Secret, [{addwindow, AW} | IntervalOpts]),
+                                Secret,
+                                [{window, W} | IntervalOpts]))
+     || W <- lists:seq(0, N), AW <- lists:seq(-N, N), W < abs(AW)].
+
+
+validating_past_future_totp_with_window(_) ->
+    Secret = <<"MFRGGZDFMZTWQ2LK">>,
+    IntervalOpts = [{timestamp, os:timestamp()}],
+    N = 5,
+    [?_assert(pot:valid_totp(pot:totp(Secret, [{addwindow, AW} | IntervalOpts]),
+                             Secret,
+                             [{window, W} | IntervalOpts]))
+     || W <- lists:seq(0, N), AW <- lists:seq(-N, N), W >= abs(AW)].