Skip to content

Commit

Permalink
Allow a limited list of paths when user has device
Browse files Browse the repository at this point in the history
This prevents users from adding or listing devices while they are not
yet verified and already have a registered device.
  • Loading branch information
nnist authored and mikedingjan committed Nov 26, 2019
1 parent aa0c404 commit a6711b2
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 12 deletions.
27 changes: 17 additions & 10 deletions src/wagtail_2fa/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@

class VerifyUserMiddleware(_OTPMiddleware):
_allowed_url_names = [
"wagtail_2fa_auth",
"wagtailadmin_login",
"wagtailadmin_logout",
]
_allowed_url_names_no_device = [
"wagtail_2fa_auth",
"wagtail_2fa_device_list",
"wagtail_2fa_device_new",
Expand Down Expand Up @@ -66,24 +71,26 @@ def _require_verified_user(self, request):
return False

# Allow the user to a fixed number of paths when not verified
if request.path in self._allowed_paths:
user_has_device = django_otp.user_has_device(user, confirmed=True)
if request.path in self._get_allowed_paths(user_has_device):
return False

# For all other cases require that the user is verfied via otp
return True

@property
def _allowed_paths(self):
"""Return the paths the user may visit when not verified via otp
This result cannot be cached since we want to be compatible with the
django-hosts package. Django-hosts alters the urlconf based on the
hostname in the request, so the urls might exist for admin.<domain> but
not for www.<domain>.
def _get_allowed_paths(self, has_device):
"""Return the paths the user may visit when not verified via otp.
If the user already has a registered device, return a limited set of
paths to prevent them from adding or listing devices to prevent them
from adding or listing devices.
"""
allowed_url_names = self._allowed_url_names
if not has_device:
allowed_url_names = self._allowed_url_names_no_device

results = []
for route_name in self._allowed_url_names:
for route_name in allowed_url_names:
try:
results.append(settings.WAGTAIL_MOUNT_PATH + reverse(route_name))
except NoReverseMatch:
Expand Down
4 changes: 2 additions & 2 deletions tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ def test_superuser_dont_require_register_device(rf, superuser, settings):

def test_not_specifiying_wagtail_mount_point_does_not_prepend_allowed_paths_with_wagtail_mount_path(settings):
settings.WAGTAIL_MOUNT_PATH = ''
allowed_paths = VerifyUserMiddleware()._allowed_paths
allowed_paths = VerifyUserMiddleware()._get_allowed_paths(has_device=False)

for allowed_path in allowed_paths:
assert allowed_path.startswith('/cms')


def test_specifiying_wagtail_mount_point_does_prepend_allowed_paths_with_wagtail_mount_path(settings):
settings.WAGTAIL_MOUNT_PATH = '/wagtail'
allowed_paths = VerifyUserMiddleware()._allowed_paths
allowed_paths = VerifyUserMiddleware()._get_allowed_paths(has_device=False)

for allowed_path in allowed_paths:
assert allowed_path.startswith(settings.WAGTAIL_MOUNT_PATH)

0 comments on commit a6711b2

Please sign in to comment.