implement unecrypted passkey login and cleanup code a bit mor

migrations/sqlite/2025-06-03-173809_create_web_authn_credentials_table/up.sql

@@ -4,8 +4,8 @@ CREATE TABLE web_authn_credentials (
     name         TEXT NOT NULL,
     credential           TEXT NOT NULL,
     supports_prf         BOOLEAN NOT NULL,
-    encrypted_user_key         TEXT NOT NULL,
-    encrypted_public_key         TEXT NOT NULL,
-    encrypted_private_key         TEXT NOT NULL,
+    encrypted_user_key         TEXT,
+    encrypted_public_key         TEXT,
+    encrypted_private_key         TEXT,
     FOREIGN KEY(user_uuid) REFERENCES users(uuid)
 );

src/api/core/mod.rs

@@ -204,6 +204,7 @@ async fn post_api_webauthn_delete(data: Json<PasswordOrOtpData>, uuid: String, h
     Ok(Status::Ok)
 }
 
+// TODO replace this with something else
 static WEBAUTHN_STATES: OnceLock<Mutex<HashMap<UserId, RegistrationState>>> = OnceLock::new();
 
 #[post("/webauthn/attestation-options", data = "<data>")]
@@ -213,7 +214,7 @@ async fn post_api_webauthn_attestation_options(data: Json<PasswordOrOtpData>, he
     
     data.validate(&user, false, &mut conn).await?;
 
-    // C# does this check as well
+    // TODO C# does this check as well, should there be an option in the admin panel to disable passkey login?
     // await ValidateIfUserCanUsePasskeyLogin(user.Id);
 
     // TODO add existing keys here when the table exists
@@ -240,71 +241,41 @@ async fn post_api_webauthn_attestation_options(data: Json<PasswordOrOtpData>, he
     let mut options = serde_json::to_value(challenge.public_key)?;
     options["status"] = "ok".into();
     options["errorMessage"] = "".into();
-    // TODO does this need to be set?
+    
+    // TODO test if the client actually expects this field to exist
     options["extensions"] = Value::Object(serde_json::Map::new());
     
-    // TODO make this nicer
-    let mut webauthn_credential_create_options = Value::Object(serde_json::Map::new());
-    webauthn_credential_create_options["options"] = options;
-    webauthn_credential_create_options["object"] = "webauthnCredentialCreateOptions".into();
-
-    // TODO this hopefully shouldn't be needed
-    // webauthn_credential_create_options["token"] = "atoken".into();
-
-    Ok(Json(webauthn_credential_create_options))
+    Ok(Json(json!({
+        "options": options,
+        "object": "webauthnCredentialCreateOptions"
+    })))
 }
 
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
-// TODO remove allow dead_code
-#[allow(dead_code)]
 struct WebAuthnLoginCredentialCreateRequest {
     device_response: RegisterPublicKeyCredentialCopy,
     name: String,
-    // TODO this is hopefully not needed
-    // token: String,
     supports_prf: bool,
-    encrypted_user_key: String,
-    encrypted_public_key: String,
-    encrypted_private_key: String,
+    encrypted_user_key: Option<String>,
+    encrypted_public_key: Option<String>,
+    encrypted_private_key: Option<String>,
 }
 
 #[post("/webauthn", data = "<data>")]
 async fn post_api_webauthn(data: Json<WebAuthnLoginCredentialCreateRequest>, headers: Headers, mut conn: DbConn) -> ApiResult<Status> {
-    // this check await ValidateIfUserCanUsePasskeyLogin(user.Id); again
     let data: WebAuthnLoginCredentialCreateRequest = data.into_inner();
-    // let data: WebAuthnLoginCredentialCreateRequest = serde_json::from_str(&data)?;
     let user = headers.user;
     
-    // TODO Retrieve and delete the saved challenge state here
-
-
     // Verify the credentials with the saved state
     let (credential, _data) = {
         let mut states = WEBAUTHN_STATES.get().unwrap().lock().unwrap();
         let state = states.remove(&user.uuid).unwrap();
 
+        // TODO make the closure check if the credential already exists
         WebauthnConfig::load(true).register_credential(&data.device_response.into(), &state, |_| Ok(false))?
     };
     
-    // TODO add existing keys here when the table exists
-    // let mut registrations: Vec<_> = get_webauthn_registrations(&user.uuid, &mut conn).await?.1;
-    // // TODO: Check for repeated ID's
-    // registrations.push(WebauthnRegistration {
-    //     id: data.id.into_i32()?,
-    //     name: data.name,
-    //     migrated: false,
-    //
-    //     credential,
-    // });
-
-    // let registrations = Vec::new();
-
-    // TODO Save the registration
-    // TwoFactor::new(user.uuid.clone(), TwoFactorType::Webauthn, serde_json::to_string(&registrations)?)
-    //     .save(&mut conn)
-    //     .await?;
-
     WebAuthnCredential::new(
         user.uuid,
         data.name,
@@ -326,10 +297,10 @@ async fn get_api_webauthn(headers: Headers, mut conn: DbConn) -> Json<Value> {
         .await
         .into_iter()
         .map(|wac| {
-            // TODO generate prfStatus from GetPrfStatus() in C#
             json!({
                 "id": wac.uuid,
                 "name": wac.name,
+                // TODO generate prfStatus like GetPrfStatus() does in the C# implementation
                 "prfStatus": 0,
                 "encryptedUserKey": wac.encrypted_user_key,
                 "encryptedPublicKey": wac.encrypted_public_key,

src/api/core/two_factor/webauthn.rs

@@ -146,7 +146,6 @@ async fn get_webauthn(data: Json<PasswordOrOtpData>, headers: Headers, mut conn:
     })))
 }
 
-// TODO Creation call
 #[post("/two-factor/get-webauthn-challenge", data = "<data>")]
 async fn generate_webauthn_challenge(data: Json<PasswordOrOtpData>, headers: Headers, mut conn: DbConn) -> JsonResult {
     let data: PasswordOrOtpData = data.into_inner();
@@ -261,7 +260,6 @@ impl From<PublicKeyCredentialCopy> for PublicKeyCredential {
     }
 }
 
-// TODO Confirmation call
 #[post("/two-factor/webauthn", data = "<data>")]
 async fn activate_webauthn(data: Json<EnableWebauthnData>, headers: Headers, mut conn: DbConn) -> JsonResult {
     let data: EnableWebauthnData = data.into_inner();

src/api/identity.rs

@@ -125,7 +125,7 @@ pub struct PublicKeyCredentialCopy {
     pub raw_id: Base64UrlSafeData,
     pub response: AuthenticatorAssertionResponseRawCopy,
     pub r#type: String,
-    // TODO think about what to do with this field, currently this is ignored in the conversion
+    // This field is unused and discarded when converted to PublicKeyCredential
     pub extensions: Option<Value>,
 }
 
@@ -199,44 +199,57 @@ async fn _webauthn_login(
     
     let web_authn_credentials = WebAuthnCredential::find_all_by_user(&user.uuid, conn).await;
 
-    let credentials = web_authn_credentials
+    let parsed_credentials = web_authn_credentials
         .iter()
         .map(|c| {
             serde_json::from_str(&c.credential)
         }).collect::<Result<Vec<Credential>, _>>()?;
     
-    let web_authn_credential = {
+    let pairs = web_authn_credentials.into_iter()
+        .zip(parsed_credentials.clone())
+        .collect::<Vec<_>>();
+    
+    let authenticator_data;
+    let (web_authn_credential, mut credential) = {
         let token = data.token.as_ref().unwrap();
         let mut states = WEBAUTHN_AUTHENTICATION_STATES.get().unwrap().lock().unwrap();
         let mut state = states.remove(token).unwrap();
         let resp = device_response.into();
 
-        state.set_allowed_credentials(credentials);
+        state.set_allowed_credentials(parsed_credentials);
         
-        // TODO update respective credential in database
-        let (credential_id, auth_data) = WebauthnConfig::load(true)
-            .authenticate_credential(&resp, &state)?;
+        let credential_id;
         
-        if !auth_data.user_verified {
-            // TODO throw an error here
-            panic!()
-        }
-
-        web_authn_credentials.into_iter()
-            .find(|c| &serde_json::from_str::<Credential>(&c.credential).unwrap().cred_id == credential_id)
-            .unwrap()
-
-        /* TODO return this error on failure
-        err!(
-                "Username or password is incorrect. Try again",
+        if let Ok((cred_id, auth_data)) = WebauthnConfig::load(true)
+            .authenticate_credential(&resp, &state) {
+            credential_id = cred_id;
+            authenticator_data = auth_data;
+        } else {
+            err!(
+                "Passkey authentication Failed.",
                 format!("IP: {}. Username: {username}.", ip.ip),
                 ErrorEvent {
                     event: EventType::UserFailedLogIn,
                 }
             )
-        */
+        }
+
+        // TODO should this check be done? Since we need to trust the client here anyway ...
+        // if !auth_data.user_verified { some_error }
+
+        pairs.into_iter()
+            .find(|(_, c)| &c.cred_id == credential_id)
+            .unwrap()
     };
     
+    // update the counter
+    credential.counter = authenticator_data.counter;
+    WebAuthnCredential::update_credential_by_uuid(
+        &web_authn_credential.uuid, 
+        serde_json::to_string(&credential)?, 
+        conn
+    ).await?;
+
     let now = Utc::now().naive_utc();
 
     if user.verified_at.is_none() && CONFIG.mail_enabled() && CONFIG.signups_verify() {
@@ -273,7 +286,7 @@ async fn _webauthn_login(
 
     let (mut device, new_device) = get_device(&data, conn, &user).await;
 
-    // TODO is this needed with passkeys?
+    // TODO is this wanted with passkeys?
     if CONFIG.mail_enabled() && new_device {
         if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, &device).await {
             error!("Error sending new device email: {e:#?}");
@@ -335,7 +348,7 @@ async fn _webauthn_login(
         json!({"Object": "masterPasswordPolicy"})
     };
 
-    let result = json!({
+    let mut result = json!({
         "access_token": access_token,
         "expires_in": expires_in,
         "token_type": "Bearer",
@@ -354,14 +367,17 @@ async fn _webauthn_login(
         "scope": scope,
         "UserDecryptionOptions": {
             "HasMasterPassword": !user.password_hash.is_empty(),
-            "WebAuthnPrfOption": {
-                "EncryptedPrivateKey": web_authn_credential.encrypted_private_key,
-                "EncryptedUserKey": web_authn_credential.encrypted_user_key,
-            },
             "Object": "userDecryptionOptions"
         },
     });
     
+    if web_authn_credential.encrypted_private_key.is_some() && web_authn_credential.encrypted_user_key.is_some() {
+        result["UserDecryptionOptions"]["WebAuthnPrfOption"] = json!({
+            "EncryptedPrivateKey": web_authn_credential.encrypted_private_key,
+            "EncryptedUserKey": web_authn_credential.encrypted_user_key,
+        })
+    }
+
     info!("User {username} logged in successfully. IP: {}", ip.ip);
     Ok(Json(result))
 }
@@ -963,6 +979,7 @@ async fn identity_register(data: Json<RegisterData>, conn: DbConn) -> JsonResult
     _register(data, false, conn).await
 }
 
+// TODO this should be removed and either use something similar to what bitwarden employs or something else
 static WEBAUTHN_AUTHENTICATION_STATES: OnceLock<Mutex<HashMap<String, AuthenticationState>>> = OnceLock::new();
 
 #[get("/accounts/webauthn/assertion-options")]
@@ -973,7 +990,6 @@ fn get_web_authn_assertion_options() -> JsonResult {
             None,
         )?;
     
-    // TODO this needs to be solved in another way to avoid DoS
     let t = util::get_uuid();
     WEBAUTHN_AUTHENTICATION_STATES.get_or_init(|| Mutex::new(HashMap::new())).lock().unwrap().insert(t.clone(), state);
 
@@ -1052,7 +1068,7 @@ async fn register_finish(data: Json<RegisterData>, conn: DbConn) -> JsonResult {
 struct ConnectData {
     #[field(name = uncased("grant_type"))]
     #[field(name = uncased("granttype"))]
-    grant_type: String, // refresh_token, password, client_credentials (API key)
+    grant_type: String, // refresh_token, password, client_credentials (API key), webauthn
 
     // Needed for grant_type="refresh_token"
     #[field(name = uncased("refresh_token"))]
@@ -1100,10 +1116,10 @@ struct ConnectData {
     #[field(name = uncased("authrequest"))]
     auth_request: Option<AuthRequestId>,
 
-    // Needed for "login with passkey"
+    // Needed for grant_type = "webauthn"
     #[field(name = uncased("deviceresponse"))]
     device_response: Option<String>,
-    // TODO this may be removed again if implemented correctly
+    // TODO this may be removed when `WEBAUTHN_AUTHENTICATION_STATES` is removed
     #[field(name = uncased("token"))]
     token: Option<String>,
 }

src/db/models/user.rs

@@ -476,5 +476,5 @@ impl Invitation {
 )]
 #[deref(forward)]
 #[from(forward)]
-// TODO create a way to construct this
+// TODO this also shouldn't be public
 pub struct UserId(pub String);

src/db/models/web_authn_credential.rs

@@ -16,9 +16,9 @@ db_object! {
         pub name: String,
         pub credential: String,
         pub supports_prf: bool,
-        pub encrypted_user_key: String,
-        pub encrypted_public_key: String,
-        pub encrypted_private_key: String,
+        pub encrypted_user_key: Option<String>,
+        pub encrypted_public_key: Option<String>,
+        pub encrypted_private_key: Option<String>,
     }
 }
 
@@ -28,9 +28,9 @@ impl WebAuthnCredential {
         name: String,
         credential: String,
         supports_prf: bool,
-        encrypted_user_key: String,
-        encrypted_public_key: String,
-        encrypted_private_key: String,
+        encrypted_user_key: Option<String>,
+        encrypted_public_key: Option<String>,
+        encrypted_private_key: Option<String>,
     ) -> Self {
         Self {
             uuid: WebAuthnCredentialId(crate::util::get_uuid()),
@@ -78,6 +78,16 @@ impl WebAuthnCredential {
             ).execute(conn).map_res("Error removing web_authn_credential for user")
         }}
     }
+    
+    pub async fn update_credential_by_uuid(uuid: &WebAuthnCredentialId, credential: String, conn: &mut DbConn) -> EmptyResult {
+        db_run! { conn: {
+            diesel::update(web_authn_credentials::table
+                .filter(web_authn_credentials::uuid.eq(uuid))
+            ).set(web_authn_credentials::credential.eq(credential))
+                .execute(conn)
+                .map_res("Error updating credential for web_authn_credential")
+        }}
+    }
 }
 
 #[derive(
@@ -96,4 +106,5 @@ impl WebAuthnCredential {
     Deserialize,
     UuidFromParam,
 )]
+// TODO this probably shouldn't need to be public
 pub struct WebAuthnCredentialId(pub String);

src/db/schemas/sqlite/schema.rs

@@ -327,9 +327,9 @@ table! {
         name -> Text,
         credential -> Text,
         supports_prf -> Bool,
-        encrypted_user_key -> Text,
-        encrypted_public_key -> Text,
-        encrypted_private_key -> Text,
+        encrypted_user_key -> Nullable<Text>,
+        encrypted_public_key -> Nullable<Text>,
+        encrypted_private_key -> Nullable<Text>,
     }
 }