feat(webauthn): make webauthn params configurable #48
Conversation
* add attachment, attestation preference and resident key requirement params to create and update tenant endpoints * add fallback values when params are not provided * add migrations for new fields in webauthn config in database * reflect changes in README.md and admin spec Closes: #45
* add login init dto with user_id as optional param * extend service to switch to BeginLogin /ValidateLogin when user_id was given * extend database to persist login state in sessiondata for login/finalize * extend openapi spec to reflect optional user_id parameter for login/initialize Closes: #33
* add config for mfa passkeys * add endpoints for mfa passkeys TODO: add docs Closes: #45
|
Also a flag or indication that a credential can only be used as a MFA credential is missing. Currently you can use a credential that was created as MFA for a passkey login. |
* mfa config has now its own database table * mfa config does not require RP config anymore * webauthn client for mfa uses rp from passkey config * update openapi specs to reflect changes Closes: #45
|
I updated all findings + openapi spec and added the missed mfa flag |
There was a problem hiding this comment.
When calling /mfa/registration/initialize the options are missing the attachment.
When trying to use a security key at /mfa/login/finalize I always get an error {"title":"failed to get user handle","details":"user not found","status":401}. When using a credential that was created through the passkey endpoints I don't get this error.
server/persistence/migrations/20240322134127_create_mfa_configs.up.fizz
Outdated
Show resolved
Hide resolved
* make MFA config optional for backwards compatibility * rename some variables for better clarity * add audit log types for MFA login for better distinction * add missing attachment option to mfa client * add api key vs tenant secrets check for mfa/non-discover login Closes: #45
I fixed all findings but was unable to reproduce your 401 error. I do not own a security key so I tried to reproduce it with an iPhone which worked fine. |
You can use the Chrome Virtual Authenticator to test it. |
|
I don't know if my attachment changes fixed them. but it works on my side: Screen.Recording.2024-04-03.at.16.39.23.movEdit: I think I found a big I have to fix and then test again. I tried the new MFA optional mechanism at the same time. And while looking into the code I think I made a copy paste error when creating the default MFA config (using all webauthn parameters from passkey config instead of defaults when mfa config is missing in dto) |
use mfa default params instead of passkey dto ones when creating a mfa default config on admin operations `create tenant` or `update config`
Use sessionData userId as userhandle when login is MFA or non-discoverable. Also check if credential is in allowed list Closes: #45
credential check will be done by go-webauthn lib Closes: #45
There was a problem hiding this comment.
When a user has one passkey and one 2FA credential and /login/initialize is called with the userID in the body, the 2FA credential is included in the allowCredentials list. This leads to an uncool UX for the user. The 2FA credential ID should not be returned for passkey logins.
* remove mfa credentials from allowedCredentials when using non mfa routes * for good measure also remove mfa credentials from FindCredentialById when used on Non-MFA routes * rename CreateApiKeyError to CheckApiKey * increase versions of openapi spec * change defaults for mfa config object in admin spec * cleanup: remove unused methods from handler/webauthn.go Closes: #45
Closes: #45, #50