Skip to content

Commit

Permalink
refactor: [#615] handlers and functions now use optional user id
Browse files Browse the repository at this point in the history
  • Loading branch information
mario-nt committed Aug 4, 2024
1 parent c22c919 commit 6010155
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 37 deletions.
8 changes: 4 additions & 4 deletions src/services/category.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ impl Service {
/// * The category name is empty.
/// * The category already exists.
/// * There is a database error.
pub async fn add_category(&self, category_name: &str, user_id: &UserId) -> Result<i64, ServiceError> {
pub async fn add_category(&self, category_name: &str, maybe_user_id: Option<UserId>) -> Result<i64, ServiceError> {
self.authorization_service
.authorize(ACTION::AddCategory, Some(*user_id))
.authorize(ACTION::AddCategory, maybe_user_id)
.await?;

let trimmed_name = category_name.trim();
Expand Down Expand Up @@ -65,9 +65,9 @@ impl Service {
///
/// * The user does not have the required permissions.
/// * There is a database error.
pub async fn delete_category(&self, category_name: &str, user_id: &UserId) -> Result<(), ServiceError> {
pub async fn delete_category(&self, category_name: &str, maybe_user_id: Option<UserId>) -> Result<(), ServiceError> {
self.authorization_service
.authorize(ACTION::DeleteCategory, Some(*user_id))
.authorize(ACTION::DeleteCategory, maybe_user_id)
.await?;

match self.category_repository.delete(category_name).await {
Expand Down
8 changes: 4 additions & 4 deletions src/services/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ impl Service {
/// # Errors
///
/// It returns an error if the user does not have the required permissions.
pub async fn get_all(&self, user_id: &UserId) -> Result<Settings, ServiceError> {
pub async fn get_all(&self, maybe_user_id: Option<UserId>) -> Result<Settings, ServiceError> {
self.authorization_service
.authorize(ACTION::GetSettings, Some(*user_id))
.authorize(ACTION::GetSettings, maybe_user_id)
.await?;

let torrust_index_configuration = self.configuration.get_all().await;
Expand All @@ -45,9 +45,9 @@ impl Service {
/// # Errors
///
/// It returns an error if the user does not have the required permissions.
pub async fn get_all_masking_secrets(&self, user_id: &UserId) -> Result<Settings, ServiceError> {
pub async fn get_all_masking_secrets(&self, maybe_user_id: Option<UserId>) -> Result<Settings, ServiceError> {
self.authorization_service
.authorize(ACTION::GetSettingsSecret, Some(*user_id))
.authorize(ACTION::GetSettingsSecret, maybe_user_id)
.await?;

let mut torrust_index_configuration = self.configuration.get_all().await;
Expand Down
10 changes: 4 additions & 6 deletions src/services/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ impl Service {
///
/// * The user does not have the required permissions.
/// * There is a database error.
pub async fn add_tag(&self, tag_name: &str, user_id: &UserId) -> Result<TagId, ServiceError> {
self.authorization_service.authorize(ACTION::AddTag, Some(*user_id)).await?;
pub async fn add_tag(&self, tag_name: &str, maybe_user_id: Option<UserId>) -> Result<TagId, ServiceError> {
self.authorization_service.authorize(ACTION::AddTag, maybe_user_id).await?;

let trimmed_name = tag_name.trim();

Expand All @@ -55,10 +55,8 @@ impl Service {
///
/// * The user does not have the required permissions.
/// * There is a database error.
pub async fn delete_tag(&self, tag_id: &TagId, user_id: &UserId) -> Result<(), ServiceError> {
self.authorization_service
.authorize(ACTION::DeleteTag, Some(*user_id))
.await?;
pub async fn delete_tag(&self, tag_id: &TagId, maybe_user_id: Option<UserId>) -> Result<(), ServiceError> {
self.authorization_service.authorize(ACTION::DeleteTag, maybe_user_id).await?;

match self.tag_repository.delete(tag_id).await {
Ok(()) => Ok(()),
Expand Down
8 changes: 6 additions & 2 deletions src/services/torrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,13 @@ impl Index {
/// * The user does not have permission to delete the torrent.
/// * Unable to get the torrent listing from it's ID.
/// * Unable to delete the torrent from the database.
pub async fn delete_torrent(&self, info_hash: &InfoHash, user_id: &UserId) -> Result<DeletedTorrentResponse, ServiceError> {
pub async fn delete_torrent(
&self,
info_hash: &InfoHash,
maybe_user_id: Option<UserId>,
) -> Result<DeletedTorrentResponse, ServiceError> {
self.authorization_service
.authorize(ACTION::DeleteTorrent, Some(*user_id))
.authorize(ACTION::DeleteTorrent, maybe_user_id)
.await?;

let torrent_listing = self.torrent_listing_generator.one_torrent_by_info_hash(info_hash).await?;
Expand Down
10 changes: 7 additions & 3 deletions src/services/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,14 @@ impl BanService {
/// * `ServiceError::InternalServerError` if unable get user from the request.
/// * An error if unable to get user profile from supplied username.
/// * An error if unable to set the ban of the user in the database.
pub async fn ban_user(&self, username_to_be_banned: &str, user_id: &UserId) -> Result<(), ServiceError> {
debug!("user with ID {user_id} banning username: {username_to_be_banned}");
#[allow(clippy::missing_panics_doc)]
pub async fn ban_user(&self, username_to_be_banned: &str, maybe_user_id: Option<UserId>) -> Result<(), ServiceError> {
debug!(
"user with ID {} banning username: {username_to_be_banned}",
maybe_user_id.unwrap()
);

self.authorization_service.authorize(ACTION::BanUser, Some(*user_id)).await?;
self.authorization_service.authorize(ACTION::BanUser, maybe_user_id).await?;

let user_profile = self
.user_profile_repository
Expand Down
17 changes: 12 additions & 5 deletions src/web/api/server/v1/contexts/category/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use super::forms::{AddCategoryForm, DeleteCategoryForm};
use super::responses::{added_category, deleted_category, Category};
use crate::common::AppData;
use crate::web::api::server::v1::extractors::optional_user_id::ExtractOptionalLoggedInUser;
use crate::web::api::server::v1::extractors::user_id::ExtractLoggedInUser;
use crate::web::api::server::v1::responses::{self};

/// It handles the request to get all the categories.
Expand Down Expand Up @@ -50,10 +49,14 @@ pub async fn get_all_handler(
#[allow(clippy::unused_async)]
pub async fn add_handler(
State(app_data): State<Arc<AppData>>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
ExtractOptionalLoggedInUser(maybe_user_id): ExtractOptionalLoggedInUser,
extract::Json(category_form): extract::Json<AddCategoryForm>,
) -> Response {
match app_data.category_service.add_category(&category_form.name, &user_id).await {
match app_data
.category_service
.add_category(&category_form.name, maybe_user_id)
.await
{
Ok(_) => added_category(&category_form.name).into_response(),
Err(error) => error.into_response(),
}
Expand All @@ -70,14 +73,18 @@ pub async fn add_handler(
#[allow(clippy::unused_async)]
pub async fn delete_handler(
State(app_data): State<Arc<AppData>>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
ExtractOptionalLoggedInUser(maybe_user_id): ExtractOptionalLoggedInUser,
extract::Json(category_form): extract::Json<DeleteCategoryForm>,
) -> Response {
// code-review: why do we need to send the whole category object to delete it?
// And we should use the ID instead of the name, because the name could change
// or we could add support for multiple languages.

match app_data.category_service.delete_category(&category_form.name, &user_id).await {
match app_data
.category_service
.delete_category(&category_form.name, maybe_user_id)
.await
{
Ok(()) => deleted_category(&category_form.name).into_response(),
Err(error) => error.into_response(),
}
Expand Down
5 changes: 2 additions & 3 deletions src/web/api/server/v1/contexts/settings/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use axum::response::{IntoResponse, Json, Response};

use crate::common::AppData;
use crate::web::api::server::v1::extractors::optional_user_id::ExtractOptionalLoggedInUser;
use crate::web::api::server::v1::extractors::user_id::ExtractLoggedInUser;
use crate::web::api::server::v1::responses;

/// Get all settings.
Expand All @@ -19,9 +18,9 @@ use crate::web::api::server::v1::responses;
#[allow(clippy::unused_async)]
pub async fn get_all_handler(
State(app_data): State<Arc<AppData>>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
ExtractOptionalLoggedInUser(maybe_user_id): ExtractOptionalLoggedInUser,
) -> Response {
let all_settings = match app_data.settings_service.get_all_masking_secrets(&user_id).await {
let all_settings = match app_data.settings_service.get_all_masking_secrets(maybe_user_id).await {
Ok(all_settings) => all_settings,
Err(error) => return error.into_response(),
};
Expand Down
9 changes: 4 additions & 5 deletions src/web/api/server/v1/contexts/tag/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use super::forms::{AddTagForm, DeleteTagForm};
use super::responses::{added_tag, deleted_tag};
use crate::common::AppData;
use crate::web::api::server::v1::extractors::optional_user_id::ExtractOptionalLoggedInUser;
use crate::web::api::server::v1::extractors::user_id::ExtractLoggedInUser;
use crate::web::api::server::v1::responses::{self};

/// It handles the request to get all the tags.
Expand Down Expand Up @@ -50,10 +49,10 @@ pub async fn get_all_handler(
#[allow(clippy::unused_async)]
pub async fn add_handler(
State(app_data): State<Arc<AppData>>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
ExtractOptionalLoggedInUser(maybe_user_id): ExtractOptionalLoggedInUser,
extract::Json(add_tag_form): extract::Json<AddTagForm>,
) -> Response {
match app_data.tag_service.add_tag(&add_tag_form.name, &user_id).await {
match app_data.tag_service.add_tag(&add_tag_form.name, maybe_user_id).await {
Ok(_) => added_tag(&add_tag_form.name).into_response(),
Err(error) => error.into_response(),
}
Expand All @@ -70,10 +69,10 @@ pub async fn add_handler(
#[allow(clippy::unused_async)]
pub async fn delete_handler(
State(app_data): State<Arc<AppData>>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
ExtractOptionalLoggedInUser(maybe_user_id): ExtractOptionalLoggedInUser,
extract::Json(delete_tag_form): extract::Json<DeleteTagForm>,
) -> Response {
match app_data.tag_service.delete_tag(&delete_tag_form.tag_id, &user_id).await {
match app_data.tag_service.delete_tag(&delete_tag_form.tag_id, maybe_user_id).await {
Ok(()) => deleted_tag(delete_tag_form.tag_id).into_response(),
Err(error) => error.into_response(),
}
Expand Down
4 changes: 2 additions & 2 deletions src/web/api/server/v1/contexts/torrent/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,14 @@ pub async fn update_torrent_info_handler(
#[allow(clippy::unused_async)]
pub async fn delete_torrent_handler(
State(app_data): State<Arc<AppData>>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
ExtractOptionalLoggedInUser(maybe_user_id): ExtractOptionalLoggedInUser,
Path(info_hash): Path<InfoHashParam>,
) -> Response {
let Ok(info_hash) = InfoHash::from_str(&info_hash.lowercase()) else {
return errors::Request::InvalidInfoHashParam.into_response();
};

match app_data.torrent_service.delete_torrent(&info_hash, &user_id).await {
match app_data.torrent_service.delete_torrent(&info_hash, maybe_user_id).await {
Ok(deleted_torrent_response) => Json(OkResponseData {
data: deleted_torrent_response,
})
Expand Down
5 changes: 2 additions & 3 deletions src/web/api/server/v1/contexts/user/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use super::forms::{ChangePasswordForm, JsonWebToken, LoginForm, RegistrationForm
use super::responses::{self};
use crate::common::AppData;
use crate::web::api::server::v1::extractors::optional_user_id::ExtractOptionalLoggedInUser;
use crate::web::api::server::v1::extractors::user_id::ExtractLoggedInUser;
use crate::web::api::server::v1::responses::OkResponseData;

// Registration
Expand Down Expand Up @@ -163,11 +162,11 @@ pub async fn change_password_handler(
pub async fn ban_handler(
State(app_data): State<Arc<AppData>>,
Path(to_be_banned_username): Path<UsernameParam>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
ExtractOptionalLoggedInUser(maybe_user_id): ExtractOptionalLoggedInUser,
) -> Response {
// todo: add reason and `date_expiry` parameters to request

match app_data.ban_service.ban_user(&to_be_banned_username.0, &user_id).await {
match app_data.ban_service.ban_user(&to_be_banned_username.0, maybe_user_id).await {
Ok(()) => Json(OkResponseData {
data: format!("Banned user: {}", to_be_banned_username.0),
})
Expand Down

0 comments on commit 6010155

Please sign in to comment.