Merge #662: #615: Authorization for public handlers

f38b628 refactor: [#615] new authorization error for guest users (Mario)
d8b3ee2 refactor: [#615] change test http return code (Mario)
0360517 refactor: [#615] changed status code so tests pass (Mario)
6010155 refactor: [#615] handlers and functions now use optional user id (Mario)
c22c919 refactor: [#615] change password handler and function now use optional user id (Mario)
dbcd0e1  refactor: [#615] added missing error to handler comments (Mario)
00c293c  refactor: [#615] added missing error to function comments (Mario)
e527867 fix: [#615] adjusted http error code for test so it pass (Mario)
42fb031 refactor: [#615] renamed error to be more human friendly (Mario)
836c94d refactor: [#615] upload torrent handler now uses an optional user id (Mario)
935eb53 refactor: [#615] rearrange actions (Mario)
93b15ac refactor: [#615] renamed variables to maybe_user_id (Mario)
e614e2f feat: [#615] authorization layer added to the change password method of the user service (Mario)
543d804 refactor: [#615] redirect to details url with infohash method now calls the torrent service that implements the authorization layer (Mario)
f8e7570 feat: [#615] authorization layer added to the get torrent info method of the torrent service (Mario)
1342dce feat: [#615] authorization layer implemented for the get torrents handler (Mario)
f264e75 fix: [#615] fix unfinished comment (Mario)
d56d66c refactor: [#615] download torrent handler now calls the torrent service to get the canonical infohash (Mario)
b3ffc9f feat: [#615] new get canonical info hash method (Mario)
7bb6ff7 feat: [#615] authorization layer implemented for the get torrent method of the torrent service (Mario)
b5171bf feat: [#615] authorization layer added for add torrent method of the torrent service (Mario)
1c607c2 refactor: [#615] added optional logged in user to public handlers and methods (Mario)
b483c8e refactor: [#615] renamed variable (Mario)
2880f7c feat: [#615] added authorization layer for get public settings method of the settings service (Mario)
530f37a feat: [#615] admin user is now authorize to see proxied images (Mario)
d1d2941 refactor: [#615] fix linting errors (Mario)
056cb83 refactor: [#615] methods used by the proxy service now use the user id instead of the user compact type (Mario)
173f43d feat: [#615] authorization layer implemented for the proxy service (Mario)
4f6ea18 refactor: [#615] new action and reordered old ones for cohesion (Mario)
0ac5f9b refactor: [#615] get all categories handler now calls the category service which uses the authorization layer (Mario)
839a00d feat: [#615] New get categories service method (Mario)
fe11c2f refactor: [#615] added comments for new functions (Mario)
cd8f609 feat: [#615] authorization implemented for get tags method (Mario)
eee3349 refactor: [#615] new get tags service method (Mario)
6f8de80 feat: [#615] authorization service implemented for the about service (Mario)
a757c5c refactor: [#615] new about service and minor refactor to existing code (Mario)

Pull request description:

  Parent issue: #615

ACKs for top commit:
  josecelano:
    ACK f38b628

Tree-SHA512: d8d710af481b2480b0035ad13feaab15e6299f5767f4d68837a4a05c237fd3e0bdbccdfa46c0c625b11e7109dcb73879036debf7a51b7d83c856403878ce70d2

Author: josecelano

src/app.rs

@@ -18,7 +18,7 @@ use crate::services::torrent::{
     DbTorrentListingGenerator, DbTorrentRepository, DbTorrentTagRepository,
 };
 use crate::services::user::{self, DbBannedUserList, DbUserProfileRepository, DbUserRepository, Repository};
-use crate::services::{authorization, proxy, settings, torrent};
+use crate::services::{about, authorization, proxy, settings, torrent};
 use crate::tracker::statistics_importer::StatisticsImporter;
 use crate::web::api::server::signals::Halted;
 use crate::web::api::server::v1::auth::Authentication;
@@ -101,7 +101,10 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
         authorization_service.clone(),
     ));
     let tag_service = Arc::new(tag::Service::new(tag_repository.clone(), authorization_service.clone()));
-    let proxy_service = Arc::new(proxy::Service::new(image_cache_service.clone(), user_repository.clone()));
+    let proxy_service = Arc::new(proxy::Service::new(
+        image_cache_service.clone(),
+        authorization_service.clone(),
+    ));
     let settings_service = Arc::new(settings::Service::new(configuration.clone(), authorization_service.clone()));
     let torrent_index = Arc::new(torrent::Index::new(
         configuration.clone(),
@@ -127,6 +130,7 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
     let profile_service = Arc::new(user::ProfileService::new(
         configuration.clone(),
         user_authentication_repository.clone(),
+        authorization_service.clone(),
     ));
     let ban_service = Arc::new(user::BanService::new(
         user_profile_repository.clone(),
@@ -141,6 +145,8 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
         user_authentication_repository.clone(),
     ));
 
+    let about_service = Arc::new(about::Service::new(authorization_service.clone()));
+
     // Build app container
 
     let app_data = Arc::new(AppData::new(
@@ -174,6 +180,7 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
         registration_service,
         profile_service,
         ban_service,
+        about_service,
     ));
 
     // Start cronjob to import tracker torrent data and updating

src/cache/image/manager.rs

@@ -7,7 +7,7 @@ use tokio::sync::RwLock;
 
 use crate::cache::BytesCache;
 use crate::config::Configuration;
-use crate::models::user::UserCompact;
+use crate::models::user::UserId;
 
 pub enum Error {
     UrlIsUnreachable,
@@ -125,33 +125,26 @@ impl ImageCacheService {
     /// # Errors
     ///
     /// Return a `Error::Unauthenticated` if the user has not been authenticated.
-    pub async fn get_image_by_url(&self, url: &str, opt_user: Option<UserCompact>) -> Result<Bytes, Error> {
+    pub async fn get_image_by_url(&self, url: &str, user_id: UserId) -> Result<Bytes, Error> {
         if let Some(entry) = self.image_cache.read().await.get(url).await {
             return Ok(entry.bytes);
         }
+        self.check_user_quota(&user_id).await?;
 
-        match opt_user {
-            None => Err(Error::Unauthenticated),
+        let image_bytes = self.get_image_from_url_as_bytes(url).await?;
 
-            Some(user) => {
-                self.check_user_quota(&user).await?;
+        self.check_image_size(&image_bytes).await?;
 
-                let image_bytes = self.get_image_from_url_as_bytes(url).await?;
+        // These two functions could be executed after returning the image to the client,
+        // but than we would need a dedicated task or thread that executes these functions.
+        // This can be problematic if a task is spawned after every user request.
+        // Since these functions execute very fast, I don't see a reason to further optimize this.
+        // For now.
+        self.update_image_cache(url, &image_bytes).await?;
 
-                self.check_image_size(&image_bytes).await?;
+        self.update_user_quota(&user_id, image_bytes.len()).await?;
 
-                // These two functions could be executed after returning the image to the client,
-                // but than we would need a dedicated task or thread that executes these functions.
-                // This can be problematic if a task is spawned after every user request.
-                // Since these functions execute very fast, I don't see a reason to further optimize this.
-                // For now.
-                self.update_image_cache(url, &image_bytes).await?;
-
-                self.update_user_quota(&user, image_bytes.len()).await?;
-
-                Ok(image_bytes)
-            }
-        }
+        Ok(image_bytes)
     }
 
     async fn get_image_from_url_as_bytes(&self, url: &str) -> Result<Bytes, Error> {
@@ -176,8 +169,8 @@ impl ImageCacheService {
         res.bytes().await.map_err(|_| Error::UrlIsNotAnImage)
     }
 
-    async fn check_user_quota(&self, user: &UserCompact) -> Result<(), Error> {
-        if let Some(quota) = self.user_quotas.read().await.get(&user.user_id) {
+    async fn check_user_quota(&self, user_id: &UserId) -> Result<(), Error> {
+        if let Some(quota) = self.user_quotas.read().await.get(user_id) {
             if quota.is_reached() {
                 return Err(Error::UserQuotaMet);
             }
@@ -211,24 +204,24 @@ impl ImageCacheService {
         Ok(())
     }
 
-    async fn update_user_quota(&self, user: &UserCompact, amount: usize) -> Result<(), Error> {
+    async fn update_user_quota(&self, user_id: &UserId, amount: usize) -> Result<(), Error> {
         let settings = self.cfg.settings.read().await;
 
         let mut quota = self
             .user_quotas
             .read()
             .await
-            .get(&user.user_id)
+            .get(user_id)
             .cloned()
             .unwrap_or(ImageCacheQuota::new(
-                user.user_id,
+                *user_id,
                 settings.image_cache.user_quota_bytes,
                 settings.image_cache.user_quota_period_seconds,
             ));
 
         let _ = quota.add_usage(amount);
 
-        let _ = self.user_quotas.write().await.insert(user.user_id, quota);
+        let _ = self.user_quotas.write().await.insert(*user_id, quota);
 
         Ok(())
     }

src/common.rs

@@ -11,7 +11,7 @@ use crate::services::torrent::{
     DbTorrentListingGenerator, DbTorrentRepository, DbTorrentTagRepository,
 };
 use crate::services::user::{self, DbBannedUserList, DbUserProfileRepository, Repository};
-use crate::services::{proxy, settings, torrent};
+use crate::services::{about, proxy, settings, torrent};
 use crate::tracker::statistics_importer::StatisticsImporter;
 use crate::web::api::server::v1::auth::Authentication;
 use crate::{mailer, tracker};
@@ -51,6 +51,7 @@ pub struct AppData {
     pub registration_service: Arc<user::RegistrationService>,
     pub profile_service: Arc<user::ProfileService>,
     pub ban_service: Arc<user::BanService>,
+    pub about_service: Arc<about::Service>,
 }
 
 impl AppData {
@@ -88,6 +89,7 @@ impl AppData {
         registration_service: Arc<user::RegistrationService>,
         profile_service: Arc<user::ProfileService>,
         ban_service: Arc<user::BanService>,
+        about_service: Arc<about::Service>,
     ) -> AppData {
         AppData {
             cfg,
@@ -122,6 +124,7 @@ impl AppData {
             registration_service,
             profile_service,
             ban_service,
+            about_service,
         }
     }
 }

src/errors.rs

@@ -109,7 +109,12 @@ pub enum ServiceError {
     InvalidTag,
 
     #[display(fmt = "Unauthorized action.")]
-    Unauthorized,
+    UnauthorizedAction,
+
+    #[display(
+        fmt = "Unauthorized actions for guest users. Try logging in to check if you have permission to perform the action"
+    )]
+    UnauthorizedActionForGuests,
 
     #[display(fmt = "This torrent already exists in our database.")]
     InfoHashAlreadyExists,
@@ -300,7 +305,8 @@ pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode {
         ServiceError::MissingMandatoryMetadataFields => StatusCode::BAD_REQUEST,
         ServiceError::InvalidCategory => StatusCode::BAD_REQUEST,
         ServiceError::InvalidTag => StatusCode::BAD_REQUEST,
-        ServiceError::Unauthorized => StatusCode::FORBIDDEN,
+        ServiceError::UnauthorizedAction => StatusCode::FORBIDDEN,
+        ServiceError::UnauthorizedActionForGuests => StatusCode::UNAUTHORIZED,
         ServiceError::InfoHashAlreadyExists => StatusCode::BAD_REQUEST,
         ServiceError::CanonicalInfoHashAlreadyExists => StatusCode::CONFLICT,
         ServiceError::OriginalInfoHashAlreadyExists => StatusCode::CONFLICT,

src/services/about.rs

@@ -1,13 +1,35 @@
 //! Templates for "about" static pages.
 
-#[must_use]
-pub fn index_page() -> String {
-    page()
+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>,
 }
 
-#[must_use]
-pub fn page() -> String {
-    r#"
+impl Service {
+    #[must_use]
+    pub fn new(authorization_service: Arc<authorization::Service>) -> Service {
+        Service { authorization_service }
+    }
+
+    /// Returns the html with the about page
+    ///
+    /// # Errors
+    ///
+    /// It returns an error if:
+    ///
+    /// * The user does not have the required permissions.
+    /// * There is an error authorizing the action.
+    pub async fn get_about_page(&self, maybe_user_id: Option<UserId>) -> Result<String, ServiceError> {
+        self.authorization_service
+            .authorize(ACTION::GetAboutPage, maybe_user_id)
+            .await?;
+
+        let html = r#"
     <html>
         <head>
             <title>About</title>
@@ -23,37 +45,52 @@ pub fn page() -> String {
             <a href="./about/license">license</a>
         </footer>
     </html>
-"#
-    .to_string()
-}
-
-#[must_use]
-pub fn license_page() -> String {
-    r#"
-    <html>
-        <head>
-            <title>Licensing</title>
-        </head>
-        <body style="margin-left: auto;margin-right: auto;max-width: 30em;">
-            <h1>Torrust Index</h1>
-
-            <h2>Licensing</h2>
-
-            <h3>Multiple Licenses</h3>
-
-            <p>This repository has multiple licenses depending on the content type, the date of contributions or stemming from external component licenses that were not developed by any of Torrust team members or Torrust repository contributors.</p>
+"#;
 
-            <p>The two main applicable license to most of its content are:</p>
+        Ok(html.to_string())
+    }
 
-            <p>- For Code -- <a href="https://github.com/torrust/torrust-index/blob/main/licensing/agpl-3.0.md">agpl-3.0</a></p>
+    /// Returns the html with the license page
+    ///
+    /// # Errors
+    ///
+    /// It returns an error if:
+    ///
+    /// * The user does not have the required permissions.
+    /// * There is an error authorizing the action.
+    pub async fn get_license_page(&self, maybe_user_id: Option<UserId>) -> Result<String, ServiceError> {
+        self.authorization_service
+            .authorize(ACTION::GetLicensePage, maybe_user_id)
+            .await?;
 
-            <p>- For Media (Images, etc.) -- <a href="https://github.com/torrust/torrust-index/blob/main/licensing/cc-by-sa.md">cc-by-sa</a></p>
+        let html = r#"
+        <html>
+            <head>
+                <title>Licensing</title>
+            </head>
+            <body style="margin-left: auto;margin-right: auto;max-width: 30em;">
+                <h1>Torrust Index</h1>
+    
+                <h2>Licensing</h2>
+    
+                <h3>Multiple Licenses</h3>
+    
+                <p>This repository has multiple licenses depending on the content type, the date of contributions or stemming from external component licenses that were not developed by any of Torrust team members or Torrust repository contributors.</p>
+    
+                <p>The two main applicable license to most of its content are:</p>
+    
+                <p>- For Code -- <a href="https://github.com/torrust/torrust-index/blob/main/licensing/agpl-3.0.md">agpl-3.0</a></p>
+    
+                <p>- For Media (Images, etc.) -- <a href="https://github.com/torrust/torrust-index/blob/main/licensing/cc-by-sa.md">cc-by-sa</a></p>
+    
+                <p>If you want to read more about all the licenses and how they apply please refer to the <a href="https://github.com/torrust/torrust-index/blob/develop/licensing/contributor_agreement_v01.md">contributor agreement</a>.</p>
+            </body>
+            <footer style="padding: 1.25em 0;border-top: dotted 1px;">
+                <a href="../about">about</a>
+            </footer>
+        </html>
+    "#;
 
-            <p>If you want to read more about all the licenses and how they apply please refer to the <a href="https://github.com/torrust/torrust-index/blob/develop/licensing/contributor_agreement_v01.md">contributor agreement</a>.</p>
-        </body>
-        <footer style="padding: 1.25em 0;border-top: dotted 1px;">
-            <a href="../about">about</a>
-        </footer>
-    </html>
-"#.to_string()
+        Ok(html.to_string())
+    }
 }