django-multifactor icon indicating copy to clipboard operation
django-multifactor copied to clipboard

Redirect too many times error when using FIDO2 and development server 'localhost'.

Open maximbelyayev opened this issue 1 year ago • 12 comments

Hello, In advance, please excuse any obvious errors as I'm relatively new to django and mfa flow.

Background: I've followed the docs and installed django-multifactor, added to INSTALLED_APPS, and I have the exact same MULTIFACTOR settings except for:

'FIDO_SERVER_ID': 'localhost', # Server ID for FIDO request

I launch via latest Chrome version and command and python manage.py runserver_plus localhost:8000 --cert-file cert.pem --key-file key.pem to access through https.

urls.py using latest decorator_include package from StevenMapes fork:

path('admin/multifactor/', include('multifactor.urls')), path('admin/', decorator_include(multifactor_protected(factors=1), admin.site.urls))

Problem: However, after logging into /admin page and setting up FIDO2. Logging out and logging back into admin results in a ERR_TOO_MANY_REDIRECTS where the redirects are between admin/multifactor/add/ and admin/multifactor/authenticate/. This doesn't occur using TOTP.

maximbelyayev avatar May 01 '24 19:05 maximbelyayev

So the main difference I can see between your setup and the ones I am using in all my projects is that you are decorating all of Django Admin using decorator include then have the multifactors URL as a subfolder to that URL which means it's also protected so that would then say "you need to have passed the MFA check and it'll redirect causing the endless loop until the browser gives up.

What I so is to setup the URLs like this

path('mfa/multifactor/', include('multifactor.urls')),
path('admin/', decorator_include(multifactor_protected(factors=1), admin.site.urls))

That way when you go to and admin/ page you will be redirect to the MFA page under mfa/multifactor/ which is not itself protected. That way the you can then complete the MFA steps and will be redirect back to the admin url you were trying to access.

Give that a go and let me know if it resolves the issue as I expect it will.

Also great to hear you are using my fork of decorator_include, at some-point soon I'm going to split it off to create a new repository so I can push it out to Pypi as it doesn't look like the maintainer of that project is responding to my request to take over the project. I use it heavily so I'm going to keep patching my fork to work with the latest Python and Django combinations

StevenMapes avatar May 01 '24 19:05 StevenMapes

Thanks for the quick response! Unfortunately, it now redirects between https://localhost:8000/mfa/multifactor/add/ and https://localhost:8000/mfa/multifactor/authenticate/. Trying to also understand the root cause here...

Yes, thanks for your work on decorator_include! A standalone on Pypi would be great.

Edit: Could it have to do with the following? Printing self.available methods results in defaultdict(<class 'list'>, {})

 if not self.available_methods:
     return redirect('multifactor:add')

maximbelyayev avatar May 01 '24 19:05 maximbelyayev

I must confess I've only used TOTP so I'd need to setup a FIFO one in order to test myself. Hopefully @oliwarner will be able to advise what's happening

StevenMapes avatar May 01 '24 20:05 StevenMapes

I think it as to do with the following lines under class Authenticate in django-multifactor views.py:

if domain != self.request.get_host():
    other_domains.add(domain)
    continue

Domain is set as localhost whereas self.request.get_host() equates to localhost:8000. This skips the append to self.available_methods later on.

Can't set 'FIDO_SERVER_ID': 'localhost:8000 as it results in ValueError: Invalid origin in CollectedClientData from fido2/server.py

maximbelyayev avatar May 01 '24 20:05 maximbelyayev

Interesting. In that case can you try upgrading the fido2 package you have installed to the latest version please to see if that resolves anything.

Looking at this PR https://github.com/Yubico/python-fido2/pull/218 it relates to localhost usage.

Update: reading further that seems to be more around allowing http for localhost so it may not actually help

StevenMapes avatar May 01 '24 20:05 StevenMapes

Some further digging

https://github.com/Yubico/python-fido2/issues/122#issuecomment-1054259584 suggests a custom verify_origin function could be used.

Then the following PR on Django-mf3 shows an example of an implementation of using the custom function. https://github.com/xi/django-mfa3/pull/17

I'm not on my laptop at the moment but based on @maximbelyayev comment this if statement could be extended to support checking for localhost

So we change

if domain != self.request.get_host():
    other_domains.add(domain)

To also check and not (domain == "localhost" and self.request.get_host().startswith("localhost:"))

If suggest forking this package, adding a check for that, seeing if that works for you then creating a PR if it does

StevenMapes avatar May 01 '24 20:05 StevenMapes

Just created PR 82. Minor parsing change to request.get_host() statements. Thanks for the help Steven

maximbelyayev avatar May 01 '24 21:05 maximbelyayev

So that solved your issue without breaking the TOTP route right

StevenMapes avatar May 01 '24 21:05 StevenMapes

Yep just tested TOTP

maximbelyayev avatar May 01 '24 22:05 maximbelyayev

Great! Could I ask one additional thing of you since you have a working FIDO2 implementation, would you be able to upgrade the fido2 dependency to the latest version and check that it still works please. This project is currently pinned to fido2 = '1.1.2' and that package is now on 1.1.3 so it would be useful to know it's still working

StevenMapes avatar May 02 '24 08:05 StevenMapes

I also just realised that this issue is the same as #70

StevenMapes avatar May 02 '24 08:05 StevenMapes

@oliwarner does v0.7 resolve this one, I believe it may so it can be closed if it does.

StevenMapes avatar Jun 07 '24 11:06 StevenMapes