fractal-analytics-platform / fractal-server

Fractal backend
https://fractal-analytics-platform.github.io/fractal-server/
BSD 3-Clause "New" or "Revised" License
11 stars 3 forks source link

Review passlib/pwdlib switch and update to fastapi-users v13 #1708

Open tcompa opened 2 months ago

tcompa commented 2 months ago

Trough fastapi-users, we rely on passlib for password hashing. Despite being a mainstream library, this went unmantained (see e.g. https://github.com/fractal-analytics-platform/fractal-server/issues/1041 and https://foss.heptapod.net/python-libs/passlib/-/issues/190). As a result, we cannot bump versions of fastapi-users (which is not an urgent issue) nor bcrypt (which maybe more of an issue).

The fastapi-users mantainer created a https://frankie567.github.io/pwdlib/, a lightweight passlib replacement. In principle the update is safe, since it goes through hash update:

Password are now hashed using the Argon2 algorithm by default. Passwords created with the previous default algorithm (bcrypt) will still be verified correctly and upgraded to Argon2 when the user logs in. (https://github.com/fastapi-users/fastapi-users/releases/tag/v13.0.0)

We should double-check this on a test instance of fractal-server, where we first create users with fastapi-users v12, then update to v13 and check how it works.

ychiucco commented 2 months ago

Switch to main, install, set database and run the app:

git checkout tags/2.3.10
poetry install -E  postgres
poetry run fractalctl set-db
poetry run fractalctl start

Run fractal-web and register a new user new@user.xy with password password:

select * from user_oauth;

 id |      email       |                       hashed_password                        | is_active | is_superuser | is_verified | slurm_user |      cache_dir      | username | slurm_accounts
----+------------------+--------------------------------------------------------------+-----------+--------------+-------------+------------+---------------------+----------+----------------
  1 | admin@fractal.xy | $2b$12$yRTnPhuDSukYt1DMjGEI7OBV2PD2/Q/lcB.RgjA3khj.PC/Gyc9wu | t         | t            | t           |            |                     | admin    | []
  2 | new@user.xy      | $2b$12$3h1rt3SZrMQPIi.9rlMEbuo2rWHnGPQ7fYj5blLbaBcRzDG62KmUC | t         | f            | t           |            | /Users/yuri/Desktop |          | []
(2 rows)

Stop the app, switch to 1708-review-passlibpwdlib-switch-and-update-to-fastapi-users-v13, install and run the app:

git switch 1708-review-passlibpwdlib-switch-and-update-to-fastapi-users-v13
poetry install -E postgres
poetry run fractalctl start

Stop the app, switch back to main, install and run the app:

git checkout tags/2.3.10
poetry install -E  postgres
poetry run fractalctl start

I cannot login anymore using the same password.

tcompa commented 2 months ago

Thanks a lot!

The example above (and https://github.com/fractal-analytics-platform/fractal-server/pull/1717) proves that this is a perfectly viable approach to updating.

With @ychiucco we decided to wait a bit longer before committing to a new (critical) dependency, to track its progress and its integration with fastapi-users (e.g. right now the latest fastapi-users does not use the very latest pwdlib - which is anyway a very minor issue).

We should look back at this issue e.g. in one or two months.


When looking back at this, let's also double-check the resources required for argon2 operations (e.g. we may want to use a single thread and not four).