Skip to content

Commit

Permalink
[server] Add remove user to admin endpoint #824 (#851)
Browse files Browse the repository at this point in the history
* fix changing roles

* add tests

* add remove db user
  • Loading branch information
michaelvlach authored Dec 11, 2023
1 parent 5e251f7 commit acf05c5
Show file tree
Hide file tree
Showing 11 changed files with 377 additions and 29 deletions.
55 changes: 55 additions & 0 deletions agdb_server/openapi/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,46 @@
]
}
},
"/api/v1/db/user/remove": {
"post": {
"tags": [
"crate::routes::db::user"
],
"operationId": "remove",
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/RemoveDbUser"
}
}
},
"required": true
},
"responses": {
"204": {
"description": "user removed"
},
"401": {
"description": "unauthorized"
},
"403": {
"description": "user must be a db admin / cannot remove last admin user"
},
"464": {
"description": "user not found"
},
"466": {
"description": "db not found"
}
},
"security": [
{
"Token": []
}
]
}
},
"/api/v1/status": {
"get": {
"tags": [
Expand Down Expand Up @@ -494,6 +534,21 @@
"read"
]
},
"RemoveDbUser": {
"type": "object",
"required": [
"database",
"user"
],
"properties": {
"database": {
"type": "string"
},
"user": {
"type": "string"
}
}
},
"ServerDatabase": {
"type": "object",
"required": [
Expand Down
2 changes: 2 additions & 0 deletions agdb_server/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ use utoipa::OpenApi;
crate::routes::db::list,
crate::routes::db::remove,
crate::routes::db::user::add,
crate::routes::db::user::remove,
),
components(schemas(
crate::routes::admin::user::UserStatus,
crate::routes::db::DbType,
crate::routes::db::ServerDatabase,
crate::routes::db::ServerDatabaseName,
crate::routes::db::user::DbUser,
crate::routes::db::user::RemoveDbUser,
crate::routes::db::user::DbUserRole,
crate::routes::user::ChangePassword,
crate::routes::user::UserCredentials,
Expand Down
4 changes: 3 additions & 1 deletion agdb_server/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ pub(crate) fn app(config: Config, shutdown_sender: Sender<()>, db_pool: DbPool)
)
.route("/login", routing::post(routes::user::login));

let db_user_router_v1 = Router::new().route("/add", routing::post(routes::db::user::add));
let db_user_router_v1 = Router::new()
.route("/add", routing::post(routes::db::user::add))
.route("/remove", routing::post(routes::db::user::remove));

let db_router_v1 = Router::new()
.route("/add", routing::post(routes::db::add))
Expand Down
95 changes: 77 additions & 18 deletions agdb_server/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl DbPool {
Ok(db_pool)
}

pub(crate) fn add_database(&self, user: DbId, database: Database) -> ServerResult {
pub(crate) fn add_db(&self, user: DbId, database: Database) -> ServerResult {
let db = ServerDb::new(&format!("{}:{}", database.db_type, database.name)).map_err(
|mut e| {
e.status = ErrorCode::DbInvalid.into();
Expand All @@ -119,16 +119,38 @@ impl DbPool {
Ok(())
}

pub(crate) fn add_database_user(&self, database: DbId, user: DbId, role: &str) -> ServerResult {
self.db_mut()?.exec_mut(
&QueryBuilder::insert()
.edges()
.from(user)
.to(database)
.values_uniform(vec![("role", role).into()])
.query(),
)?;
Ok(())
pub(crate) fn add_db_user(&self, db: DbId, user: DbId, role: &str) -> ServerResult {
self.db_mut()?.transaction_mut(|t| {
let existing_role = t.exec(
&QueryBuilder::search()
.from(user)
.to(db)
.limit(1)
.where_()
.keys(vec!["role".into()])
.query(),
)?;

if existing_role.result == 1 {
t.exec_mut(
&QueryBuilder::insert()
.values(vec![vec![("role", role).into()]])
.ids(existing_role)
.query(),
)?;
} else {
t.exec_mut(
&QueryBuilder::insert()
.edges()
.from(user)
.to(db)
.values_uniform(vec![("role", role).into()])
.query(),
)?;
}

Ok(())
})
}

pub(crate) fn create_user(&self, user: DbUser) -> ServerResult {
Expand All @@ -146,8 +168,8 @@ impl DbPool {
Ok(())
}

pub(crate) fn delete_database(&self, db: Database) -> ServerResult {
let filename = self.remove_database(db)?.get()?.filename().to_string();
pub(crate) fn delete_db(&self, db: Database) -> ServerResult {
let filename = self.remove_db(db)?.get()?.filename().to_string();
let path = Path::new(&filename);

if path.exists() {
Expand All @@ -162,7 +184,7 @@ impl DbPool {
Ok(())
}

pub(crate) fn find_databases(&self) -> ServerResult<Vec<Database>> {
pub(crate) fn find_dbs(&self) -> ServerResult<Vec<Database>> {
Ok(self
.db()?
.exec(
Expand All @@ -179,7 +201,7 @@ impl DbPool {
.try_into()?)
}

pub(crate) fn find_database_id(&self, name: &str) -> ServerResult<DbId> {
pub(crate) fn find_db_id(&self, name: &str) -> ServerResult<DbId> {
Ok(self
.db()?
.exec(
Expand Down Expand Up @@ -225,7 +247,7 @@ impl DbPool {
.collect())
}

pub(crate) fn find_user_databases(&self, user: DbId) -> ServerResult<Vec<Database>> {
pub(crate) fn find_user_dbs(&self, user: DbId) -> ServerResult<Vec<Database>> {
Ok(self
.db()?
.exec(
Expand All @@ -242,7 +264,7 @@ impl DbPool {
.try_into()?)
}

pub(crate) fn find_user_database(&self, user: DbId, db: &str) -> ServerResult<Database> {
pub(crate) fn find_user_db(&self, user: DbId, db: &str) -> ServerResult<Database> {
Ok(self
.0
.server_db
Expand Down Expand Up @@ -341,6 +363,26 @@ impl DbPool {
.id)
}

pub(crate) fn db_admins(&self, db: DbId) -> ServerResult<Vec<DbId>> {
Ok(self
.db()?
.exec(
&QueryBuilder::search()
.to(db)
.where_()
.distance(CountComparison::Equal(2))
.and()
.beyond()
.where_()
.node()
.or()
.key("role")
.value(Comparison::Equal("admin".into()))
.query(),
)?
.ids())
}

pub(crate) fn is_db_admin(&self, user: DbId, db: DbId) -> ServerResult<bool> {
Ok(self
.db()?
Expand All @@ -360,7 +402,24 @@ impl DbPool {
== 1)
}

pub(crate) fn remove_database(&self, db: Database) -> ServerResult<ServerDb> {
pub(crate) fn remove_db_user(&self, db: DbId, user: DbId) -> ServerResult {
self.db_mut()?.exec_mut(
&QueryBuilder::remove()
.ids(
QueryBuilder::search()
.from(user)
.to(db)
.limit(1)
.where_()
.edge()
.query(),
)
.query(),
)?;
Ok(())
}

pub(crate) fn remove_db(&self, db: Database) -> ServerResult<ServerDb> {
self.db_mut()?
.exec_mut(&QueryBuilder::remove().ids(db.db_id.unwrap()).query())?;

Expand Down
2 changes: 1 addition & 1 deletion agdb_server/src/routes/admin/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub(crate) async fn list(
State(db_pool): State<DbPool>,
) -> ServerResponse<(StatusCode, Json<Vec<ServerDatabase>>)> {
let dbs = db_pool
.find_databases()?
.find_dbs()?
.into_iter()
.map(|db| db.into())
.collect();
Expand Down
14 changes: 7 additions & 7 deletions agdb_server/src/routes/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub(crate) async fn add(
State(db_pool): State<DbPool>,
Json(request): Json<ServerDatabase>,
) -> ServerResponse {
if db_pool.find_database_id(&request.name).is_ok() {
if db_pool.find_db_id(&request.name).is_ok() {
return Err(ErrorCode::DbExists.into());
}

Expand All @@ -81,7 +81,7 @@ pub(crate) async fn add(
db_type: request.db_type.to_string(),
};

db_pool.add_database(user.0, db)?;
db_pool.add_db(user.0, db)?;

Ok(StatusCode::CREATED)
}
Expand All @@ -102,13 +102,13 @@ pub(crate) async fn delete(
State(db_pool): State<DbPool>,
Json(request): Json<ServerDatabaseName>,
) -> ServerResponse {
let db = db_pool.find_user_database(user.0, &request.name)?;
let db = db_pool.find_user_db(user.0, &request.name)?;

if !db_pool.is_db_admin(user.0, db.db_id.unwrap())? {
return Ok(StatusCode::FORBIDDEN);
}

db_pool.delete_database(db)?;
db_pool.delete_db(db)?;

Ok(StatusCode::NO_CONTENT)
}
Expand All @@ -126,7 +126,7 @@ pub(crate) async fn list(
State(db_pool): State<DbPool>,
) -> ServerResponse<(StatusCode, Json<Vec<ServerDatabase>>)> {
let dbs = db_pool
.find_user_databases(user.0)?
.find_user_dbs(user.0)?
.into_iter()
.map(|db| db.into())
.collect();
Expand All @@ -149,13 +149,13 @@ pub(crate) async fn remove(
State(db_pool): State<DbPool>,
Json(request): Json<ServerDatabaseName>,
) -> ServerResponse {
let db = db_pool.find_user_database(user.0, &request.name)?;
let db = db_pool.find_user_db(user.0, &request.name)?;

if !db_pool.is_db_admin(user.0, db.db_id.unwrap())? {
return Ok(StatusCode::FORBIDDEN);
}

db_pool.remove_database(db)?;
db_pool.remove_db(db)?;

Ok(StatusCode::NO_CONTENT)
}
42 changes: 40 additions & 2 deletions agdb_server/src/routes/db/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ pub(crate) struct DbUser {
pub(crate) role: DbUserRole,
}

#[derive(Deserialize, ToSchema)]
pub(crate) struct RemoveDbUser {
pub(crate) database: String,
pub(crate) user: String,
}

impl Display for DbUserRole {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand Down Expand Up @@ -50,14 +56,46 @@ pub(crate) async fn add(
State(db_pool): State<DbPool>,
Json(request): Json<DbUser>,
) -> ServerResponse {
let db = db_pool.find_database_id(&request.database)?;
let db = db_pool.find_db_id(&request.database)?;
let db_user = db_pool.find_user_id(&request.user)?;

if !db_pool.is_db_admin(user.0, db)? || db_user == user.0 {
return Ok(StatusCode::FORBIDDEN);
}

db_pool.add_database_user(db, db_user, &request.role.to_string())?;
db_pool.add_db_user(db, db_user, &request.role.to_string())?;

Ok(StatusCode::CREATED)
}

#[utoipa::path(post,
path = "/api/v1/db/user/remove",
request_body = RemoveDbUser,
security(("Token" = [])),
responses(
(status = 204, description = "user removed"),
(status = 401, description = "unauthorized"),
(status = 403, description = "user must be a db admin / cannot remove last admin user"),
(status = 464, description = "user not found"),
(status = 466, description = "db not found"),
)
)]
pub(crate) async fn remove(
user: UserId,
State(db_pool): State<DbPool>,
Json(request): Json<RemoveDbUser>,
) -> ServerResponse {
let db = db_pool.find_db_id(&request.database)?;
let db_user = db_pool.find_user_id(&request.user)?;
let admins = db_pool.db_admins(db)?;

println!("{:?} == {:?}", admins, vec![db_user]);

if (!admins.contains(&user.0) && user.0 != db_user) || admins == vec![db_user] {
return Ok(StatusCode::FORBIDDEN);
}

db_pool.remove_db_user(db, db_user)?;

Ok(StatusCode::NO_CONTENT)
}
1 change: 1 addition & 0 deletions agdb_server/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub const USER_CHANGE_PASSWORD_URI: &str = "/user/change_password";
pub const ADMIN_USER_CREATE_URI: &str = "/admin/user/create";
pub const DB_ADD_URI: &str = "/db/add";
pub const DB_USER_ADD_URI: &str = "/db/user/add";
pub const DB_USER_REMOVE_URI: &str = "/db/user/remove";
pub const DB_DELETE_URI: &str = "/db/delete";
pub const DB_LIST_URI: &str = "/db/list";
pub const ADMIN_DB_LIST_URI: &str = "/admin/db/list";
Expand Down
Loading

0 comments on commit acf05c5

Please sign in to comment.