From 677ab60cc117677bf53dc4887a6ff1c094e17cd0 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 4 Jan 2022 18:59:17 +0100 Subject: Small changes in key model and refactoring --- src/api/s3_bucket.rs | 8 ++-- src/api/signature.rs | 3 +- src/garage/admin.rs | 21 ++++++--- src/garage/cli/util.rs | 31 +++++++------ src/model/bucket_table.rs | 29 +++++++----- src/model/key_table.rs | 116 +++++++++++++++++++++++----------------------- src/model/permission.rs | 14 +++--- 7 files changed, 119 insertions(+), 103 deletions(-) diff --git a/src/api/s3_bucket.rs b/src/api/s3_bucket.rs index 50aeb173..27208ffa 100644 --- a/src/api/s3_bucket.rs +++ b/src/api/s3_bucket.rs @@ -38,8 +38,8 @@ pub fn handle_get_bucket_versioning() -> Result, Error> { } pub async fn handle_list_buckets(garage: &Garage, api_key: &Key) -> Result, Error> { - let key_state = api_key.state.as_option().ok_or_internal_error( - "Key should not be in deleted state at this point (internal error)", + let key_p = api_key.params().ok_or_internal_error( + "Key should not be in deleted state at this point (in handle_list_buckets)", )?; // Collect buckets user has access to @@ -74,7 +74,7 @@ pub async fn handle_list_buckets(garage: &Garage, api_key: &Key) -> Result Result>(); Ok(AdminRpc::KeyList(key_ids)) } @@ -454,7 +460,7 @@ impl AdminRpcHandler { } async fn handle_create_key(&self, query: &KeyNewOpt) -> Result { - let key = Key::new(query.name.clone()); + let key = Key::new(&query.name); self.garage.key_table.insert(&key).await?; self.key_info_result(key).await } @@ -465,7 +471,10 @@ impl AdminRpcHandler { .bucket_helper() .get_existing_matching_key(&query.key_pattern) .await?; - key.name.update(query.new_name.clone()); + key.params_mut() + .unwrap() + .name + .update(query.new_name.clone()); self.garage.key_table.insert(&key).await?; self.key_info_result(key).await } @@ -500,7 +509,7 @@ impl AdminRpcHandler { // 2. Remove permissions on all authorized buckets for (ab_id, _auth) in state.authorized_buckets.items().iter() { helper - .set_bucket_key_permissions(*ab_id, &key.key_id, BucketKeyPerm::no_permissions()) + .set_bucket_key_permissions(*ab_id, &key.key_id, BucketKeyPerm::NO_PERMISSIONS) .await?; } diff --git a/src/garage/cli/util.rs b/src/garage/cli/util.rs index 7a7d0e9b..365831c4 100644 --- a/src/garage/cli/util.rs +++ b/src/garage/cli/util.rs @@ -65,11 +65,11 @@ pub fn print_key_info(key: &Key, relevant_buckets: &HashMap) { "".to_string() }; - println!("Key name: {}", key.name.get()); - println!("Key ID: {}", key.key_id); - println!("Secret key: {}", key.secret_key); match &key.state { Deletable::Present(p) => { + println!("Key name: {}", p.name.get()); + println!("Key ID: {}", key.key_id); + println!("Secret key: {}", p.secret_key); println!("Can create buckets: {}", p.allow_create_bucket.get()); println!("\nKey-specific bucket aliases:"); let mut table = vec![]; @@ -112,12 +112,19 @@ pub fn print_key_info(key: &Key, relevant_buckets: &HashMap) { format_table(table); } Deletable::Deleted => { - println!("\nKey is deleted."); + println!("Key {} is deleted.", key.key_id); } } } pub fn print_bucket_info(bucket: &Bucket, relevant_keys: &HashMap) { + let key_name = |k| { + relevant_keys + .get(k) + .map(|k| k.params().unwrap().name.get().as_str()) + .unwrap_or("") + }; + println!("Bucket: {}", hex::encode(bucket.id)); match &bucket.state { Deletable::Deleted => println!("Bucket is deleted."), @@ -135,11 +142,7 @@ pub fn print_bucket_info(bucket: &Bucket, relevant_keys: &HashMap) let mut table = vec![]; for ((key_id, alias), _, active) in p.local_aliases.items().iter() { if *active { - let key_name = relevant_keys - .get(key_id) - .map(|k| k.name.get().as_str()) - .unwrap_or(""); - table.push(format!("\t{} ({})\t{}", key_id, key_name, alias)); + table.push(format!("\t{} ({})\t{}", key_id, key_name(key_id), alias)); } } format_table(table); @@ -150,13 +153,13 @@ pub fn print_bucket_info(bucket: &Bucket, relevant_keys: &HashMap) let rflag = if perm.allow_read { "R" } else { " " }; let wflag = if perm.allow_write { "W" } else { " " }; let oflag = if perm.allow_owner { "O" } else { " " }; - let key_name = relevant_keys - .get(k) - .map(|k| k.name.get().as_str()) - .unwrap_or(""); table.push(format!( "\t{}{}{}\t{}\t{}", - rflag, wflag, oflag, k, key_name + rflag, + wflag, + oflag, + k, + key_name(k) )); } format_table(table); diff --git a/src/model/bucket_table.rs b/src/model/bucket_table.rs index d687e774..db7cec18 100644 --- a/src/model/bucket_table.rs +++ b/src/model/bucket_table.rs @@ -97,27 +97,32 @@ impl Bucket { self.state.is_deleted() } + /// Returns an option representing the parameters (None if in deleted state) + pub fn params(&self) -> Option<&BucketParams> { + self.state.as_option() + } + + /// Mutable version of `.params()` + pub fn params_mut(&mut self) -> Option<&mut BucketParams> { + self.state.as_option_mut() + } + /// Return the list of authorized keys, when each was updated, and the permission associated to /// the key pub fn authorized_keys(&self) -> &[(String, BucketKeyPerm)] { - match &self.state { - crdt::Deletable::Deleted => &[], - crdt::Deletable::Present(state) => state.authorized_keys.items(), - } + self.params() + .map(|s| s.authorized_keys.items()) + .unwrap_or(&[]) } pub fn aliases(&self) -> &[(String, u64, bool)] { - match &self.state { - crdt::Deletable::Deleted => &[], - crdt::Deletable::Present(state) => state.aliases.items(), - } + self.params().map(|s| s.aliases.items()).unwrap_or(&[]) } pub fn local_aliases(&self) -> &[((String, String), u64, bool)] { - match &self.state { - crdt::Deletable::Deleted => &[], - crdt::Deletable::Present(state) => state.local_aliases.items(), - } + self.params() + .map(|s| s.local_aliases.items()) + .unwrap_or(&[]) } } diff --git a/src/model/key_table.rs b/src/model/key_table.rs index c25f2da4..d5e30f3f 100644 --- a/src/model/key_table.rs +++ b/src/model/key_table.rs @@ -14,29 +14,37 @@ pub struct Key { /// The id of the key (immutable), used as partition key pub key_id: String, - /// The secret_key associated + /// Internal state of the key + pub state: crdt::Deletable, +} + +/// Configuration for a key +#[derive(PartialEq, Clone, Debug, Serialize, Deserialize)] +pub struct KeyParams { + /// The secret_key associated (immutable) pub secret_key: String, /// Name for the key pub name: crdt::Lww, + /// Flag to allow users having this key to create buckets + pub allow_create_bucket: crdt::Lww, + /// If the key is present: it gives some permissions, /// a map of bucket IDs (uuids) to permissions. /// Otherwise no permissions are granted to key - pub state: crdt::Deletable, -} - -/// Configuration for a key -#[derive(PartialEq, Clone, Debug, Serialize, Deserialize)] -pub struct KeyParams { - pub allow_create_bucket: crdt::Lww, pub authorized_buckets: crdt::Map, + + /// A key can have a local view of buckets names it is + /// the only one to see, this is the namespace for these aliases pub local_aliases: crdt::LwwMap>, } impl KeyParams { - pub fn new() -> Self { + fn new(secret_key: &str, name: &str) -> Self { KeyParams { + secret_key: secret_key.to_string(), + name: crdt::Lww::new(name.to_string()), allow_create_bucket: crdt::Lww::new(false), authorized_buckets: crdt::Map::new(), local_aliases: crdt::LwwMap::new(), @@ -44,14 +52,9 @@ impl KeyParams { } } -impl Default for KeyParams { - fn default() -> Self { - Self::new() - } -} - impl Crdt for KeyParams { fn merge(&mut self, o: &Self) { + self.name.merge(&o.name); self.allow_create_bucket.merge(&o.allow_create_bucket); self.authorized_buckets.merge(&o.authorized_buckets); self.local_aliases.merge(&o.local_aliases); @@ -60,14 +63,12 @@ impl Crdt for KeyParams { impl Key { /// Initialize a new Key, generating a random identifier and associated secret key - pub fn new(name: String) -> Self { + pub fn new(name: &str) -> Self { let key_id = format!("GK{}", hex::encode(&rand::random::<[u8; 12]>()[..])); let secret_key = hex::encode(&rand::random::<[u8; 32]>()[..]); Self { key_id, - secret_key, - name: crdt::Lww::new(name), - state: crdt::Deletable::present(KeyParams::new()), + state: crdt::Deletable::present(KeyParams::new(&secret_key, name)), } } @@ -75,9 +76,7 @@ impl Key { pub fn import(key_id: &str, secret_key: &str, name: &str) -> Self { Self { key_id: key_id.to_string(), - secret_key: secret_key.to_string(), - name: crdt::Lww::new(name.to_string()), - state: crdt::Deletable::present(KeyParams::new()), + state: crdt::Deletable::present(KeyParams::new(secret_key, name)), } } @@ -85,49 +84,47 @@ impl Key { pub fn delete(key_id: String) -> Self { Self { key_id, - secret_key: "".into(), - name: crdt::Lww::new("".to_string()), state: crdt::Deletable::Deleted, } } + /// Returns true if this represents a deleted bucket + pub fn is_deleted(&self) -> bool { + self.state.is_deleted() + } + + /// Returns an option representing the params (None if in deleted state) + pub fn params(&self) -> Option<&KeyParams> { + self.state.as_option() + } + + /// Mutable version of `.state()` + pub fn params_mut(&mut self) -> Option<&mut KeyParams> { + self.state.as_option_mut() + } + + /// Get permissions for a bucket + pub fn bucket_permissions(&self, bucket: &Uuid) -> BucketKeyPerm { + self.params() + .map(|params| params.authorized_buckets.get(bucket)) + .flatten() + .cloned() + .unwrap_or(BucketKeyPerm::NO_PERMISSIONS) + } + /// Check if `Key` is allowed to read in bucket pub fn allow_read(&self, bucket: &Uuid) -> bool { - if let crdt::Deletable::Present(params) = &self.state { - params - .authorized_buckets - .get(bucket) - .map(|x| x.allow_read) - .unwrap_or(false) - } else { - false - } + self.bucket_permissions(bucket).allow_read } /// Check if `Key` is allowed to write in bucket pub fn allow_write(&self, bucket: &Uuid) -> bool { - if let crdt::Deletable::Present(params) = &self.state { - params - .authorized_buckets - .get(bucket) - .map(|x| x.allow_write) - .unwrap_or(false) - } else { - false - } + self.bucket_permissions(bucket).allow_write } /// Check if `Key` is owner of bucket pub fn allow_owner(&self, bucket: &Uuid) -> bool { - if let crdt::Deletable::Present(params) = &self.state { - params - .authorized_buckets - .get(bucket) - .map(|x| x.allow_owner) - .unwrap_or(false) - } else { - false - } + self.bucket_permissions(bucket).allow_owner } } @@ -142,7 +139,6 @@ impl Entry for Key { impl Crdt for Key { fn merge(&mut self, other: &Self) { - self.name.merge(&other.name); self.state.merge(&other.state); } } @@ -168,15 +164,20 @@ impl TableSchema for KeyTable { KeyFilter::Deleted(df) => df.apply(entry.state.is_deleted()), KeyFilter::MatchesAndNotDeleted(pat) => { let pat = pat.to_lowercase(); - !entry.state.is_deleted() - && (entry.key_id.to_lowercase().starts_with(&pat) - || entry.name.get().to_lowercase() == pat) + entry + .params() + .map(|p| { + entry.key_id.to_lowercase().starts_with(&pat) + || p.name.get().to_lowercase() == pat + }) + .unwrap_or(false) } } } fn try_migrate(bytes: &[u8]) -> Option { let old_k = rmp_serde::decode::from_read_ref::<_, old::Key>(bytes).ok()?; + let name = crdt::Lww::raw(old_k.name.timestamp(), old_k.name.get().clone()); let state = if old_k.deleted.get() { crdt::Deletable::Deleted @@ -185,16 +186,15 @@ impl TableSchema for KeyTable { // migration is performed in specific migration code in // garage/migrate.rs crdt::Deletable::Present(KeyParams { + secret_key: old_k.secret_key, + name, allow_create_bucket: crdt::Lww::new(false), authorized_buckets: crdt::Map::new(), local_aliases: crdt::LwwMap::new(), }) }; - let name = crdt::Lww::raw(old_k.name.timestamp(), old_k.name.get().clone()); Some(Key { key_id: old_k.key_id, - secret_key: old_k.secret_key, - name, state, }) } diff --git a/src/model/permission.rs b/src/model/permission.rs index b8f7dd71..67527ed0 100644 --- a/src/model/permission.rs +++ b/src/model/permission.rs @@ -21,14 +21,12 @@ pub struct BucketKeyPerm { } impl BucketKeyPerm { - pub fn no_permissions() -> Self { - Self { - timestamp: 0, - allow_read: false, - allow_write: false, - allow_owner: false, - } - } + pub const NO_PERMISSIONS: Self = Self { + timestamp: 0, + allow_read: false, + allow_write: false, + allow_owner: false, + }; } impl Crdt for BucketKeyPerm { -- cgit v1.2.3