dani-garcia / vaultwarden

Unofficial Bitwarden compatible server written in Rust, formerly known as bitwarden_rs
GNU Affero General Public License v3.0
38.47k stars 1.87k forks source link

U2F not working when using Postgresql #635

Closed TechnicLab closed 4 years ago

TechnicLab commented 5 years ago

An error when trying to login with U2F and using postgresql db: [CAUSE] DatabaseError( bitwarden bitwarden_rs[108]: UniqueViolation, bitwarden bitwarden_rs[108]: "duplicate key value violates unique constraint \"twofactor_user_uuid_atyp bitwarden bitwarden_rs[108]: )

TechnicLab commented 4 years ago

Update: this issue comes into the effect when two 2FA methods are used on the same time.

TechnicLab commented 4 years ago

Any news about this problem?

dabde commented 4 years ago

Hi, have the same issue.

Think the issue is, that the key-data and the challenge are tried to stored in the same database.

https://github.com/dani-garcia/bitwarden_rs/blob/master/src/api/core/two_factor/u2f.rs#L246 https://github.com/dani-garcia/bitwarden_rs/blob/master/src/api/core/two_factor/u2f.rs#L253

both try to use the user_uuid and type the same values. type is 4 for u2f keys. And each user should already be uniq.

In all three databases, we have the uniq key mysql: https://github.com/dani-garcia/bitwarden_rs/blob/master/migrations/mysql/2018-07-11-181453_create_u2f_twofactor/up.sql#L8 sqlite: https://github.com/dani-garcia/bitwarden_rs/blob/master/migrations/sqlite/2018-07-11-181453_create_u2f_twofactor/up.sql#L8 postgres: https://github.com/dani-garcia/bitwarden_rs/blob/master/migrations/postgresql/2019-09-12-100000_create_tables/up.sql#L116

the only difference is, postgres use for save an insert_into: https://github.com/dani-garcia/bitwarden_rs/blob/master/src/db/models/two_factor.rs#L75 and mysql/sqlite an replace_into: https://github.com/dani-garcia/bitwarden_rs/blob/master/src/db/models/two_factor.rs#L87

related to the diesel support of replace_into: https://docs.rs/diesel/1.4.3/diesel/fn.replace_into.html

so fix would be, store challenge somewhere else, or remove the entry before using insert_into

Maybe @swedishborgie can give some support/input how to solve this issue.

thanks in advance ;)

swedishborgie commented 4 years ago

I concur with @dabde's assessment; the issue is that the unique key is being violated and the PostgreSQL version of the function will only do an upsert if the primary key is violated (missing the unique constraint). The replace_into function for MySQL/sqlite covers both.

This should hopefully be as simple as adding another .on_conflict() clause for the unique constraint. I will hopefully have some time tonight to throw together a patch and send over a PR.