Enable k2v feature flag in CI

- [x] Better distinguish error types
- [x] Parse error messages received from server
- [x] Remove `src/` folder layer, we don't have that for other crates

Co-authored-by: Alex Auvolat <alex@adnab.me>
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/307
Co-authored-by: Alex <alex@adnab.me>
Co-committed-by: Alex <alex@adnab.me>

Cargo.lock

@@ -1584,6 +1584,7 @@ dependencies = [
  "clap 3.1.18",
  "garage_util 0.7.0",
  "http",
+ "log",
  "rusoto_core",
  "rusoto_credential",
  "rusoto_signature",

Cargo.nix

@@ -688,7 +688,7 @@ in
     registry = "registry+https://github.com/rust-lang/crates.io-index";
     src = fetchCratesIo { inherit name version; sha256 = "59a6001667ab124aebae2a495118e11d30984c3a653e99d86d58971708cf5e4b"; };
     dependencies = {
-      ${ if hostPlatform.config == "aarch64-linux-android" || hostPlatform.parsed.cpu.name == "aarch64" && hostPlatform.parsed.kernel.name == "linux" || hostPlatform.config == "aarch64-apple-darwin" then "libc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".libc."0.2.121" { inherit profileName; };
+      ${ if hostPlatform.config == "aarch64-linux-android" || hostPlatform.config == "aarch64-apple-darwin" || hostPlatform.parsed.cpu.name == "aarch64" && hostPlatform.parsed.kernel.name == "linux" then "libc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".libc."0.2.121" { inherit profileName; };
     };
   });
   
@@ -2117,6 +2117,7 @@ in
       clap = rustPackages."registry+https://github.com/rust-lang/crates.io-index".clap."3.1.18" { inherit profileName; };
       garage_util = rustPackages."unknown".garage_util."0.7.0" { inherit profileName; };
       http = rustPackages."registry+https://github.com/rust-lang/crates.io-index".http."0.2.6" { inherit profileName; };
+      log = rustPackages."registry+https://github.com/rust-lang/crates.io-index".log."0.4.16" { inherit profileName; };
       rusoto_core = rustPackages."registry+https://github.com/rust-lang/crates.io-index".rusoto_core."0.48.0" { inherit profileName; };
       rusoto_credential = rustPackages."registry+https://github.com/rust-lang/crates.io-index".rusoto_credential."0.48.0" { inherit profileName; };
       rusoto_signature = rustPackages."registry+https://github.com/rust-lang/crates.io-index".rusoto_signature."0.48.0" { inherit profileName; };
@@ -5029,7 +5030,7 @@ in
       [ "default" ]
     ];
     dependencies = {
-      ${ if hostPlatform.config == "aarch64-uwp-windows-msvc" || hostPlatform.config == "aarch64-pc-windows-msvc" then "windows_aarch64_msvc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_aarch64_msvc."0.32.0" { inherit profileName; };
+      ${ if hostPlatform.config == "aarch64-pc-windows-msvc" || hostPlatform.config == "aarch64-uwp-windows-msvc" then "windows_aarch64_msvc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_aarch64_msvc."0.32.0" { inherit profileName; };
       ${ if hostPlatform.config == "i686-pc-windows-gnu" || hostPlatform.config == "i686-uwp-windows-gnu" then "windows_i686_gnu" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_i686_gnu."0.32.0" { inherit profileName; };
       ${ if hostPlatform.config == "i686-uwp-windows-msvc" || hostPlatform.config == "i686-pc-windows-msvc" then "windows_i686_msvc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_i686_msvc."0.32.0" { inherit profileName; };
       ${ if hostPlatform.config == "x86_64-pc-windows-gnu" || hostPlatform.config == "x86_64-uwp-windows-gnu" then "windows_x86_64_gnu" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_x86_64_gnu."0.32.0" { inherit profileName; };

doc/drafts/admin-api.md

@@ -1,21 +1,30 @@
-# Access control
+# Specification of Garage's administration API
+
+
+**WARNING.** At this point, there is no comittement to stability of the APIs described in this document.
+We will bump the version numbers prefixed to each API endpoint at each time the syntax
+or semantics change, meaning that code that relies on these endpoint will break
+when changes are introduced.
+
+
+## Access control
 
 The admin API uses two different tokens for acces control, that are specified in the config file's `[admin]` section:
 
 - `metrics_token`: the token for accessing the Metrics endpoint (if this token is not set in the config file, the Metrics endpoint can be accessed without access control);
-- `admin_token`: the token for accessing all of the other administration endpoints (if this token is not set in the config file, these endpoints can be accessed without access control).
+- `admin_token`: the token for accessing all of the other administration endpoints (if this token is not set in the config file, access to these endpoints is disabled entirely).
 
-# Administration API endpoints
+## Administration API endpoints
 
-## Metrics-related endpoints
+### Metrics-related endpoints
 
-### Metrics `GET /metrics`
+#### Metrics `GET /metrics`
 
 Returns internal Garage metrics in Prometheus format.
 
-## Cluster operations
+### Cluster operations
 
-### GetClusterStatus `GET /v0/status`
+#### GetClusterStatus `GET /v0/status`
 
 Returns the cluster's current status in JSON, including:
 
@@ -94,7 +103,7 @@ Example response body:
 }
 ```
 
-### ConnectClusterNodes `POST /v0/connect`
+#### ConnectClusterNodes `POST /v0/connect`
 
 Instructs this Garage node to connect to other Garage nodes at specified addresses.
 
@@ -115,16 +124,16 @@ Example response:
 [
 	{
 		"success": true,
-		"error": null,
+		"error": null
 	},
 	{
 		"success": false,
-		"error": "Handshake error",
+		"error": "Handshake error"
 	}
 ]
 ```
 
-### GetClusterLayout `GET /v0/layout`
+#### GetClusterLayout `GET /v0/layout`
 
 Returns the cluster's current layout in JSON, including:
 
@@ -173,7 +182,7 @@ Example response body:
 }
 ```
 
-### UpdateClusterLayout `POST /v0/layout`
+#### UpdateClusterLayout `POST /v0/layout`
 
 Send modifications to the cluster layout. These modifications will
 be included in the staged role changes, visible in subsequent calls
@@ -204,7 +213,7 @@ Contrary to the CLI that may update only a subset of the fields
 values must be specified.
 
 
-### ApplyClusterLayout `POST /v0/layout/apply`
+#### ApplyClusterLayout `POST /v0/layout/apply`
 
 Applies to the cluster the layout changes currently registered as
 staged layout changes.
@@ -221,7 +230,7 @@ Similarly to the CLI, the body must include the version of the new layout
 that will be created, which MUST be 1 + the value of the currently
 existing layout in the cluster.
 
-### RevertClusterLayout `POST /v0/layout/revert`
+#### RevertClusterLayout `POST /v0/layout/revert`
 
 Clears all of the staged layout changes.
 
@@ -240,9 +249,9 @@ version number, which MUST be 1 + the value of the currently
 existing layout in the cluster.
 
 
-## Access key operations
+### Access key operations
 
-### ListKeys `GET /v0/key`
+#### ListKeys `GET /v0/key`
 
 Returns all API access keys in the cluster.
 
@@ -261,7 +270,7 @@ Example response:
 ]
 ```
 
-### CreateKey `POST /v0/key`
+#### CreateKey `POST /v0/key`
 
 Creates a new API access key.
 
@@ -273,7 +282,7 @@ Request body format:
 }
 ```
 
-### ImportKey `POST /v0/key/import`
+#### ImportKey `POST /v0/key/import`
 
 Imports an existing API key.
 
@@ -287,8 +296,8 @@ Request body format:
 }
 ```
 
-### GetKeyInfo `GET /v0/key?id=<acces key id>`
-### GetKeyInfo `GET /v0/key?search=<pattern>`
+#### GetKeyInfo `GET /v0/key?id=<acces key id>`
+#### GetKeyInfo `GET /v0/key?search=<pattern>`
 
 Returns information about the requested API access key.
 
@@ -359,11 +368,11 @@ Example response:
 }
 ```
 
-### DeleteKey `DELETE /v0/key?id=<acces key id>`
+#### DeleteKey `DELETE /v0/key?id=<acces key id>`
 
 Deletes an API access key.
 
-### UpdateKey `POST /v0/key?id=<acces key id>`
+#### UpdateKey `POST /v0/key?id=<acces key id>`
 
 Updates information about the specified API access key.
 
@@ -384,9 +393,9 @@ If they are present, the corresponding modifications are applied to the key, oth
 The possible flags in `allow` and `deny` are: `createBucket`.
 
 
-## Bucket operations
+### Bucket operations
 
-### ListBuckets `GET /v0/bucket`
+#### ListBuckets `GET /v0/bucket`
 
 Returns all storage buckets in the cluster.
 
@@ -428,8 +437,8 @@ Example response:
 ]
 ```
 
-### GetBucketInfo `GET /v0/bucket?id=<bucket id>`
-### GetBucketInfo `GET /v0/bucket?globalAlias=<alias>`
+#### GetBucketInfo `GET /v0/bucket?id=<bucket id>`
+#### GetBucketInfo `GET /v0/bucket?globalAlias=<alias>`
 
 Returns information about the requested storage bucket.
 
@@ -462,7 +471,7 @@ Example response:
 }
 ```
 
-### CreateBucket `POST /v0/bucket`
+#### CreateBucket `POST /v0/bucket`
 
 Creates a new storage bucket.
 
@@ -481,7 +490,11 @@ OR
 	"localAlias": {
 		"accessKeyId": "GK31c2f218a2e44f485b94239e",
 		"alias": "NameOfMyBucket",
-		"allPermissions": true
+		"allow": {
+			"read": true,
+			"write": true,
+			"owner": false
+		}
 	}
 }
 ```
@@ -495,13 +508,16 @@ OR
 Creates a new bucket, either with a global alias, a local one,
 or no alias at all.
 
-### DeleteBucket `DELETE /v0/bucket?id=<bucket id>`
+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 /v0/bucket?id=<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!
 
-### PutBucketWebsite `PUT /v0/bucket/website?id=<bucket id>`
+#### PutBucketWebsite `PUT /v0/bucket/website?id=<bucket id>`
 
 Sets the website configuration for a bucket (this also enables website access for this bucket).
 
@@ -510,21 +526,21 @@ Request body format:
 ```json
 {
 	"indexDocument": "index.html",
-	"errorDocument": "404.html",
+	"errorDocument": "404.html"
 }
 ```
 
 The field `errorDocument` is optional, if no error document is set a generic error message is displayed when errors happen.
 
 
-### DeleteBucketWebsite `DELETE /v0/bucket/website?id=<bucket id>`
+#### DeleteBucketWebsite `DELETE /v0/bucket/website?id=<bucket id>`
 
 Deletes the website configuration for a bucket (disables website access for this bucket).
 
 
-## Operations on permissions for keys on buckets
+### Operations on permissions for keys on buckets
 
-### BucketAllowKey `POST /v0/bucket/allow`
+#### BucketAllowKey `POST /v0/bucket/allow`
 
 Allows a key to do read/write/owner operations on a bucket.
 
@@ -545,7 +561,7 @@ Request body format:
 Flags in `permissions` which have the value `true` will be activated.
 Other flags will remain unchanged.
 
-### BucketDenyKey `POST /v0/bucket/deny`
+#### BucketDenyKey `POST /v0/bucket/deny`
 
 Denies a key from doing read/write/owner operations on a bucket.
 
@@ -567,21 +583,21 @@ Flags in `permissions` which have the value `true` will be deactivated.
 Other flags will remain unchanged.
 
 
-## Operations on bucket aliases
+### Operations on bucket aliases
 
-### GlobalAliasBucket `PUT /v0/bucket/alias/global?id=<bucket id>&alias=<global alias>`
+#### GlobalAliasBucket `PUT /v0/bucket/alias/global?id=<bucket id>&alias=<global alias>`
 
 Empty body. Creates a global alias for a bucket.
 
-### GlobalUnaliasBucket `DELETE /v0/bucket/alias/global?id=<bucket id>&alias=<global alias>`
+#### GlobalUnaliasBucket `DELETE /v0/bucket/alias/global?id=<bucket id>&alias=<global alias>`
 
 Removes a global alias for a bucket.
 
-### LocalAliasBucket `PUT /v0/bucket/alias/local?id=<bucket id>&accessKeyId=<access key ID>&alias=<local alias>`
+#### LocalAliasBucket `PUT /v0/bucket/alias/local?id=<bucket id>&accessKeyId=<access key ID>&alias=<local alias>`
 
 Empty body. Creates a local alias for a bucket in the namespace of a specific access key.
 
-### LocalUnaliasBucket `DELETE /v0/bucket/alias/local?id=<bucket id>&accessKeyId<access key ID>&alias=<local alias>`
+#### LocalUnaliasBucket `DELETE /v0/bucket/alias/local?id=<bucket id>&accessKeyId<access key ID>&alias=<local alias>`
 
 Removes a local alias for a bucket in the namespace of a specific access key.
 

src/api/admin/api_server.rs

@@ -107,17 +107,27 @@ impl ApiHandler for AdminApiServer {
 		req: Request<Body>,
 		endpoint: Endpoint,
 	) -> Result<Response<Body>, Error> {
-		let expected_auth_header = match endpoint.authorization_type() {
-			Authorization::MetricsToken => self.metrics_token.as_ref(),
-			Authorization::AdminToken => self.admin_token.as_ref(),
-		};
+		let expected_auth_header =
+			match endpoint.authorization_type() {
+				Authorization::MetricsToken => self.metrics_token.as_ref(),
+				Authorization::AdminToken => match &self.admin_token {
+					None => return Err(Error::forbidden(
+						"Admin token isn't configured, admin API access is disabled for security.",
+					)),
+					Some(t) => Some(t),
+				},
+			};
 
 		if let Some(h) = expected_auth_header {
 			match req.headers().get("Authorization") {
-				None => Err(Error::forbidden("Authorization token must be provided")),
-				Some(v) if v.to_str().map(|hv| hv == h).unwrap_or(false) => Ok(()),
-				_ => Err(Error::forbidden("Invalid authorization token provided")),
-			}?;
+				None => return Err(Error::forbidden("Authorization token must be provided")),
+				Some(v) => {
+					let authorized = v.to_str().map(|hv| hv.trim() == h).unwrap_or(false);
+					if !authorized {
+						return Err(Error::forbidden("Invalid authorization token provided"));
+					}
+				}
+			}
 		}
 
 		match endpoint {

src/api/admin/bucket.rs

@@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize};
 use garage_util::crdt::*;
 use garage_util::data::*;
 use garage_util::error::Error as GarageError;
+use garage_util::time::*;
 
 use garage_table::*;
 
@@ -283,13 +284,19 @@ pub async fn handle_create_bucket(
 			.bucket_helper()
 			.set_local_bucket_alias(bucket.id, &la.access_key_id, &la.alias)
 			.await?;
-		if la.all_permissions {
+
+		if la.allow.read || la.allow.write || la.allow.owner {
 			garage
 				.bucket_helper()
 				.set_bucket_key_permissions(
 					bucket.id,
 					&la.access_key_id,
-					BucketKeyPerm::ALL_PERMISSIONS,
+					BucketKeyPerm {
+						timestamp: now_msec(),
+						allow_read: la.allow.read,
+						allow_write: la.allow.write,
+						allow_owner: la.allow.owner,
+					},
 				)
 				.await?;
 		}
@@ -311,7 +318,7 @@ struct CreateBucketLocalAlias {
 	access_key_id: String,
 	alias: String,
 	#[serde(default)]
-	all_permissions: bool,
+	allow: ApiBucketKeyPerm,
 }
 
 pub async fn handle_delete_bucket(

src/k2v-client/Cargo.toml

@@ -6,6 +6,7 @@ edition = "2018"
 [dependencies]
 base64 = "0.13.0"
 http = "0.2.6"
+log = "0.4"
 rusoto_core = "0.48.0"
 rusoto_credential = "0.48.0"
 rusoto_signature = "0.48.0"
@@ -22,6 +23,10 @@ garage_util = { path = "../util", optional = true }
 [features]
 cli = ["clap", "tokio/fs", "tokio/io-std", "garage_util"]
 
+[lib]
+path = "lib.rs"
+
 [[bin]]
 name = "k2v-cli"
+path = "bin/k2v-cli.rs"
 required-features = ["cli"]

src/k2v-client/error.rs

@@ -5,6 +5,13 @@ use thiserror::Error;
 /// Errors returned by this crate
 #[derive(Error, Debug)]
 pub enum Error {
+	#[error("{0}, {1}: {2} (path = {3})")]
+	Remote(
+		http::StatusCode,
+		Cow<'static, str>,
+		Cow<'static, str>,
+		Cow<'static, str>,
+	),
 	#[error("received invalid response: {0}")]
 	InvalidResponse(Cow<'static, str>),
 	#[error("not found")]

src/k2v-client/lib.rs

@@ -4,6 +4,7 @@ use std::time::Duration;
 use http::header::{ACCEPT, CONTENT_LENGTH, CONTENT_TYPE};
 use http::status::StatusCode;
 use http::HeaderMap;
+use log::{debug, error};
 
 use rusoto_core::{ByteStream, DispatchSignedRequest, HttpClient};
 use rusoto_credential::AwsCredentials;
@@ -310,12 +311,47 @@ impl K2vClient {
 			StatusCode::NO_CONTENT => Vec::new(),
 			StatusCode::NOT_FOUND => return Err(Error::NotFound),
 			StatusCode::NOT_MODIFIED => Vec::new(),
-			_ => {
-				return Err(Error::InvalidResponse(
-					format!("invalid error code: {}", res.status).into(),
-				))
+			s => {
+				let err_body = read_body(&mut res.headers, res.body)
+					.await
+					.unwrap_or_default();
+				let err_body_str = std::str::from_utf8(&err_body)
+					.map(String::from)
+					.unwrap_or_else(|_| base64::encode(&err_body));
+
+				if s.is_client_error() || s.is_server_error() {
+					error!("Error response {}: {}", res.status, err_body_str);
+					let err = match serde_json::from_slice::<ErrorResponse>(&err_body) {
+						Ok(err) => Error::Remote(
+							res.status,
+							err.code.into(),
+							err.message.into(),
+							err.path.into(),
+						),
+						Err(_) => Error::Remote(
+							res.status,
+							"unknown".into(),
+							err_body_str.into(),
+							"?".into(),
+						),
+					};
+					return Err(err);
+				} else {
+					let msg = format!(
+						"Unexpected response code {}. Response body: {}",
+						res.status, err_body_str
+					);
+					error!("{}", msg);
+					return Err(Error::InvalidResponse(msg.into()));
+				}
 			}
 		};
+		debug!(
+			"Response body: {}",
+			std::str::from_utf8(&body)
+				.map(String::from)
+				.unwrap_or_else(|_| base64::encode(&body))
+		);
 
 		Ok(Response {
 			body,
@@ -558,6 +594,15 @@ struct BatchDeleteResponse<'a> {
 	deleted_items: u64,
 }
 
+#[derive(Deserialize)]
+struct ErrorResponse {
+	code: String,
+	message: String,
+	#[allow(dead_code)]
+	region: String,
+	path: String,
+}
+
 struct Response {
 	body: Vec<u8>,
 	status: StatusCode,