Check authorization when attempting to delete a database (#96)

In the ClientAPI /database/delete/:address endpoint, require authorization before deleting a database.

crates/client-api/src/routes/database.rs

@@ -29,7 +29,7 @@ use spacetimedb::database_logger::DatabaseLogger;
 use spacetimedb::host::DescribedEntityType;
 use spacetimedb::identity::Identity;
 use spacetimedb::json::client_api::StmtResultJson;
-use spacetimedb::messages::control_db::{DatabaseInstance, HostType};
+use spacetimedb::messages::control_db::{Database, DatabaseInstance, HostType};
 
 use crate::util::{ByteStringBody, NameOrAddress};
 use crate::{log_and_500, ControlCtx, ControlNodeDelegate, WorkerCtx};
@@ -69,14 +69,10 @@ pub async fn call(
     let args = ReducerArgs::Json(body);
 
     let address = name_or_address.resolve(&*worker_ctx).await?;
-    let database = worker_ctx
-        .get_database_by_address(&address)
-        .await
-        .map_err(log_and_500)?
-        .ok_or_else(|| {
-            log::error!("Could not find database: {}", address.to_hex());
-            (StatusCode::NOT_FOUND, "No such database.")
-        })?;
+    let database = worker_ctx_find_database(&*worker_ctx, &address).await?.ok_or_else(|| {
+        log::error!("Could not find database: {}", address.to_hex());
+        (StatusCode::NOT_FOUND, "No such database.")
+    })?;
     let identity = database.identity;
     let database_instance = worker_ctx
         .get_leader_database_instance_by_database(database.id)
@@ -189,14 +185,10 @@ async fn extract_db_call_info(
 ) -> Result<DatabaseInformation, ErrorResponse> {
     let auth = auth.get_or_create(ctx).await?;
 
-    let database = ctx
-        .get_database_by_address(address)
-        .await
-        .map_err(log_and_500)?
-        .ok_or_else(|| {
-            log::error!("Could not find database: {}", address.to_hex());
-            (StatusCode::NOT_FOUND, "No such database.")
-        })?;
+    let database = worker_ctx_find_database(ctx, address).await?.ok_or_else(|| {
+        log::error!("Could not find database: {}", address.to_hex());
+        (StatusCode::NOT_FOUND, "No such database.")
+    })?;
 
     let database_instance = ctx.get_leader_database_instance_by_database(database.id).await.ok_or((
         StatusCode::NOT_FOUND,
@@ -262,10 +254,8 @@ pub async fn describe(
     auth: SpacetimeAuthHeader,
 ) -> axum::response::Result<impl IntoResponse> {
     let address = name_or_address.resolve(&*worker_ctx).await?;
-    let database = worker_ctx
-        .get_database_by_address(&address)
-        .await
-        .map_err(log_and_500)?
+    let database = worker_ctx_find_database(&*worker_ctx, &address)
+        .await?
         .ok_or((StatusCode::NOT_FOUND, "No such database."))?;
 
     let call_info = extract_db_call_info(&*worker_ctx, auth, &address).await?;
@@ -318,10 +308,8 @@ pub async fn catalog(
     auth: SpacetimeAuthHeader,
 ) -> axum::response::Result<impl IntoResponse> {
     let address = name_or_address.resolve(&*worker_ctx).await?;
-    let database = worker_ctx
-        .get_database_by_address(&address)
-        .await
-        .map_err(log_and_500)?
+    let database = worker_ctx_find_database(&*worker_ctx, &address)
+        .await?
         .ok_or((StatusCode::NOT_FOUND, "No such database."))?;
 
     let call_info = extract_db_call_info(&*worker_ctx, auth, &address).await?;
@@ -366,10 +354,8 @@ pub async fn info(
     Path(InfoParams { name_or_address }): Path<InfoParams>,
 ) -> axum::response::Result<impl IntoResponse> {
     let address = name_or_address.resolve(&*worker_ctx).await?;
-    let database = worker_ctx
-        .get_database_by_address(&address)
-        .await
-        .map_err(log_and_500)?
+    let database = worker_ctx_find_database(&*worker_ctx, &address)
+        .await?
         .ok_or((StatusCode::NOT_FOUND, "No such database."))?;
 
     let host_type = match database.host_type {
@@ -397,21 +383,28 @@ pub struct LogsQuery {
     follow: bool,
 }
 
+fn auth_or_unauth(auth: SpacetimeAuthHeader) -> axum::response::Result<SpacetimeAuth> {
+    auth.get()
+        .ok_or((StatusCode::UNAUTHORIZED, "Invalid credentials").into())
+}
+
 pub async fn logs(
     State(worker_ctx): State<Arc<dyn WorkerCtx>>,
     Path(LogsParams { name_or_address }): Path<LogsParams>,
     Query(LogsQuery { num_lines, follow }): Query<LogsQuery>,
     auth: SpacetimeAuthHeader,
 ) -> axum::response::Result<impl IntoResponse> {
     // You should not be able to read the logs from a database that you do not own
-    // so, unless you are the owner, this will fail, hence using get() and not get_or_create
-    let auth = auth.get().ok_or((StatusCode::UNAUTHORIZED, "Invalid credentials."))?;
+    // so, unless you are the owner, this will fail.
+    // TODO: This returns `UNAUTHORIZED` on failure,
+    //       while everywhere else we return `BAD_REQUEST`.
+    //       Is this special in some way? Should this change?
+    //       Should all the others change?
+    let auth = auth_or_unauth(auth)?;
 
     let address = name_or_address.resolve(&*worker_ctx).await?;
-    let database = worker_ctx
-        .get_database_by_address(&address)
-        .await
-        .map_err(log_and_500)?
+    let database = worker_ctx_find_database(&*worker_ctx, &address)
+        .await?
         .ok_or((StatusCode::NOT_FOUND, "No such database."))?;
 
     if database.identity != auth.identity {
@@ -483,6 +476,13 @@ fn mime_ndjson() -> mime::Mime {
     "application/x-ndjson".parse().unwrap()
 }
 
+async fn worker_ctx_find_database(
+    worker_ctx: &dyn WorkerCtx,
+    address: &Address,
+) -> Result<Option<Database>, StatusCode> {
+    worker_ctx.get_database_by_address(address).await.map_err(log_and_500)
+}
+
 #[derive(Deserialize)]
 pub struct SqlParams {
     name_or_address: NameOrAddress,
@@ -503,10 +503,8 @@ pub async fn sql(
     let auth = auth.get_or_create(&*worker_ctx).await?;
 
     let address = name_or_address.resolve(&*worker_ctx).await?;
-    let database = worker_ctx
-        .get_database_by_address(&address)
-        .await
-        .map_err(log_and_500)?
+    let database = worker_ctx_find_database(&*worker_ctx, &address)
+        .await?
         .ok_or((StatusCode::NOT_FOUND, "No such database."))?;
 
     let auth = AuthCtx::new(database.identity, auth.identity);
@@ -613,14 +611,19 @@ pub struct RegisterTldParams {
     tld: String,
 }
 
+fn auth_or_bad_request(auth: SpacetimeAuthHeader) -> axum::response::Result<SpacetimeAuth> {
+    auth.get()
+        .ok_or((StatusCode::BAD_REQUEST, "Invalid credentials.").into())
+}
+
 pub async fn register_tld(
     State(ctx): State<Arc<dyn ControlCtx>>,
     Query(RegisterTldParams { tld }): Query<RegisterTldParams>,
     auth: SpacetimeAuthHeader,
 ) -> axum::response::Result<impl IntoResponse> {
     // You should not be able to publish to a database that you do not own
     // so, unless you are the owner, this will fail, hence not using get_or_create
-    let auth = auth.get().ok_or((StatusCode::BAD_REQUEST, "Invalid credentials."))?;
+    let auth = auth_or_bad_request(auth)?;
 
     let tld = tld.parse::<DomainName>().map_err(DomainParsingRejection)?.into_tld();
     let result = ctx
@@ -739,6 +742,13 @@ pub async fn confirm_recovery_code(
     Ok(axum::Json(result))
 }
 
+async fn control_ctx_find_database(ctx: &dyn ControlCtx, address: &Address) -> Result<Option<Database>, StatusCode> {
+    ctx.control_db()
+        .get_database_by_address(address)
+        .await
+        .map_err(log_and_500)
+}
+
 #[derive(Deserialize)]
 pub struct PublishDatabaseParams {}
 
@@ -779,8 +789,8 @@ pub async fn publish(
     } = query_params;
 
     // You should not be able to publish to a database that you do not own
-    // so, unless you are the owner, this will fail, hence not using get_or_create
-    let auth = auth.get().ok_or((StatusCode::BAD_REQUEST, "Invalid credentials."))?;
+    // so, unless you are the owner, this will fail.
+    let auth = auth_or_bad_request(auth)?;
 
     let specified_address = matches!(name_or_address, Some(NameOrAddress::Address(_)));
 
@@ -831,12 +841,7 @@ pub async fn publish(
 
     let trace_log = should_trace(trace_log);
 
-    let op = match ctx
-        .control_db()
-        .get_database_by_address(&db_address)
-        .await
-        .map_err(log_and_500)?
-    {
+    let op = match control_ctx_find_database(&*ctx, &db_address).await? {
         Some(db) => {
             if Identity::from_slice(db.identity.as_slice()) != auth.identity {
                 return Err((StatusCode::BAD_REQUEST, "Identity does not own this database.").into());
@@ -929,12 +934,23 @@ pub struct DeleteDatabaseParams {
 pub async fn delete_database(
     State(ctx): State<Arc<dyn ControlCtx>>,
     Path(DeleteDatabaseParams { address }): Path<DeleteDatabaseParams>,
+    auth: SpacetimeAuthHeader,
 ) -> axum::response::Result<impl IntoResponse> {
-    // TODO(cloutiertyler): Validate that the creator has credentials for the identity of this database
-
-    ctx.delete_database(&address).await.map_err(log_and_500)?;
+    let auth = auth_or_bad_request(auth)?;
 
-    Ok(())
+    match control_ctx_find_database(&*ctx, &address).await? {
+        Some(db) => {
+            if db.identity != auth.identity {
+                Err((StatusCode::BAD_REQUEST, "Identity does not own this database.").into())
+            } else {
+                ctx.delete_database(&address)
+                    .await
+                    .map_err(log_and_500)
+                    .map_err(Into::into)
+            }
+        }
+        None => Ok(()),
+    }
 }
 
 #[derive(Deserialize)]
@@ -954,7 +970,7 @@ pub async fn set_name(
     }): Query<SetNameQueryParams>,
     auth: SpacetimeAuthHeader,
 ) -> axum::response::Result<impl IntoResponse> {
-    let auth = auth.get().ok_or((StatusCode::BAD_REQUEST, "Invalid credentials."))?;
+    let auth = auth_or_bad_request(auth)?;
 
     let database = ctx
         .control_db()

crates/client-api/src/routes/subscribe.rs

@@ -97,7 +97,9 @@ pub async fn handle_websocket(
         };
 
         match forwarded_for {
-            Some(TypedHeader(XForwardedFor(ip))) => log::debug!("New client connected from ip {}", ip),
+            Some(TypedHeader(XForwardedFor(ip))) => {
+                log::debug!("New client connected from ip {}", ip)
+            }
             None => log::debug!("New client connected from unknown ip"),
         }
 

test/tests/permissions-delete.sh

@@ -0,0 +1,19 @@
+#!/bin/bash
+
+if [ "$DESCRIBE_TEST" = 1 ] ; then
+        echo "This test checks to make sure that you cannot delete a database that you do not own."
+        exit
+fi
+
+set -euox pipefail
+
+source "./test/lib.include"
+
+run_test cargo run identity new --no-email
+IDENT=$(grep IDENTITY "$TEST_OUT" | awk '{print $2}')
+run_test cargo run identity set-default "$IDENT"
+run_test cargo run publish -s -d --project-path="$PROJECT_PATH" --clear-database
+ADDRESS="$(grep "reated new database" "$TEST_OUT" | awk 'NF>1{print $NF}')"
+
+reset_config
+if cargo run delete "$ADDRESS"; then exit 1; fi