diff --git a/migrations/sqlite/2025-06-03-173809_create_web_authn_credentials_table/up.sql b/migrations/sqlite/2025-06-03-173809_create_web_authn_credentials_table/up.sql index 9851088c..9f885c34 100644 --- a/migrations/sqlite/2025-06-03-173809_create_web_authn_credentials_table/up.sql +++ b/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) ); diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index 0b46dfa8..7238fa4a 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -204,6 +204,7 @@ async fn post_api_webauthn_delete(data: Json, uuid: String, h Ok(Status::Ok) } +// TODO replace this with something else static WEBAUTHN_STATES: OnceLock>> = OnceLock::new(); #[post("/webauthn/attestation-options", data = "")] @@ -213,7 +214,7 @@ async fn post_api_webauthn_attestation_options(data: Json, 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, 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, + encrypted_public_key: Option, + encrypted_private_key: Option, } #[post("/webauthn", data = "")] async fn post_api_webauthn(data: Json, headers: Headers, mut conn: DbConn) -> ApiResult { - // 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(®istrations)?) - // .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 { .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, diff --git a/src/api/core/two_factor/webauthn.rs b/src/api/core/two_factor/webauthn.rs index 9600573e..fe9f84d3 100644 --- a/src/api/core/two_factor/webauthn.rs +++ b/src/api/core/two_factor/webauthn.rs @@ -146,7 +146,6 @@ async fn get_webauthn(data: Json, headers: Headers, mut conn: }))) } -// TODO Creation call #[post("/two-factor/get-webauthn-challenge", data = "")] async fn generate_webauthn_challenge(data: Json, headers: Headers, mut conn: DbConn) -> JsonResult { let data: PasswordOrOtpData = data.into_inner(); @@ -261,7 +260,6 @@ impl From for PublicKeyCredential { } } -// TODO Confirmation call #[post("/two-factor/webauthn", data = "")] async fn activate_webauthn(data: Json, headers: Headers, mut conn: DbConn) -> JsonResult { let data: EnableWebauthnData = data.into_inner(); diff --git a/src/api/identity.rs b/src/api/identity.rs index fca0aced..b71697e2 100644 --- a/src/api/identity.rs +++ b/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, } @@ -199,43 +199,56 @@ 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::, _>>()?; - - let web_authn_credential = { + + let pairs = web_authn_credentials.into_iter() + .zip(parsed_credentials.clone()) + .collect::>(); + + 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); - - // TODO update respective credential in database - let (credential_id, auth_data) = WebauthnConfig::load(true) - .authenticate_credential(&resp, &state)?; - - if !auth_data.user_verified { - // TODO throw an error here - panic!() - } - - web_authn_credentials.into_iter() - .find(|c| &serde_json::from_str::(&c.credential).unwrap().cred_id == credential_id) - .unwrap() - - /* TODO return this error on failure - err!( - "Username or password is incorrect. Try again", + state.set_allowed_credentials(parsed_credentials); + + let credential_id; + + 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(); @@ -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,13 +367,16 @@ 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, 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>> = OnceLock::new(); #[get("/accounts/webauthn/assertion-options")] @@ -972,8 +989,7 @@ fn get_web_authn_assertion_options() -> JsonResult { Vec::new(), 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, 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, - // Needed for "login with passkey" + // Needed for grant_type = "webauthn" #[field(name = uncased("deviceresponse"))] device_response: Option, - // 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, } diff --git a/src/db/models/user.rs b/src/db/models/user.rs index d38ea5ba..f3f54720 100644 --- a/src/db/models/user.rs +++ b/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); diff --git a/src/db/models/web_authn_credential.rs b/src/db/models/web_authn_credential.rs index a548aaaf..75b1f792 100644 --- a/src/db/models/web_authn_credential.rs +++ b/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, + pub encrypted_public_key: Option, + pub encrypted_private_key: Option, } } @@ -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, + encrypted_public_key: Option, + encrypted_private_key: Option, ) -> 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); diff --git a/src/db/schemas/sqlite/schema.rs b/src/db/schemas/sqlite/schema.rs index 6674cdba..194b3f91 100644 --- a/src/db/schemas/sqlite/schema.rs +++ b/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, + encrypted_public_key -> Nullable, + encrypted_private_key -> Nullable, } }