Implement authorization service and refactor services to use it

- Added `AuthorizationService` and its implementation `AuthorizationServiceImpl` to handle permission checks across various services.
- Refactored `AlbumServiceImpl`, `MediaServiceImpl`, `PersonServiceImpl`, and `TagServiceImpl` to utilize the new authorization service for permission checks.
- Removed direct permission checks from services and replaced them with calls to the `AuthorizationService`.
- Updated repository interfaces to include new methods for checking media permissions in shared albums.
- Enhanced the `authz` module with new permission types for better granularity in access control.
- Adjusted the `AppState` struct to include the new `authorization_service`.

libertas_api/src/factory.rs

@@ -1,17 +1,22 @@
 use std::sync::Arc;
 
 use libertas_core::{
-    config::{AppConfig},
+    config::AppConfig,
     error::{CoreError, CoreResult},
 };
 use libertas_infra::factory::{
-    build_album_repository, build_album_share_repository, build_database_pool, build_face_region_repository, build_media_metadata_repository, build_media_repository, build_person_repository, build_person_share_repository, build_tag_repository, build_user_repository
+    build_album_repository, build_album_share_repository, build_database_pool,
+    build_face_region_repository, build_media_metadata_repository, build_media_repository,
+    build_person_repository, build_person_share_repository, build_tag_repository,
+    build_user_repository,
 };
 
 use crate::{
     security::{Argon2Hasher, JwtGenerator},
     services::{
-        album_service::AlbumServiceImpl, media_service::MediaServiceImpl, person_service::PersonServiceImpl, tag_service::TagServiceImpl, user_service::UserServiceImpl
+        album_service::AlbumServiceImpl, authorization_service::AuthorizationServiceImpl,
+        media_service::MediaServiceImpl, person_service::PersonServiceImpl,
+        tag_service::TagServiceImpl, user_service::UserServiceImpl,
     },
     state::AppState,
 };
@@ -33,11 +38,22 @@ pub async fn build_app_state(config: AppConfig) -> CoreResult<AppState> {
     let tag_repo = build_tag_repository(&config.database, db_pool.clone()).await?;
     let person_repo = build_person_repository(&config.database, db_pool.clone()).await?;
     let face_region_repo = build_face_region_repository(&config.database, db_pool.clone()).await?;
-    let person_share_repo = build_person_share_repository(&config.database, db_pool.clone()).await?;
+    let person_share_repo =
+        build_person_share_repository(&config.database, db_pool.clone()).await?;
 
     let hasher = Arc::new(Argon2Hasher::default());
     let tokenizer = Arc::new(JwtGenerator::new(config.jwt_secret.clone()));
 
+    let authorization_service = Arc::new(AuthorizationServiceImpl::new(
+        media_repo.clone(),
+        album_repo.clone(),
+        album_share_repo.clone(),
+        person_repo.clone(),
+        person_share_repo.clone(),
+        face_region_repo.clone(),
+        user_repo.clone(),
+    ));
+
     let user_service = Arc::new(UserServiceImpl::new(
         user_repo.clone(),
         hasher,
@@ -47,25 +63,25 @@ pub async fn build_app_state(config: AppConfig) -> CoreResult<AppState> {
     let media_service = Arc::new(MediaServiceImpl::new(
         media_repo.clone(),
         user_repo.clone(),
-        album_share_repo.clone(),
         media_metadata_repo.clone(),
+        authorization_service.clone(),
         config.clone(),
         nats_client.clone(),
     ));
     let album_service = Arc::new(AlbumServiceImpl::new(
-        album_repo,
+        album_repo.clone(),
-        media_repo.clone(),
+        album_share_repo.clone(),
-        album_share_repo,
+        authorization_service.clone(),
     ));
     let tag_service = Arc::new(TagServiceImpl::new(
-        tag_repo,
+        tag_repo.clone(),
-        media_repo.clone(),
+        authorization_service.clone(),
     ));
     let person_service = Arc::new(PersonServiceImpl::new(
-        person_repo,
+        person_repo.clone(),
-        face_region_repo,
+        face_region_repo.clone(),
-        media_repo.clone(),
+        person_share_repo.clone(),
-        person_share_repo,
+        authorization_service.clone(),
     ));
 
     Ok(AppState {
@@ -74,6 +90,7 @@ pub async fn build_app_state(config: AppConfig) -> CoreResult<AppState> {
         album_service,
         tag_service,
         person_service,
+        authorization_service,
         token_generator: tokenizer,
         nats_client,
         config,

libertas_api/src/services/album_service.rs

@@ -3,31 +3,31 @@ use std::sync::Arc;
 use async_trait::async_trait;
 use chrono::Utc;
 use libertas_core::{
-    authz,
+    authz::{self, Permission},
     error::{CoreError, CoreResult},
     models::Album,
-    repositories::{AlbumRepository, AlbumShareRepository, MediaRepository},
+    repositories::{AlbumRepository, AlbumShareRepository},
     schema::{AddMediaToAlbumData, CreateAlbumData, ShareAlbumData, UpdateAlbumData},
-    services::AlbumService,
+    services::{AlbumService, AuthorizationService},
 };
 use uuid::Uuid;
 
 pub struct AlbumServiceImpl {
     album_repo: Arc<dyn AlbumRepository>,
-    media_repo: Arc<dyn MediaRepository>,
     album_share_repo: Arc<dyn AlbumShareRepository>,
+    auth_service: Arc<dyn AuthorizationService>,
 }
 
 impl AlbumServiceImpl {
     pub fn new(
         album_repo: Arc<dyn AlbumRepository>,
-        media_repo: Arc<dyn MediaRepository>,
         album_share_repo: Arc<dyn AlbumShareRepository>,
+        auth_service: Arc<dyn AuthorizationService>,
     ) -> Self {
         Self {
             album_repo,
-            media_repo,
             album_share_repo,
+            auth_service,
         }
     }
 }
@@ -56,55 +56,28 @@ impl AlbumService for AlbumServiceImpl {
     }
 
     async fn get_album_details(&self, album_id: Uuid, user_id: Uuid) -> CoreResult<Album> {
+        self.auth_service
+            .check_permission(user_id, Permission::ViewAlbum(album_id))
+            .await?;
+
         let album = self
             .album_repo
             .find_by_id(album_id)
             .await?
             .ok_or(CoreError::NotFound("Album".to_string(), album_id))?;
 
-        let share_permission = self
-            .album_share_repo
-            .get_user_permission(album_id, user_id)
-            .await?;
-
-        if !authz::can_view_album(user_id, &album, share_permission) {
-            return Err(CoreError::Auth("Access denied to album".to_string()));
-        }
-
         Ok(album)
     }
 
     async fn add_media_to_album(&self, data: AddMediaToAlbumData, user_id: Uuid) -> CoreResult<()> {
-        let album = self
+        self.auth_service
-            .album_repo
+            .check_permission(user_id, Permission::AddToAlbum(data.album_id))
-            .find_by_id(data.album_id)
-            .await?
-            .ok_or(CoreError::NotFound("Album".to_string(), data.album_id))?;
-
-        let share_permission = self
-            .album_share_repo
-            .get_user_permission(data.album_id, user_id)
             .await?;
 
-        if !authz::can_contribute_to_album(user_id, &album, share_permission) {
-            return Err(CoreError::Auth(
-                "User does not have permission to add media to this album".to_string(),
-            ));
-        }
-
         for media_id in &data.media_ids {
-            let media = self
+            self.auth_service
-                .media_repo
+                .check_permission(*media_id, Permission::ViewMedia(*media_id))
-                .find_by_id(*media_id)
+                .await?;
-                .await?
-                .ok_or(CoreError::NotFound("Media".to_string(), *media_id))?;
-
-            if !authz::is_owner(user_id, &media) {
-                return Err(CoreError::Auth(format!(
-                    "Access denied to media item {}",
-                    media_id
-                )));
-            }
         }
 
         self.album_repo
@@ -117,17 +90,9 @@ impl AlbumService for AlbumServiceImpl {
     }
 
     async fn share_album(&self, data: ShareAlbumData, owner_id: Uuid) -> CoreResult<()> {
-        let album = self
+        self.auth_service
-            .album_repo
+            .check_permission(owner_id, Permission::ShareAlbum(data.album_id))
-            .find_by_id(data.album_id)
+            .await?;
-            .await?
-            .ok_or(CoreError::NotFound("Album".to_string(), data.album_id))?;
-
-        if !authz::is_owner(owner_id, &album) {
-            return Err(CoreError::Auth(
-                "Only the album owner can share the album".to_string(),
-            ));
-        }
 
         if data.target_user_id == owner_id {
             return Err(CoreError::Validation(
@@ -146,23 +111,16 @@ impl AlbumService for AlbumServiceImpl {
         user_id: Uuid,
         data: UpdateAlbumData<'_>,
     ) -> CoreResult<Album> {
+        self.auth_service
+            .check_permission(user_id, Permission::EditAlbum(album_id))
+            .await?;
+
         let mut album = self
             .album_repo
             .find_by_id(album_id)
             .await?
             .ok_or(CoreError::NotFound("Album".to_string(), album_id))?;
 
-        let share_permission = self
-            .album_share_repo
-            .get_user_permission(album_id, user_id)
-            .await?;
-
-        if !authz::can_contribute_to_album(user_id, &album, share_permission) {
-            return Err(CoreError::Auth(
-                "User does not have permission to update this album".to_string(),
-            ));
-        }
-
         if let Some(name) = data.name {
             if name.is_empty() {
                 return Err(CoreError::Validation(
@@ -191,17 +149,9 @@ impl AlbumService for AlbumServiceImpl {
     }
 
     async fn delete_album(&self, album_id: Uuid, user_id: Uuid) -> CoreResult<()> {
-        let album = self
+        self.auth_service
-            .album_repo
+            .check_permission(user_id, Permission::DeleteAlbum(album_id))
-            .find_by_id(album_id)
+            .await?;
-            .await?
-            .ok_or(CoreError::NotFound("Album".to_string(), album_id))?;
-
-        if !authz::is_owner(user_id, &album) {
-            return Err(CoreError::Auth(
-                "Only the album owner can delete the album".to_string(),
-            ));
-        }
 
         self.album_repo.delete(album_id).await
     }

libertas_api/src/services/authorization_service.rs

@@ -0,0 +1,271 @@
+use std::sync::Arc;
+
+use async_trait::async_trait;
+use libertas_core::{
+    authz::{self, Permission},
+    error::{CoreError, CoreResult},
+    models::{Album, AlbumPermission, Media, Person, PersonPermission, User},
+    repositories::{
+        AlbumRepository, AlbumShareRepository, FaceRegionRepository, MediaRepository,
+        PersonRepository, PersonShareRepository, UserRepository,
+    },
+    services::AuthorizationService,
+};
+use uuid::Uuid;
+
+pub struct AuthorizationServiceImpl {
+    media_repo: Arc<dyn MediaRepository>,
+    album_repo: Arc<dyn AlbumRepository>,
+    album_share_repo: Arc<dyn AlbumShareRepository>,
+    person_repo: Arc<dyn PersonRepository>,
+    person_share_repo: Arc<dyn PersonShareRepository>,
+    face_repo: Arc<dyn FaceRegionRepository>,
+    user_repo: Arc<dyn UserRepository>,
+}
+
+impl AuthorizationServiceImpl {
+    pub fn new(
+        media_repo: Arc<dyn MediaRepository>,
+        album_repo: Arc<dyn AlbumRepository>,
+        album_share_repo: Arc<dyn AlbumShareRepository>,
+        person_repo: Arc<dyn PersonRepository>,
+        person_share_repo: Arc<dyn PersonShareRepository>,
+        face_repo: Arc<dyn FaceRegionRepository>,
+        user_repo: Arc<dyn UserRepository>,
+    ) -> Self {
+        Self {
+            media_repo,
+            album_repo,
+            album_share_repo,
+            person_repo,
+            person_share_repo,
+            face_repo,
+            user_repo,
+        }
+    }
+
+    async fn get_user(&self, user_id: Uuid) -> CoreResult<User> {
+        let user = self
+            .user_repo
+            .find_by_id(user_id)
+            .await?
+            .ok_or(CoreError::NotFound("User".to_string(), user_id))?;
+        Ok(user)
+    }
+
+    async fn get_media(&self, media_id: Uuid) -> CoreResult<Media> {
+        let media = self
+            .media_repo
+            .find_by_id(media_id)
+            .await?
+            .ok_or(CoreError::NotFound("Media".to_string(), media_id))?;
+        Ok(media)
+    }
+
+    async fn get_album(&self, album_id: Uuid) -> CoreResult<Album> {
+        let album = self
+            .album_repo
+            .find_by_id(album_id)
+            .await?
+            .ok_or(CoreError::NotFound("Album".to_string(), album_id))?;
+        Ok(album)
+    }
+
+    async fn get_album_share_permission(
+        &self,
+        album_id: Uuid,
+        user_id: Uuid,
+    ) -> CoreResult<Option<AlbumPermission>> {
+        let permission = self
+            .album_share_repo
+            .get_user_permission(album_id, user_id)
+            .await?;
+        Ok(permission)
+    }
+
+    async fn get_person_share_permission(
+        &self,
+        person_id: Uuid,
+        user_id: Uuid,
+    ) -> CoreResult<Option<PersonPermission>> {
+        let permission = self
+            .person_share_repo
+            .get_user_permission(person_id, user_id)
+            .await?;
+        Ok(permission)
+    }
+
+    async fn get_person(&self, person_id: Uuid) -> CoreResult<Person> {
+        let person = self
+            .person_repo
+            .find_by_id(person_id)
+            .await?
+            .ok_or(CoreError::NotFound("Person".to_string(), person_id))?;
+        Ok(person)
+    }
+}
+
+#[async_trait]
+impl AuthorizationService for AuthorizationServiceImpl {
+    async fn check_permission(&self, user_id: Uuid, permission: Permission) -> CoreResult<()> {
+        let user = self.get_user(user_id).await?;
+
+        if authz::is_admin(&user) {
+            return Ok(());
+        }
+
+        match permission {
+            Permission::ViewMedia(media_id) => {
+                let media = self.get_media(media_id).await?;
+                if authz::is_owner(user_id, &media) {
+                    return Ok(());
+                }
+
+                let is_shared = self
+                    .album_share_repo
+                    .is_media_in_shared_album(media_id, user_id)
+                    .await?;
+
+                if is_shared {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to view this media.".into(),
+                ))
+            }
+
+            Permission::DeleteMedia(media_id) | Permission::EditMedia(media_id) => {
+                let media = self.get_media(media_id).await?;
+                if authz::is_owner(user_id, &media) {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to modify this media.".into(),
+                ))
+            }
+
+            Permission::AddTags(media_id)
+            | Permission::RemoveTags(media_id)
+            | Permission::EditTags(media_id) => {
+                let media = self.get_media(media_id).await?;
+
+                if authz::is_owner(user_id, &media) {
+                    return Ok(());
+                }
+
+                let can_contribute = self
+                    .album_share_repo
+                    .is_media_in_contributable_album(media_id, user_id)
+                    .await?;
+
+                if can_contribute {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to modify tags for this media.".into(),
+                ))
+            }
+
+            Permission::ViewAlbum(album_id) => {
+                let album = self.get_album(album_id).await?;
+
+                let share_permission = self.get_album_share_permission(album_id, user_id).await?;
+
+                if authz::can_view_album(user_id, &album, share_permission) {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to view this album.".into(),
+                ))
+            }
+
+            Permission::AddToAlbum(album_id) | Permission::EditAlbum(album_id) => {
+                let album = self.get_album(album_id).await?;
+                let share_permission = self.get_album_share_permission(album_id, user_id).await?;
+
+                if authz::can_contribute_to_album(user_id, &album, share_permission) {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to modify this album.".into(),
+                ))
+            }
+
+            Permission::ShareAlbum(album_id) | Permission::DeleteAlbum(album_id) => {
+                let album = self.get_album(album_id).await?;
+
+                if authz::is_owner(user_id, &album) {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to share or delete this album.".into(),
+                ))
+            }
+
+            Permission::ViewPerson(person_id) => {
+                let person = self.get_person(person_id).await?;
+                let share_permission = self.get_person_share_permission(person_id, user_id).await?;
+
+                if authz::can_access_person(user_id, &person, share_permission) {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to view this person.".into(),
+                ))
+            }
+
+            Permission::EditPerson(person_id)
+            | Permission::SharePerson(person_id)
+            | Permission::DeletePerson(person_id) => {
+                let person = self.get_person(person_id).await?;
+
+                if authz::is_owner(user_id, &person) {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to modify this person.".into(),
+                ))
+            }
+
+            Permission::UsePerson(person_id) => {
+                let person = self.get_person(person_id).await?;
+                let share_permission = self.get_person_share_permission(person_id, user_id).await?;
+
+                if authz::can_use_person(user_id, &person, share_permission) {
+                    return Ok(());
+                }
+
+                Err(CoreError::Auth(
+                    "User does not have permission to use this person.".into(),
+                ))
+            }
+
+            Permission::ViewFaces(media_id) => {
+                self.check_permission(user_id, Permission::ViewMedia(media_id))
+                    .await
+            }
+
+            Permission::AssignFace(face_region_id) => {
+                let face =
+                    self.face_repo
+                        .find_by_id(face_region_id)
+                        .await?
+                        .ok_or(CoreError::NotFound(
+                            "FaceRegion".to_string(),
+                            face_region_id,
+                        ))?;
+
+                self.check_permission(user_id, Permission::AddTags(face.media_id))
+                    .await
+            }
+        }
+    }
+}

libertas_api/src/services/media_service.rs

@@ -1,9 +1,19 @@
-use std::{path::{Path, PathBuf}, sync::Arc};
+use std::{
+    path::{Path, PathBuf},
+    sync::Arc,
+};
 
 use async_trait::async_trait;
 use futures::stream::StreamExt;
 use libertas_core::{
-    authz, config::AppConfig, error::{CoreError, CoreResult}, media_utils::{ExtractedExif, extract_exif_data_from_bytes, get_storage_path_and_date}, models::{Media, MediaBundle, MediaMetadata}, repositories::{AlbumShareRepository, MediaMetadataRepository, MediaRepository, UserRepository}, schema::{ListMediaOptions, UploadMediaData}, services::MediaService
+    authz,
+    config::AppConfig,
+    error::{CoreError, CoreResult},
+    media_utils::{ExtractedExif, extract_exif_data_from_bytes, get_storage_path_and_date},
+    models::{Media, MediaBundle, MediaMetadata},
+    repositories::{MediaMetadataRepository, MediaRepository, UserRepository},
+    schema::{ListMediaOptions, UploadMediaData},
+    services::{AuthorizationService, MediaService},
 };
 use serde_json::json;
 use sha2::{Digest, Sha256};
@@ -13,8 +23,8 @@ use uuid::Uuid;
 pub struct MediaServiceImpl {
     repo: Arc<dyn MediaRepository>,
     user_repo: Arc<dyn UserRepository>,
-    album_share_repo: Arc<dyn AlbumShareRepository>,
     metadata_repo: Arc<dyn MediaMetadataRepository>,
+    auth_service: Arc<dyn AuthorizationService>,
     config: AppConfig,
     nats_client: async_nats::Client,
 }
@@ -23,16 +33,16 @@ impl MediaServiceImpl {
     pub fn new(
         repo: Arc<dyn MediaRepository>,
         user_repo: Arc<dyn UserRepository>,
-        album_share_repo: Arc<dyn AlbumShareRepository>,
         metadata_repo: Arc<dyn MediaMetadataRepository>,
+        auth_service: Arc<dyn AuthorizationService>,
         config: AppConfig,
         nats_client: async_nats::Client,
     ) -> Self {
         Self {
             repo,
             user_repo,
-            album_share_repo,
             metadata_repo,
+            auth_service,
             config,
             nats_client,
         }
@@ -52,21 +62,27 @@ impl MediaService for MediaServiceImpl {
             .await?;
 
         let file_bytes_clone = file_bytes.clone();
-        let extracted_data = tokio::task::spawn_blocking(move || {
+        let extracted_data =
-            extract_exif_data_from_bytes(&file_bytes_clone)
+            tokio::task::spawn_blocking(move || extract_exif_data_from_bytes(&file_bytes_clone))
-        })
+                .await
-        .await
+                .unwrap()?;
-        .unwrap()?;
 
-        let (storage_path_buf, _date_taken) =
+        let (storage_path_buf, _date_taken) = get_storage_path_and_date(&extracted_data, &filename);
-            get_storage_path_and_date(&extracted_data, &filename);
 
         let storage_path_str = self
             .persist_media_file(&file_bytes, &storage_path_buf)
             .await?;
 
         let media = self
-            .persist_media_metadata(owner_id, filename, mime_type, storage_path_str, hash, file_size, extracted_data)
+            .persist_media_metadata(
+                owner_id,
+                filename,
+                mime_type,
+                storage_path_str,
+                hash,
+                file_size,
+                extracted_data,
+            )
             .await?;
 
         self.publish_new_media_job(media.id).await?;
@@ -75,71 +91,48 @@ impl MediaService for MediaServiceImpl {
     }
 
     async fn get_media_details(&self, id: Uuid, user_id: Uuid) -> CoreResult<MediaBundle> {
+        self.auth_service
+            .check_permission(user_id, authz::Permission::ViewMedia(id))
+            .await?;
+
         let media = self
             .repo
             .find_by_id(id)
             .await?
             .ok_or(CoreError::NotFound("Media".to_string(), id))?;
 
-        let user = self
-            .user_repo
-            .find_by_id(user_id)
-            .await?
-            .ok_or(CoreError::NotFound("User".to_string(), user_id))?;
-
-        if !authz::is_owner(user_id, &media) && !authz::is_admin(&user) {
-             let is_shared = self
-            .album_share_repo
-            .is_media_in_shared_album(id, user_id)
-            .await?;
-
-            tracing::warn!("User {} attempted to access media {} without permission, media owner is: {}", user_id, id, media.owner_id);
-
-            if !is_shared {
-                tracing::warn!("User {} attempted to access media {} without permission, media owner is: {}", user_id, id, media.owner_id);
-                return Err(CoreError::Auth("Access denied".to_string()));
-            }
-        }
-
         let metadata = self.metadata_repo.find_by_media_id(id).await?;
 
         Ok(MediaBundle { media, metadata })
     }
 
-    async fn list_user_media(&self, user_id: Uuid, options: ListMediaOptions) -> CoreResult<Vec<Media>> {
+    async fn list_user_media(
+        &self,
+        user_id: Uuid,
+        options: ListMediaOptions,
+    ) -> CoreResult<Vec<Media>> {
         self.repo.list_by_user(user_id, &options).await
     }
 
     async fn get_media_filepath(&self, id: Uuid, user_id: Uuid) -> CoreResult<String> {
+        self.auth_service
+            .check_permission(user_id, authz::Permission::ViewMedia(id))
+            .await?;
+
         let media = self
             .repo
             .find_by_id(id)
             .await?
             .ok_or(CoreError::NotFound("Media".to_string(), id))?;
 
-        let user = self
+        Ok(media.storage_path)
-            .user_repo
-            .find_by_id(user_id)
-            .await?
-            .ok_or(CoreError::NotFound("User".to_string(), user_id))?;
-
-        if authz::is_owner(user_id, &media) || authz::is_admin(&user) {
-            return Ok(media.storage_path);
-        }
-
-        let is_shared = self
-            .album_share_repo
-            .is_media_in_shared_album(id, user_id)
-            .await?;
-
-        if is_shared {
-            return Ok(media.storage_path);
-        }
-
-        Err(CoreError::Auth("Access denied".to_string()))
     }
 
     async fn delete_media(&self, id: Uuid, user_id: Uuid) -> CoreResult<()> {
+        self.auth_service
+            .check_permission(user_id, authz::Permission::DeleteMedia(id))
+            .await?;
+
         let media = self
             .repo
             .find_by_id(id)
@@ -152,10 +145,6 @@ impl MediaService for MediaServiceImpl {
             .await?
             .ok_or(CoreError::NotFound("User".to_string(), user_id))?;
 
-        if !authz::is_owner(user_id, &media) && !authz::is_admin(&user) {
-            return Err(CoreError::Auth("Access denied".to_string()));
-        }
-
         let full_path = PathBuf::from(&self.config.media_library_path).join(&media.storage_path);
         self.repo.delete(id).await?;
 
@@ -227,7 +216,11 @@ impl MediaServiceImpl {
         Ok(())
     }
 
-    async fn persist_media_file(&self, file_bytes: &[u8], storage_path: &Path) -> CoreResult<String> {
+    async fn persist_media_file(
+        &self,
+        file_bytes: &[u8],
+        storage_path: &Path,
+    ) -> CoreResult<String> {
         let mut dest_path = PathBuf::from(&self.config.media_library_path);
         dest_path.push(storage_path);
 

libertas_api/src/services/mod.rs

@@ -3,3 +3,4 @@ pub mod media_service;
 pub mod user_service;
 pub mod tag_service;
 pub mod person_service;
+pub mod authorization_service;

libertas_api/src/services/person_service.rs

@@ -1,119 +1,50 @@
 use std::sync::Arc;
 
 use async_trait::async_trait;
-use libertas_core::{authz, error::{CoreError, CoreResult}, models::{FaceRegion, Media, Person, PersonPermission}, repositories::{FaceRegionRepository, MediaRepository, PersonRepository, PersonShareRepository}, services::PersonService};
+use libertas_core::{
+    authz,
+    error::{CoreError, CoreResult},
+    models::{FaceRegion, Person, PersonPermission},
+    repositories::{FaceRegionRepository, PersonRepository, PersonShareRepository},
+    services::{AuthorizationService, PersonService},
+};
 use uuid::Uuid;
 
 pub struct PersonServiceImpl {
     person_repo: Arc<dyn PersonRepository>,
     face_repo: Arc<dyn FaceRegionRepository>,
-    media_repo: Arc<dyn MediaRepository>,
     person_share_repo: Arc<dyn PersonShareRepository>,
+    auth_service: Arc<dyn AuthorizationService>,
 }
 
 impl PersonServiceImpl {
     pub fn new(
         person_repo: Arc<dyn PersonRepository>,
         face_repo: Arc<dyn FaceRegionRepository>,
-        media_repo: Arc<dyn MediaRepository>,
         person_share_repo: Arc<dyn PersonShareRepository>,
+        auth_service: Arc<dyn AuthorizationService>,
     ) -> Self {
         Self {
             person_repo,
             face_repo,
-            media_repo,
             person_share_repo,
+            auth_service,
         }
     }
 
-    async fn get_and_authorize_person_owner(
+    async fn get_person(&self, person_id: Uuid) -> CoreResult<Person> {
-        &self,
-        person_id: Uuid,
-        user_id: Uuid,
-    ) -> CoreResult<Person> {
         let person = self
-        .person_repo
+            .person_repo
-        .find_by_id(person_id)
+            .find_by_id(person_id)
-        .await?
+            .await?
-        .ok_or(CoreError::NotFound("Person".to_string(), person_id))?;
+            .ok_or(CoreError::NotFound("Person".to_string(), person_id))?;
-
-        if person.owner_id != user_id {
-            return Err(CoreError::Auth(
-                "User must be the owner to perform this action".to_string(),
-            ));
-        }
-        
         Ok(person)
     }
-
-    async fn get_and_authorize_person_access(
-        &self,
-        person_id: Uuid,
-        user_id: Uuid,
-    ) -> CoreResult<Person> {
-        let person = self
-        .person_repo
-        .find_by_id(person_id)
-        .await?
-        .ok_or(CoreError::NotFound("Person".to_string(), person_id))?;
-
-        let share_permission = self.person_share_repo.get_user_permission(person_id, user_id).await?;
-
-        if !authz::can_access_person(user_id, &person, share_permission) {
-           return Err(CoreError::Auth(
-                "User does not have permission to access this person".to_string(),
-            ));
-        }
-
-        Ok(person)
-    }
-
-    async fn get_and_authorize_person_usage(
-        &self,
-        person_id: Uuid,
-        user_id: Uuid,
-    ) -> CoreResult<Person> {
-        let person = self
-        .person_repo
-        .find_by_id(person_id)
-        .await?
-        .ok_or(CoreError::NotFound("Person".to_string(), person_id))?;
-
-        let share_permission = self.person_share_repo.get_user_permission(person_id, user_id).await?;
-
-        if !authz::can_edit_person(user_id, &person, share_permission) {
-           return Err(CoreError::Auth(
-                "User does not have permission to use this person".to_string(),
-            ));
-        }
-
-        Ok(person)
-    }
-
-    async fn authorize_media_access(
-        &self,
-        media_id: Uuid,
-        user_id: Uuid,
-    ) -> CoreResult<Media> {
-        let media = self.media_repo.find_by_id(media_id).await?.ok_or(CoreError::NotFound("Media".to_string(), media_id))?;
-
-        if !authz::is_owner(user_id, &media) {
-           return Err(CoreError::Auth(
-                "User does not have permission to access this media".to_string(),
-            ));
-        }
-
-        Ok(media)
-    }
 }
 
 #[async_trait]
 impl PersonService for PersonServiceImpl {
-    async fn create_person(
+    async fn create_person(&self, name: &str, owner_id: Uuid) -> CoreResult<Person> {
-        &self,
-        name: &str,
-        owner_id: Uuid,
-    ) -> CoreResult<Person> {
         let person = Person {
             id: Uuid::new_v4(),
             owner_id,
@@ -127,21 +58,28 @@ impl PersonService for PersonServiceImpl {
     }
 
     async fn get_person(&self, person_id: Uuid, user_id: Uuid) -> CoreResult<Person> {
-        self.get_and_authorize_person_access(person_id, user_id).await
+        self.auth_service
+            .check_permission(user_id, authz::Permission::ViewPerson(person_id))
+            .await?;
+
+        self.person_repo
+            .find_by_id(person_id)
+            .await?
+            .ok_or(CoreError::NotFound("Person".to_string(), person_id))
     }
 
     async fn list_people(&self, user_id: Uuid) -> CoreResult<Vec<Person>> {
         let mut owned_people = self.person_repo.list_by_user(user_id).await?;
 
         let shared_people_with_perms = self
-        .person_share_repo
+            .person_share_repo
-        .list_people_shared_with_user(user_id)
+            .list_people_shared_with_user(user_id)
-        .await?;
+            .await?;
 
         let shared_people = shared_people_with_perms
-        .into_iter()
+            .into_iter()
-        .map(|(person, _permission)| person)
+            .map(|(person, _permission)| person)
-        .collect::<Vec<Person>>();
+            .collect::<Vec<Person>>();
 
         owned_people.extend(shared_people);
 
@@ -154,14 +92,22 @@ impl PersonService for PersonServiceImpl {
         name: &str,
         user_id: Uuid,
     ) -> CoreResult<Person> {
-        let mut person = self.get_and_authorize_person_owner(person_id, user_id).await?;
+        self.auth_service
+            .check_permission(user_id, authz::Permission::EditPerson(person_id))
+            .await?;
+
+        let mut person = self.get_person(person_id).await?;
+
         person.name = name.to_string();
         self.person_repo.update(person.clone()).await?;
         Ok(person)
     }
 
     async fn delete_person(&self, person_id: Uuid, user_id: Uuid) -> CoreResult<()> {
-        self.get_and_authorize_person_owner(person_id, user_id).await?;
+        self.auth_service
+            .check_permission(user_id, authz::Permission::DeletePerson(person_id))
+            .await?;
+
         self.person_repo.delete(person_id).await
     }
 
@@ -171,15 +117,21 @@ impl PersonService for PersonServiceImpl {
         person_id: Uuid,
         user_id: Uuid,
     ) -> CoreResult<FaceRegion> {
-        self.get_and_authorize_person_usage(person_id, user_id).await?;
+        self.auth_service
+            .check_permission(user_id, authz::Permission::UsePerson(person_id))
+            .await?;
+        self.auth_service
+            .check_permission(user_id, authz::Permission::AssignFace(face_region_id))
+            .await?;
 
-        let mut face = self
+        let mut face =
-        .face_repo
+            self.face_repo
-        .find_by_id(face_region_id)
+                .find_by_id(face_region_id)
-        .await?
+                .await?
-        .ok_or(CoreError::NotFound("FaceRegion".to_string(), face_region_id))?;
+                .ok_or(CoreError::NotFound(
-
+                    "FaceRegion".to_string(),
-        self.authorize_media_access(face.media_id, user_id).await?;
+                    face_region_id,
+                ))?;
 
         self.face_repo
             .update_person_id(face_region_id, person_id)
@@ -194,7 +146,9 @@ impl PersonService for PersonServiceImpl {
         media_id: Uuid,
         user_id: Uuid,
     ) -> CoreResult<Vec<FaceRegion>> {
-        self.authorize_media_access(media_id, user_id).await?;
+        self.auth_service
+            .check_permission(user_id, authz::Permission::ViewFaces(media_id))
+            .await?;
 
         self.face_repo.find_by_media_id(media_id).await
     }
@@ -206,7 +160,9 @@ impl PersonService for PersonServiceImpl {
         permission: PersonPermission,
         owner_id: Uuid,
     ) -> CoreResult<()> {
-        self.get_and_authorize_person_owner(person_id, owner_id).await?;
+        self.auth_service
+            .check_permission(owner_id, authz::Permission::SharePerson(person_id))
+            .await?;
 
         self.person_share_repo
             .create_or_update_share(person_id, target_user_id, permission)
@@ -219,7 +175,9 @@ impl PersonService for PersonServiceImpl {
         target_user_id: Uuid,
         owner_id: Uuid,
     ) -> CoreResult<()> {
-        self.get_and_authorize_person_owner(person_id, owner_id).await?;
+        self.auth_service
+            .check_permission(owner_id, authz::Permission::SharePerson(person_id))
+            .await?;
 
         self.person_share_repo
             .remove_share(person_id, target_user_id)

libertas_api/src/services/tag_service.rs

@@ -1,40 +1,24 @@
 use std::sync::Arc;
 
 use async_trait::async_trait;
-use libertas_core::{authz, error::{CoreError, CoreResult}, models::{Media, Tag}, repositories::{MediaRepository, TagRepository}, services::TagService};
+use libertas_core::{authz::Permission, error::CoreResult, models::Tag, repositories::TagRepository, services::{AuthorizationService, TagService}};
 use uuid::Uuid;
 
 pub struct TagServiceImpl {
     tag_repo: Arc<dyn TagRepository>,
-    media_repo: Arc<dyn MediaRepository>,
+    auth_service: Arc<dyn AuthorizationService>,
 }
 
 impl TagServiceImpl {
     pub fn new(
         tag_repo: Arc<dyn TagRepository>,
-        media_repo: Arc<dyn MediaRepository>,
+        auth_service: Arc<dyn AuthorizationService>,
     ) -> Self {
         Self {
             tag_repo,
-            media_repo,
+            auth_service,
         }
     }
-
-    async fn authorize_media_access(
-        &self,
-        media_id: Uuid,
-        user_id: Uuid,
-    ) -> CoreResult<Media> {
-        let media = self.media_repo.find_by_id(media_id).await?.ok_or(CoreError::NotFound("Media".to_string(), media_id))?;
-
-        if !authz::is_owner(user_id, &media) {
-           return Err(CoreError::Auth(
-                "User does not have permission to access this media".to_string(),
-            ));
-        }
-
-        Ok(media)
-    }
 }
 
 #[async_trait]
@@ -46,7 +30,7 @@ impl TagService for TagServiceImpl {
         user_id: Uuid,
 
     ) -> CoreResult<Vec<Tag>> {
-        self.authorize_media_access(media_id, user_id).await?;
+        self.auth_service.check_permission(user_id, Permission::AddTags(media_id)).await?;
 
         let mut tag_ids = Vec::new();
         let tags = self.tag_repo.find_or_create_tags(tag_names).await?;
@@ -65,7 +49,7 @@ impl TagService for TagServiceImpl {
         tag_names: &[String],
         user_id: Uuid,
     ) -> CoreResult<()> {
-        self.authorize_media_access(media_id, user_id).await?;
+        self.auth_service.check_permission(user_id, Permission::RemoveTags(media_id)).await?;
 
         let tags = self.tag_repo.find_or_create_tags(tag_names).await?;
         let mut tag_ids = Vec::new();
@@ -83,7 +67,7 @@ impl TagService for TagServiceImpl {
         media_id: Uuid,
         user_id: Uuid,
     ) -> CoreResult<Vec<Tag>> {
-        self.authorize_media_access(media_id, user_id).await?;
+        self.auth_service.check_permission(user_id, Permission::ViewMedia(media_id)).await?;
 
         let tags = self.tag_repo.list_tags_for_media(media_id).await?;
 

libertas_api/src/state.rs

@@ -2,7 +2,7 @@ use std::sync::Arc;
 
 use libertas_core::{
     config::AppConfig,
-    services::{AlbumService, MediaService, PersonService, TagService, UserService},
+    services::{AlbumService, AuthorizationService, MediaService, PersonService, TagService, UserService},
 };
 
 use crate::security::TokenGenerator;
@@ -14,6 +14,7 @@ pub struct AppState {
     pub album_service: Arc<dyn AlbumService>,
     pub tag_service: Arc<dyn TagService>,
     pub person_service: Arc<dyn PersonService>,
+    pub authorization_service: Arc<dyn AuthorizationService>,
     pub token_generator: Arc<dyn TokenGenerator>,
     pub nats_client: async_nats::Client,
     pub config: AppConfig,

libertas_core/src/authz.rs

@@ -2,6 +2,34 @@ use uuid::Uuid;
 
 use crate::models::{Album, AlbumPermission, Media, Person, PersonPermission, Role, User};
 
+pub enum Permission {
+    // Media
+    ViewMedia(Uuid),
+    EditMedia(Uuid),
+    AddTags(Uuid),
+    RemoveTags(Uuid),
+    EditTags(Uuid),
+    DeleteMedia(Uuid),
+
+    // Albums
+    ViewAlbum(Uuid),
+    AddToAlbum(Uuid),
+    EditAlbum(Uuid),
+    ShareAlbum(Uuid),
+    DeleteAlbum(Uuid),
+
+    // People
+    ViewPerson(Uuid),
+    EditPerson(Uuid),
+    UsePerson(Uuid),
+    SharePerson(Uuid),
+    DeletePerson(Uuid),
+
+    // Faces
+    ViewFaces(Uuid),
+    AssignFace(Uuid),
+}
+
 pub trait Ownable {
     fn owner_id(&self) -> Uuid;
 }
@@ -48,10 +76,18 @@ pub fn can_contribute_to_album(
     is_owner(user_id, album) || share_permission == Some(AlbumPermission::Contribute)
 }
 
-pub fn can_access_person(user_id: Uuid, person: &Person, share_permission: Option<PersonPermission>) -> bool {
+pub fn can_access_person(
+    user_id: Uuid,
+    person: &Person,
+    share_permission: Option<PersonPermission>,
+) -> bool {
     is_owner(user_id, person) || share_permission.is_some()
 }
 
-pub fn can_edit_person(user_id: Uuid, person: &Person, share_permission: Option<PersonPermission>) -> bool {
+pub fn can_use_person(
+    user_id: Uuid,
+    person: &Person,
+    share_permission: Option<PersonPermission>,
+) -> bool {
     is_owner(user_id, person) || share_permission == Some(PersonPermission::CanUse)
 }

libertas_core/src/repositories.rs

@@ -3,7 +3,11 @@ use uuid::Uuid;
 
 use crate::{
     error::CoreResult,
-    models::{Album, AlbumPermission, FaceRegion, Media, MediaMetadata, Person, PersonPermission, Tag, User}, schema::ListMediaOptions,
+    models::{
+        Album, AlbumPermission, FaceRegion, Media, MediaMetadata, Person, PersonPermission, Tag,
+        User,
+    },
+    schema::ListMediaOptions,
 };
 
 #[async_trait]
@@ -11,7 +15,11 @@ pub trait MediaRepository: Send + Sync {
     async fn find_by_hash(&self, hash: &str) -> CoreResult<Option<Media>>;
     async fn create(&self, media: &Media) -> CoreResult<()>;
     async fn find_by_id(&self, id: Uuid) -> CoreResult<Option<Media>>;
-    async fn list_by_user(&self, user_id: Uuid, options: &ListMediaOptions) -> CoreResult<Vec<Media>>;
+    async fn list_by_user(
+        &self,
+        user_id: Uuid,
+        options: &ListMediaOptions,
+    ) -> CoreResult<Vec<Media>>;
     async fn update_thumbnail_path(&self, id: Uuid, thumbnail_path: String) -> CoreResult<()>;
     async fn delete(&self, id: Uuid) -> CoreResult<()>;
 }
@@ -51,6 +59,11 @@ pub trait AlbumShareRepository: Send + Sync {
     ) -> CoreResult<Option<AlbumPermission>>;
 
     async fn is_media_in_shared_album(&self, media_id: Uuid, user_id: Uuid) -> CoreResult<bool>;
+    async fn is_media_in_contributable_album(
+        &self,
+        media_id: Uuid,
+        user_id: Uuid,
+    ) -> CoreResult<bool>;
 }
 
 #[async_trait]

libertas_core/src/services.rs

@@ -2,11 +2,9 @@ use async_trait::async_trait;
 use uuid::Uuid;
 
 use crate::{
-    error::CoreResult,
+    authz::Permission, error::CoreResult, models::{Album, FaceRegion, Media, MediaBundle, Person, PersonPermission, Tag, User}, schema::{
-    models::{Album, FaceRegion, Media, MediaBundle, Person, PersonPermission, Tag, User},
-    schema::{
         AddMediaToAlbumData, CreateAlbumData, CreateUserData, ListMediaOptions, LoginUserData, ShareAlbumData, UpdateAlbumData, UploadMediaData
-    },
+    }
 };
 
 #[async_trait]
@@ -85,3 +83,8 @@ pub trait PersonService: Send + Sync {
         owner_id: Uuid,
     ) -> CoreResult<()>;
 }
+
+#[async_trait]
+pub trait AuthorizationService: Send + Sync {
+    async fn check_permission(&self, user_id: Uuid, permission: Permission) -> CoreResult<()>;
+}

libertas_infra/src/repositories/album_share_repository.rs

@@ -1,10 +1,12 @@
+use crate::db_models::PostgresAlbumPermission;
 use async_trait::async_trait;
 use libertas_core::{
-    error::{CoreError, CoreResult}, models::AlbumPermission, repositories::AlbumShareRepository
+    error::{CoreError, CoreResult},
+    models::AlbumPermission,
+    repositories::AlbumShareRepository,
 };
 use sqlx::PgPool;
 use uuid::Uuid;
-use crate::db_models::PostgresAlbumPermission;
 
 #[derive(Clone)]
 pub struct PostgresAlbumShareRepository {
@@ -83,4 +85,30 @@ impl AlbumShareRepository for PostgresAlbumShareRepository {
 
         Ok(result.exists.unwrap_or(false))
     }
+
+    async fn is_media_in_contributable_album(
+        &self,
+        media_id: Uuid,
+        user_id: Uuid,
+    ) -> CoreResult<bool> {
+        let result = sqlx::query!(
+            r#"
+            SELECT EXISTS (
+                SELECT 1
+                FROM album_media am
+                JOIN album_shares ash ON am.album_id = ash.album_id
+                WHERE am.media_id = $1 AND ash.user_id = $2
+                  AND ash.permission = $3
+            )
+            "#,
+            media_id,
+            user_id,
+            PostgresAlbumPermission::Contribute as PostgresAlbumPermission,
+        )
+        .fetch_one(&self.pool)
+        .await
+        .map_err(|e| CoreError::Database(e.to_string()))?;
+
+        Ok(result.exists.unwrap_or(false))
+    }
 }