refactor: [#615] added optional logged in user to public handlers and…

… methods
torrust · Aug 3, 2024 · 1c607c2 · 1c607c2
1 parent b483c8e
commit 1c607c2
Show file tree

Hide file tree

Showing 9 changed files with 50 additions and 31 deletions.
diff --git a/src/services/about.rs b/src/services/about.rs
@@ -4,6 +4,7 @@ use std::sync::Arc;
 
 use super::authorization::{self, ACTION};
 use crate::errors::ServiceError;
+use crate::models::user::UserId;
 
 pub struct Service {
     authorization_service: Arc<authorization::Service>,
@@ -23,8 +24,10 @@ impl Service {
     ///
     /// * The user does not have the required permissions.
     /// * There is an error authorizing the action.
-    pub async fn get_about_page(&self) -> Result<String, ServiceError> {
-        self.authorization_service.authorize(ACTION::GetAboutPage, None).await?;
+    pub async fn get_about_page(&self, opt_user_id: Option<UserId>) -> Result<String, ServiceError> {
+        self.authorization_service
+            .authorize(ACTION::GetAboutPage, opt_user_id)
+            .await?;
 
         let html = r#"
     <html>
@@ -55,8 +58,10 @@ impl Service {
     ///
     /// * The user does not have the required permissions.
     /// * There is an error authorizing the action.
-    pub async fn get_license_page(&self) -> Result<String, ServiceError> {
-        self.authorization_service.authorize(ACTION::GetLicensePage, None).await?;
+    pub async fn get_license_page(&self, opt_user_id: Option<UserId>) -> Result<String, ServiceError> {
+        self.authorization_service
+            .authorize(ACTION::GetLicensePage, opt_user_id)
+            .await?;
 
         let html = r#"
         <html>

diff --git a/src/services/authorization.rs b/src/services/authorization.rs
@@ -173,17 +173,21 @@ impl CasbinConfiguration {
             ),
             policy: String::from(
                 "
+                admin, GetCategories
                 admin, AddCategory
                 admin, DeleteCategory
                 admin, GetPublicSettings
                 admin, GetSettingsSecret
+                admin, GetTags
                 admin, AddTag
                 admin, DeleteTag
                 admin, DeleteTorrent
                 admin, BanUser
                 admin, GetImageByUrl
+                registered, GetCategories
                 registered, GetImageByUrl
                 registered, GetPublicSettings
+                registered, GetTags
                 guest, GetCategories
                 guest, GetTags
                 guest, GetAboutPage

diff --git a/src/services/category.rs b/src/services/category.rs
@@ -87,8 +87,10 @@ impl Service {
     ///
     /// * The user does not have the required permissions.
     /// * There is a database error retrieving the categories.
-    pub async fn get_categories(&self) -> Result<Vec<Category>, ServiceError> {
-        self.authorization_service.authorize(ACTION::GetCategories, None).await?;
+    pub async fn get_categories(&self, opt_user_id: Option<UserId>) -> Result<Vec<Category>, ServiceError> {
+        self.authorization_service
+            .authorize(ACTION::GetCategories, opt_user_id)
+            .await?;
 
         self.category_repository
             .get_all()

diff --git a/src/services/proxy.rs b/src/services/proxy.rs
@@ -38,12 +38,14 @@ impl Service {
     /// * The image URL is not an image.
     /// * The image is too big.
     /// * The user quota is met.
-    pub async fn get_image_by_url(&self, url: &str, user_id: &UserId) -> Result<Bytes, Error> {
+    #[allow(clippy::missing_panics_doc)]
+    pub async fn get_image_by_url(&self, url: &str, opt_user_id: Option<UserId>) -> Result<Bytes, Error> {
         self.authorization_service
-            .authorize(ACTION::GetImageByUrl, Some(*user_id))
+            .authorize(ACTION::GetImageByUrl, opt_user_id)
             .await
             .map_err(|_| Error::Unauthenticated)?;
 
-        self.image_cache_service.get_image_by_url(url, *user_id).await
+        // The unwrap should never panic as if the opt_user_id is none, an authorization error will be returned and handled at the method above
+        self.image_cache_service.get_image_by_url(url, opt_user_id.unwrap()).await
     }
 }
diff --git a/src/services/tag.rs b/src/services/tag.rs
@@ -77,8 +77,8 @@ impl Service {
     ///
     /// * The user does not have the required permissions.
     /// * There is a database error retrieving the tags.
-    pub async fn get_tags(&self) -> Result<Vec<TorrentTag>, ServiceError> {
-        self.authorization_service.authorize(ACTION::GetTags, None).await?;
+    pub async fn get_tags(&self, opt_user_id: Option<UserId>) -> Result<Vec<TorrentTag>, ServiceError> {
+        self.authorization_service.authorize(ACTION::GetTags, opt_user_id).await?;
 
         self.tag_repository.get_all().await.map_err(|_| ServiceError::DatabaseError)
     }

diff --git a/src/web/api/server/v1/contexts/about/handlers.rs b/src/web/api/server/v1/contexts/about/handlers.rs
@@ -7,18 +7,25 @@ use axum::http::{header, StatusCode};
 use axum::response::{IntoResponse, Response};
 
 use crate::common::AppData;
+use crate::web::api::server::v1::extractors::optional_user_id::ExtractOptionalLoggedInUser;
 
 #[allow(clippy::unused_async)]
-pub async fn about_page_handler(State(app_data): State<Arc<AppData>>) -> Response {
-    match app_data.about_service.get_about_page().await {
+pub async fn about_page_handler(
+    State(app_data): State<Arc<AppData>>,
+    ExtractOptionalLoggedInUser(opt_user_id): ExtractOptionalLoggedInUser,
+) -> Response {
+    match app_data.about_service.get_about_page(opt_user_id).await {
         Ok(html) => (StatusCode::OK, [(header::CONTENT_TYPE, "text/html; charset=utf-8")], html).into_response(),
         Err(error) => error.into_response(),
     }
 }
 
 #[allow(clippy::unused_async)]
-pub async fn license_page_handler(State(app_data): State<Arc<AppData>>) -> Response {
-    match app_data.about_service.get_license_page().await {
+pub async fn license_page_handler(
+    State(app_data): State<Arc<AppData>>,
+    ExtractOptionalLoggedInUser(opt_user_id): ExtractOptionalLoggedInUser,
+) -> Response {
+    match app_data.about_service.get_license_page(opt_user_id).await {
         Ok(html) => (StatusCode::OK, [(header::CONTENT_TYPE, "text/html; charset=utf-8")], html)
             .into_response()
             .into_response(),

diff --git a/src/web/api/server/v1/contexts/category/handlers.rs b/src/web/api/server/v1/contexts/category/handlers.rs
@@ -8,6 +8,7 @@ use axum::response::{IntoResponse, Json, Response};
 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};
 
@@ -25,8 +26,11 @@ use crate::web::api::server::v1::responses::{self};
 ///
 /// It returns an error if there is a database error.
 #[allow(clippy::unused_async)]
-pub async fn get_all_handler(State(app_data): State<Arc<AppData>>) -> Response {
-    match app_data.category_service.get_categories().await {
+pub async fn get_all_handler(
+    State(app_data): State<Arc<AppData>>,
+    ExtractOptionalLoggedInUser(opt_user_id): ExtractOptionalLoggedInUser,
+) -> Response {
+    match app_data.category_service.get_categories(opt_user_id).await {
         Ok(categories) => {
             let categories: Vec<Category> = categories.into_iter().map(Category::from).collect();
             Json(responses::OkResponseData { data: categories }).into_response()

diff --git a/src/web/api/server/v1/contexts/proxy/handlers.rs b/src/web/api/server/v1/contexts/proxy/handlers.rs
@@ -6,26 +6,17 @@ use axum::extract::{Path, State};
 use axum::response::Response;
 
 use super::responses::png_image;
-use crate::cache::image::manager::Error;
 use crate::common::AppData;
 use crate::ui::proxy::map_error_to_image;
-use crate::web::api::server::v1::extractors::bearer_token::Extract;
+use crate::web::api::server::v1::extractors::optional_user_id::ExtractOptionalLoggedInUser;
 
 /// Get the remote image. It uses the cached image if available.
 #[allow(clippy::unused_async)]
 pub async fn get_proxy_image_handler(
     State(app_data): State<Arc<AppData>>,
-    Extract(maybe_bearer_token): Extract,
+    ExtractOptionalLoggedInUser(opt_user_id): ExtractOptionalLoggedInUser,
     Path(url): Path<String>,
 ) -> Response {
-    if maybe_bearer_token.is_none() {
-        return png_image(map_error_to_image(&Error::Unauthenticated));
-    }
-
-    let Ok(user_id) = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await else {
-        return png_image(map_error_to_image(&Error::Unauthenticated));
-    };
-
     // code-review: Handling status codes in the index-gui other tan OK is quite a pain.
     // Return OK for now.
 
@@ -36,7 +27,7 @@ pub async fn get_proxy_image_handler(
     // Get image URL from URL path parameter.
     let image_url = urlencoding::decode(&url).unwrap_or_default().into_owned();
 
-    match app_data.proxy_service.get_image_by_url(&image_url, &user_id).await {
+    match app_data.proxy_service.get_image_by_url(&image_url, opt_user_id).await {
         Ok(image_bytes) => {
             // Returns the cached image.
             png_image(image_bytes)

diff --git a/src/web/api/server/v1/contexts/tag/handlers.rs b/src/web/api/server/v1/contexts/tag/handlers.rs
@@ -8,6 +8,7 @@ use axum::response::{IntoResponse, Json, Response};
 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};
 
@@ -25,8 +26,11 @@ use crate::web::api::server::v1::responses::{self};
 ///
 /// It returns an error if there is a database error.
 #[allow(clippy::unused_async)]
-pub async fn get_all_handler(State(app_data): State<Arc<AppData>>) -> Response {
-    match app_data.tag_service.get_tags().await {
+pub async fn get_all_handler(
+    State(app_data): State<Arc<AppData>>,
+    ExtractOptionalLoggedInUser(opt_user_id): ExtractOptionalLoggedInUser,
+) -> Response {
+    match app_data.tag_service.get_tags(opt_user_id).await {
         Ok(tags) => Json(responses::OkResponseData { data: tags }).into_response(),
         Err(error) => error.into_response(),
     }