From 53e38605784fa1cdd71596d0499401888cf21c90 Mon Sep 17 00:00:00 2001 From: Szepesi Tibor Date: Mon, 29 Jul 2024 11:11:20 +0200 Subject: [PATCH 1/6] Remove UUID v1 --- Cargo.lock | 18 ------------------ Cargo.toml | 3 +-- iam-common/Cargo.toml | 1 - iam-common/src/id.rs | 30 ++---------------------------- iam/Cargo.toml | 1 - 5 files changed, 3 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c1d8dbb..1e3f60c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -206,15 +206,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "atomic" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d818003e740b63afc82337e3160717f4f63078720a810b7b903e70a5d1d2994" -dependencies = [ - "bytemuck", -] - [[package]] name = "atomic-waker" version = "1.1.2" @@ -412,12 +403,6 @@ version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" -[[package]] -name = "bytemuck" -version = "1.16.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b236fc92302c97ed75b38da1f4917b5cdda4984745740f153a5d3059e48d725e" - [[package]] name = "byteorder" version = "1.5.0" @@ -1234,7 +1219,6 @@ dependencies = [ "iam-common", "iam-entity", "mime", - "once_cell", "pin-project-lite", "rand", "sea-orm", @@ -1283,7 +1267,6 @@ dependencies = [ "jose-jwk", "jsonwebtoken", "mime", - "once_cell", "rand", "rust-argon2", "sea-orm", @@ -3471,7 +3454,6 @@ version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81dfa00651efa65069b0b6b651f4aaa31ba9e3c3ce0137aaad053604ee7e0314" dependencies = [ - "atomic", "getrandom", ] diff --git a/Cargo.toml b/Cargo.toml index 9d7adbb..332a146 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,11 +34,10 @@ iam-entity = { path = "./iam-entity" } iam-macros = { path = "./iam-macros" } jsonwebtoken = "9.3.0" mime = "0.3.17" -once_cell = "1.19.0" rand = { version = "0.8.5", default-features = false, features = ["std", "std_rng"] } sea-orm = { version = "0.12.15", default-features = false, features = ["macros", "runtime-actix-rustls", "sqlx-mysql", "with-chrono"] } serde = { version = "1.0.204", features = ["derive"] } serde_json = "1.0.120" tokio = { version = "1.38.0", features = ["macros", "rt-multi-thread", "signal"] } tracing = { version = "0.1.40", default-features = false } -uuid = { version = "1.10.0", features = ["v1", "v4", "rng"] } +uuid = { version = "1.10.0", default-features = false, features = ["v4"] } diff --git a/iam-common/Cargo.toml b/iam-common/Cargo.toml index a4b52c4..ef5a34e 100644 --- a/iam-common/Cargo.toml +++ b/iam-common/Cargo.toml @@ -13,7 +13,6 @@ rand.workspace = true rust-argon2 = { version = "2.1.0", default-features = false } bcrypt = "0.15.1" serde.workspace = true -once_cell.workspace = true jsonwebtoken.workspace = true axum.workspace = true iam-macros.workspace = true diff --git a/iam-common/src/id.rs b/iam-common/src/id.rs index 8ee5245..1947bfa 100644 --- a/iam-common/src/id.rs +++ b/iam-common/src/id.rs @@ -1,10 +1,6 @@ -use once_cell::sync::Lazy; use serde::{Deserialize, Serialize}; use std::fmt::{self, Display}; -use uuid::{ - v1::{Context, Timestamp}, - Uuid, -}; +use uuid::Uuid; #[derive(Debug, Clone, Copy)] #[repr(u8)] @@ -21,32 +17,10 @@ pub struct Id { ty: IdType, } -static CONTEXT: Lazy = Lazy::new(Context::new_random); - impl Id { fn new(ty: IdType) -> Self { - let date = chrono::Utc::now(); - let timestamp = Timestamp::from_unix( - &*CONTEXT, - date.timestamp() as u64, - date.timestamp_subsec_nanos(), - ); - let hostname = std::env::var("HOSTNAME").unwrap_or_else(|_| "dev".to_string()); - - let mut buf = [0u8; 6]; - let mut buf_iter = buf.iter_mut(); - let mut iter = hostname.as_bytes().iter(); - - while let Some(x) = iter.next_back() { - if let Some(y) = buf_iter.next() { - *y = *x; - } else { - break; - } - } - Self { - uuid: Uuid::new_v1(timestamp, &buf), + uuid: Uuid::new_v4(), ty, } } diff --git a/iam/Cargo.toml b/iam/Cargo.toml index 8c9f49d..855c056 100644 --- a/iam/Cargo.toml +++ b/iam/Cargo.toml @@ -19,7 +19,6 @@ chrono.workspace = true validator = { version = "0.18.1", features = ["derive"] } async-trait.workspace = true futures-util = "0.3.30" -once_cell.workspace = true tokio.workspace = true tower = { version = "0.4.13", features = ["timeout"] } tower-http = { version = "0.5.2", features = ["add-extension", "auth", "compression-full", "cors", "decompression-full", "request-id", "sensitive-headers", "trace", "util"] } From c3fa9a49a0aff80f139404d4cc1efae88b2b3746 Mon Sep 17 00:00:00 2001 From: Szepesi Tibor Date: Mon, 29 Jul 2024 13:03:41 +0200 Subject: [PATCH 2/6] Rework Id type Implement FromStr for Id and add some tests --- Cargo.lock | 1 + iam-common/Cargo.toml | 1 + iam-common/src/id.rs | 152 ++++++++++++++++++++++++++++++++---------- 3 files changed, 120 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1e3f60c..633b4b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1256,6 +1256,7 @@ dependencies = [ name = "iam-common" version = "0.1.0" dependencies = [ + "anyhow", "axum", "base64 0.22.1", "bcrypt", diff --git a/iam-common/Cargo.toml b/iam-common/Cargo.toml index ef5a34e..5df7923 100644 --- a/iam-common/Cargo.toml +++ b/iam-common/Cargo.toml @@ -23,3 +23,4 @@ mime.workspace = true base64.workspace = true ed25519-dalek = { version = "2.1.1", features = ["pkcs8", "rand_core"] } jose-jwk = { version = "0.1.2", default-features = false } +anyhow.workspace = true diff --git a/iam-common/src/id.rs b/iam-common/src/id.rs index 1947bfa..d6868eb 100644 --- a/iam-common/src/id.rs +++ b/iam-common/src/id.rs @@ -1,9 +1,12 @@ +use anyhow::bail; use serde::{Deserialize, Serialize}; -use std::fmt::{self, Display}; +use std::{ + fmt::{self, Display}, + str::FromStr, +}; use uuid::Uuid; -#[derive(Debug, Clone, Copy)] -#[repr(u8)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum IdType { Action, Group, @@ -11,6 +14,33 @@ pub enum IdType { App, } +impl IdType { + const fn as_str(&self) -> &'static str { + match self { + IdType::Action => "ActionID", + IdType::Group => "GroupID", + IdType::User => "UserID", + IdType::App => "AppID", + } + } +} + +impl FromStr for IdType { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + let ty = match s { + "ActionID" => IdType::Action, + "UserID" => IdType::User, + "AppID" => IdType::App, + "GroupID" => IdType::Group, + _ => bail!("invalid type"), + }; + + Ok(ty) + } +} + #[derive(Debug, Clone, Copy)] pub struct Id { uuid: Uuid, @@ -46,26 +76,37 @@ impl Id { } #[inline] - const fn get_prefix(&self) -> &'static str { - match self.ty { - IdType::Action => "ActionID", - IdType::Group => "GroupID", - IdType::User => "UserID", - IdType::App => "AppID", - } + pub const fn get_type(&self) -> IdType { + self.ty } } impl Display for Id { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "{}-{}", - self.get_prefix(), - self.uuid - .as_hyphenated() - .encode_lower(&mut Uuid::encode_buffer()) - ) + let ty = self.get_type().as_str(); + + let mut buf = Uuid::encode_buffer(); + let uuid = self.uuid.as_hyphenated().encode_lower(&mut buf); + + write!(f, "{}-{}", ty, uuid) + } +} + +impl FromStr for Id { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + let Some((ty, id_str)) = s.split_once('-') else { + bail!("missing type"); + }; + + let ty = IdType::from_str(ty)?; + + let Ok(uuid) = Uuid::parse_str(id_str) else { + bail!("invalid uuid"); + }; + + Ok(Self { uuid, ty }) } } @@ -99,25 +140,68 @@ impl<'de> Deserialize<'de> for Id { where E: de::Error, { - let (ty, id_str) = if let Some(i) = v.strip_prefix("ActionID-") { - (IdType::Action, i) - } else if let Some(i) = v.strip_prefix("GroupID-") { - (IdType::Group, i) - } else if let Some(i) = v.strip_prefix("UserID-") { - (IdType::User, i) - } else if let Some(i) = v.strip_prefix("AppID-") { - (IdType::App, i) - } else { - return Err(de::Error::invalid_value(de::Unexpected::Str(v), &self)); - }; - - let uuid = Uuid::parse_str(id_str) - .map_err(|_| de::Error::invalid_value(de::Unexpected::Str(id_str), &self))?; - - Ok(Id { ty, uuid }) + match Id::from_str(v) { + Ok(id) => Ok(id), + Err(err) => Err(de::Error::custom(err)), + } } } deserializer.deserialize_str(Visitor) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn from_str() { + let id = Id::from_str("UserID-00000000-0000-0000-0000-000000000000").unwrap(); + assert_eq!(id.get_type(), IdType::User); + + let id = Id::from_str("ActionID-00000000-0000-0000-0000-000000000000").unwrap(); + assert_eq!(id.get_type(), IdType::Action); + + let id = Id::from_str("UserID-00000000-0000-0000-0000-000000000000").unwrap(); + assert_eq!(id.get_type(), IdType::User); + + let id = Id::from_str("GroupID-00000000-0000-0000-0000-000000000000").unwrap(); + assert_eq!(id.get_type(), IdType::Group); + } + + #[test] + #[should_panic] + fn from_str_invalid_type() { + Id::from_str("Invalid-00000000-0000-0000-0000-000000000000").unwrap(); + } + + #[test] + #[should_panic] + fn from_str_invalid_uuid() { + Id::from_str("UserID-invalid").unwrap(); + } + + #[test] + fn to_string() { + let str_id = "UserID-00000000-0000-0000-0000-000000000000"; + let id = Id::from_str(str_id).unwrap(); + + assert_eq!(id.to_string(), str_id); + + let str_id = "ActionID-00000000-0000-0000-0000-000000000000"; + let id = Id::from_str(str_id).unwrap(); + + assert_eq!(id.to_string(), str_id); + + let str_id = "AppID-00000000-0000-0000-0000-000000000000"; + let id = Id::from_str(str_id).unwrap(); + + assert_eq!(id.to_string(), str_id); + + let str_id = "GroupID-00000000-0000-0000-0000-000000000000"; + let id = Id::from_str(str_id).unwrap(); + + assert_eq!(id.to_string(), str_id); + } +} From 419dc555e56c2dc27dafcae8c4b50b3b5d3a0a38 Mon Sep 17 00:00:00 2001 From: Szepesi Tibor Date: Mon, 29 Jul 2024 13:04:03 +0200 Subject: [PATCH 3/6] Remove id deserialize hack from libiam --- libiam/src/user.rs | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/libiam/src/user.rs b/libiam/src/user.rs index 545e9fb..fbf6797 100644 --- a/libiam/src/user.rs +++ b/libiam/src/user.rs @@ -4,7 +4,7 @@ use crate::{ }; use iam_common::{keys::jwt::Claims, Id}; use jsonwebtoken::{Algorithm, DecodingKey, Validation}; -use std::sync::Arc; +use std::{str::FromStr, sync::Arc}; #[derive(Debug)] pub struct UserInner { @@ -48,31 +48,20 @@ impl User { let api = iam.inner.api.with_token(token.clone()); + // TODO: this should be done with `Jwt::get_claims()` + let claims = + jsonwebtoken::decode::(token.as_str(), &DecodingKey::from_secret(&[]), &{ + let mut v = Validation::new(Algorithm::EdDSA); + v.insecure_disable_signature_validation(); + v.set_audience(&["https://verseghy-gimnazium.net"]); + v + })? + .claims; + Ok(Self { inner: Arc::new(UserInner { token: token.clone(), - id: serde_json::from_str::( - format!( - // HACK: impl FromStr - "\"{}\"", - jsonwebtoken::decode::( - token.as_str(), - &DecodingKey::from_secret(&[]), - &{ - let mut v = Validation::new(Algorithm::RS256); - v.insecure_disable_signature_validation(); - v.set_audience(&["https://verseghy-gimnazium.net"]); - v - }, - ) - .unwrap() - .claims - .sub - .as_str() - ) - .as_str(), - ) - .unwrap(), + id: Id::from_str(&claims.sub)?, _api: api, }), }) From e475066f9a9f2759cf239a21c3a2a4bb2f097cc0 Mon Sep 17 00:00:00 2001 From: Szepesi Tibor Date: Mon, 29 Jul 2024 20:08:05 +0200 Subject: [PATCH 4/6] Change error message --- iam-common/src/id.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iam-common/src/id.rs b/iam-common/src/id.rs index d6868eb..a378c4d 100644 --- a/iam-common/src/id.rs +++ b/iam-common/src/id.rs @@ -97,7 +97,7 @@ impl FromStr for Id { fn from_str(s: &str) -> Result { let Some((ty, id_str)) = s.split_once('-') else { - bail!("missing type"); + bail!("invalid format"); }; let ty = IdType::from_str(ty)?; From 3a027bb32ff47d260e9c125e006ba584a0006c4a Mon Sep 17 00:00:00 2001 From: Szepesi Tibor Date: Mon, 29 Jul 2024 20:57:53 +0200 Subject: [PATCH 5/6] Add more test --- iam-common/src/id.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/iam-common/src/id.rs b/iam-common/src/id.rs index a378c4d..cf24625 100644 --- a/iam-common/src/id.rs +++ b/iam-common/src/id.rs @@ -182,6 +182,12 @@ mod tests { Id::from_str("UserID-invalid").unwrap(); } + #[test] + #[should_panic] + fn from_str_invalid_format() { + Id::from_str("invalid").unwrap(); + } + #[test] fn to_string() { let str_id = "UserID-00000000-0000-0000-0000-000000000000"; From b2760d74a01fab5da6efd213d1700eb2fe20a50f Mon Sep 17 00:00:00 2001 From: Szepesi Tibor Date: Mon, 29 Jul 2024 20:59:38 +0200 Subject: [PATCH 6/6] Don't use should_panic --- iam-common/src/id.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/iam-common/src/id.rs b/iam-common/src/id.rs index cf24625..8b526d8 100644 --- a/iam-common/src/id.rs +++ b/iam-common/src/id.rs @@ -171,21 +171,21 @@ mod tests { } #[test] - #[should_panic] fn from_str_invalid_type() { - Id::from_str("Invalid-00000000-0000-0000-0000-000000000000").unwrap(); + let id = Id::from_str("Invalid-00000000-0000-0000-0000-000000000000"); + assert!(id.is_err()); } #[test] - #[should_panic] fn from_str_invalid_uuid() { - Id::from_str("UserID-invalid").unwrap(); + let id = Id::from_str("UserID-invalid"); + assert!(id.is_err()); } #[test] - #[should_panic] fn from_str_invalid_format() { - Id::from_str("invalid").unwrap(); + let id = Id::from_str("invalid"); + assert!(id.is_err()); } #[test]