From 08021e254739c1b8c490ea292594f45445bbda79 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Tue, 28 Nov 2023 23:56:44 +0000 Subject: [PATCH] Add CLI commands to create users and change passwords --- .envrc | 2 + ...3ce2b95fb61fe8bd43f49b1488d0a1bdc7151.json | 24 ++++ ...bea7a0fc5fb55c587a5ac37e519081612e436.json | 46 +++++++ ...ac52c8067c61d6992f8f5045b908eb73bab8b.json | 15 +++ Cargo.lock | 9 ++ Cargo.toml | 3 +- flake.lock | 8 +- flake.nix | 2 +- migrations/20231030092146_users_and_roles.sql | 8 +- src/bin/idcoop.rs | 119 +++++++++++++++--- src/config.rs | 20 +++ src/lib.rs | 1 + src/passwords.rs | 27 ++++ src/store.rs | 56 ++++++++- 14 files changed, 313 insertions(+), 27 deletions(-) create mode 100644 .sqlx/query-113f68e43d5ccd98f47ac9f451f3ce2b95fb61fe8bd43f49b1488d0a1bdc7151.json create mode 100644 .sqlx/query-c105886b934bce762e2bda6488fbea7a0fc5fb55c587a5ac37e519081612e436.json create mode 100644 .sqlx/query-eae27786a7c81ee2199fe3d5c10ac52c8067c61d6992f8f5045b908eb73bab8b.json create mode 100644 src/passwords.rs diff --git a/.envrc b/.envrc index cffc922..f92a1f0 100644 --- a/.envrc +++ b/.envrc @@ -1 +1,3 @@ use flake . --impure +unset LD_LIBRARY_PATH + diff --git a/.sqlx/query-113f68e43d5ccd98f47ac9f451f3ce2b95fb61fe8bd43f49b1488d0a1bdc7151.json b/.sqlx/query-113f68e43d5ccd98f47ac9f451f3ce2b95fb61fe8bd43f49b1488d0a1bdc7151.json new file mode 100644 index 0000000..03c85c1 --- /dev/null +++ b/.sqlx/query-113f68e43d5ccd98f47ac9f451f3ce2b95fb61fe8bd43f49b1488d0a1bdc7151.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO users (user_name, user_id, created_at_utc, password_hash, locked) VALUES ($1, gen_random_uuid(), NOW(), $2, $3) RETURNING user_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "user_id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Bool" + ] + }, + "nullable": [ + false + ] + }, + "hash": "113f68e43d5ccd98f47ac9f451f3ce2b95fb61fe8bd43f49b1488d0a1bdc7151" +} diff --git a/.sqlx/query-c105886b934bce762e2bda6488fbea7a0fc5fb55c587a5ac37e519081612e436.json b/.sqlx/query-c105886b934bce762e2bda6488fbea7a0fc5fb55c587a5ac37e519081612e436.json new file mode 100644 index 0000000..3f02f55 --- /dev/null +++ b/.sqlx/query-c105886b934bce762e2bda6488fbea7a0fc5fb55c587a5ac37e519081612e436.json @@ -0,0 +1,46 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT user_id, user_name, password_hash, locked, created_at_utc FROM users WHERE user_name = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "user_id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "user_name", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "password_hash", + "type_info": "Text" + }, + { + "ordinal": 3, + "name": "locked", + "type_info": "Bool" + }, + { + "ordinal": 4, + "name": "created_at_utc", + "type_info": "Timestamp" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false, + true, + false, + false + ] + }, + "hash": "c105886b934bce762e2bda6488fbea7a0fc5fb55c587a5ac37e519081612e436" +} diff --git a/.sqlx/query-eae27786a7c81ee2199fe3d5c10ac52c8067c61d6992f8f5045b908eb73bab8b.json b/.sqlx/query-eae27786a7c81ee2199fe3d5c10ac52c8067c61d6992f8f5045b908eb73bab8b.json new file mode 100644 index 0000000..d69d932 --- /dev/null +++ b/.sqlx/query-eae27786a7c81ee2199fe3d5c10ac52c8067c61d6992f8f5045b908eb73bab8b.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE users SET password_hash = $1 WHERE user_id = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Uuid" + ] + }, + "nullable": [] + }, + "hash": "eae27786a7c81ee2199fe3d5c10ac52c8067c61d6992f8f5045b908eb73bab8b" +} diff --git a/Cargo.lock b/Cargo.lock index 259d8b8..211a927 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1554,6 +1554,7 @@ dependencies = [ name = "idcoop" version = "0.1.0" dependencies = [ + "argon2", "async-trait", "axum", "axum_csrf", @@ -2948,6 +2949,7 @@ dependencies = [ "atoi", "byteorder", "bytes", + "chrono", "crc", "crossbeam-queue", "dotenvy", @@ -2978,6 +2980,7 @@ dependencies = [ "tokio-stream", "tracing", "url", + "uuid", "webpki-roots", ] @@ -3031,6 +3034,7 @@ dependencies = [ "bitflags 2.4.1", "byteorder", "bytes", + "chrono", "crc", "digest", "dotenvy", @@ -3059,6 +3063,7 @@ dependencies = [ "stringprep", "thiserror", "tracing", + "uuid", "whoami", ] @@ -3072,6 +3077,7 @@ dependencies = [ "base64", "bitflags 2.4.1", "byteorder", + "chrono", "crc", "dotenvy", "etcetera", @@ -3098,6 +3104,7 @@ dependencies = [ "stringprep", "thiserror", "tracing", + "uuid", "whoami", ] @@ -3108,6 +3115,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d59dc83cf45d89c555a577694534fcd1b55c545a816c816ce51f20bbe56a4f3f" dependencies = [ "atoi", + "chrono", "flume 0.11.0", "futures-channel", "futures-core", @@ -3121,6 +3129,7 @@ dependencies = [ "sqlx-core", "tracing", "url", + "uuid", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index b2e483c..4f1ecfd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +argon2 = "0.5.2" async-trait = "0.1.74" axum = { version = "0.6.20", features = ["tracing", "macros", "headers"] } axum_csrf = { version = "0.7.2", features = ["layer"] } @@ -27,7 +28,7 @@ serde = { version = "1.0.188", features = ["derive"] } serde_json = "1.0.108" serde_urlencoded = "0.7.1" sha2 = "0.10.8" -sqlx = { version = "0.7.2", features = ["postgres", "runtime-tokio-rustls", "macros", "migrate"] } +sqlx = { version = "0.7.2", features = ["postgres", "runtime-tokio-rustls", "macros", "migrate", "uuid", "chrono"] } subtle = "2.5.0" time = "0.3.30" tokio = { version = "1.33.0", features = ["rt", "macros"] } diff --git a/flake.lock b/flake.lock index 79435c3..36b513e 100644 --- a/flake.lock +++ b/flake.lock @@ -217,16 +217,16 @@ }, "nixpkgs_3": { "locked": { - "lastModified": 1690927903, - "narHash": "sha256-D5gCaCROnjEKDOel//8TO/pOP87pAEtT0uT8X+0Bj/U=", + "lastModified": 1700794826, + "narHash": "sha256-RyJTnTNKhO0yqRpDISk03I/4A67/dp96YRxc86YOPgU=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "bd836ac5e5a7358dea73cb74a013ca32864ccb86", + "rev": "5a09cb4b393d58f9ed0d9ca1555016a8543c2ac8", "type": "github" }, "original": { "id": "nixpkgs", - "ref": "nixos-23.05", + "ref": "nixos-unstable", "type": "indirect" } }, diff --git a/flake.nix b/flake.nix index d90d552..40fc947 100644 --- a/flake.nix +++ b/flake.nix @@ -9,7 +9,7 @@ url = "github:nix-community/fenix"; inputs.nixpkgs.follows = "nixpkgs"; }; - nixpkgs.url = "nixpkgs/nixos-23.05"; + nixpkgs.url = "nixpkgs/nixos-unstable"; devenv.url = "github:cachix/devenv/v0.6.3"; }; diff --git a/migrations/20231030092146_users_and_roles.sql b/migrations/20231030092146_users_and_roles.sql index 5ece1ab..d5ec879 100644 --- a/migrations/20231030092146_users_and_roles.sql +++ b/migrations/20231030092146_users_and_roles.sql @@ -2,8 +2,8 @@ CREATE TABLE users ( user_id UUID PRIMARY KEY NOT NULL, - username TEXT NOT NULL UNIQUE, - password_hash BYTEA NOT NULL, + user_name TEXT NOT NULL UNIQUE, + password_hash TEXT, created_at_utc TIMESTAMP NOT NULL, last_login_utc TIMESTAMP, locked BOOLEAN NOT NULL DEFAULT FALSE @@ -13,9 +13,9 @@ COMMENT ON TABLE users IS 'All users known to the system.'; COMMENT ON COLUMN users.user_id IS 'A UUID given to the user. Stays with the user even after the user is renamed.'; -COMMENT ON COLUMN users.username IS 'A textual user name for the user. Might be kept stable but some deployments may support renames.'; +COMMENT ON COLUMN users.user_name IS 'A textual user name for the user. Might be kept stable but some deployments may support renames.'; -COMMENT ON COLUMN users.password_hash IS 'Hash of the user''s password.'; +COMMENT ON COLUMN users.password_hash IS 'Hash of the user''s password. Null if the user does not have a password set.'; COMMENT ON COLUMN users.created_at_utc IS 'When the user was created, in UTC.'; diff --git a/src/bin/idcoop.rs b/src/bin/idcoop.rs index f2fb2a8..562b5f9 100644 --- a/src/bin/idcoop.rs +++ b/src/bin/idcoop.rs @@ -1,3 +1,4 @@ +use std::io::stdin; use std::sync::Arc; use std::{net::SocketAddr, path::PathBuf}; @@ -5,7 +6,8 @@ use clap::Parser; use confique::{Config, Partial}; use eyre::{bail, Context}; use idcoop::config::SecretConfig; -use idcoop::store::IdCoopStore; +use idcoop::passwords::create_password_hash; +use idcoop::store::{CreateUser, IdCoopStore}; use idcoop::{config::Configuration, web}; use tracing_subscriber::fmt::format::FmtSpan; use tracing_subscriber::layer::SubscriberExt; @@ -31,17 +33,11 @@ enum Subcommand { bind: Option, }, - /// Hashes a password from stdin. (TODO decide exact interface including user etc) - HashPassword {}, - - /// TODO - UserAdd {}, - - /// TODO - UserDel {}, - - /// TODO - UserLock {}, + /// Manage users. + User { + #[clap(subcommand)] + cmd: UserCommand, + }, } #[tokio::main(flavor = "current_thread")] @@ -84,11 +80,102 @@ async fn main() -> eyre::Result<()> { let secrets = SecretConfig::try_new(&config).await?; web::serve(bind, Arc::new(store), Arc::new(config), Arc::new(secrets)).await? } - Subcommand::HashPassword {} => todo!(), - Subcommand::UserAdd {} => todo!(), - Subcommand::UserDel {} => todo!(), - Subcommand::UserLock {} => todo!(), + Subcommand::User { cmd } => handle_user_command(cmd, &config).await?, } Ok(()) } + +/// Commands for user management. +#[derive(Clone, Parser)] +enum UserCommand { + /// Add a user. + #[clap(alias = "new", alias = "create")] + Add { + /// The login name of the user. + // TODO this should be a richer newtype with validation + username: String, + + #[clap(long = "locked")] + locked: bool, + }, + /// Deletes a user. + /// Consider whether this is what you really want: in most cases locking a user is more appropriate. + #[clap(alias = "remove", alias = "rm", alias = "del")] + Delete { + /// The login name of the user. + username: String, + }, + /// Locks a user, preventing them from logging in. + Lock { + /// The login name of the user. + username: String, + }, + /// Unlocks a user, letting them log in once more. + Unlock { + /// The login name of the user. + username: String, + }, + /// Changes a user's password. + #[clap(alias = "chpass", alias = "passwd")] + ChangePassword { + /// The login name of the user. + username: String, + }, + /// Lists all users that are registered. + #[clap(alias = "ls")] + ListAll {}, +} + +async fn handle_user_command(command: UserCommand, config: &Configuration) -> eyre::Result<()> { + let store = IdCoopStore::connect(&config.postgres.connect) + .await + .context("Failed to connect to Postgres")?; + match command { + UserCommand::Add { username, locked } => { + store + .txn(|mut txn| { + Box::pin(async move { + txn.create_user(CreateUser { + user_login_name: username, + password_hash: None, + locked, + }) + .await + }) + }) + .await + .context("failed to add user")?; + } + UserCommand::Delete { username } => todo!(), + UserCommand::Lock { username } => todo!(), + UserCommand::Unlock { username } => todo!(), + UserCommand::ChangePassword { username } => { + let Some(user) = store.txn(|mut txn| { Box::pin(async move { + txn.lookup_user_by_name(username).await + })}).await? else { + bail!("No user by that name."); + }; + print!( + "Change password for {} ({}):\n=> ", + user.user_name, user.user_id + ); + let mut buf_line = String::new(); + stdin() + .read_line(&mut buf_line) + .context("failed to read password")?; + let raw_password = buf_line.trim(); + let hash = create_password_hash(raw_password, &config.password_hashing) + .context("unable to hash password!")?; + store + .txn(|mut txn| { + Box::pin( + async move { txn.change_user_password(user.user_id, Some(hash)).await }, + ) + }) + .await?; + } + UserCommand::ListAll {} => todo!(), + } + Ok(()) +} diff --git a/src/config.rs b/src/config.rs index ae1c14c..13781d5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,6 +19,9 @@ pub struct Configuration { #[config(nested)] pub oidc: OidcConfiguration, + + #[config(nested)] + pub password_hashing: PasswordHashingConfig, } #[derive(Config)] @@ -92,3 +95,20 @@ impl SecretConfig { Ok(Self { rsa_key_pair }) } } + +/// Configuration for the password hashing algorithm. Argon2id is in use. +/// OWASP shows how you can trade-off the different parameters whilst still retaining, to some extent, a similar level of security. +#[derive(Config)] +pub struct PasswordHashingConfig { + /// Kibibytes of memory to use whilst hashing. Recommendation from OWASP is at least 19 Mebibytes. + /// I don't want to bump this above 20 MiB in the default setting since we are aiming for low memory use in idCoop. + /// Very keen users could probably reduce this safely as long as the number of iterations is increased, but consider your own research first. + #[config(default = 20480)] + pub memory: u32, + /// Number of iterations. Recommmendation from OWASP is at least 2. I find 32 plenty fast on my machine; I expect it would be reasonable to bump up more too. + #[config(default = 32)] + pub iterations: u32, + /// Degree of parallelism. Recommendation from OWASP is (at least?) 1. + #[config(default = 1)] + pub parallelism: u32, +} diff --git a/src/lib.rs b/src/lib.rs index 5ee31c0..c406667 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ pub mod config; +pub mod passwords; pub mod store; pub mod web; diff --git a/src/passwords.rs b/src/passwords.rs new file mode 100644 index 0000000..f464b47 --- /dev/null +++ b/src/passwords.rs @@ -0,0 +1,27 @@ +use argon2::{password_hash::SaltString, Algorithm, Argon2, Params, PasswordHasher, Version}; +use eyre::eyre; +use rand::rngs::OsRng; + +use crate::config::PasswordHashingConfig; + +pub fn create_password_hash( + password: &str, + config: &PasswordHashingConfig, +) -> eyre::Result { + eprintln!("hash {password:?}"); + let salt = SaltString::generate(&mut OsRng); + let argon2 = Argon2::new( + Algorithm::Argon2id, + Version::default(), + Params::new(config.memory, config.iterations, config.parallelism, None) + .map_err(|err| eyre!("bad Argon2 params: {err:?}"))?, + ); + argon2 + .hash_password(password.as_bytes(), &salt) + .map_err(|err| eyre!("failed argon2 password hash: {err:?}")) + .map(|h| h.to_string()) +} + +pub fn check_hash(password: String, hash: &str) -> eyre::Result { + todo!() +} diff --git a/src/store.rs b/src/store.rs index 20ef2ee..13db09d 100644 --- a/src/store.rs +++ b/src/store.rs @@ -1,6 +1,7 @@ +use chrono::NaiveDateTime; use eyre::Context; use futures::future::BoxFuture; -use sqlx::{Connection, PgPool, Postgres, Transaction}; +use sqlx::{types::Uuid, Connection, PgPool, Postgres, Transaction}; use tracing::error; /// Postgres-backed storage for IdCoop @@ -48,6 +49,21 @@ impl IdCoopStore { } } +#[derive(Debug)] +pub struct User { + pub user_id: Uuid, + pub user_name: String, + pub created_at_utc: NaiveDateTime, + pub password_hash: Option, + pub locked: bool, +} + +pub struct CreateUser { + pub user_login_name: String, + pub password_hash: Option, + pub locked: bool, +} + pub struct IdCoopStoreTxn<'a, 'txn> { txn: &'a mut Transaction<'txn, Postgres>, } @@ -64,4 +80,42 @@ impl<'a, 'txn> IdCoopStoreTxn<'a, 'txn> { error!("TODO: issue access token"); Ok(()) } + + /// Creates a user and returns the user ID. + pub async fn create_user(&mut self, cu: CreateUser) -> eyre::Result { + let r = sqlx::query!( + "INSERT INTO users (user_name, user_id, created_at_utc, password_hash, locked) VALUES ($1, gen_random_uuid(), NOW(), $2, $3) RETURNING user_id", + &cu.user_login_name, cu.password_hash.as_ref(), cu.locked + ) + .fetch_one(&mut **self.txn) + .await.context("failed to create user in DB")?; + + Ok(r.user_id) + } + + pub async fn lookup_user_by_name(&mut self, name: String) -> eyre::Result> { + sqlx::query_as!( + User, + "SELECT user_id, user_name, password_hash, locked, created_at_utc FROM users WHERE user_name = $1", + name + ) + .fetch_optional(&mut **self.txn) + .await.context("failed to lookup user from DB") + } + + pub async fn change_user_password( + &mut self, + user_id: Uuid, + new_password_hash: Option, + ) -> eyre::Result<()> { + sqlx::query!( + "UPDATE users SET password_hash = $1 WHERE user_id = $2", + new_password_hash, + user_id, + ) + .execute(&mut **self.txn) + .await + .context("failed to set user password in DB")?; + Ok(()) + } }