Merge pull request #182 from GSA/sms-code-rigor

Use cryptographically secure random number for sms codes

app/dao/users_dao.py

@@ -1,6 +1,6 @@
 import uuid
 from datetime import datetime, timedelta
-from random import SystemRandom
+from secrets import randbelow
 
 from sqlalchemy import func
 from sqlalchemy.orm import joinedload
@@ -19,16 +19,9 @@ def _remove_values_for_keys_if_present(dict, keys):
         dict.pop(key, None)
 
 
-def create_secret_code():
-    return ''.join(get_non_repeating_random_digits(5))
-
-
-def get_non_repeating_random_digits(length):
-    output = [None] * length
-    for index in range(length):
-        while output[index] in {None, output[index - 1]}:
-            output[index] = str(SystemRandom().randrange(10))
-    return output
+def create_secret_code(length=6):
+    random_number = randbelow(10 ** length)
+    return f"{random_number:06d}"
 
 
 def save_user_attribute(usr, update_dict=None):

tests/app/dao/test_users_dao.py

@@ -183,21 +183,9 @@ def test_create_secret_code_different_subsequent_codes():
     assert code1 != code2
 
 
-def test_create_secret_code_returns_5_digits():
+def test_create_secret_code_returns_6_digits():
     code = create_secret_code()
-    assert len(str(code)) == 5
-
-
-def test_create_secret_code_never_repeats_consecutive_digits(mocker):
-    mocker.patch('app.dao.users_dao.SystemRandom.randrange', side_effect=[
-        1, 1, 1,
-        2,
-        3,
-        4, 4,
-        1,  # Repeated allowed if not consecutive
-        9, 9,  # Not called because we have 5 digits now
-    ])
-    assert create_secret_code() == '12341'
+    assert len(str(code)) == 6
 
 
 @freeze_time('2018-07-07 12:00:00')