Upgrade to webauthn 1.6.0

[Natim]

No description provided.

[dunderrrrrr]

Is this about to be merged into main when checks pass? @justinmayer
Would much appreciate webauthn version 1.6.0 😄

[Natim]

@dunderrrrrr did you had a chance to test kagi with this new branch? I am a bit worried that the migration is not going to be easy.

[dunderrrrrr]

@Natim not yet, ill give it a shot when I have time. Thanks!

[justinmayer]

@dunderrrrrr wrote: Is this about to be merged into main when checks pass?

I am traveling for the next week, and when I return I will be able to test these changes on a production application. In the interim, as Rémy suggested, any testing you can do would be much appreciated!

[inbalzelinger]

If you want to allow the user to register new usb nfc yubikey, one that it's pin code wasn't defined before on this specific browser you should define the
_authenticator_selection.resident_key=ResidentKeyRequirement.PREFERRED see

[rphlo]

Should not use hardcoded URLs, instead use window.Kagi.verify_credential_info
Same for other hardcoded URLs in this file.

[justinmayer]

After testing the contents of this upgrade-webauthn-1.6.0 branch on an existing production application that relocates the Kagi views from its default path, I encountered an error when trying to add a WebAuthn key. I finally found the time to reproduce this error using the bundled test project, which I will document here.

First, starting on the current main branch, I followed the steps in the README to set up the test project, including running the initial migrations, creating a superuser, etc. Then I logged into http://localhost:8000 using Firefox 112.0.1, added a new WebAuthn key successfully, logged out, logged back in using the WebAuthn key, and finally removed the WebAuthn key. In short, I confirmed that basic WebAuthn behavior functions correctly in the current main branch. In a separate terminal session and virtual environment, I switched to the upgrade-webauthn-1.6.0 branch, installed the upgraded dependencies and code via poetry upgrade && poetry install, ran migrations, and confirmed that all of the above steps continue to function as expected. So far, so good.

Then, switching back to my other terminal session running the current main branch, I modified testproj/testproj/urls.py, adding these imports…

from kagi.views import api as kagi_api_views
from kagi.views.login import KagiLoginView

… these urlpatterns …

path("login/", KagiLoginView.as_view(), name="login"),
path("settings/security/", include("kagi.urls", namespace="kagi")),

… and I commented out this line, which is obviated by the line above:

# path("kagi/", include("kagi.urls", namespace="kagi")),

After making these changes, I confirmed that all of the above WebAuthn-related behavior continues to function as expected on the current main branch, even though the Kagi views have been relocated from /kagi/ to /settings/security/.

However, when I switch back to the upgrade-webauthn-1.6.0 branch, attempting to add a WebAuthn key results in the following error in the browser console:

Uncaught (in promise) SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data

    ProvisionWebAuthn http://localhost:8000/static/kagi/webauthn.js:211
    AsyncFunctionThrow self-hosted:814
    (Async: async)
    doWebAuthn http://localhost:8000/static/kagi/webauthn.js:47

GET | http://localhost:8000/kagi/api/begin-activate/
Status: 404 Not Found

In short, it seems the new branch depends on the /kagi/ prefix in a way that the current main branch does not, which at least for me feels like a regression.

@Natim: Do you have any ideas about what is going on here, and perhaps how it might be resolved?

(Copying @apollo13, @carltongibson & @MarkusH)

[rphlo]

After testing the contents of this upgrade-webauthn-1.6.0 branch on an existing production application that relocates the Kagi views from its default path, I encountered an error when trying to add a WebAuthn key. I finally found the time to reproduce this error using the bundled test project, which I will document here.

First, starting on the current main branch, I followed the steps in the README to set up the test project, including running the initial migrations, creating a superuser, etc. Then I logged into http://localhost:8000 using Firefox 112.0.1, added a new WebAuthn key successfully, logged out, logged back in using the WebAuthn key, and finally removed the WebAuthn key. In short, I confirmed that basic WebAuthn behavior functions correctly in the current main branch. In a separate terminal session and virtual environment, I switched to the upgrade-webauthn-1.6.0 branch, installed the upgraded dependencies and code via poetry upgrade && poetry install, ran migrations, and confirmed that all of the above steps continue to function as expected. So far, so good.

Then, switching back to my other terminal session running the current main branch, I modified testproj/testproj/urls.py, adding these imports…

from kagi.views import api as kagi_api_views

from kagi.views.login import KagiLoginView

… these urlpatterns …

path("login/", KagiLoginView.as_view(), name="login"),

path("settings/security/", include("kagi.urls", namespace="kagi")),

… and I commented out this line, which is obviated by the line above:

# path("kagi/", include("kagi.urls", namespace="kagi")),

After making these changes, I confirmed that all of the above WebAuthn-related behavior continues to function as expected on the current main branch, even though the Kagi views have been relocated from /kagi/ to /settings/security/.

However, when I switch back to the upgrade-webauthn-1.6.0 branch, attempting to add a WebAuthn key results in the following error in the browser console:

Uncaught (in promise) SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data



    ProvisionWebAuthn http://localhost:8000/static/kagi/webauthn.js:211

    AsyncFunctionThrow self-hosted:814

    (Async: async)

    doWebAuthn http://localhost:8000/static/kagi/webauthn.js:47



GET | http://localhost:8000/kagi/api/begin-activate/

Status: 404 Not Found

In short, it seems the new branch depends on the /kagi/ prefix in a way that the current main branch does not, which at least for me feels like a regression.

See my earlier comment that point out the origin of this issue.

[justinmayer]

@rphlo: Around the time of your previous comment, I tried using window.Kagi.verify_credential_info, but I could not figure out how to use it to resolve the problem. Is there any chance you could propose a specific diff that would successfully resolve the problem in question?

[rphlo]

@rphlo: Around the time of your previous comment, I tried using window.Kagi.verify_credential_info, but I could not figure out how to use it to resolve the problem. Is there any chance you could propose a specific diff that would successfully resolve the problem in question?

I have successfully ran this branch with the following change in webauthn.js rphlo@4db7f51 (this includes some linting too)

[apollo13]

Hi @rphlo, thank you so much for this. I will pulling your chances into this branch shortly and will start testing.

[MarkusH]

@rphlo: Around the time of your previous comment, I tried using window.Kagi.verify_credential_info, but I could not figure out how to use it to resolve the problem. Is there any chance you could propose a specific diff that would successfully resolve the problem in question?

As a follow-up to this, I opened #65

[rphlo]

@rphlo: Around the time of your previous comment, I tried using window.Kagi.verify_credential_info, but I could not figure out how to use it to resolve the problem. Is there any chance you could propose a specific diff that would successfully resolve the problem in question?

As a follow-up to this, I opened #65

@MarkusH This is an issue I also had in my project. I fixed it somehow similarly, however this is something that I believe is outside the scope of this PR and can be fixed in separate one as it affect also other branches.

[MarkusH]

@rphlo: Around the time of your previous comment, I tried using window.Kagi.verify_credential_info, but I could not figure out how to use it to resolve the problem. Is there any chance you could propose a specific diff that would successfully resolve the problem in question?

As a follow-up to this, I opened #65

@MarkusH This is an issue I also had in my project. I fixed it somehow similarly, however this is something that I believe is outside the scope of this PR and can be fixed in separate one as it affect also other branches.

Yes, absolutely!

[justinmayer]

Many sincere thanks to @Natim for all his work on this endeavor, as well as to everyone else who helped review and improve it! 🌟