Skip to content

Commit

Permalink
Removed wrapping of django-otp random_hex function. (jazzband#392)
Browse files Browse the repository at this point in the history
From django-otp 0.8.0 and up, random_hex is always a string. We don't need to wrap it any longer.
  • Loading branch information
claudep authored Oct 3, 2020
1 parent 44b5064 commit 5ccfbfb
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 31 deletions.
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
packages=find_packages(exclude=('example', 'tests')),
install_requires=[
'Django>=2.2',
'django_otp>=0.6.0',
'django_otp>=0.8.0',
'qrcode>=4.0.0,<6.99',
'django-phonenumber-field>=1.1.0,<3.99',
'django-formtools',
Expand Down
7 changes: 4 additions & 3 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@

from django.contrib.auth.hashers import make_password
from django.test import TestCase, override_settings
from django_otp.util import random_hex

from two_factor.models import PhoneDevice, random_hex_str
from two_factor.models import PhoneDevice
from two_factor.utils import (
backup_phones, default_device, get_otpauth_url, totp_digits,
)
Expand Down Expand Up @@ -95,8 +96,8 @@ def test_get_totp_digits(self):
self.assertEqual(totp_digits(), no_digits)

def test_random_hex(self):
# test that returned random_hex_str is string
h = random_hex_str()
# test that returned random_hex is string
h = random_hex()
self.assertIsInstance(h, str)
# hex string must be 40 characters long. If cannot be longer, because CharField max_length=40
self.assertEqual(len(h), 40)
Expand Down
23 changes: 11 additions & 12 deletions tests/test_views_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
from django.urls import reverse
from django_otp import DEVICE_ID_SESSION_KEY
from django_otp.oath import totp

from two_factor.models import random_hex_str
from django_otp.util import random_hex

from .utils import UserMixin

Expand Down Expand Up @@ -95,7 +94,7 @@ def test_valid_login_primary_key_stored(self, mock_time):
mock_time.time.return_value = 12345.12
user = self.create_user()
user.totpdevice_set.create(name='default',
key=random_hex_str())
key=random_hex())

response = self._post({'auth-username': '[email protected]',
'auth-password': 'secret',
Expand All @@ -113,7 +112,7 @@ def test_valid_login_post_auth_session_clear_of_form_data(self, mock_time):
mock_time.time.return_value = 12345.12
user = self.create_user()
user.totpdevice_set.create(name='default',
key=random_hex_str())
key=random_hex())

response = self._post({'auth-username': '[email protected]',
'auth-password': 'secret',
Expand All @@ -132,7 +131,7 @@ def test_valid_login_expired(self, mock_time, mock_logger):
mock_time.time.return_value = 12345.12
user = self.create_user()
device = user.totpdevice_set.create(name='default',
key=random_hex_str())
key=random_hex())

response = self._post({'auth-username': '[email protected]',
'auth-password': 'secret',
Expand Down Expand Up @@ -165,7 +164,7 @@ def test_valid_login_no_timeout(self, mock_time):
mock_time.time.return_value = 12345.12
user = self.create_user()
device = user.totpdevice_set.create(name='default',
key=random_hex_str())
key=random_hex())

response = self._post({'auth-username': '[email protected]',
'auth-password': 'secret',
Expand Down Expand Up @@ -210,7 +209,7 @@ def test_valid_login_with_redirect_authenticated_user_loop(self):
def test_with_generator(self, mock_signal):
user = self.create_user()
device = user.totpdevice_set.create(name='default',
key=random_hex_str())
key=random_hex())

response = self._post({'auth-username': '[email protected]',
'auth-password': 'secret',
Expand Down Expand Up @@ -240,7 +239,7 @@ def test_with_generator(self, mock_signal):
def test_throttle_with_generator(self, mock_signal):
user = self.create_user()
device = user.totpdevice_set.create(name='default',
key=random_hex_str())
key=random_hex())

self._post({'auth-username': '[email protected]',
'auth-password': 'secret',
Expand All @@ -265,11 +264,11 @@ def test_with_backup_phone(self, mock_signal, fake):
user = self.create_user()
for no_digits in (6, 8):
with self.settings(TWO_FACTOR_TOTP_DIGITS=no_digits):
user.totpdevice_set.create(name='default', key=random_hex_str(),
user.totpdevice_set.create(name='default', key=random_hex(),
digits=no_digits)
device = user.phonedevice_set.create(name='backup', number='+31101234567',
method='sms',
key=random_hex_str())
key=random_hex())

# Backup phones should be listed on the login form
response = self._post({'auth-username': '[email protected]',
Expand Down Expand Up @@ -322,7 +321,7 @@ def test_with_backup_phone(self, mock_signal, fake):
@mock.patch('two_factor.views.core.signals.user_verified.send')
def test_with_backup_token(self, mock_signal):
user = self.create_user()
user.totpdevice_set.create(name='default', key=random_hex_str())
user.totpdevice_set.create(name='default', key=random_hex())
device = user.staticdevice_set.create(name='backup')
device.token_set.create(token='abcdef123')

Expand Down Expand Up @@ -475,7 +474,7 @@ def setUp(self):
super().setUp()
self.user = self.create_user()
self.device = self.user.totpdevice_set.create(name='default',
key=random_hex_str())
key=random_hex())

def _post(self, data=None):
return self.client.post(reverse('two_factor:login'), data=data)
Expand Down
7 changes: 4 additions & 3 deletions tests/test_views_phone.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
from django.test.utils import override_settings
from django.urls import reverse, reverse_lazy
from django_otp.oath import totp
from django_otp.util import random_hex

from two_factor.models import PhoneDevice, random_hex_str
from two_factor.models import PhoneDevice
from two_factor.utils import backup_phones
from two_factor.validators import validate_international_phonenumber
from two_factor.views.core import PhoneDeleteView, PhoneSetupView
Expand Down Expand Up @@ -176,7 +177,7 @@ class PhoneDeviceTest(UserMixin, TestCase):
def test_verify(self):
for no_digits in (6, 8):
with self.settings(TWO_FACTOR_TOTP_DIGITS=no_digits):
device = PhoneDevice(key=random_hex_str())
device = PhoneDevice(key=random_hex())
self.assertFalse(device.verify_token(-1))
self.assertFalse(device.verify_token('foobar'))
self.assertTrue(device.verify_token(totp(device.bin_key, digits=no_digits)))
Expand All @@ -189,7 +190,7 @@ def test_verify_token_as_string(self):
"""
for no_digits in (6, 8):
with self.settings(TWO_FACTOR_TOTP_DIGITS=no_digits):
device = PhoneDevice(key=random_hex_str())
device = PhoneDevice(key=random_hex())
self.assertTrue(device.verify_token(str(totp(device.bin_key, digits=no_digits))))

def test_unicode(self):
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ deps =
coverage

django-formtools
django_otp>=0.6.0
django_otp>=0.8.0
django-phonenumber-field>=1.1.0,<2.99
phonenumbers>=7.0.9,<8.99
qrcode>=4.0.0,<6.99
Expand Down
3 changes: 2 additions & 1 deletion two_factor/migrations/0006_phonedevice_key_default.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import django_otp.util
from django.db import migrations, models

import two_factor.models
Expand All @@ -13,6 +14,6 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name='phonedevice',
name='key',
field=models.CharField(default=two_factor.models.random_hex_str, help_text='Hex-encoded secret key', max_length=40, validators=[two_factor.models.key_validator]),
field=models.CharField(default=django_otp.util.random_hex, help_text='Hex-encoded secret key', max_length=40, validators=[two_factor.models.key_validator]),
),
]
8 changes: 1 addition & 7 deletions two_factor/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

from django.conf import settings
from django.db import models
from django.utils.encoding import force_str
from django.utils.translation import gettext_lazy as _
from django_otp.models import Device
from django_otp.oath import totp
Expand Down Expand Up @@ -54,11 +53,6 @@ def key_validator(*args, **kwargs):
return hex_validator()(*args, **kwargs)


def random_hex_str(length=20):
# Could be removed once we depend on django_otp > 0.7.5
return force_str(random_hex(length=length))


class PhoneDevice(Device):
"""
Model with phone number and token seed linked to a user.
Expand All @@ -69,7 +63,7 @@ class Meta:
number = PhoneNumberField()
key = models.CharField(max_length=40,
validators=[key_validator],
default=random_hex_str,
default=random_hex,
help_text="Hex-encoded secret key")
method = models.CharField(max_length=4, choices=PHONE_METHODS,
verbose_name=_('method'))
Expand Down
7 changes: 4 additions & 3 deletions two_factor/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@
from django_otp import devices_for_user
from django_otp.decorators import otp_required
from django_otp.plugins.otp_static.models import StaticDevice, StaticToken
from django_otp.util import random_hex

from two_factor import signals
from two_factor.models import get_available_methods, random_hex_str
from two_factor.models import get_available_methods
from two_factor.utils import totp_digits

from ..forms import (
Expand Down Expand Up @@ -532,7 +533,7 @@ def get_key(self, step):
self.storage.extra_data.setdefault('keys', {})
if step in self.storage.extra_data['keys']:
return self.storage.extra_data['keys'].get(step)
key = random_hex_str(20)
key = random_hex(20)
self.storage.extra_data['keys'][step] = key
return key

Expand Down Expand Up @@ -662,7 +663,7 @@ def get_key(self):
The key is preserved between steps and stored as ascii in the session.
"""
if self.key_name not in self.storage.extra_data:
key = random_hex_str(20)
key = random_hex(20)
self.storage.extra_data[self.key_name] = key
return self.storage.extra_data[self.key_name]

Expand Down

0 comments on commit 5ccfbfb

Please sign in to comment.