From 8ef42c9609bcefc642cc9739acb921dffba49b89 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 14 Jun 2023 17:13:41 +0200 Subject: [PATCH] admin docs: reformatting, key admin: add check --- doc/drafts/admin-api.md | 83 +++++++++++++++++++++------------------- src/api/admin/cluster.rs | 18 ++++----- src/api/admin/key.rs | 3 +- src/garage/admin/key.rs | 15 ++------ src/model/key_table.rs | 14 +++++-- 5 files changed, 69 insertions(+), 64 deletions(-) diff --git a/doc/drafts/admin-api.md b/doc/drafts/admin-api.md index cb491945..572595db 100644 --- a/doc/drafts/admin-api.md +++ b/doc/drafts/admin-api.md @@ -363,6 +363,7 @@ existing layout in the cluster. This returns the new cluster layout with all changes reverted, as returned by GetClusterLayout. + ### Access key operations #### ListKeys `GET /v1/key` @@ -384,37 +385,6 @@ Example response: ] ``` -#### CreateKey `POST /v1/key` - -Creates a new API access key. - -Request body format: - -```json -{ - "name": "NameOfMyKey" -} -``` - -This returns the key info, including the created secret key, -in the same format as the result of GetKeyInfo. - -#### ImportKey `POST /v1/key/import` - -Imports an existing API key. - -Request body format: - -```json -{ - "accessKeyId": "GK31c2f218a2e44f485b94239e", - "secretAccessKey": "b892c0665f0ada8a4755dae98baa3b133590e11dae3bcc1f9d769d67f16c3835", - "name": "NameOfMyKey" -} -``` - -This returns the key info in the same format as the result of GetKeyInfo. - #### GetKeyInfo `GET /v1/key?id=` #### GetKeyInfo `GET /v1/key?search=` @@ -490,9 +460,38 @@ Example response: } ``` -#### DeleteKey `DELETE /v1/key?id=` +#### CreateKey `POST /v1/key` -Deletes an API access key. +Creates a new API access key. + +Request body format: + +```json +{ + "name": "NameOfMyKey" +} +``` + +This returns the key info, including the created secret key, +in the same format as the result of GetKeyInfo. + +#### ImportKey `POST /v1/key/import` + +Imports an existing API key. +This will check that the imported key is in the valid format, i.e. +is a key that could have been generated by Garage. + +Request body format: + +```json +{ + "accessKeyId": "GK31c2f218a2e44f485b94239e", + "secretAccessKey": "b892c0665f0ada8a4755dae98baa3b133590e11dae3bcc1f9d769d67f16c3835", + "name": "NameOfMyKey" +} +``` + +This returns the key info in the same format as the result of GetKeyInfo. #### UpdateKey `POST /v1/key?id=` @@ -516,6 +515,11 @@ The possible flags in `allow` and `deny` are: `createBucket`. This returns the key info in the same format as the result of GetKeyInfo. +#### DeleteKey `DELETE /v1/key?id=` + +Deletes an API access key. + + ### Bucket operations #### ListBuckets `GET /v1/bucket` @@ -644,12 +648,6 @@ or no alias at all. Technically, you can also specify both `globalAlias` and `localAlias` and that would create two aliases, but I don't see why you would want to do that. -#### DeleteBucket `DELETE /v1/bucket?id=` - -Deletes a storage bucket. A bucket cannot be deleted if it is not empty. - -Warning: this will delete all aliases associated with the bucket! - #### UpdateBucket `PUT /v1/bucket?id=` Updates configuration of the given bucket. @@ -682,6 +680,13 @@ In `quotas`: new values of `maxSize` and `maxObjects` must both be specified, or to remove the quotas. An absent value will be considered the same as a `null`. It is not possible to change only one of the two quotas. +#### DeleteBucket `DELETE /v1/bucket?id=` + +Deletes a storage bucket. A bucket cannot be deleted if it is not empty. + +Warning: this will delete all aliases associated with the bucket! + + ### Operations on permissions for keys on buckets #### BucketAllowKey `POST /v1/bucket/allow` diff --git a/src/api/admin/cluster.rs b/src/api/admin/cluster.rs index ae4a1cc3..c8107b82 100644 --- a/src/api/admin/cluster.rs +++ b/src/api/admin/cluster.rs @@ -1,7 +1,7 @@ use std::net::SocketAddr; use std::sync::Arc; -use hyper::{Body, Request, Response, StatusCode}; +use hyper::{Body, Request, Response}; use serde::{Deserialize, Serialize}; use garage_util::crdt::*; @@ -161,8 +161,8 @@ struct GetClusterStatusResponse { #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct ApplyClusterLayoutResponse { - message: Vec, - layout: GetClusterLayoutResponse, + message: Vec, + layout: GetClusterLayoutResponse, } #[derive(Serialize)] @@ -238,7 +238,7 @@ pub async fn handle_update_cluster_layout( garage.system.update_cluster_layout(&layout).await?; let res = format_cluster_layout(&layout); - Ok(json_ok_response(&res)?) + Ok(json_ok_response(&res)?) } pub async fn handle_apply_cluster_layout( @@ -253,10 +253,10 @@ pub async fn handle_apply_cluster_layout( garage.system.update_cluster_layout(&layout).await?; let res = ApplyClusterLayoutResponse { - message: msg, - layout: format_cluster_layout(&layout), - }; - Ok(json_ok_response(&res)?) + message: msg, + layout: format_cluster_layout(&layout), + }; + Ok(json_ok_response(&res)?) } pub async fn handle_revert_cluster_layout( @@ -270,7 +270,7 @@ pub async fn handle_revert_cluster_layout( garage.system.update_cluster_layout(&layout).await?; let res = format_cluster_layout(&layout); - Ok(json_ok_response(&res)?) + Ok(json_ok_response(&res)?) } // ---- diff --git a/src/api/admin/key.rs b/src/api/admin/key.rs index 0d1f799b..8d1c6890 100644 --- a/src/api/admin/key.rs +++ b/src/api/admin/key.rs @@ -93,7 +93,8 @@ pub async fn handle_import_key( &req.access_key_id, &req.secret_access_key, req.name.as_deref().unwrap_or("Imported key"), - ); + ) + .ok_or_bad_request("Invalid key format")?; garage.key_table.insert(&imported_key).await?; key_info_results(garage, imported_key, false).await diff --git a/src/garage/admin/key.rs b/src/garage/admin/key.rs index 908986fa..1c92670c 100644 --- a/src/garage/admin/key.rs +++ b/src/garage/admin/key.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use garage_table::*; -use garage_model::helper::error::Error; +use garage_model::helper::error::*; use garage_model::key_table::*; use crate::cli::*; @@ -127,22 +127,13 @@ impl AdminRpcHandler { return Err(Error::BadRequest("This command is intended to re-import keys that were previously generated by Garage. If you want to create a new key, use `garage key new` instead. Add the --yes flag if you really want to re-import a key.".to_string())); } - if query.key_id.len() != 26 - || &query.key_id[..2] != "GK" - || hex::decode(&query.key_id[2..]).is_err() - { - return Err(Error::BadRequest(format!("The specified key ID is not a valid Garage key ID (starts with `GK`, followed by 12 hex-encoded bytes)"))); - } - if query.secret_key.len() != 64 || hex::decode(&query.secret_key).is_err() { - return Err(Error::BadRequest(format!("The specified secret key is not a valid Garage secret key (composed of 32 hex-encoded bytes)"))); - } - let prev_key = self.garage.key_table.get(&EmptyKey, &query.key_id).await?; if prev_key.is_some() { return Err(Error::BadRequest(format!("Key {} already exists in data store. Even if it is deleted, we can't let you create a new key with the same ID. Sorry.", query.key_id))); } - let imported_key = Key::import(&query.key_id, &query.secret_key, &query.name); + let imported_key = Key::import(&query.key_id, &query.secret_key, &query.name) + .ok_or_bad_request("Invalid key format")?; self.garage.key_table.insert(&imported_key).await?; self.key_info_result(imported_key).await diff --git a/src/model/key_table.rs b/src/model/key_table.rs index bb5334a3..a9762f1b 100644 --- a/src/model/key_table.rs +++ b/src/model/key_table.rs @@ -149,11 +149,19 @@ impl Key { } /// Import a key from it's parts - pub fn import(key_id: &str, secret_key: &str, name: &str) -> Self { - Self { + pub fn import(key_id: &str, secret_key: &str, name: &str) -> Result { + if key_id.len() != 26 || &key_id[..2] != "GK" || hex::decode(&key_id[2..]).is_err() { + return Err("The specified key ID is not a valid Garage key ID (starts with `GK`, followed by 12 hex-encoded bytes)"); + } + + if secret_key.len() != 64 || hex::decode(&secret_key).is_err() { + return Err("The specified secret key is not a valid Garage secret key (composed of 32 hex-encoded bytes)"); + } + + Ok(Self { key_id: key_id.to_string(), state: crdt::Deletable::present(KeyParams::new(secret_key, name)), - } + }) } /// Create a new Key which can me merged to mark an existing key deleted