Rough add and remove endpoints for groups

oxidecomputer · Sep 29, 2023 · cfb143d · cfb143d
1 parent df2f140
commit cfb143d
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 36 deletions.
diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs
@@ -925,6 +925,50 @@ impl ApiContext {
     ) -> Result<Option<AccessGroup<ApiPermission>>, StoreError> {
         AccessGroupStore::delete(&*self.storage, &group_id).await
     }
+
+    pub async fn add_api_user_to_group(
+        &self,
+        api_user_id: &Uuid,
+        group_id: &Uuid,
+    ) -> Result<Option<ApiUser<ApiPermission>>, StoreError> {
+        // TODO: This needs to be wrapped in a transaction. That requires reworking the way the
+        // store traits are handled. Ideally we could have an API that still abstracts away the
+        // underlying connection management while allowing for transactions. Possibly something
+        // that takes a closure and passes in a connection that implements all of the expected
+        // data store traits
+        let user = ApiUserStore::get(&*self.storage, api_user_id, false).await?;
+
+        Ok(if let Some(user) = user {
+            let mut update: NewApiUser<ApiPermission> = user.into();
+            update.groups.push(*group_id);
+
+            Some(ApiUserStore::upsert(&*self.storage, update).await?)
+        } else {
+            None
+        })
+    }
+
+    pub async fn remove_api_user_from_group(
+        &self,
+        api_user_id: &Uuid,
+        group_id: &Uuid,
+    ) -> Result<Option<ApiUser<ApiPermission>>, StoreError> {
+        // TODO: This needs to be wrapped in a transaction. That requires reworking the way the
+        // store traits are handled. Ideally we could have an API that still abstracts away the
+        // underlying connection management while allowing for transactions. Possibly something
+        // that takes a closure and passes in a connection that implements all of the expected
+        // data store traits
+        let user = ApiUserStore::get(&*self.storage, api_user_id, false).await?;
+
+        Ok(if let Some(user) = user {
+            let mut update: NewApiUser<ApiPermission> = user.into();
+            update.groups.retain(|id| id != group_id);
+
+            Some(ApiUserStore::upsert(&*self.storage, update).await?)
+        } else {
+            None
+        })
+    }
 }
 
 #[cfg(test)]

diff --git a/rfd-api/src/endpoints/api_user.rs b/rfd-api/src/endpoints/api_user.rs
@@ -2,7 +2,6 @@ use chrono::{DateTime, Utc};
 use dropshot::{
     endpoint, HttpError, HttpResponseCreated, HttpResponseOk, Path, RequestContext, TypedBody,
 };
-use http::StatusCode;
 use partial_struct::partial;
 use rfd_model::{storage::ListPagination, ApiUser, NewApiKey, NewApiUser};
 use schemars::JsonSchema;
@@ -16,7 +15,7 @@ use crate::{
     context::ApiContext,
     error::ApiError,
     permissions::ApiPermission,
-    util::response::{client_error, not_found, to_internal_error},
+    util::response::{forbidden, not_found, to_internal_error},
     ApiCaller, ApiPermissions, User,
 };
 
@@ -81,7 +80,7 @@ async fn get_api_user_op(
             Err(not_found("Failed to find"))
         }
     } else {
-        Err(client_error(StatusCode::FORBIDDEN, "Unauthorized"))
+        Err(forbidden())
     }
 }
 
@@ -125,7 +124,7 @@ async fn create_api_user_op(
 
         Ok(HttpResponseCreated(user))
     } else {
-        Err(client_error(StatusCode::FORBIDDEN, "Unauthorized"))
+        Err(forbidden())
     }
 }
 
@@ -174,7 +173,7 @@ async fn update_api_user_op(
 
         Ok(HttpResponseOk(user))
     } else {
-        Err(client_error(StatusCode::FORBIDDEN, "Unauthorized"))
+        Err(forbidden())
     }
 }
 
@@ -222,7 +221,7 @@ async fn list_api_user_tokens_op(
                 .collect(),
         ))
     } else {
-        Err(client_error(StatusCode::FORBIDDEN, "Unauthorized"))
+        Err(forbidden())
     }
 }
 
@@ -308,7 +307,7 @@ async fn create_api_user_token_op(
             Err(not_found("Failed to find api user"))
         }
     } else {
-        Err(client_error(StatusCode::FORBIDDEN, "Unauthorized"))
+        Err(forbidden())
     }
 }
 
@@ -357,7 +356,7 @@ async fn get_api_user_token_op(
             Err(not_found("Failed to find token"))
         }
     } else {
-        Err(client_error(StatusCode::FORBIDDEN, "Unauthorized"))
+        Err(forbidden())
     }
 }
 
@@ -400,7 +399,72 @@ async fn delete_api_user_token_op(
             Err(not_found("Failed to find token"))
         }
     } else {
-        Err(client_error(StatusCode::FORBIDDEN, "Unauthorized"))
+        Err(forbidden())
+    }
+}
+
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct AddGroupBody {
+    group_id: Uuid,
+}
+
+#[trace_request]
+#[endpoint {
+    method = POST,
+    path = "/api-user/{identifier}/group",
+}]
+#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))]
+pub async fn add_api_user_to_group(
+    rqctx: RequestContext<ApiContext>,
+    path: Path<ApiUserPath>,
+    body: TypedBody<AddGroupBody>,
+) -> Result<HttpResponseOk<ApiUser<ApiPermission>>, HttpError> {
+    let ctx = rqctx.context();
+    let auth = ctx.authn_token(&rqctx).await?;
+    let caller = ctx.get_caller(&auth).await?;
+    let path = path.into_inner();
+    let body = body.into_inner();
+
+    if caller.can(&ApiPermission::AddToGroup(body.group_id)) {
+        ctx.add_api_user_to_group(&path.identifier, &body.group_id)
+            .await
+            .map_err(ApiError::Storage)?
+            .map(HttpResponseOk)
+            .ok_or_else(|| not_found("User does not exist"))
+    } else {
+        Err(forbidden())
+    }
+}
+
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct ApiUserRemoveGroupPath {
+    identifier: Uuid,
+    group_id: Uuid,
+}
+
+#[trace_request]
+#[endpoint {
+    method = DELETE,
+    path = "/api-user/{identifier}/group/{group_id}",
+}]
+#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))]
+pub async fn remove_api_user_from_group(
+    rqctx: RequestContext<ApiContext>,
+    path: Path<ApiUserRemoveGroupPath>,
+) -> Result<HttpResponseOk<ApiUser<ApiPermission>>, HttpError> {
+    let ctx = rqctx.context();
+    let auth = ctx.authn_token(&rqctx).await?;
+    let caller = ctx.get_caller(&auth).await?;
+    let path = path.into_inner();
+
+    if caller.can(&ApiPermission::RemoveFromGroup(path.group_id)) {
+        ctx.remove_api_user_from_group(&path.identifier, &path.group_id)
+            .await
+            .map_err(ApiError::Storage)?
+            .map(HttpResponseOk)
+            .ok_or_else(|| not_found("User does not exist"))
+    } else {
+        Err(forbidden())
     }
 }
 

diff --git a/rfd-api/src/endpoints/groups.rs → rfd-api/src/endpoints/group.rs b/rfd-api/src/endpoints/groups.rs → rfd-api/src/endpoints/group.rs
@@ -7,7 +7,7 @@ use tracing::instrument;
 use uuid::Uuid;
 
 use crate::{
-    context::ApiContext, error::ApiError, permissions::ApiPermission, util::response::unauthorized,
+    context::ApiContext, error::ApiError, permissions::ApiPermission, util::response::forbidden,
     ApiPermissions,
 };
 
@@ -21,15 +21,15 @@ pub async fn get_groups(
     rqctx: RequestContext<ApiContext>,
 ) -> Result<HttpResponseOk<Vec<AccessGroup<ApiPermission>>>, HttpError> {
     let ctx = rqctx.context();
-    let auth = ctx.authn_token(&rqctx).await;
-    let caller = ctx.get_caller(&auth?).await?;
+    let auth = ctx.authn_token(&rqctx).await?;
+    let caller = ctx.get_caller(&auth).await?;
 
     if caller.can(&ApiPermission::ListGroups) {
         Ok(HttpResponseOk(
             ctx.get_groups().await.map_err(ApiError::Storage)?,
         ))
     } else {
-        Err(unauthorized())
+        Err(forbidden())
     }
 }
 
@@ -50,8 +50,8 @@ pub async fn create_group(
     body: TypedBody<AccessGroupUpdateParams>,
 ) -> Result<HttpResponseOk<AccessGroup<ApiPermission>>, HttpError> {
     let ctx = rqctx.context();
-    let auth = ctx.authn_token(&rqctx).await;
-    let caller = ctx.get_caller(&auth?).await?;
+    let auth = ctx.authn_token(&rqctx).await?;
+    let caller = ctx.get_caller(&auth).await?;
 
     if caller.can(&ApiPermission::CreateGroup) {
         let body = body.into_inner();
@@ -65,7 +65,7 @@ pub async fn create_group(
             .map_err(ApiError::Storage)?,
         ))
     } else {
-        Err(unauthorized())
+        Err(forbidden())
     }
 }
 
@@ -86,8 +86,8 @@ pub async fn update_group(
     body: TypedBody<AccessGroupUpdateParams>,
 ) -> Result<HttpResponseOk<AccessGroup<ApiPermission>>, HttpError> {
     let ctx = rqctx.context();
-    let auth = ctx.authn_token(&rqctx).await;
-    let caller = ctx.get_caller(&auth?).await?;
+    let auth = ctx.authn_token(&rqctx).await?;
+    let caller = ctx.get_caller(&auth).await?;
     let path = path.into_inner();
 
     if caller.can(&ApiPermission::UpdateGroup(path.group_id)) {
@@ -102,7 +102,7 @@ pub async fn update_group(
             .map_err(ApiError::Storage)?,
         ))
     } else {
-        Err(unauthorized())
+        Err(forbidden())
     }
 }
 
@@ -117,8 +117,8 @@ pub async fn delete_group(
     path: Path<AccessGroupPath>,
 ) -> Result<HttpResponseOk<Option<AccessGroup<ApiPermission>>>, HttpError> {
     let ctx = rqctx.context();
-    let auth = ctx.authn_token(&rqctx).await;
-    let caller = ctx.get_caller(&auth?).await?;
+    let auth = ctx.authn_token(&rqctx).await?;
+    let caller = ctx.get_caller(&auth).await?;
     let path = path.into_inner();
 
     if caller.can(&ApiPermission::DeleteGroup(path.group_id)) {
@@ -128,6 +128,6 @@ pub async fn delete_group(
                 .map_err(ApiError::Storage)?,
         ))
     } else {
-        Err(unauthorized())
+        Err(forbidden())
     }
 }
diff --git a/rfd-api/src/endpoints/mod.rs b/rfd-api/src/endpoints/mod.rs
@@ -1,5 +1,5 @@
 pub mod api_user;
-pub mod groups;
+pub mod group;
 pub mod login;
 pub mod rfd;
 pub mod webhook;
diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs
@@ -10,10 +10,11 @@ use crate::{
     context::ApiContext,
     endpoints::{
         api_user::{
-            create_api_user, create_api_user_token, delete_api_user_token, get_api_user,
-            get_api_user_token, get_self, list_api_user_tokens, update_api_user,
+            add_api_user_to_group, create_api_user, create_api_user_token, delete_api_user_token,
+            get_api_user, get_api_user_token, get_self, list_api_user_tokens,
+            remove_api_user_from_group, update_api_user,
         },
-        groups::{create_group, delete_group, get_groups, update_group},
+        group::{create_group, delete_group, get_groups, update_group},
         login::oauth::{
             client::{
                 create_oauth_client, create_oauth_client_redirect_uri, create_oauth_client_secret,
@@ -88,6 +89,8 @@ pub fn server(
     api.register(get_api_user_token).unwrap();
     api.register(create_api_user_token).unwrap();
     api.register(delete_api_user_token).unwrap();
+    api.register(add_api_user_to_group).unwrap();
+    api.register(remove_api_user_from_group).unwrap();
 
     // Group Management
     api.register(get_groups).unwrap();

diff --git a/rfd-api/src/util.rs b/rfd-api/src/util.rs
@@ -44,6 +44,10 @@ pub mod response {
         client_error(StatusCode::UNAUTHORIZED, "Unauthorized")
     }
 
+    pub fn forbidden() -> HttpError {
+        client_error(StatusCode::FORBIDDEN, "Unauthorized")
+    }
+
     pub fn client_error<S>(status_code: StatusCode, message: S) -> HttpError
     where
         S: ToString,

diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs
@@ -1,8 +1,3 @@
-use std::{
-    collections::{BTreeMap, BTreeSet},
-    time::Duration,
-};
-
 use async_bb8_diesel::{AsyncRunQueryDsl, ConnectionError, ConnectionManager, OptionalExtension};
 use async_trait::async_trait;
 use bb8::Pool;
@@ -15,7 +10,10 @@ use diesel::{
     upsert::{excluded, on_constraint},
     ExpressionMethods, PgArrayExpressionMethods,
 };
-
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    time::Duration,
+};
 use uuid::Uuid;
 
 use crate::{
@@ -74,10 +72,6 @@ impl PostgresStore {
                 .await?,
         })
     }
-
-    pub fn connection(&self) -> &DbPool {
-        &self.conn
-    }
 }
 
 #[async_trait]
@@ -559,11 +553,13 @@ where
             .values((
                 api_user::id.eq(user.id),
                 api_user::permissions.eq(user.permissions.clone()),
+                api_user::groups.eq(user.groups.clone()),
             ))
             .on_conflict(api_user::id)
             .do_update()
             .set((
                 api_user::permissions.eq(excluded(api_user::permissions)),
+                api_user::groups.eq(excluded(api_user::groups)),
                 api_user::updated_at.eq(Utc::now()),
             ))
             .get_result_async(&self.conn)