Some bugs in FIDO2

[d3cline]

Line 72 template FIDO/recheck.html

      }).then(function (response) {if (response.ok) return res = response.json()}).then(function (res) {
          if (res.status=="OK")
          {

'res is undefined'

My fix is to remove the extra 'then' and just not dump it as json,

      }).then(function (response) {
          if (response.statusText=="OK")

Later in the same template 'res is used again for the redirect,

          {%  if mode == "auth" %}
          window.location.href=res.redirect;
          {% elif mode == "recheck" %}

I never actually saw one in the object when I console logged it, and was able to hard code it for my use case, but this should also be checked.

I am happy to fix the above but it should be reviewed for context/intent and see if I am missing something.

Line 139 of FIDO2.py is a bool not a callable

138                     request.session["mfa"] = mfa
139                     if not request.user.is_authenticated():
140                         res=login(request)

should be

138                     request.session["mfa"] = mfa
139                     if not request.user.is_authenticated:
140                         res=login(request)

Please review and if these fixes are OK I can do a PR for them. I feel like I am forgetting one more...

[mkalioby]

This is very strange, as FIDO2 is working fine on our production applications. Can you give me the env you are using? And the use case and the authenticator you are using Thanks Mohamed

…

On Tue, Jun 16, 2020, 21:15 John Spounias ***@***.***> wrote: 1. Line 72 template FIDO/recheck.html }).then(function (response) {if (response.ok) return res = response.json()}).then(function (res) { if (res.status=="OK") { 'res is undefined' My fix is to remove the extra 'then' and just not dump it as json, }).then(function (response) { if (response.statusText=="OK") Later in the same template 'res is used again for the redirect, {% if mode == "auth" %} window.location.href=res.redirect; {% elif mode == "recheck" %} I never actually saw one in the object when I console logged it, and was able to hard code it for my use case, but this should also be checked. I am happy to fix the above but it should be reviewed for context/intent and see if I am missing something. 1. Line 139 of FIDO2.py is a bool not a callable 138 request.session["mfa"] = mfa 139 if not request.user.is_authenticated(): 140 res=login(request) should be 138 request.session["mfa"] = mfa 139 if not request.user.is_authenticated: 140 res=login(request) Please review and if these fixes are OK I can do a PR for them. I feel like I am forgetting one more... — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#13>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPOPRGJUUJBZACCTRT7IT3RW6ZD5ANCNFSM4N73TSTA> .

[d3cline]

I am using Django 3.0.7
Python 3.6.8
Postgresql 11

Its a generic FIDO2 usb auth device which functions for other use cases, and works once I made these adjustments.

[mkalioby]

Which browser are you testing against?

For is_authenticated(), the change is part if Django 2.0, so we need to try both

[d3cline]

I have been testing with the latest versions of both Firefox and Chrome. Most of my debugging is Chrome for FIDO2 so far, but I could try other browsers if required.

[mkalioby]

Can you please give me the test case for js issues? So I reproduce it

[d3cline]

The test case was authentication using a FIDO2 USB, the failure will trigger during auth.
I have a feeling thinking about how I was working on it that the first error is caused by the second, that the fact I wasn't getting request.ok may have been causing res to also be undefined, but I am not sure. I was getting a 500 because of the django version issue. The error was that the 'return res =' was not passing to the next line correctly, and by moving it into 1 function as described I was able to bypass needing to pass it at all. (I admit its hard to describe that line of code, it has a lot going on)

[mkalioby]

So to have clear,

• You registered your key successfully 
• Logout 
• Failure at login

Do I miss something?

[d3cline]

Correct.

[d3cline]

Trying on linux vs windows I am getting different results... Will do more testing and post back when I have more info.

[d3cline]

      }).then(function (response) {
          if (response.statusText=="OK")

Changed to

      }).then(function (response) {
          if (response.ok)

for some reason between os/browser 'statusText' was empty vs 'OK' on the other platform.

[mkalioby]

@d3cline Can you provide a demo app to show the problem on github and send me the link as I tried and I can reproduce the templates issue

[d3cline]

Not sure I can. Let me try to explain a bit.
In my integration I have to override the templates a lot and some functions since we are 100% JSON API based. It would be difficult to isolate this code from the rest of the app.
Having spent some more time thinking about it, and knowing you can't reproduce it,
I feel if the is authenticated bool is fixed this should resolve itself. and even if not at least the template code has a django cannon way to approach it via override. The core module working means I as a dev can deal with the template. There might be some better error handling for installed.ok and status in those lines which I feel would help in writing custom templates, or debugging issues in the future.
Tldr I think the 500 from the failed upstream may have been causing it, fixing the error handling will aid in any future triggering of the underlying problem.