mkalioby / django-passkeys

Django Authentication Backend For Passkeys
MIT License
187 stars 20 forks source link

Setup instruction are not working #6

Open kakulukia opened 1 year ago

kakulukia commented 1 year ago

Hi!

I just wanted to try to add passkeys to my app, but following the instructions i get some errors.

  1. "KEY_ATTACHMENT = NONE | passkeys.Attachment.CROSS_PLATFORM | passkeys.Attachment.PLATFORM"

NONE is not defined or referenced. And if i change it to the Python internal None i get this: TypeError: unsupported operand type(s) for |: 'NoneType' and 'AuthenticatorAttachment'

Anyway, even using or operand this instruction seems to have no effect since None or anything will always be anything, so stripped it down to "KEY_ATTACHMENT = passkeys.Attachment.CROSS_PLATFORM | passkeys.Attachment.PLATFORM". I currently down know what it is doing, but i guess it should work the same as before.

  1. url() is outdated and should be replaced with path()

  2. The passkeys/ index view is annotated with @login_required .. to me this makes no sense, since this IS or at least SHOULD BE the login view! Having passkeys working after i logged in the normal way kinda defeats the whole purpose. So i removed the annotation. But now i get "Field 'id' expected a number but got <SimpleLazyObject: <django.contrib.auth.models.AnonymousUser object at 0x108df3ad0>>". So it locks like the login view is actually expecting a logged in user.

I dont get it. Please clarify! 🥹

mkalioby commented 1 year ago

Hi!

I just wanted to try to add passkeys to my app, but following the instructions i get some errors.

1. "KEY_ATTACHMENT = NONE | passkeys.Attachment.CROSS_PLATFORM | passkeys.Attachment.PLATFORM"

If you want to use both Platform and Cross Platform don't include this to the settings file.

NONE is not defined or referenced. And if i change it to the Python internal None i get this: TypeError: unsupported operand type(s) for |: 'NoneType' and 'AuthenticatorAttachment'

Anyway, even using or operand this instruction seems to have no effect since None or anything will always be anything, so stripped it down to "KEY_ATTACHMENT = passkeys.Attachment.CROSS_PLATFORM | passkeys.Attachment.PLATFORM". I currently down know what it is doing, but i guess it should work the same as before.

2. url() is outdated and should be replaced with path()

Which file are you talking about?

3. The passkeys/ index view is annotated with @login_required .. to me this makes no sense, since this IS or at least SHOULD BE the login view! Having passkeys working after i logged in the normal way kinda defeats the whole purpose. So i removed the annotation. But now i get "Field 'id' expected a number but got <SimpleLazyObject: <django.contrib.auth.models.AnonymousUser object at 0x108df3ad0>>". So it locks like the login view is actually expecting a logged in user.

I dont get it. Please clarify! 🥹

Nope, this passkeys/ is for the user to manage passkeys not login. Login is done by changing your login as stated in points 8,9.

kakulukia commented 1 year ago

Sorry, for not reading until the end. I would recommend structuring the setup instructions differently, tho.

  1. Please inform about the options first. What implications do i face choosing one or the other? If both are the default anyway, please mention it. For simplicity i would recommend to remove that KEY_ATTACHMENT from the setup instructions.

  2. im Talking about step 5 of adding the url to my app. url() is actually not available in Django 4 anymore. Please update to path().

  3. okay, understood .. again sorry for not reading until the end. Id like to test whatever im doing step by step, which is actually not possible with the setup instructions. What about moving step number 7 to the very end? First login via passkey and only then it makes sense to have a link to the passkey management page. And add that small hint to step 7 (passkey management page).

mkalioby commented 1 year ago

Sure. You can do a PR request if you like.

kakulukia commented 1 year ago

Okay, let me finish the integration first. :D

But you have to help out for point one. I have no clue what KEY_ATTACHMENT does and what the different options are.

mkalioby commented 1 year ago

In webAuthn, there are 2 types of authenticators

  1. Cross-Platform which allows the user to roam between devices as this is traditionally the Security Keys as Yubico keys
  2. Platform Authenticators which as the name states linked to the device (e.g Apple TouchID and FaceID, Andriod SafetyNet and Windows Hello)

Note: Currently in Passkeys all the platform authicators keys are synced to the vendors cloud so the user can use it on another device. e.g Apple Device in case of apple id or the user Google account if Android or Chrome.

kakulukia commented 1 year ago

So that actually means even when I choose platform it will actually be cross platform? This kinda also says: remove the config parameter, it makes no sense to change it.

Did I understand that correctly?

mkalioby commented 1 year ago

Easier to say, use cross-platform if you want to only use security keys like Yubico. Use Platform when you use the mobile or laptop authenticators.

kakulukia commented 1 year ago

But above you said "allows the user to roam between devices" which means you are not forced to. So cross platform means platform plus 3rd party devices like Yubico?

Is that a correct statement?

mkalioby commented 1 year ago

Cross platform means the device shall be able to roam like Yubico.

Platform authenticator are the ones connected to the devices.

kakulukia commented 1 year ago

okay, finally i was able to login via passkey! 🥳 But there is not just one undocumented step to be added to actually be able to use this lib without trouble.

Cleaning this up will take some time. But lets see what i end up with. I definitely want to add passkeys to my new app! (maybe also all old ones :D)

Here are my key findings as a short reminder:

  1. SSL is needed
  2. getUserCredentials - dont know why, but its broken for me - this version works
def getUserCredentials(username):
    return [AttestedCredentialData(websafe_decode(uk.token)) for uk in UserPasskey.objects.filter(
        user__username=username)]
  1. Bootstrap is not really needed, but not loading it is causing the invisible login button problem. Can be fixed with a colored background:
    button.btn.btn-block.btn-dark(type="button" onclick="authn('loginForm')")
      img(src="{% static 'passkeys/imgs/FIDO-Passkey_Icon-White.png' %}" style="width: 24px; background: grey")
  2. The logon for actually needs to have the ID loginForm. Better give full demo code of the form for easier usage.
  3. jQuery needs to be available for the keypass registration to work, should be removed
  4. Project also depending on fontawesome - cant delete my key due to delete linked rendered with no size (icon not available)
  5. passkey view is supposed to be running in a modal which causes additional JS errors
  6. KEY_ATTACHMENT = passkeys.Attachment.PLATFORM should be the project default as it has the easiest UX and probably is the right choice for most projects
  7. path() instead of url()
  8. {%include 'passkeys.js' %} including script tags in a VueJS app totally breaks things - and actually the file is not even a JS but an HTML file. The wrong extension breaks syntax highlighting in editors.

I hope i didnt forget mayor things, but with all the above considered the onboarding for django-passkeys should be much smoother!

I need some more time to actually create a PR from this.

ThomasWaldmann commented 1 year ago

I also had troubles with KEY_ATTACHMENT. Devs new to passkeys usually do not have a clue what to use and just copy and pasting from the readme resulted in incorrect Python.

So this should look like that in the readme:

# allow all authenticator attachments:
KEY_ATTACHMENT = None
# use *only* external security keys like YubiKeys:
KEY_ATTACHMENT = passkeys.Attachment.CROSS_PLATFORM
# use *only* mobile/laptop builtin authenticators like FaceID, TouchID, Fingerprint, Windows Hello, ...:
KEY_ATTACHMENT = passkeys.Attachment.PLATFORM
kmahelona commented 10 months ago

Some explanation on what FIDO_SERVER_ID and FIDO_SERVER_NAME are and how to set them properly them would also be helpful.

mkalioby commented 10 months ago

Some explanation on what FIDO_SERVER_ID and FIDO_SERVER_NAME are and how to set them properly them would also be helpful.

It is there in README

FIDO_SERVER_ID="localhost"      # Server rp id for FIDO2, it the full domain of your project
FIDO_SERVER_NAME="TestApp"
kmahelona commented 10 months ago

So the django app is the FIDO server? If this were a production site, then it would be e.g. "mydjangosite.com"? Is the server name important? Apologies as I have very little knowledge about the passkeys except what I know about ssl keys and how we use them e.g. to log into servers.

mkalioby commented 10 months ago

Server_ID shall match or submatch for your current domain so if you run your application on app.example.com it shall be the same or example.com

Sever_Name is how the key named on the authenticator so the user can find it in the keychain or authenticator keys list