cedarcode / webauthn-rails-demo-app

Rails app demonstrating a WebAuthn password-less login
https://webauthn.cedarcode.com
Apache License 2.0
100 stars 41 forks source link

Callback handling - why base64 encode rawId instead of using id #119

Closed truongnmt closed 1 year ago

truongnmt commented 3 years ago

First, thanks for the implementation! Carefully written, also the test was really helpful!

By the way, when I read the credential callback implementation: https://github.com/cedarcode/webauthn-rails-demo-app/blob/d9b73e20a7272e8d9f7a26c48ec49e5100295939/app/controllers/credentials_controller.rb#L27

Why do we have to base64 encode the rawId? Why not use webauthn_credential.id straight away? As written in spec, id is base64(rawId) already. By manually base64(rawId) and save it to the database, what benefit do we have over saving id? https://www.w3.org/TR/credential-management-1/#dom-credential-id

Beside, in session_controller we use the base64 encoded rawId to create allow list and send to authenticator. https://github.com/cedarcode/webauthn-rails-demo-app/blob/d9b73e20a7272e8d9f7a26c48ec49e5100295939/app/controllers/sessions_controller.rb#L11

So overall, save id returned from authenticator then using that id to send back to authenticator make sense to me.

Brantron commented 2 years ago

Great callout, after using this template as inspiration for my own implementation I came back to make this same issue. Hopefully folks in the future will see this and understand that they can simplify the implementation a bit by using id and not manually encoding the raw_id

rromanchuk commented 1 year ago

@Brantron have you seen a refreshed implementation on the client side anywhere? I don't use UJS, only Turbo + Stimulus, so I need to refactor all the xhr eventing.

I suppose I should just start fresh, using https://github.com/github/webauthn-json/blob/main/src/dev/demo/index.ts as a guide