Add return_interval option to valid_hotp

There is a flaw in the `valid_hotp` logic introduced in #15 that prevents it from being effectively used: because it returns a raw `boolean`, there is no way to know *which* interval matched: the next one, the 1000th one, or somewhere in between? In order to effectively implement a proper HOTP scheme, you need to track the interval number of the last valid HOTP to prevent its reuse, OR the reuse of a HOTP from any previous interval. For backwards compatibility, I have retained the default `boolean()` response, but if the `return_interval` option is set, the interval will be returned when a hotp is valid, like `{true, interval()}`.
yuce · Feb 19, 2020 · 62b8289 · 62b8289
1 parent c9f5e18
commit 62b8289
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 14 deletions.
diff --git a/README.md b/README.md
@@ -204,14 +204,14 @@ Erlang:
 
 ```
 pot:valid_hotp(Token, Secret) -> Boolean
-pot:valid_hotp(Token, Secret, Options) -> Boolean
+pot:valid_hotp(Token, Secret, Options) -> Boolean | {true, interval()}
 ```
 
 Elixir:
 
 ```
 :pot.valid_hotp(Token, Secret) -> Boolean
-:pot.valid_hotp(Token, Secret, Options) -> Boolean
+:pot.valid_hotp(Token, Secret, Options) -> Boolean | {true, interval()}
 ```
 
 The following `Options` are allowed:
@@ -220,11 +220,13 @@ The following `Options` are allowed:
 |-------------------|-------------|--------------------------|
 | `digest_method`   | atom        | from [hotp/2,3](#hotp23) |
 | `last`            | integer     | 1                        |
+| `return_interval` | boolean     | false                     |
 | `token_length`    | integer > 0 | from [hotp/2,3](#hotp23) |
 | `trials`          | integer > 0 | 1000                     |
 
 - `last` is the `Interval` value of the previous valid `Token`; the next `Interval` after `last` is used as the first candidate for validating the `Token`.
 - `trials` controls the number of incremental `Interval` values after `last` to try when validating the `Token`. If a matching candidate is not found within `trials` attempts, the `Token` is considered invalid.
+- `return_interval` controls whether the matching `Interval` of a valid `Token` is returned with the result. if set to `true`, then `valid_hotp/2` will return `{true, Interval}` (e.g., `{true, 123}`) when a valid `Token` is provided.
 
 #### `valid_totp/2,3`
 
@@ -297,6 +299,20 @@ IsValid = pot:valid_hotp(Token, Secret, [{last, LastUsed}]),
 % Do something
 ```
 
+Alternatively, to get the last interval from a validated token:
+
+```erlang
+Secret = <<"MFRGGZDFMZTWQ2LK">>,
+Token = <<"123456">>,
+LastUsed = 5,  % last successful trial
+Options = [{last, LastUsed}, {return_interval, true}],
+NewLastUsed = case pot:valid_hotp(Token, Secret, Options) of
+                  {true, LastInterval} -> LastInterval;
+                  false -> LastUsed
+              end,
+% Do something
+```
+
 ### Create a time based token with 30 seconds ahead
 
 ```erlang
@@ -362,6 +378,22 @@ is_valid = :pot.valid_hotp(token, secret, [{:last, last_used}])
 # Do something
 ```
 
+Alternatively, to get the last interval from a validated token:
+
+```elixir
+secret = "MFRGGZDFMZTWQ2LK"
+token = "123456"
+last_used = 5  # last successful trial
+options = [{:last, last_used}, {:return_token, true}]
+new_last_used =
+    case :pot.valid_hotp(token, secret, options) do
+        {true, last_interval} -> last_interval
+        false -> last_used
+    end
+# Do something
+```
+
+
 ### Create a time based token with 30 seconds ahead
 
 ```elixir

diff --git a/src/pot.erl b/src/pot.erl
@@ -59,6 +59,7 @@
 
 -type valid_hotp_option() :: hotp_option() |
                              {last, interval()} |
+                             return_interval | {return_interval, boolean()} |
                              {trials, pos_integer()}.
 
 -type valid_totp_option() :: totp_option() |
@@ -73,6 +74,8 @@
 
 -type time_interval_options() :: [time_interval_option()].
 
+-type valid_hotp_return() :: boolean() | {true, LastInterval :: interval()}.
+
 %%==============================================================================
 %% Token generation
 %%==============================================================================
@@ -127,7 +130,7 @@ valid_token(Token, Opts) when is_binary(Token) ->
 valid_hotp(Token, Secret) ->
     valid_hotp(Token, Secret, []).
 
--spec valid_hotp(token(), secret(), valid_hotp_options()) -> boolean().
+-spec valid_hotp(token(), secret(), valid_hotp_options()) -> valid_hotp_return().
 valid_hotp(Token, Secret, Opts) ->
     Last = proplists:get_value(last, Opts, 1),
     Trials = proplists:get_value(trials, Opts, 1000),
@@ -137,8 +140,8 @@ valid_hotp(Token, Secret, Opts) ->
             case check_candidate(Token, Secret, Last + 1, Last + Trials, Opts) of
                 false ->
                     false;
-                _ ->
-                    true
+                LastInterval ->
+                    valid_hotp_return(LastInterval, proplists:get_value(return_interval, Opts, false))
             end;
         _ ->
             false
@@ -196,3 +199,9 @@ check_candidate(_Token, _Secret, _Current, _Last, _Opts) ->
 prepend_zeros(Token, N) ->
     Padding = << <<48:8>> || _ <- lists:seq(1, N) >>,
     <<Padding/binary, Token/binary>>.
+
+-spec valid_hotp_return(interval(), boolean()) -> valid_hotp_return().
+valid_hotp_return(LastInterval, true = _ReturnInterval) ->
+    {true, LastInterval};
+valid_hotp_return(_LastInterval, _ReturnInterval) ->
+    true.
diff --git a/test/hotp_validity_tests.erl b/test/hotp_validity_tests.erl
@@ -9,6 +9,7 @@ checking_hotp_validity_without_range_test_() ->
      fun start/0,
      fun stop/1,
      [fun checking_hotp_validity_without_range/1,
+      fun checking_hotp_validity_without_range_return_interval/1,
       fun checking_hotp_validity_max_default_range/1,
       fun checking_hotp_validity_past_max_default_range/1,
       fun validating_invalid_token_hotp/1,
@@ -31,6 +32,11 @@ checking_hotp_validity_without_range(Secret) ->
     [?_assert(pot:valid_hotp(pot:hotp(Secret, 123), Secret))].
 
 
+checking_hotp_validity_without_range_return_interval(Secret) ->
+    [?_assertEqual({true, 123},
+                   pot:valid_hotp(pot:hotp(Secret, 123), Secret, [return_interval]))].
+
+
 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))}].
@@ -54,20 +60,32 @@ validating_correct_totp_as_hotp(Secret) ->
 
 
 retrieving_proper_interval_from_validator(Secret) ->
-    Totp = <<"713385">>,
-    Result = pot:valid_hotp(Totp, Secret, [{last, 1}, {trials, 5}]),
-    [?_assert(Result),
-     ?_assertEqual(pot:hotp(Secret, 4), Totp)].
+    Hotp = <<"713385">>,
+    [?_assert(pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 5}])),
+     ?_assert(pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 5},
+                                            {return_interval, false}])),
+     ?_assertEqual({true, 4},
+                   pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 5},
+                                                 {return_interval, true}])),
+     ?_assertEqual(pot:hotp(Secret, 4), Hotp)].
 
 
 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 = <<"816065">>,
+    [?_assert(pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 1}])),
+     ?_assert(pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 1},
+                                            {return_interval, false}])),
+     ?_assertEqual({true, 2},
+                   pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 1},
+                                                 {return_interval, true}])),
+     ?_assertEqual(pot:hotp(Secret, 2), Hotp)].
 
 
 hotp_for_range_preceding_match(_) ->
     Secret = <<"MFRGGZDFMZTWQ2LK">>,
-    [?_assertNot(pot:valid_hotp(<<"713385">>, Secret, [{last, 1}, {trials, 2}]))].
+    Hotp = <<"713385">>,
+    [?_assertNot(pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 2}])),
+     ?_assertNot(pot:valid_hotp(Hotp, Secret, [{last, 1}, {trials, 2},
+                                               {return_interval, true}])),
+     ?_assertEqual(pot:hotp(Secret, 4), Hotp)].