From 098dd653d13ab91afb64c5585f340682325f0edc Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Sun, 25 Oct 2026 14:04:44 -0700 Subject: [PATCH 01/16] fix(api): reject an OIDC login whose access group sync fails Reconciling a user's OIDC-sourced access group memberships used to log a warning and let the login continue. A first-time OIDC user whose sync failed ended up in no groups, and a user with no groups sees the whole library, so a database hiccup quietly turned into full visibility. The callback now fails the login instead, and the reconcile runs in one transaction so a failure part-way never leaves half the IdP's groups applied. The users list also shows each user's access groups, with the membership source, so an admin can see at a glance who is restricted and spot an account that landed in the wrong group. Memberships for the whole page are fetched in two queries rather than one per user. --- crates/codex-api/src/docs.rs | 1 + crates/codex-api/src/routes/v1/dto/user.rs | 20 ++++ .../codex-api/src/routes/v1/handlers/oidc.rs | 112 +++++++++++++----- .../codex-api/src/routes/v1/handlers/users.rs | 104 +++++++++------- .../codex-db/src/repositories/access_group.rs | 57 ++++++++- docs/api/openapi.json | 37 +++++- tests/api/access_groups.rs | 50 ++++++++ web/openapi.json | 37 +++++- .../users/UserAccessGroupBadges.test.tsx | 39 ++++++ .../users/UserAccessGroupBadges.tsx | 40 +++++++ web/src/components/users/index.ts | 1 + web/src/mocks/data/factories.ts | 1 + web/src/pages/settings/UsersSettings.tsx | 11 +- web/src/types/api.generated.ts | 21 ++++ 14 files changed, 454 insertions(+), 77 deletions(-) create mode 100644 web/src/components/users/UserAccessGroupBadges.test.tsx create mode 100644 web/src/components/users/UserAccessGroupBadges.tsx diff --git a/crates/codex-api/src/docs.rs b/crates/codex-api/src/docs.rs index 7aae46b06..dd68c4a67 100644 --- a/crates/codex-api/src/docs.rs +++ b/crates/codex-api/src/docs.rs @@ -1008,6 +1008,7 @@ The following paths are exempt from rate limiting: v1::dto::RetryErrorsResponse, v1::dto::PageDto, v1::dto::UserDto, + v1::dto::UserAccessGroupDto, v1::dto::UserDetailDto, v1::dto::CreateUserRequest, v1::dto::UpdateUserRequest, diff --git a/crates/codex-api/src/routes/v1/dto/user.rs b/crates/codex-api/src/routes/v1/dto/user.rs index f3b3af9cc..f7fa1adbe 100644 --- a/crates/codex-api/src/routes/v1/dto/user.rs +++ b/crates/codex-api/src/routes/v1/dto/user.rs @@ -44,6 +44,26 @@ pub struct UserDto { /// Last account update timestamp #[schema(example = "2024-01-15T10:30:00Z")] pub updated_at: DateTime, + + /// Access groups the user belongs to, sorted by name + pub access_groups: Vec, +} + +/// An access group membership as shown on a user +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct UserAccessGroupDto { + /// Access group ID + #[schema(example = "550e8400-e29b-41d4-a716-446655440000")] + pub id: uuid::Uuid, + + /// Access group name + #[schema(example = "Kids")] + pub name: String, + + /// Membership source (manual or oidc) + #[schema(example = "manual")] + pub source: String, } /// User detail DTO with sharing tag grants (for single user view) diff --git a/crates/codex-api/src/routes/v1/handlers/oidc.rs b/crates/codex-api/src/routes/v1/handlers/oidc.rs index c73765639..fa7735ff7 100644 --- a/crates/codex-api/src/routes/v1/handlers/oidc.rs +++ b/crates/codex-api/src/routes/v1/handlers/oidc.rs @@ -294,20 +294,7 @@ pub async fn callback( .await .map_err(|e| ApiError::Internal(format!("Failed to update OIDC connection: {}", e)))?; - // Reconcile OIDC-sourced access group memberships - if let Err(e) = AccessGroupRepository::reconcile_oidc_group_memberships( - &state.db, - user.id, - &auth_result.groups, - ) - .await - { - warn!( - user_id = %user.id, - error = %e, - "Failed to reconcile OIDC access group memberships (login continues)" - ); - } + sync_access_groups_from_oidc(&state.db, user.id, &auth_result.groups).await?; // Sync role from IdP groups on every login let user = sync_role_from_groups(&state.db, user, &auth_result.mapped_role).await?; @@ -415,20 +402,7 @@ pub async fn callback( .await .map_err(|e| ApiError::Internal(format!("Failed to create OIDC connection: {}", e)))?; - // Reconcile OIDC-sourced access group memberships for new connection - if let Err(e) = AccessGroupRepository::reconcile_oidc_group_memberships( - &state.db, - user.id, - &auth_result.groups, - ) - .await - { - warn!( - user_id = %user.id, - error = %e, - "Failed to reconcile OIDC access group memberships (login continues)" - ); - } + sync_access_groups_from_oidc(&state.db, user.id, &auth_result.groups).await?; (user, is_new) }; @@ -559,6 +533,28 @@ fn redirect_response(url: &str) -> Result { }) } +/// Reconcile the user's OIDC-sourced access group memberships with the IdP's groups. +/// +/// A failure here fails the login. Continuing would leave the user with stale or +/// missing memberships, and a user with no groups sees the whole library. +async fn sync_access_groups_from_oidc( + db: &sea_orm::DatabaseConnection, + user_id: Uuid, + oidc_groups: &[String], +) -> Result<(), ApiError> { + AccessGroupRepository::reconcile_oidc_group_memberships(db, user_id, oidc_groups) + .await + .map(|_| ()) + .map_err(|e| { + warn!( + user_id = %user_id, + error = %e, + "Failed to reconcile OIDC access group memberships, rejecting login" + ); + ApiError::Internal("Failed to sync access groups".to_string()) + }) +} + /// Sync user role from IdP group mapping on every OIDC login. /// /// If the mapped role differs from the current role, updates the user in the database. @@ -960,5 +956,65 @@ mod tests { let updated = sync_role_from_groups(&db, created, "reader").await.unwrap(); assert_eq!(updated.role, "reader"); } + + #[tokio::test] + async fn test_sync_access_groups_from_oidc_maps_groups() { + let db = setup_test_db().await; + let user = UserRepository::create(&db, &test_user("mapped")) + .await + .unwrap(); + let group = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + AccessGroupRepository::add_oidc_mapping(&db, group.id, "idp-kids") + .await + .unwrap(); + + sync_access_groups_from_oidc(&db, user.id, &["idp-kids".to_string()]) + .await + .unwrap(); + + let groups = AccessGroupRepository::list_for_user(&db, user.id) + .await + .unwrap(); + assert_eq!(groups.len(), 1); + assert_eq!(groups[0].id, group.id); + } + + /// A user whose group sync fails would otherwise log in with no + /// groups, which under default-open visibility means seeing the whole + /// library. The sync failure has to fail the login instead. + #[tokio::test] + async fn test_sync_access_groups_from_oidc_fails_when_reconcile_fails() { + use sea_orm::ConnectionTrait; + + let db = setup_test_db().await; + let user = UserRepository::create(&db, &test_user("broken")) + .await + .unwrap(); + db.execute_unprepared("DROP TABLE access_group_oidc_mappings") + .await + .unwrap(); + + let result = + sync_access_groups_from_oidc(&db, user.id, &["idp-kids".to_string()]).await; + assert!(matches!(result, Err(ApiError::Internal(_)))); + } + + fn test_user(username: &str) -> users::Model { + users::Model { + id: Uuid::new_v4(), + username: username.to_string(), + email: format!("{username}@example.com"), + password_hash: "hash".to_string(), + role: "reader".to_string(), + is_active: true, + email_verified: false, + permissions: serde_json::json!([]), + created_at: Utc::now(), + updated_at: Utc::now(), + last_login_at: None, + } + } } } diff --git a/crates/codex-api/src/routes/v1/handlers/users.rs b/crates/codex-api/src/routes/v1/handlers/users.rs index 3c999728b..a812e699b 100644 --- a/crates/codex-api/src/routes/v1/handlers/users.rs +++ b/crates/codex-api/src/routes/v1/handlers/users.rs @@ -1,6 +1,6 @@ use super::super::dto::{ - CreateUserRequest, PaginatedResponse, UpdateUserRequest, UserDetailDto, UserDto, - UserListParams, UserSharingTagGrantDto, common::PaginationLinkBuilder, + CreateUserRequest, PaginatedResponse, UpdateUserRequest, UserAccessGroupDto, UserDetailDto, + UserDto, UserListParams, UserSharingTagGrantDto, common::PaginationLinkBuilder, }; use super::paginated_response; use crate::require_permission; @@ -16,8 +16,11 @@ use axum::{ }; use chrono::Utc; use codex_db::entities::users; -use codex_db::repositories::{SharingTagRepository, UserListFilter, UserRepository}; +use codex_db::repositories::{ + AccessGroupRepository, SharingTagRepository, UserListFilter, UserRepository, +}; use codex_utils::password; +use std::collections::HashMap; use std::sync::Arc; use uuid::Uuid; @@ -121,23 +124,14 @@ pub async fn list_users( .await .map_err(|e| ApiError::Internal(format!("Failed to fetch users: {}", e)))?; + let user_ids: Vec = result.users.iter().map(|u| u.id).collect(); + let mut groups = access_groups_by_user(&state.db, &user_ids).await?; let dtos: Vec = result .users .into_iter() .map(|user| { - let role = user.get_role(); - let permissions = parse_permissions_json(&user.permissions); - UserDto { - id: user.id, - username: user.username, - email: user.email, - role, - permissions, - is_active: user.is_active, - last_login_at: user.last_login_at, - created_at: user.created_at, - updated_at: user.updated_at, - } + let user_groups = groups.remove(&user.id).unwrap_or_default(); + user_to_dto(user, user_groups) }) .collect(); @@ -287,19 +281,11 @@ pub async fn create_user( .await .map_err(|e| ApiError::Internal(format!("Failed to create user: {}", e)))?; - let role = user.get_role(); - let permissions = parse_permissions_json(&user.permissions); - let dto = UserDto { - id: user.id, - username: user.username, - email: user.email, - role, - permissions, - is_active: user.is_active, - last_login_at: user.last_login_at, - created_at: user.created_at, - updated_at: user.updated_at, - }; + let user_groups = access_groups_by_user(&state.db, &[user.id]) + .await? + .remove(&user.id) + .unwrap_or_default(); + let dto = user_to_dto(user, user_groups); Ok(Json(dto)) } @@ -392,19 +378,11 @@ pub async fn update_user( .await .map_err(|e| ApiError::Internal(format!("Failed to update user: {}", e)))?; - let role = updated.get_role(); - let permissions = parse_permissions_json(&updated.permissions); - let dto = UserDto { - id: updated.id, - username: updated.username, - email: updated.email, - role, - permissions, - is_active: updated.is_active, - last_login_at: updated.last_login_at, - created_at: updated.created_at, - updated_at: updated.updated_at, - }; + let user_groups = access_groups_by_user(&state.db, &[updated.id]) + .await? + .remove(&updated.id) + .unwrap_or_default(); + let dto = user_to_dto(updated, user_groups); Ok(Json(dto)) } @@ -447,3 +425,45 @@ pub async fn delete_user( Ok(()) } + +/// Build the list-level user DTO from a user row and their group memberships. +fn user_to_dto(user: users::Model, access_groups: Vec) -> UserDto { + let role = user.get_role(); + let permissions = parse_permissions_json(&user.permissions); + UserDto { + id: user.id, + username: user.username, + email: user.email, + role, + permissions, + is_active: user.is_active, + last_login_at: user.last_login_at, + created_at: user.created_at, + updated_at: user.updated_at, + access_groups, + } +} + +/// Access group memberships for a batch of users, keyed by user ID. +async fn access_groups_by_user( + db: &sea_orm::DatabaseConnection, + user_ids: &[Uuid], +) -> Result>, ApiError> { + let memberships = AccessGroupRepository::list_memberships_for_users(db, user_ids) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch access groups: {}", e)))?; + Ok(memberships + .into_iter() + .map(|(user_id, entries)| { + let dtos = entries + .into_iter() + .map(|(group, membership)| UserAccessGroupDto { + id: group.id, + name: group.name, + source: membership.source, + }) + .collect(); + (user_id, dtos) + }) + .collect()) +} diff --git a/crates/codex-db/src/repositories/access_group.rs b/crates/codex-db/src/repositories/access_group.rs index 3e5b0aa3a..c2f9ae763 100644 --- a/crates/codex-db/src/repositories/access_group.rs +++ b/crates/codex-db/src/repositories/access_group.rs @@ -7,8 +7,10 @@ use anyhow::Result; use chrono::Utc; use sea_orm::{ - ActiveModelTrait, ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter, QueryOrder, Set, + ActiveModelTrait, ColumnTrait, ConnectionTrait, DatabaseConnection, EntityTrait, QueryFilter, + QueryOrder, Set, TransactionTrait, }; +use std::collections::{HashMap, HashSet}; use uuid::Uuid; use crate::entities::{ @@ -101,7 +103,7 @@ impl AccessGroupRepository { /// Add a user to a group. Idempotent: if the membership already exists /// returns the existing row. Updates the `source` if it differs. pub async fn add_member( - db: &DatabaseConnection, + db: &impl ConnectionTrait, group_id: Uuid, user_id: Uuid, source: MembershipSource, @@ -131,7 +133,7 @@ impl AccessGroupRepository { /// Remove a user from a group. Returns true if a row was deleted. pub async fn remove_member( - db: &DatabaseConnection, + db: &impl ConnectionTrait, group_id: Uuid, user_id: Uuid, ) -> Result { @@ -192,6 +194,49 @@ impl AccessGroupRepository { .await?) } + /// Group memberships for many users in two queries, keyed by user ID. + /// + /// Each user's groups are sorted by name and paired with the membership + /// row, which carries the source. Users with no memberships are absent. + pub async fn list_memberships_for_users( + db: &DatabaseConnection, + user_ids: &[Uuid], + ) -> Result>> { + if user_ids.is_empty() { + return Ok(HashMap::new()); + } + let memberships = user_access_groups::Entity::find() + .filter(user_access_groups::Column::UserId.is_in(user_ids.to_vec())) + .all(db) + .await?; + if memberships.is_empty() { + return Ok(HashMap::new()); + } + let group_ids: HashSet = memberships.iter().map(|m| m.access_group_id).collect(); + let groups: HashMap = access_groups::Entity::find() + .filter(access_groups::Column::Id.is_in(group_ids)) + .all(db) + .await? + .into_iter() + .map(|g| (g.id, g)) + .collect(); + + let mut by_user: HashMap> = + HashMap::new(); + for membership in memberships { + if let Some(group) = groups.get(&membership.access_group_id) { + by_user + .entry(membership.user_id) + .or_default() + .push((group.clone(), membership)); + } + } + for entries in by_user.values_mut() { + entries.sort_by(|a, b| a.0.name.cmp(&b.0.name)); + } + Ok(by_user) + } + // ==================== Grants ==================== /// Set a group's grant for a sharing tag (upsert). If a row exists with a @@ -325,7 +370,10 @@ impl AccessGroupRepository { user_id: Uuid, oidc_groups: &[String], ) -> Result<(usize, usize)> { - use std::collections::HashSet; + // One transaction, so a failure part-way never leaves the user with + // half the IdP's groups applied. + let txn = db.begin().await?; + let db = &txn; // 1. Desired: access groups mapped to any of the user's OIDC group names let desired_group_ids: HashSet = if oidc_groups.is_empty() { @@ -373,6 +421,7 @@ impl AccessGroupRepository { Self::remove_member(db, *group_id, user_id).await?; } + txn.commit().await?; Ok((to_add.len(), to_remove.len())) } } diff --git a/docs/api/openapi.json b/docs/api/openapi.json index 0995dcbc4..0bbdac19d 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -47543,6 +47543,33 @@ } } }, + "UserAccessGroupDto": { + "type": "object", + "description": "An access group membership as shown on a user", + "required": [ + "id", + "name", + "source" + ], + "properties": { + "id": { + "type": "string", + "format": "uuid", + "description": "Access group ID", + "example": "550e8400-e29b-41d4-a716-446655440000" + }, + "name": { + "type": "string", + "description": "Access group name", + "example": "Kids" + }, + "source": { + "type": "string", + "description": "Membership source (manual or oidc)", + "example": "manual" + } + } + }, "UserDetailDto": { "type": "object", "description": "User detail DTO with sharing tag grants (for single user view)", @@ -47631,9 +47658,17 @@ "permissions", "isActive", "createdAt", - "updatedAt" + "updatedAt", + "accessGroups" ], "properties": { + "accessGroups": { + "type": "array", + "items": { + "$ref": "#/components/schemas/UserAccessGroupDto" + }, + "description": "Access groups the user belongs to, sorted by name" + }, "createdAt": { "type": "string", "format": "date-time", diff --git a/tests/api/access_groups.rs b/tests/api/access_groups.rs index 6d8376d7a..8f5dae2fa 100644 --- a/tests/api/access_groups.rs +++ b/tests/api/access_groups.rs @@ -518,6 +518,56 @@ async fn test_get_user_access_groups() { assert!(names.contains(&"Group B")); } +#[tokio::test] +async fn test_list_users_includes_access_groups() { + use codex::db::entities::user_access_groups::MembershipSource; + use codex::db::repositories::AccessGroupRepository; + + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let (alice_id, _) = create_reader_and_token(&db, &state, "alice").await; + let (bob_id, _) = create_reader_and_token(&db, &state, "bob").await; + + let kids = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + let manga = AccessGroupRepository::create(&db, "Manga", None) + .await + .unwrap(); + AccessGroupRepository::add_member(&db, manga.id, alice_id, MembershipSource::Oidc) + .await + .unwrap(); + AccessGroupRepository::add_member(&db, kids.id, alice_id, MembershipSource::Manual) + .await + .unwrap(); + + let app = create_test_router(state).await; + let request = get_request_with_auth("/api/v1/users", &token); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + let users = response.unwrap()["data"].as_array().unwrap().clone(); + + let groups_of = |id: uuid::Uuid| { + users.iter().find(|u| u["id"] == id.to_string()).unwrap()["accessGroups"] + .as_array() + .unwrap() + .clone() + }; + + // Sorted by name, with the membership source + let alice_groups = groups_of(alice_id); + assert_eq!(alice_groups.len(), 2); + assert_eq!(alice_groups[0]["name"], "Kids"); + assert_eq!(alice_groups[0]["source"], "manual"); + assert_eq!(alice_groups[0]["id"], kids.id.to_string()); + assert_eq!(alice_groups[1]["name"], "Manga"); + assert_eq!(alice_groups[1]["source"], "oidc"); + + assert!(groups_of(bob_id).is_empty()); +} + // ============================================================================ // Effective Grants Tests // ============================================================================ diff --git a/web/openapi.json b/web/openapi.json index 0995dcbc4..0bbdac19d 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -47543,6 +47543,33 @@ } } }, + "UserAccessGroupDto": { + "type": "object", + "description": "An access group membership as shown on a user", + "required": [ + "id", + "name", + "source" + ], + "properties": { + "id": { + "type": "string", + "format": "uuid", + "description": "Access group ID", + "example": "550e8400-e29b-41d4-a716-446655440000" + }, + "name": { + "type": "string", + "description": "Access group name", + "example": "Kids" + }, + "source": { + "type": "string", + "description": "Membership source (manual or oidc)", + "example": "manual" + } + } + }, "UserDetailDto": { "type": "object", "description": "User detail DTO with sharing tag grants (for single user view)", @@ -47631,9 +47658,17 @@ "permissions", "isActive", "createdAt", - "updatedAt" + "updatedAt", + "accessGroups" ], "properties": { + "accessGroups": { + "type": "array", + "items": { + "$ref": "#/components/schemas/UserAccessGroupDto" + }, + "description": "Access groups the user belongs to, sorted by name" + }, "createdAt": { "type": "string", "format": "date-time", diff --git a/web/src/components/users/UserAccessGroupBadges.test.tsx b/web/src/components/users/UserAccessGroupBadges.test.tsx new file mode 100644 index 000000000..4af4e7a2f --- /dev/null +++ b/web/src/components/users/UserAccessGroupBadges.test.tsx @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { renderWithProviders, screen } from "@/test/utils"; +import { UserAccessGroupBadges } from "./UserAccessGroupBadges"; + +describe("UserAccessGroupBadges", () => { + it("renders a badge per group", () => { + renderWithProviders( + , + ); + + expect(screen.getByText("Kids")).toBeInTheDocument(); + expect(screen.getByText("Manga")).toBeInTheDocument(); + }); + + it("labels memberships that come from the identity provider", () => { + renderWithProviders( + , + ); + + expect(screen.getByLabelText("Manga (from OIDC)")).toBeInTheDocument(); + expect(screen.queryByLabelText("Kids (from OIDC)")).not.toBeInTheDocument(); + }); + + it("shows a placeholder when the user has no groups", () => { + renderWithProviders(); + + expect(screen.getByText("None")).toBeInTheDocument(); + }); +}); diff --git a/web/src/components/users/UserAccessGroupBadges.tsx b/web/src/components/users/UserAccessGroupBadges.tsx new file mode 100644 index 000000000..bb75b9165 --- /dev/null +++ b/web/src/components/users/UserAccessGroupBadges.tsx @@ -0,0 +1,40 @@ +import { Badge, Group, Text } from "@mantine/core"; +import { IconCloud } from "@tabler/icons-react"; +import type { components } from "@/types/api.generated"; + +type UserAccessGroup = components["schemas"]["UserAccessGroupDto"]; + +interface UserAccessGroupBadgesProps { + groups: UserAccessGroup[]; +} + +/** Compact list of the access groups a user belongs to, for table cells. */ +export function UserAccessGroupBadges({ groups }: UserAccessGroupBadgesProps) { + if (groups.length === 0) { + return ( + + None + + ); + } + + return ( + + {groups.map((group) => { + const fromOidc = group.source === "oidc"; + return ( + : undefined} + aria-label={fromOidc ? `${group.name} (from OIDC)` : undefined} + title={fromOidc ? "Assigned from an OIDC group" : undefined} + > + {group.name} + + ); + })} + + ); +} diff --git a/web/src/components/users/index.ts b/web/src/components/users/index.ts index 414a37a79..723419ef4 100644 --- a/web/src/components/users/index.ts +++ b/web/src/components/users/index.ts @@ -1,2 +1,3 @@ +export { UserAccessGroupBadges } from "./UserAccessGroupBadges"; export { UserEffectiveGrants } from "./UserEffectiveGrants"; export { UserSharingTagGrants } from "./UserSharingTagGrants"; diff --git a/web/src/mocks/data/factories.ts b/web/src/mocks/data/factories.ts index 5a64a5717..4fd670426 100644 --- a/web/src/mocks/data/factories.ts +++ b/web/src/mocks/data/factories.ts @@ -66,6 +66,7 @@ export const createUser = (overrides: Partial = {}): UserDto => ({ lastLoginAt: faker.date.recent().toISOString(), createdAt: faker.date.past().toISOString(), updatedAt: faker.date.recent().toISOString(), + accessGroups: [], ...overrides, }); diff --git a/web/src/pages/settings/UsersSettings.tsx b/web/src/pages/settings/UsersSettings.tsx index 5043cb317..39cc24248 100644 --- a/web/src/pages/settings/UsersSettings.tsx +++ b/web/src/pages/settings/UsersSettings.tsx @@ -41,7 +41,11 @@ import { type UserDto, type UserListParams, usersApi } from "@/api/users"; import { PermissionPicker } from "@/components/common"; import { TableSkeleton } from "@/components/skeletons"; import { ResponsiveTable, type ResponsiveTableColumn } from "@/components/ui"; -import { UserEffectiveGrants, UserSharingTagGrants } from "@/components/users"; +import { + UserAccessGroupBadges, + UserEffectiveGrants, + UserSharingTagGrants, +} from "@/components/users"; import { useShowSkeleton } from "@/lib/motion/useShowSkeleton"; import { useAuthStore } from "@/store/authStore"; import { type Permission, ROLE_PERMISSIONS } from "@/types/permissions"; @@ -362,6 +366,11 @@ export function UsersSettings() { ), }, + { + key: "accessGroups", + header: "Access Groups", + accessor: (user) => , + }, { key: "status", header: "Status", diff --git a/web/src/types/api.generated.ts b/web/src/types/api.generated.ts index eafd0369a..2f77e488b 100644 --- a/web/src/types/api.generated.ts +++ b/web/src/types/api.generated.ts @@ -21677,6 +21677,25 @@ export interface components { */ username?: string | null; }; + /** @description An access group membership as shown on a user */ + UserAccessGroupDto: { + /** + * Format: uuid + * @description Access group ID + * @example 550e8400-e29b-41d4-a716-446655440000 + */ + id: string; + /** + * @description Access group name + * @example Kids + */ + name: string; + /** + * @description Membership source (manual or oidc) + * @example manual + */ + source: string; + }; /** @description User detail DTO with sharing tag grants (for single user view) */ UserDetailDto: { /** @@ -21727,6 +21746,8 @@ export interface components { }; /** @description User data transfer object */ UserDto: { + /** @description Access groups the user belongs to, sorted by name */ + accessGroups: components["schemas"]["UserAccessGroupDto"][]; /** * Format: date-time * @description Account creation timestamp From 18fdf5f0e537c3452c29b8479f06e655b2e90859 Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Sun, 25 Oct 2026 14:24:17 -0700 Subject: [PATCH 02/16] feat: let one access group be the default that new users join A user with no access group sees the whole library, and there was no way to give new accounts a baseline: every kid account had to be added to its group by hand, and forgetting meant full visibility. An access group can now be marked as the default. Every new account joins it, inside the same transaction as the account itself, so registration, admin-created users and first OIDC login all get the baseline and an account can never exist without it. Joining is a one-off at creation, not a rule applied to everyone: the membership is an ordinary row (source `default`) that an admin can remove, and it stays removed. Making a group default does not touch existing users; a separate action adds everyone who is not yet a member, and it never rewrites an existing membership's source, since turning an OIDC membership into a default one would stop the IdP from removing it. At most one group can be the default. Choosing a new one clears the old one, and a partial unique index refuses a second default on both SQLite and PostgreSQL if two requests race. --- crates/codex-api/src/docs.rs | 2 + .../src/routes/v1/dto/access_group.rs | 24 ++ .../src/routes/v1/handlers/access_groups.rs | 57 +++- .../codex-api/src/routes/v1/routes/admin.rs | 4 + crates/codex-db/src/entities/access_groups.rs | 3 + .../src/entities/user_access_groups.rs | 4 + .../codex-db/src/repositories/access_group.rs | 295 +++++++++++++++++- crates/codex-db/src/repositories/user.rs | 8 +- docs/api/openapi.json | 87 +++++- migration/src/lib.rs | 3 + .../m20261025_000116_access_group_default.rs | 83 +++++ tests/api/access_groups.rs | 198 ++++++++++++ tests/db/access_groups.rs | 62 ++++ tests/db/mod.rs | 1 + web/openapi.json | 87 +++++- web/src/api/accessGroups.ts | 9 + .../access-groups/DefaultGroupCard.test.tsx | 92 ++++++ .../access-groups/DefaultGroupCard.tsx | 123 ++++++++ web/src/pages/settings/AccessGroupDetail.tsx | 13 + .../pages/settings/AccessGroupsSettings.tsx | 5 + web/src/types/api.generated.ts | 86 ++++- 21 files changed, 1238 insertions(+), 8 deletions(-) create mode 100644 migration/src/m20261025_000116_access_group_default.rs create mode 100644 tests/db/access_groups.rs create mode 100644 web/src/components/access-groups/DefaultGroupCard.test.tsx create mode 100644 web/src/components/access-groups/DefaultGroupCard.tsx diff --git a/crates/codex-api/src/docs.rs b/crates/codex-api/src/docs.rs index dd68c4a67..7240c4cce 100644 --- a/crates/codex-api/src/docs.rs +++ b/crates/codex-api/src/docs.rs @@ -599,6 +599,7 @@ The following paths are exempt from rate limiting: v1::handlers::access_groups::update_access_group, v1::handlers::access_groups::delete_access_group, v1::handlers::access_groups::add_access_group_members, + v1::handlers::access_groups::add_all_users_to_access_group, v1::handlers::access_groups::remove_access_group_member, v1::handlers::access_groups::get_user_access_groups, v1::handlers::access_groups::add_access_group_grant, @@ -962,6 +963,7 @@ The following paths are exempt from rate limiting: v1::dto::AccessGroupSummaryDto, v1::dto::CreateAccessGroupRequest, v1::dto::UpdateAccessGroupRequest, + v1::dto::AddAllUsersResponse, v1::dto::AddAccessGroupMembersRequest, v1::dto::AddAccessGroupGrantRequest, v1::dto::AddAccessGroupOidcMappingRequest, diff --git a/crates/codex-api/src/routes/v1/dto/access_group.rs b/crates/codex-api/src/routes/v1/dto/access_group.rs index 203984df1..469614750 100644 --- a/crates/codex-api/src/routes/v1/dto/access_group.rs +++ b/crates/codex-api/src/routes/v1/dto/access_group.rs @@ -23,6 +23,10 @@ pub struct AccessGroupDto { #[schema(example = "Users who can access manga content")] pub description: Option, + /// Whether new users join this group when their account is created + #[schema(example = false)] + pub is_default: bool, + /// Number of members in the group #[schema(example = 5)] pub member_count: u64, @@ -56,6 +60,10 @@ pub struct AccessGroupDetailDto { #[schema(example = "Users who can access manga content")] pub description: Option, + /// Whether new users join this group when their account is created + #[schema(example = false)] + pub is_default: bool, + /// Members of the group pub members: Vec, @@ -157,6 +165,21 @@ pub struct UpdateAccessGroupRequest { /// New description (set to null to remove) #[schema(example = "Updated description")] pub description: Option>, + + /// Make this the default group (clearing any other default), or stop it + /// being the default. Only affects accounts created afterwards. + #[serde(default)] + #[schema(example = true)] + pub is_default: Option, +} + +/// Result of adding every user to an access group +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct AddAllUsersResponse { + /// Users who were not members yet and have been added + #[schema(example = 12)] + pub added: u64, } /// Add members to an access group request @@ -268,6 +291,7 @@ impl AccessGroupDto { id: model.id, name: model.name, description: model.description, + is_default: model.is_default, member_count, grant_count, created_at: model.created_at, diff --git a/crates/codex-api/src/routes/v1/handlers/access_groups.rs b/crates/codex-api/src/routes/v1/handlers/access_groups.rs index bc66f9fc5..c7da2a447 100644 --- a/crates/codex-api/src/routes/v1/handlers/access_groups.rs +++ b/crates/codex-api/src/routes/v1/handlers/access_groups.rs @@ -6,8 +6,9 @@ use super::super::dto::{ AccessGroupDetailDto, AccessGroupDto, AccessGroupGrantDto, AccessGroupMemberDto, AccessGroupOidcMappingDto, AccessGroupSummaryDto, AddAccessGroupGrantRequest, - AddAccessGroupMembersRequest, AddAccessGroupOidcMappingRequest, CreateAccessGroupRequest, - EffectiveGrantDto, EffectiveGrantsResponse, GrantSourceDto, UpdateAccessGroupRequest, + AddAccessGroupMembersRequest, AddAccessGroupOidcMappingRequest, AddAllUsersResponse, + CreateAccessGroupRequest, EffectiveGrantDto, EffectiveGrantsResponse, GrantSourceDto, + UpdateAccessGroupRequest, }; use crate::{ error::ApiError, @@ -200,12 +201,20 @@ pub async fn update_access_group( } } - let group = + let mut group = AccessGroupRepository::update(&state.db, group_id, request.name, request.description) .await .map_err(|e| ApiError::Internal(format!("Failed to update access group: {}", e)))? .ok_or_else(|| ApiError::NotFound("Access group not found".to_string()))?; + if let Some(is_default) = request.is_default + && is_default != group.is_default + { + group = AccessGroupRepository::set_default(&state.db, group_id, is_default) + .await + .map_err(|e| ApiError::Internal(format!("Failed to set default group: {}", e)))?; + } + let member_count = AccessGroupRepository::list_members(&state.db, group.id) .await .map(|m| m.len() as u64) @@ -304,6 +313,47 @@ pub async fn add_access_group_members( Ok(Json(members)) } +/// Add every user who is not yet a member to an access group (admin only) +/// +/// Meant for a group that has just become the default, since the default only +/// applies to accounts created afterwards. Existing memberships keep their +/// source; new ones are recorded as `default`. +#[utoipa::path( + post, + path = "/api/v1/access-groups/{group_id}/add-all-users", + params( + ("group_id" = Uuid, Path, description = "Access group ID") + ), + responses( + (status = 200, description = "Users added", body = AddAllUsersResponse), + (status = 404, description = "Access group not found"), + (status = 403, description = "Forbidden - Missing permission"), + ), + security( + ("jwt_bearer" = []), + ("api_key" = []) + ), + tag = "Access Groups" +)] +pub async fn add_all_users_to_access_group( + State(state): State>, + auth: AuthContext, + Path(group_id): Path, +) -> Result, ApiError> { + auth.require_permission(&Permission::SystemAdmin)?; + + AccessGroupRepository::get_by_id(&state.db, group_id) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch access group: {}", e)))? + .ok_or_else(|| ApiError::NotFound("Access group not found".to_string()))?; + + let added = AccessGroupRepository::add_all_users(&state.db, group_id) + .await + .map_err(|e| ApiError::Internal(format!("Failed to add users: {}", e)))?; + + Ok(Json(AddAllUsersResponse { added })) +} + /// Remove a user from an access group (admin only) #[utoipa::path( delete, @@ -728,6 +778,7 @@ async fn build_group_detail( id: group.id, name: group.name, description: group.description, + is_default: group.is_default, members, grants, oidc_mappings, diff --git a/crates/codex-api/src/routes/v1/routes/admin.rs b/crates/codex-api/src/routes/v1/routes/admin.rs index e96e4fbc3..8d7784c0a 100644 --- a/crates/codex-api/src/routes/v1/routes/admin.rs +++ b/crates/codex-api/src/routes/v1/routes/admin.rs @@ -156,6 +156,10 @@ pub fn routes(_state: Arc) -> Router> { "/access-groups/{group_id}/members", post(handlers::access_groups::add_access_group_members), ) + .route( + "/access-groups/{group_id}/add-all-users", + post(handlers::access_groups::add_all_users_to_access_group), + ) .route( "/access-groups/{group_id}/members/{user_id}", delete(handlers::access_groups::remove_access_group_member), diff --git a/crates/codex-db/src/entities/access_groups.rs b/crates/codex-db/src/entities/access_groups.rs index 1a4d03e5f..6e40d03f6 100644 --- a/crates/codex-db/src/entities/access_groups.rs +++ b/crates/codex-db/src/entities/access_groups.rs @@ -17,6 +17,9 @@ pub struct Model { #[sea_orm(unique)] pub name: String, pub description: Option, + /// New users join the default group when their account is created. + /// At most one group has this set (partial unique index). + pub is_default: bool, pub created_at: DateTime, pub updated_at: DateTime, } diff --git a/crates/codex-db/src/entities/user_access_groups.rs b/crates/codex-db/src/entities/user_access_groups.rs index 6048ca019..d5a984efa 100644 --- a/crates/codex-db/src/entities/user_access_groups.rs +++ b/crates/codex-db/src/entities/user_access_groups.rs @@ -71,6 +71,8 @@ pub enum MembershipSource { Manual, /// Auto-assigned from an OIDC IdP group claim Oidc, + /// Added at account creation because the group was the default group + Default, } impl MembershipSource { @@ -78,6 +80,7 @@ impl MembershipSource { match self { MembershipSource::Manual => "manual", MembershipSource::Oidc => "oidc", + MembershipSource::Default => "default", } } } @@ -89,6 +92,7 @@ impl FromStr for MembershipSource { match s.to_lowercase().as_str() { "manual" => Ok(MembershipSource::Manual), "oidc" => Ok(MembershipSource::Oidc), + "default" => Ok(MembershipSource::Default), _ => Err(format!("Unknown membership source: {}", s)), } } diff --git a/crates/codex-db/src/repositories/access_group.rs b/crates/codex-db/src/repositories/access_group.rs index c2f9ae763..3e0c1f03f 100644 --- a/crates/codex-db/src/repositories/access_group.rs +++ b/crates/codex-db/src/repositories/access_group.rs @@ -8,7 +8,7 @@ use anyhow::Result; use chrono::Utc; use sea_orm::{ ActiveModelTrait, ColumnTrait, ConnectionTrait, DatabaseConnection, EntityTrait, QueryFilter, - QueryOrder, Set, TransactionTrait, + QueryOrder, Set, TransactionTrait, sea_query::Expr, }; use std::collections::{HashMap, HashSet}; use uuid::Uuid; @@ -34,6 +34,7 @@ impl AccessGroupRepository { id: Set(Uuid::new_v4()), name: Set(name.trim().to_string()), description: Set(description), + is_default: Set(false), created_at: Set(now), updated_at: Set(now), }; @@ -194,6 +195,126 @@ impl AccessGroupRepository { .await?) } + // ==================== Default group ==================== + + /// The group new users join at account creation, if one is set. + pub async fn get_default(db: &impl ConnectionTrait) -> Result> { + Ok(access_groups::Entity::find() + .filter(access_groups::Column::IsDefault.eq(true)) + .one(db) + .await?) + } + + /// Make a group the default, or stop it being the default. + /// + /// Setting a new default clears the previous one in the same transaction. + /// Neither direction changes any membership: the default only applies to + /// accounts created afterwards (see [`Self::add_all_users`] for the rest). + pub async fn set_default( + db: &DatabaseConnection, + group_id: Uuid, + is_default: bool, + ) -> Result { + let txn = db.begin().await?; + let group = access_groups::Entity::find_by_id(group_id) + .one(&txn) + .await? + .ok_or_else(|| anyhow::anyhow!("Access group not found"))?; + + if is_default { + access_groups::Entity::update_many() + .col_expr(access_groups::Column::IsDefault, Expr::value(false)) + .filter(access_groups::Column::IsDefault.eq(true)) + .filter(access_groups::Column::Id.ne(group_id)) + .exec(&txn) + .await?; + } + + let mut active: access_groups::ActiveModel = group.into(); + active.is_default = Set(is_default); + active.updated_at = Set(Utc::now()); + let updated = active.update(&txn).await?; + txn.commit().await?; + Ok(updated) + } + + /// Add a user to the default group, if there is one. + /// + /// Called inside the user-creation transaction so every way of creating + /// an account gets the same baseline. + pub async fn join_default_group(db: &impl ConnectionTrait, user_id: Uuid) -> Result<()> { + if let Some(group) = Self::get_default(db).await? { + Self::add_member_if_absent(db, group.id, user_id, MembershipSource::Default).await?; + } + Ok(()) + } + + /// Add every user who is not yet a member, with source `default`. + /// Returns how many users were added. + pub async fn add_all_users(db: &DatabaseConnection, group_id: Uuid) -> Result { + let txn = db.begin().await?; + let existing: HashSet = user_access_groups::Entity::find() + .filter(user_access_groups::Column::AccessGroupId.eq(group_id)) + .all(&txn) + .await? + .into_iter() + .map(|m| m.user_id) + .collect(); + let user_ids: Vec = users::Entity::find() + .all(&txn) + .await? + .into_iter() + .map(|u| u.id) + .filter(|id| !existing.contains(id)) + .collect(); + + let now = Utc::now(); + for user_id in &user_ids { + user_access_groups::ActiveModel { + id: Set(Uuid::new_v4()), + user_id: Set(*user_id), + access_group_id: Set(group_id), + source: Set(MembershipSource::Default.as_str().to_string()), + created_at: Set(now), + } + .insert(&txn) + .await?; + } + txn.commit().await?; + Ok(user_ids.len() as u64) + } + + /// Insert a membership unless the user is already in the group. + /// + /// Unlike [`Self::add_member`], this never rewrites the source of an + /// existing row. Turning an `oidc` membership into `default` would stop + /// the IdP from ever removing it. Returns true if a row was inserted. + pub async fn add_member_if_absent( + db: &impl ConnectionTrait, + group_id: Uuid, + user_id: Uuid, + source: MembershipSource, + ) -> Result { + let existing = user_access_groups::Entity::find() + .filter(user_access_groups::Column::UserId.eq(user_id)) + .filter(user_access_groups::Column::AccessGroupId.eq(group_id)) + .one(db) + .await?; + if existing.is_some() { + return Ok(false); + } + user_access_groups::ActiveModel { + id: Set(Uuid::new_v4()), + user_id: Set(user_id), + access_group_id: Set(group_id), + source: Set(source.as_str().to_string()), + created_at: Set(Utc::now()), + } + .insert(db) + .await?; + Ok(true) + } + /// Group memberships for many users in two queries, keyed by user ID. /// /// Each user's groups are sorted by name and paired with the membership @@ -450,6 +571,178 @@ mod tests { UserRepository::create(db, &model).await.unwrap() } + // ---------- Default group ---------- + + #[tokio::test] + async fn set_default_keeps_a_single_default() { + let db = setup_test_db().await; + let a = AccessGroupRepository::create(&db, "A", None).await.unwrap(); + let b = AccessGroupRepository::create(&db, "B", None).await.unwrap(); + assert!(!a.is_default); + assert!( + AccessGroupRepository::get_default(&db) + .await + .unwrap() + .is_none() + ); + + AccessGroupRepository::set_default(&db, a.id, true) + .await + .unwrap(); + assert_eq!( + AccessGroupRepository::get_default(&db) + .await + .unwrap() + .unwrap() + .id, + a.id + ); + + // Moving the default clears it from the previous group + AccessGroupRepository::set_default(&db, b.id, true) + .await + .unwrap(); + assert_eq!( + AccessGroupRepository::get_default(&db) + .await + .unwrap() + .unwrap() + .id, + b.id + ); + let a = AccessGroupRepository::get_by_id(&db, a.id) + .await + .unwrap() + .unwrap(); + assert!(!a.is_default); + + AccessGroupRepository::set_default(&db, b.id, false) + .await + .unwrap(); + assert!( + AccessGroupRepository::get_default(&db) + .await + .unwrap() + .is_none() + ); + } + + /// The partial unique index is the backstop if two requests race. + #[tokio::test] + async fn database_rejects_a_second_default() { + use sea_orm::ActiveModelTrait; + + let db = setup_test_db().await; + let a = AccessGroupRepository::create(&db, "A", None).await.unwrap(); + let b = AccessGroupRepository::create(&db, "B", None).await.unwrap(); + AccessGroupRepository::set_default(&db, a.id, true) + .await + .unwrap(); + + let mut active: access_groups::ActiveModel = b.into(); + active.is_default = Set(true); + assert!(active.update(&db).await.is_err()); + } + + #[tokio::test] + async fn new_user_joins_the_default_group() { + let db = setup_test_db().await; + let kids = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + AccessGroupRepository::set_default(&db, kids.id, true) + .await + .unwrap(); + + let user = create_test_user(&db, "newbie").await; + + let members = AccessGroupRepository::list_members(&db, kids.id) + .await + .unwrap(); + assert_eq!(members.len(), 1); + assert_eq!(members[0].user_id, user.id); + assert_eq!(members[0].source, "default"); + } + + #[tokio::test] + async fn new_user_without_a_default_group_joins_nothing() { + let db = setup_test_db().await; + AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + + let user = create_test_user(&db, "newbie").await; + + assert!( + AccessGroupRepository::list_for_user(&db, user.id) + .await + .unwrap() + .is_empty() + ); + } + + /// Joining at creation is a one-off: once removed, the user stays out, + /// and making a group default later does not pull existing users in. + #[tokio::test] + async fn default_membership_is_removable_and_not_retroactive() { + let db = setup_test_db().await; + let existing = create_test_user(&db, "existing").await; + let kids = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + AccessGroupRepository::set_default(&db, kids.id, true) + .await + .unwrap(); + assert!( + AccessGroupRepository::list_for_user(&db, existing.id) + .await + .unwrap() + .is_empty() + ); + + let newbie = create_test_user(&db, "newbie").await; + AccessGroupRepository::remove_member(&db, kids.id, newbie.id) + .await + .unwrap(); + assert!( + AccessGroupRepository::list_for_user(&db, newbie.id) + .await + .unwrap() + .is_empty() + ); + } + + #[tokio::test] + async fn add_all_users_keeps_existing_memberships_as_they_are() { + let db = setup_test_db().await; + let group = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + let via_oidc = create_test_user(&db, "via_oidc").await; + let outsider = create_test_user(&db, "outsider").await; + AccessGroupRepository::add_member(&db, group.id, via_oidc.id, MembershipSource::Oidc) + .await + .unwrap(); + + let added = AccessGroupRepository::add_all_users(&db, group.id) + .await + .unwrap(); + assert_eq!(added, 1); + + let members = AccessGroupRepository::list_members(&db, group.id) + .await + .unwrap(); + let source_of = |id: Uuid| { + members + .iter() + .find(|m| m.user_id == id) + .map(|m| m.source.clone()) + }; + // An OIDC membership must stay OIDC, or the IdP could no longer remove it + assert_eq!(source_of(via_oidc.id).as_deref(), Some("oidc")); + assert_eq!(source_of(outsider.id).as_deref(), Some("default")); + } + // ---------- Group CRUD ---------- #[tokio::test] diff --git a/crates/codex-db/src/repositories/user.rs b/crates/codex-db/src/repositories/user.rs index d27175167..bc4352212 100644 --- a/crates/codex-db/src/repositories/user.rs +++ b/crates/codex-db/src/repositories/user.rs @@ -1,4 +1,5 @@ use crate::entities::{sharing_tags, user_sharing_tags, users, users::Entity as User}; +use crate::repositories::AccessGroupRepository; use crate::trace::db_system_str; use anyhow::Result; use chrono::Utc; @@ -53,7 +54,12 @@ impl UserRepository { last_login_at: Set(model.last_login_at), }; - let result = active_model.insert(db).await?; + // The default-group membership is written in the same transaction, so + // an account can never exist without the baseline it was meant to get. + let txn = db.begin().await?; + let result = active_model.insert(&txn).await?; + AccessGroupRepository::join_default_group(&txn, result.id).await?; + txn.commit().await?; Ok(result) } diff --git a/docs/api/openapi.json b/docs/api/openapi.json index 0bbdac19d..806c148d1 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -231,6 +231,54 @@ ] } }, + "/api/v1/access-groups/{group_id}/add-all-users": { + "post": { + "tags": [ + "Access Groups" + ], + "summary": "Add every user who is not yet a member to an access group (admin only)", + "description": "Meant for a group that has just become the default, since the default only\napplies to accounts created afterwards. Existing memberships keep their\nsource; new ones are recorded as `default`.", + "operationId": "add_all_users_to_access_group", + "parameters": [ + { + "name": "group_id", + "in": "path", + "description": "Access group ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "Users added", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AddAllUsersResponse" + } + } + } + }, + "403": { + "description": "Forbidden - Missing permission" + }, + "404": { + "description": "Access group not found" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + } + }, "/api/v1/access-groups/{group_id}/grants": { "post": { "tags": [ @@ -23542,6 +23590,7 @@ "required": [ "id", "name", + "isDefault", "members", "grants", "oidcMappings", @@ -23576,6 +23625,11 @@ "description": "Unique access group identifier", "example": "550e8400-e29b-41d4-a716-446655440000" }, + "isDefault": { + "type": "boolean", + "description": "Whether new users join this group when their account is created", + "example": false + }, "members": { "type": "array", "items": { @@ -23609,6 +23663,7 @@ "required": [ "id", "name", + "isDefault", "memberCount", "grantCount", "createdAt", @@ -23642,6 +23697,11 @@ "description": "Unique access group identifier", "example": "550e8400-e29b-41d4-a716-446655440000" }, + "isDefault": { + "type": "boolean", + "description": "Whether new users join this group when their account is created", + "example": false + }, "memberCount": { "type": "integer", "format": "int64", @@ -23848,6 +23908,22 @@ } } }, + "AddAllUsersResponse": { + "type": "object", + "description": "Result of adding every user to an access group", + "required": [ + "added" + ], + "properties": { + "added": { + "type": "integer", + "format": "int64", + "description": "Users who were not members yet and have been added", + "example": 12, + "minimum": 0 + } + } + }, "AddBookGenreRequest": { "type": "object", "description": "Request to add a single genre to a book", @@ -35166,7 +35242,8 @@ "description": "Provenance of a group membership", "enum": [ "manual", - "oidc" + "oidc", + "default" ] }, "MessageResponse": { @@ -46465,6 +46542,14 @@ "description": "New description (set to null to remove)", "example": "Updated description" }, + "isDefault": { + "type": [ + "boolean", + "null" + ], + "description": "Make this the default group (clearing any other default), or stop it\nbeing the default. Only affects accounts created afterwards.", + "example": true + }, "name": { "type": [ "string", diff --git a/migration/src/lib.rs b/migration/src/lib.rs index 709f71572..44268051f 100644 --- a/migration/src/lib.rs +++ b/migration/src/lib.rs @@ -217,6 +217,7 @@ mod m20260826_000112_create_user_series_reader_settings; mod m20260828_000113_relax_renumber_series_task_dedup; mod m20260927_000114_reading_history_survives_book_delete; mod m20261018_000115_create_news; +mod m20261025_000116_access_group_default; pub struct Migrator; @@ -421,6 +422,8 @@ impl MigratorTrait for Migrator { Box::new(m20260927_000114_reading_history_survives_book_delete::Migration), // News for admins, imported from the Codex release feeds. Box::new(m20261018_000115_create_news::Migration), + // At most one default access group, which new users join. + Box::new(m20261025_000116_access_group_default::Migration), ] } } diff --git a/migration/src/m20261025_000116_access_group_default.rs b/migration/src/m20261025_000116_access_group_default.rs new file mode 100644 index 000000000..a0222f6a1 --- /dev/null +++ b/migration/src/m20261025_000116_access_group_default.rs @@ -0,0 +1,83 @@ +//! Default access group. +//! +//! At most one access group may be the default. New users are added to it +//! when their account is created (membership source `default`, which fits the +//! existing `varchar(10)` source column). The membership is an ordinary row an +//! admin can remove later, not a rule that applies to everyone. +//! +//! "At most one" is a partial unique index over the `true` rows, which both +//! SQLite and PostgreSQL support. SeaQuery's index builder has no partial +//! form, so it is raw SQL; the statement is identical on both backends. + +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +const SINGLE_DEFAULT_INDEX: &str = "idx_access_groups_single_default"; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .alter_table( + Table::alter() + .table(AccessGroups::Table) + .add_column( + ColumnDef::new(AccessGroups::IsDefault) + .boolean() + .not_null() + .default(false), + ) + .to_owned(), + ) + .await?; + + manager + .get_connection() + .execute_unprepared(&format!( + "CREATE UNIQUE INDEX {SINGLE_DEFAULT_INDEX} \ + ON access_groups (is_default) WHERE is_default" + )) + .await?; + + Ok(()) + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .drop_index( + Index::drop() + .name(SINGLE_DEFAULT_INDEX) + .table(AccessGroups::Table) + .to_owned(), + ) + .await?; + + // Default memberships stay, as manual ones: the user is still in the + // group, only the reason is gone. + manager + .get_connection() + .execute_unprepared( + "UPDATE user_access_groups SET source = 'manual' WHERE source = 'default'", + ) + .await?; + + manager + .alter_table( + Table::alter() + .table(AccessGroups::Table) + .drop_column(AccessGroups::IsDefault) + .to_owned(), + ) + .await?; + + Ok(()) + } +} + +#[derive(DeriveIden)] +enum AccessGroups { + Table, + IsDefault, +} diff --git a/tests/api/access_groups.rs b/tests/api/access_groups.rs index 8f5dae2fa..4c6df5d12 100644 --- a/tests/api/access_groups.rs +++ b/tests/api/access_groups.rs @@ -173,6 +173,7 @@ async fn test_update_access_group() { let update = UpdateAccessGroupRequest { name: Some("New Name".to_string()), description: Some(Some("Now with description".to_string())), + is_default: None, }; let request = patch_json_request_with_auth( &format!("/api/v1/access-groups/{}", group.id), @@ -853,3 +854,200 @@ async fn test_list_access_groups_with_counts() { assert_eq!(groups[0].member_count, 1); assert_eq!(groups[0].grant_count, 1); } + +// ============================================================================ +// Default Group Tests +// ============================================================================ + +async fn create_group( + state: &std::sync::Arc, + token: &str, + name: &str, +) -> AccessGroupDto { + let app = create_test_router(state.clone()).await; + let body = CreateAccessGroupRequest { + name: name.to_string(), + description: None, + }; + let request = post_json_request_with_auth("/api/v1/access-groups", &body, token); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::CREATED); + response.unwrap() +} + +async fn set_default( + state: &std::sync::Arc, + token: &str, + group_id: uuid::Uuid, + is_default: bool, +) -> AccessGroupDto { + let app = create_test_router(state.clone()).await; + let request = patch_json_request_with_auth( + &format!("/api/v1/access-groups/{}", group_id), + &serde_json::json!({ "isDefault": is_default }), + token, + ); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + response.unwrap() +} + +async fn group_members( + state: &std::sync::Arc, + token: &str, + group_id: uuid::Uuid, +) -> AccessGroupDetailDto { + let app = create_test_router(state.clone()).await; + let request = get_request_with_auth(&format!("/api/v1/access-groups/{}", group_id), token); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + response.unwrap() +} + +#[tokio::test] +async fn test_mark_group_default_moves_the_default() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let kids = create_group(&state, &token, "Kids").await; + let teens = create_group(&state, &token, "Teens").await; + assert!(!kids.is_default); + + assert!(set_default(&state, &token, kids.id, true).await.is_default); + assert!(set_default(&state, &token, teens.id, true).await.is_default); + + assert!(!group_members(&state, &token, kids.id).await.is_default); + assert!(group_members(&state, &token, teens.id).await.is_default); + + assert!( + !set_default(&state, &token, teens.id, false) + .await + .is_default + ); +} + +#[tokio::test] +async fn test_admin_created_user_joins_default_group() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let kids = create_group(&state, &token, "Kids").await; + set_default(&state, &token, kids.id, true).await; + + let app = create_test_router(state.clone()).await; + let request = post_json_request_with_auth( + "/api/v1/users", + &serde_json::json!({ + "username": "kid", + "email": "kid@example.com", + "password": "securepassword123" + }), + &token, + ); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + let created = response.unwrap(); + assert_eq!(created["accessGroups"][0]["name"], "Kids"); + assert_eq!(created["accessGroups"][0]["source"], "default"); + + let detail = group_members(&state, &token, kids.id).await; + assert_eq!(detail.members.len(), 1); + assert_eq!(detail.members[0].username, "kid"); +} + +#[tokio::test] +async fn test_registered_user_joins_default_group() { + use codex::db::repositories::SettingsRepository; + + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (admin_id, token) = create_admin_and_token(&db, &state).await; + let kids = create_group(&state, &token, "Kids").await; + set_default(&state, &token, kids.id, true).await; + SettingsRepository::set( + &db, + "auth.registration_enabled", + "true".to_string(), + admin_id, + None, + None, + ) + .await + .unwrap(); + + let app = create_test_router(state.clone()).await; + let request = post_json_request( + "/api/v1/auth/register", + &serde_json::json!({ + "username": "kid", + "email": "kid@example.com", + "password": "securepassword123" + }), + ); + let (status, _): (StatusCode, Option) = + make_json_request(app, request).await; + assert!(status.is_success(), "registration failed: {status}"); + + let detail = group_members(&state, &token, kids.id).await; + assert_eq!(detail.members.len(), 1); + assert_eq!(detail.members[0].username, "kid"); + assert_eq!(detail.members[0].source, "default"); +} + +#[tokio::test] +async fn test_add_all_users_to_group() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + create_reader_and_token(&db, &state, "alice").await; + create_reader_and_token(&db, &state, "bob").await; + let kids = create_group(&state, &token, "Kids").await; + + let app = create_test_router(state.clone()).await; + let request = post_request_with_auth( + &format!("/api/v1/access-groups/{}/add-all-users", kids.id), + &token, + ); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + // The admin, alice and bob + assert_eq!(response.unwrap()["added"], 3); + + // Running it again adds nobody + let app = create_test_router(state.clone()).await; + let request = post_request_with_auth( + &format!("/api/v1/access-groups/{}/add-all-users", kids.id), + &token, + ); + let (_, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(response.unwrap()["added"], 0); + + assert_eq!( + group_members(&state, &token, kids.id).await.members.len(), + 3 + ); +} + +#[tokio::test] +async fn test_add_all_users_requires_admin() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let (_, reader_token) = create_reader_and_token(&db, &state, "alice").await; + let kids = create_group(&state, &token, "Kids").await; + + let app = create_test_router(state).await; + let request = post_request_with_auth( + &format!("/api/v1/access-groups/{}/add-all-users", kids.id), + &reader_token, + ); + let (status, _): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::FORBIDDEN); +} diff --git a/tests/db/access_groups.rs b/tests/db/access_groups.rs new file mode 100644 index 000000000..310439d06 --- /dev/null +++ b/tests/db/access_groups.rs @@ -0,0 +1,62 @@ +#[path = "../common/mod.rs"] +mod common; + +// The single-default rule for access groups is a partial unique index written +// as raw SQL, so it is checked on both backends rather than trusted to SeaQuery. + +use codex::db::entities::access_groups; +use codex::db::repositories::AccessGroupRepository; +use common::*; +use sea_orm::{ActiveModelTrait, DatabaseConnection, Set}; +use uuid::Uuid; + +async fn exercise_single_default(db: &DatabaseConnection) { + let suffix = Uuid::new_v4(); + let a = AccessGroupRepository::create(db, &format!("Default A {suffix}"), None) + .await + .unwrap(); + let b = AccessGroupRepository::create(db, &format!("Default B {suffix}"), None) + .await + .unwrap(); + + AccessGroupRepository::set_default(db, a.id, true) + .await + .unwrap(); + + // Bypassing set_default, a second default must be refused by the index + let mut active: access_groups::ActiveModel = b.clone().into(); + active.is_default = Set(true); + let second = active.update(db).await; + + // Non-default rows are unconstrained + AccessGroupRepository::set_default(db, a.id, false) + .await + .unwrap(); + let c = AccessGroupRepository::create(db, &format!("Default C {suffix}"), None) + .await + .unwrap(); + + // Clean up before asserting: on the shared PostgreSQL database a default + // left behind would pull other tests' new users into it. + for id in [a.id, b.id, c.id] { + AccessGroupRepository::delete(db, id).await.unwrap(); + } + + assert!(second.is_err(), "a second default group was accepted"); +} + +#[tokio::test] +async fn test_single_default_group_sqlite() { + let (db, _t) = setup_test_db().await; + exercise_single_default(&db).await; +} + +#[tokio::test] +#[ignore] // Requires PostgreSQL test database +async fn test_single_default_group_postgres() { + let Some(db) = setup_test_db_postgres().await else { + eprintln!("PostgreSQL test database not available, skipping"); + return; + }; + exercise_single_default(&db).await; +} diff --git a/tests/db/mod.rs b/tests/db/mod.rs index 1546c88dc..79100e2fd 100644 --- a/tests/db/mod.rs +++ b/tests/db/mod.rs @@ -2,6 +2,7 @@ // // Tests repositories, authentication utilities, and database operations. +mod access_groups; mod auth; mod book_duplicates; mod collections; diff --git a/web/openapi.json b/web/openapi.json index 0bbdac19d..806c148d1 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -231,6 +231,54 @@ ] } }, + "/api/v1/access-groups/{group_id}/add-all-users": { + "post": { + "tags": [ + "Access Groups" + ], + "summary": "Add every user who is not yet a member to an access group (admin only)", + "description": "Meant for a group that has just become the default, since the default only\napplies to accounts created afterwards. Existing memberships keep their\nsource; new ones are recorded as `default`.", + "operationId": "add_all_users_to_access_group", + "parameters": [ + { + "name": "group_id", + "in": "path", + "description": "Access group ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "Users added", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AddAllUsersResponse" + } + } + } + }, + "403": { + "description": "Forbidden - Missing permission" + }, + "404": { + "description": "Access group not found" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + } + }, "/api/v1/access-groups/{group_id}/grants": { "post": { "tags": [ @@ -23542,6 +23590,7 @@ "required": [ "id", "name", + "isDefault", "members", "grants", "oidcMappings", @@ -23576,6 +23625,11 @@ "description": "Unique access group identifier", "example": "550e8400-e29b-41d4-a716-446655440000" }, + "isDefault": { + "type": "boolean", + "description": "Whether new users join this group when their account is created", + "example": false + }, "members": { "type": "array", "items": { @@ -23609,6 +23663,7 @@ "required": [ "id", "name", + "isDefault", "memberCount", "grantCount", "createdAt", @@ -23642,6 +23697,11 @@ "description": "Unique access group identifier", "example": "550e8400-e29b-41d4-a716-446655440000" }, + "isDefault": { + "type": "boolean", + "description": "Whether new users join this group when their account is created", + "example": false + }, "memberCount": { "type": "integer", "format": "int64", @@ -23848,6 +23908,22 @@ } } }, + "AddAllUsersResponse": { + "type": "object", + "description": "Result of adding every user to an access group", + "required": [ + "added" + ], + "properties": { + "added": { + "type": "integer", + "format": "int64", + "description": "Users who were not members yet and have been added", + "example": 12, + "minimum": 0 + } + } + }, "AddBookGenreRequest": { "type": "object", "description": "Request to add a single genre to a book", @@ -35166,7 +35242,8 @@ "description": "Provenance of a group membership", "enum": [ "manual", - "oidc" + "oidc", + "default" ] }, "MessageResponse": { @@ -46465,6 +46542,14 @@ "description": "New description (set to null to remove)", "example": "Updated description" }, + "isDefault": { + "type": [ + "boolean", + "null" + ], + "description": "Make this the default group (clearing any other default), or stop it\nbeing the default. Only affects accounts created afterwards.", + "example": true + }, "name": { "type": [ "string", diff --git a/web/src/api/accessGroups.ts b/web/src/api/accessGroups.ts index 979610896..745c87a50 100644 --- a/web/src/api/accessGroups.ts +++ b/web/src/api/accessGroups.ts @@ -26,6 +26,7 @@ export type EffectiveGrantsResponse = components["schemas"]["EffectiveGrantsResponse"]; export type EffectiveGrantDto = components["schemas"]["EffectiveGrantDto"]; export type GrantSourceDto = components["schemas"]["GrantSourceDto"]; +export type AddAllUsersResponse = components["schemas"]["AddAllUsersResponse"]; export const accessGroupsApi = { // ============================================ @@ -85,6 +86,14 @@ export const accessGroupsApi = { await api.delete(`/access-groups/${groupId}/members/${userId}`); }, + /** Add every user who is not yet a member (existing memberships unchanged). */ + addAllUsers: async (groupId: string): Promise => { + const response = await api.post( + `/access-groups/${groupId}/add-all-users`, + ); + return response.data; + }, + // ============================================ // Grants // ============================================ diff --git a/web/src/components/access-groups/DefaultGroupCard.test.tsx b/web/src/components/access-groups/DefaultGroupCard.test.tsx new file mode 100644 index 000000000..6e26f32c3 --- /dev/null +++ b/web/src/components/access-groups/DefaultGroupCard.test.tsx @@ -0,0 +1,92 @@ +import { notifications } from "@mantine/notifications"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { AccessGroupDetailDto } from "@/api/accessGroups"; +import { accessGroupsApi } from "@/api/accessGroups"; +import { renderWithProviders, screen, userEvent, waitFor } from "@/test/utils"; +import { DefaultGroupCard } from "./DefaultGroupCard"; + +vi.mock("@mantine/notifications", () => ({ + notifications: { show: vi.fn() }, +})); + +vi.mock("@/api/accessGroups", () => ({ + accessGroupsApi: { + update: vi.fn(), + addAllUsers: vi.fn(), + }, +})); + +const makeGroup = ( + overrides: Partial = {}, +): AccessGroupDetailDto => ({ + id: "group-1", + name: "Kids", + description: null, + isDefault: false, + members: [], + grants: [], + oidcMappings: [], + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + ...overrides, +}); + +describe("DefaultGroupCard", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("reflects whether the group is the default", () => { + renderWithProviders( + , + ); + + expect( + screen.getByRole("switch", { name: /default group for new users/i }), + ).toBeChecked(); + }); + + it("makes the group the default when switched on", async () => { + const user = userEvent.setup(); + vi.mocked(accessGroupsApi.update).mockResolvedValue({ + ...makeGroup({ isDefault: true }), + memberCount: 0, + grantCount: 0, + }); + renderWithProviders(); + + await user.click( + screen.getByRole("switch", { name: /default group for new users/i }), + ); + + await waitFor(() => + expect(accessGroupsApi.update).toHaveBeenCalledWith("group-1", { + isDefault: true, + }), + ); + }); + + it("adds all existing users only after confirmation", async () => { + const user = userEvent.setup(); + vi.mocked(accessGroupsApi.addAllUsers).mockResolvedValue({ added: 4 }); + renderWithProviders(); + + await user.click( + screen.getByRole("button", { name: /add all existing users/i }), + ); + expect(accessGroupsApi.addAllUsers).not.toHaveBeenCalled(); + + await user.click( + await screen.findByRole("button", { name: /^add all users$/i }), + ); + + await waitFor(() => + expect(accessGroupsApi.addAllUsers).toHaveBeenCalledWith("group-1"), + ); + await waitFor(() => + expect(notifications.show).toHaveBeenCalledWith( + expect.objectContaining({ message: "Added 4 users" }), + ), + ); + }); +}); diff --git a/web/src/components/access-groups/DefaultGroupCard.tsx b/web/src/components/access-groups/DefaultGroupCard.tsx new file mode 100644 index 000000000..ee2883761 --- /dev/null +++ b/web/src/components/access-groups/DefaultGroupCard.tsx @@ -0,0 +1,123 @@ +import { Button, Card, Group, Modal, Stack, Switch, Text } from "@mantine/core"; +import { useDisclosure } from "@mantine/hooks"; +import { notifications } from "@mantine/notifications"; +import { IconUserPlus } from "@tabler/icons-react"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { type AccessGroupDetailDto, accessGroupsApi } from "@/api/accessGroups"; + +interface DefaultGroupCardProps { + group: AccessGroupDetailDto; +} + +/** + * Default-group controls for an access group: the "new users join this group" + * switch, and a one-off action to add everyone who already has an account. + */ +export function DefaultGroupCard({ group }: DefaultGroupCardProps) { + const queryClient = useQueryClient(); + const [confirmOpen, { open: openConfirm, close: closeConfirm }] = + useDisclosure(false); + + const invalidate = () => { + queryClient.invalidateQueries({ queryKey: ["access-group", group.id] }); + queryClient.invalidateQueries({ queryKey: ["access-groups"] }); + queryClient.invalidateQueries({ queryKey: ["users"] }); + }; + + const setDefaultMutation = useMutation({ + mutationFn: (isDefault: boolean) => + accessGroupsApi.update(group.id, { isDefault }), + onSuccess: (_, isDefault) => { + invalidate(); + notifications.show({ + title: "Success", + message: isDefault + ? `New users will join ${group.name}` + : `${group.name} is no longer the default group`, + color: "green", + }); + }, + onError: (error: { message?: string }) => { + notifications.show({ + title: "Error", + message: error.message || "Failed to update the default group", + color: "red", + }); + }, + }); + + const addAllMutation = useMutation({ + mutationFn: () => accessGroupsApi.addAllUsers(group.id), + onSuccess: ({ added }) => { + invalidate(); + closeConfirm(); + notifications.show({ + title: "Success", + message: `Added ${added} ${added === 1 ? "user" : "users"}`, + color: "green", + }); + }, + onError: (error: { message?: string }) => { + notifications.show({ + title: "Error", + message: error.message || "Failed to add users", + color: "red", + }); + }, + }); + + return ( + + + + setDefaultMutation.mutate(event.currentTarget.checked) + } + disabled={setDefaultMutation.isPending} + /> + + + + The default only applies to accounts created from now on. To bring + existing users in, add them all at once. + + + + + + + + + Every user who is not already a member will be added to this group, + including administrators. Existing memberships are left as they are. + + + + + + + + + ); +} diff --git a/web/src/pages/settings/AccessGroupDetail.tsx b/web/src/pages/settings/AccessGroupDetail.tsx index dc29f5959..36439a74a 100644 --- a/web/src/pages/settings/AccessGroupDetail.tsx +++ b/web/src/pages/settings/AccessGroupDetail.tsx @@ -31,6 +31,7 @@ import { Link, useParams } from "react-router-dom"; import { type AccessGroupDetailDto, accessGroupsApi } from "@/api/accessGroups"; import { type AccessMode, sharingTagsApi } from "@/api/sharingTags"; import { usersApi } from "@/api/users"; +import { DefaultGroupCard } from "@/components/access-groups/DefaultGroupCard"; export function AccessGroupDetail() { const { groupId } = useParams<{ groupId: string }>(); @@ -86,6 +87,11 @@ export function AccessGroupDetail() { {group.name} + {group.isDefault && ( + + Default + + )} {group.description && ( @@ -95,6 +101,8 @@ export function AccessGroupDetail() { + + {/* Members Section */} )} + {member.source === "default" && ( + + Default + + )} {group.name} + {group.isDefault && ( + + Default + + )} ), }, diff --git a/web/src/types/api.generated.ts b/web/src/types/api.generated.ts index 2f77e488b..350ded8cd 100644 --- a/web/src/types/api.generated.ts +++ b/web/src/types/api.generated.ts @@ -41,6 +41,28 @@ export interface paths { patch: operations["update_access_group"]; trace?: never; }; + "/api/v1/access-groups/{group_id}/add-all-users": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** + * Add every user who is not yet a member to an access group (admin only) + * @description Meant for a group that has just become the default, since the default only + * applies to accounts created afterwards. Existing memberships keep their + * source; new ones are recorded as `default`. + */ + post: operations["add_all_users_to_access_group"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/access-groups/{group_id}/grants": { parameters: { query?: never; @@ -8313,6 +8335,11 @@ export interface components { * @example 550e8400-e29b-41d4-a716-446655440000 */ id: string; + /** + * @description Whether new users join this group when their account is created + * @example false + */ + isDefault: boolean; /** @description Members of the group */ members: components["schemas"]["AccessGroupMemberDto"][]; /** @@ -8354,6 +8381,11 @@ export interface components { * @example 550e8400-e29b-41d4-a716-446655440000 */ id: string; + /** + * @description Whether new users join this group when their account is created + * @example false + */ + isDefault: boolean; /** * Format: int64 * @description Number of members in the group @@ -8492,6 +8524,15 @@ export interface components { */ oidcGroupName: string; }; + /** @description Result of adding every user to an access group */ + AddAllUsersResponse: { + /** + * Format: int64 + * @description Users who were not members yet and have been added + * @example 12 + */ + added: number; + }; /** @description Request to add a single genre to a book */ AddBookGenreRequest: { /** @@ -14682,7 +14723,7 @@ export interface components { * @description Provenance of a group membership * @enum {string} */ - MembershipSource: "manual" | "oidc"; + MembershipSource: "manual" | "oidc" | "default"; MessageResponse: { /** * @description Response message @@ -21132,6 +21173,12 @@ export interface components { * @example Updated description */ description?: string | null; + /** + * @description Make this the default group (clearing any other default), or stop it + * being the default. Only affects accounts created afterwards. + * @example true + */ + isDefault?: boolean | null; /** * @description New display name (must be unique) * @example Manga & Comic Readers @@ -22451,6 +22498,43 @@ export interface operations { }; }; }; + add_all_users_to_access_group: { + parameters: { + query?: never; + header?: never; + path: { + /** @description Access group ID */ + group_id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Users added */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["AddAllUsersResponse"]; + }; + }; + /** @description Forbidden - Missing permission */ + 403: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description Access group not found */ + 404: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; add_access_group_grant: { parameters: { query?: never; From 1f914eff805aa0cb384a065d3271e103befd900b Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Sun, 25 Oct 2026 16:04:41 -0700 Subject: [PATCH 03/16] fix(db): keep a group membership's source when it is added again Adding a user to an access group they were already in rewrote the membership's source. An admin adding someone the IdP had already placed turned the membership into a manual one, which the IdP can never remove, so leaving the IdP group no longer took them out. The reverse happened on login: an IdP group mapping to a group an admin had assigned turned that membership into an OIDC one, and leaving the IdP group then removed what the admin had set up. Whoever adds the user first now owns the membership, and adding them again from another source leaves the row as it is. --- .../codex-db/src/repositories/access_group.rs | 124 ++++++++++++------ 1 file changed, 81 insertions(+), 43 deletions(-) diff --git a/crates/codex-db/src/repositories/access_group.rs b/crates/codex-db/src/repositories/access_group.rs index 3e0c1f03f..7a5350d0d 100644 --- a/crates/codex-db/src/repositories/access_group.rs +++ b/crates/codex-db/src/repositories/access_group.rs @@ -101,8 +101,13 @@ impl AccessGroupRepository { // ==================== Memberships ==================== - /// Add a user to a group. Idempotent: if the membership already exists - /// returns the existing row. Updates the `source` if it differs. + /// Add a user to a group. Idempotent: if the membership already exists, + /// the existing row is returned unchanged. + /// + /// The source of an existing row is never rewritten. Whoever added the + /// user first owns the membership: the IdP can only remove `oidc` rows, so + /// turning one into `manual` would make it permanent, and turning a manual + /// one into `oidc` would let the IdP remove what an admin assigned. pub async fn add_member( db: &impl ConnectionTrait, group_id: Uuid, @@ -115,12 +120,7 @@ impl AccessGroupRepository { .one(db) .await?; if let Some(row) = existing { - if row.source == source.as_str() { - return Ok(row); - } - let mut active: user_access_groups::ActiveModel = row.into(); - active.source = Set(source.as_str().to_string()); - return Ok(active.update(db).await?); + return Ok(row); } let active = user_access_groups::ActiveModel { id: Set(Uuid::new_v4()), @@ -244,7 +244,7 @@ impl AccessGroupRepository { /// an account gets the same baseline. pub async fn join_default_group(db: &impl ConnectionTrait, user_id: Uuid) -> Result<()> { if let Some(group) = Self::get_default(db).await? { - Self::add_member_if_absent(db, group.id, user_id, MembershipSource::Default).await?; + Self::add_member(db, group.id, user_id, MembershipSource::Default).await?; } Ok(()) } @@ -284,37 +284,6 @@ impl AccessGroupRepository { Ok(user_ids.len() as u64) } - /// Insert a membership unless the user is already in the group. - /// - /// Unlike [`Self::add_member`], this never rewrites the source of an - /// existing row. Turning an `oidc` membership into `default` would stop - /// the IdP from ever removing it. Returns true if a row was inserted. - pub async fn add_member_if_absent( - db: &impl ConnectionTrait, - group_id: Uuid, - user_id: Uuid, - source: MembershipSource, - ) -> Result { - let existing = user_access_groups::Entity::find() - .filter(user_access_groups::Column::UserId.eq(user_id)) - .filter(user_access_groups::Column::AccessGroupId.eq(group_id)) - .one(db) - .await?; - if existing.is_some() { - return Ok(false); - } - user_access_groups::ActiveModel { - id: Set(Uuid::new_v4()), - user_id: Set(user_id), - access_group_id: Set(group_id), - source: Set(source.as_str().to_string()), - created_at: Set(Utc::now()), - } - .insert(db) - .await?; - Ok(true) - } - /// Group memberships for many users in two queries, keyed by user ID. /// /// Each user's groups are sorted by name and paired with the membership @@ -855,13 +824,82 @@ mod tests { 1 ); - // Different source: same row, source updated + // Different source: same row, original source kept let m3 = AccessGroupRepository::add_member(&db, group.id, user.id, MembershipSource::Oidc) .await .unwrap(); assert_eq!(m1.id, m3.id); - assert_eq!(m3.source, "oidc"); - assert_eq!(m3.get_source(), MembershipSource::Oidc); + assert_eq!(m3.get_source(), MembershipSource::Manual); + } + + /// An admin adding someone the IdP already put in the group must not take + /// the membership away from the IdP, or leaving the IdP group would no + /// longer remove them. + #[tokio::test] + async fn reconcile_still_removes_oidc_member_after_manual_add() { + let db = setup_test_db().await; + let user = create_test_user(&db, "alice").await; + let group = AccessGroupRepository::create(&db, "Team", None) + .await + .unwrap(); + AccessGroupRepository::add_oidc_mapping(&db, group.id, "idp-team") + .await + .unwrap(); + AccessGroupRepository::reconcile_oidc_group_memberships( + &db, + user.id, + &["idp-team".to_string()], + ) + .await + .unwrap(); + + AccessGroupRepository::add_member(&db, group.id, user.id, MembershipSource::Manual) + .await + .unwrap(); + AccessGroupRepository::reconcile_oidc_group_memberships(&db, user.id, &[]) + .await + .unwrap(); + + assert!( + AccessGroupRepository::list_for_user(&db, user.id) + .await + .unwrap() + .is_empty() + ); + } + + /// The reverse: a manual membership in a group the IdP also maps must + /// survive the user leaving the IdP group. + #[tokio::test] + async fn reconcile_keeps_manual_member_when_idp_group_also_maps() { + let db = setup_test_db().await; + let user = create_test_user(&db, "alice").await; + let group = AccessGroupRepository::create(&db, "Team", None) + .await + .unwrap(); + AccessGroupRepository::add_member(&db, group.id, user.id, MembershipSource::Manual) + .await + .unwrap(); + AccessGroupRepository::add_oidc_mapping(&db, group.id, "idp-team") + .await + .unwrap(); + + AccessGroupRepository::reconcile_oidc_group_memberships( + &db, + user.id, + &["idp-team".to_string()], + ) + .await + .unwrap(); + AccessGroupRepository::reconcile_oidc_group_memberships(&db, user.id, &[]) + .await + .unwrap(); + + let members = AccessGroupRepository::list_members(&db, group.id) + .await + .unwrap(); + assert_eq!(members.len(), 1); + assert_eq!(members[0].source, "manual"); } #[tokio::test] From 8e17e0267b6d093c52a6dc009978f0c82e29d26e Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Sun, 25 Oct 2026 16:05:54 -0700 Subject: [PATCH 04/16] docs: format the web code with biome, the only formatter it has CLAUDE.md pointed at `npm run format`, which web/package.json does not define. --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 244e4fcb8..00742bf7b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -230,7 +230,7 @@ cargo test -- --nocapture ```bash # Format code cargo fmt -cd web && npm run format +cd web && npx biome check --write . # Lint code cargo clippy -- -D warnings From f26cb5b6038c4ea6460f79fe1e7b36b28f8caf49 Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Sun, 25 Oct 2026 17:03:31 -0700 Subject: [PATCH 05/16] feat: let access groups allow or deny libraries, genres and tags Access groups could only allow or deny sharing tags, so hiding adult content from a kids account meant sharing-tagging every series by hand, and a series added later was uncovered until someone tagged it. A group can now allow or deny libraries, genres and regular tags as well as sharing tags. How rules combine: - inside a group, values of one type match as "any of" (switchable to "all of" for genres, tags and sharing tags) and different types must all match, so "library Manga + genre Kids" is Kids manga; - across groups, what each group allows adds up, and personal sharing-tag grants count as one more group; - every deny, from any group or grant, is applied globally and wins, so no other group can grant denied content back. Existing sharing-tag setups resolve exactly as before. Genre and tag rules match by normalized name rather than id. Cleanup deletes genres and tags no series uses, and the next series to carry the name recreates it under a new id; an id-keyed rule would cascade away and silently drop a deny. Admins and maintainers are no longer restricted by groups or grants. Maintainers curate metadata, covers and scans, and every one of those endpoints acts on any id; filtering their browsing while letting them edit anything was inconsistent. An unknown user now sees nothing rather than everything. The filter is built in a fixed number of queries whatever the number of groups. The rule set is replaced atomically through a new rules endpoint, and the admin UI edits all four rule types and says who is exempt. The old visibility helpers on the sharing tag repository are removed: nothing called them, and they encoded older rules (untagged series always visible, groups ignored) that a new caller could have picked up by mistake. --- crates/codex-api/src/docs.rs | 10 + .../src/routes/v1/dto/access_group.rs | 119 ++- .../src/routes/v1/handlers/access_groups.rs | 269 +++++- .../codex-api/src/routes/v1/routes/admin.rs | 5 + .../src/entities/access_group_genre_rules.rs | 52 ++ .../entities/access_group_library_rules.rs | 61 ++ .../src/entities/access_group_tag_rules.rs | 52 ++ crates/codex-db/src/entities/access_groups.rs | 58 +- crates/codex-db/src/entities/mod.rs | 3 + crates/codex-db/src/entities/prelude.rs | 5 + .../codex-db/src/repositories/access_group.rs | 13 +- .../src/repositories/access_group_rules.rs | 611 +++++++++++++ crates/codex-db/src/repositories/genre.rs | 7 +- crates/codex-db/src/repositories/mod.rs | 5 + .../codex-db/src/repositories/sharing_tag.rs | 581 +----------- crates/codex-db/src/repositories/tag.rs | 7 +- crates/codex-migrate/src/registry.rs | 3 + crates/codex-services/src/content_filter.rs | 843 ++++++++++++++++-- docs/api/openapi.json | 351 +++++++- docs/docs/users/access-groups.md | 308 +++---- docs/docs/users/permissions.md | 4 +- docs/docs/users/sharing-tags.md | 6 + migration/src/lib.rs | 3 + .../m20261025_000117_access_group_rules.rs | 259 ++++++ tests/api/access_groups.rs | 163 ++++ tests/api/reading_progress_transfer.rs | 23 +- tests/db/reading_history_retention.rs | 9 +- web/openapi.json | 351 +++++++- web/src/api/accessGroups.ts | 27 + .../AccessGroupRulesCard.test.tsx | 179 ++++ .../access-groups/AccessGroupRulesCard.tsx | 425 +++++++++ .../users/UserAccessGroupBadges.test.tsx | 13 + .../users/UserAccessGroupBadges.tsx | 13 +- web/src/pages/settings/AccessGroupDetail.tsx | 186 +--- web/src/pages/settings/UsersSettings.tsx | 7 +- web/src/types/api.generated.ts | 244 ++++- 36 files changed, 4242 insertions(+), 1033 deletions(-) create mode 100644 crates/codex-db/src/entities/access_group_genre_rules.rs create mode 100644 crates/codex-db/src/entities/access_group_library_rules.rs create mode 100644 crates/codex-db/src/entities/access_group_tag_rules.rs create mode 100644 crates/codex-db/src/repositories/access_group_rules.rs create mode 100644 migration/src/m20261025_000117_access_group_rules.rs create mode 100644 web/src/components/access-groups/AccessGroupRulesCard.test.tsx create mode 100644 web/src/components/access-groups/AccessGroupRulesCard.tsx diff --git a/crates/codex-api/src/docs.rs b/crates/codex-api/src/docs.rs index 7240c4cce..bcf010ccf 100644 --- a/crates/codex-api/src/docs.rs +++ b/crates/codex-api/src/docs.rs @@ -602,6 +602,8 @@ The following paths are exempt from rate limiting: v1::handlers::access_groups::add_all_users_to_access_group, v1::handlers::access_groups::remove_access_group_member, v1::handlers::access_groups::get_user_access_groups, + v1::handlers::access_groups::get_access_group_rules, + v1::handlers::access_groups::replace_access_group_rules, v1::handlers::access_groups::add_access_group_grant, v1::handlers::access_groups::remove_access_group_grant, v1::handlers::access_groups::add_access_group_oidc_mapping, @@ -964,6 +966,14 @@ The following paths are exempt from rate limiting: v1::dto::CreateAccessGroupRequest, v1::dto::UpdateAccessGroupRequest, v1::dto::AddAllUsersResponse, + v1::dto::AccessGroupRulesDto, + v1::dto::SharingTagRuleDto, + v1::dto::NameRuleDto, + v1::dto::LibraryRuleDto, + v1::dto::ReplaceAccessGroupRulesRequest, + v1::dto::SharingTagRuleInput, + v1::dto::LibraryRuleInput, + codex_db::entities::access_groups::RuleMatch, v1::dto::AddAccessGroupMembersRequest, v1::dto::AddAccessGroupGrantRequest, v1::dto::AddAccessGroupOidcMappingRequest, diff --git a/crates/codex-api/src/routes/v1/dto/access_group.rs b/crates/codex-api/src/routes/v1/dto/access_group.rs index 469614750..088a81450 100644 --- a/crates/codex-api/src/routes/v1/dto/access_group.rs +++ b/crates/codex-api/src/routes/v1/dto/access_group.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; use uuid::Uuid; +use codex_db::entities::access_groups::RuleMatch; use codex_db::entities::user_sharing_tags::AccessMode; /// Access group data transfer object @@ -31,10 +32,14 @@ pub struct AccessGroupDto { #[schema(example = 5)] pub member_count: u64, - /// Number of tag grants in the group + /// Number of sharing tag grants in the group #[schema(example = 3)] pub grant_count: u64, + /// Number of rules in the group, across sharing tags, tags, genres and libraries + #[schema(example = 5)] + pub rule_count: u64, + /// Creation timestamp #[schema(example = "2024-01-01T00:00:00Z")] pub created_at: DateTime, @@ -286,6 +291,7 @@ impl AccessGroupDto { model: codex_db::entities::access_groups::Model, member_count: u64, grant_count: u64, + rule_count: u64, ) -> Self { Self { id: model.id, @@ -294,6 +300,7 @@ impl AccessGroupDto { is_default: model.is_default, member_count, grant_count, + rule_count, created_at: model.created_at, updated_at: model.updated_at, } @@ -309,3 +316,113 @@ impl From for AccessGroupSummaryDto { } } } + +// ==================== Rules ==================== + +/// Every rule of an access group +/// +/// Within a group, values of the same type combine as "any of" or "all of" +/// (per the match field for that type; libraries are always "any of"), and +/// different types combine with AND. Across groups, what each group allows is +/// combined with OR. Denies from every group always win. Admins and +/// maintainers are never restricted. +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct AccessGroupRulesDto { + /// How allowed sharing tags combine + pub sharing_tag_match: RuleMatch, + /// How allowed tags combine + pub tag_match: RuleMatch, + /// How allowed genres combine + pub genre_match: RuleMatch, + /// Rules on sharing tags + pub sharing_tags: Vec, + /// Rules on regular tags, matched by name + pub tags: Vec, + /// Rules on genres, matched by name + pub genres: Vec, + /// Rules on libraries + pub libraries: Vec, +} + +/// A rule on a sharing tag +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct SharingTagRuleDto { + /// Sharing tag ID + pub sharing_tag_id: Uuid, + /// Sharing tag name + #[schema(example = "Kids")] + pub name: String, + /// Allow or deny + pub access_mode: AccessMode, +} + +/// A rule on a regular tag or a genre +/// +/// Matched by name, case-insensitively, so the rule keeps applying when the +/// tag or genre is removed by cleanup and created again later. +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct NameRuleDto { + /// Tag or genre name + #[schema(example = "Adult")] + pub name: String, + /// Allow or deny + pub access_mode: AccessMode, +} + +/// A rule on a library +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct LibraryRuleDto { + /// Library ID + pub library_id: Uuid, + /// Library name + #[schema(example = "Manga")] + pub name: String, + /// Allow or deny + pub access_mode: AccessMode, +} + +/// Replace every rule of an access group +/// +/// Omitted lists are treated as empty and omitted match fields as "any". +#[derive(Debug, Default, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase", default)] +pub struct ReplaceAccessGroupRulesRequest { + /// How allowed sharing tags combine + pub sharing_tag_match: RuleMatch, + /// How allowed tags combine + pub tag_match: RuleMatch, + /// How allowed genres combine + pub genre_match: RuleMatch, + /// Rules on sharing tags + pub sharing_tags: Vec, + /// Rules on regular tags, matched by name + pub tags: Vec, + /// Rules on genres, matched by name + pub genres: Vec, + /// Rules on libraries + pub libraries: Vec, +} + +/// A sharing tag rule in a replace request +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct SharingTagRuleInput { + /// Sharing tag ID + pub sharing_tag_id: Uuid, + /// Allow or deny + pub access_mode: AccessMode, +} + +/// A library rule in a replace request +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct LibraryRuleInput { + /// Library ID + pub library_id: Uuid, + /// Allow or deny + pub access_mode: AccessMode, +} diff --git a/crates/codex-api/src/routes/v1/handlers/access_groups.rs b/crates/codex-api/src/routes/v1/handlers/access_groups.rs index c7da2a447..f824d6047 100644 --- a/crates/codex-api/src/routes/v1/handlers/access_groups.rs +++ b/crates/codex-api/src/routes/v1/handlers/access_groups.rs @@ -1,13 +1,15 @@ //! Handlers for access group operations //! -//! Access groups bundle a set of sharing-tag allow/deny rules that can be -//! assigned to multiple users. All endpoints require admin permissions. +//! Access groups bundle allow/deny rules on sharing tags, tags, genres and +//! libraries, and are assigned to many users. All endpoints require admin +//! permissions. use super::super::dto::{ AccessGroupDetailDto, AccessGroupDto, AccessGroupGrantDto, AccessGroupMemberDto, - AccessGroupOidcMappingDto, AccessGroupSummaryDto, AddAccessGroupGrantRequest, - AddAccessGroupMembersRequest, AddAccessGroupOidcMappingRequest, AddAllUsersResponse, - CreateAccessGroupRequest, EffectiveGrantDto, EffectiveGrantsResponse, GrantSourceDto, + AccessGroupOidcMappingDto, AccessGroupRulesDto, AccessGroupSummaryDto, + AddAccessGroupGrantRequest, AddAccessGroupMembersRequest, AddAccessGroupOidcMappingRequest, + AddAllUsersResponse, CreateAccessGroupRequest, EffectiveGrantDto, EffectiveGrantsResponse, + GrantSourceDto, LibraryRuleDto, NameRuleDto, ReplaceAccessGroupRulesRequest, SharingTagRuleDto, UpdateAccessGroupRequest, }; use crate::{ @@ -20,8 +22,14 @@ use axum::{ extract::{Path, State}, http::StatusCode, }; +use codex_db::entities::sharing_tags; use codex_db::entities::user_access_groups::MembershipSource; -use codex_db::repositories::{AccessGroupRepository, SharingTagRepository}; +use codex_db::repositories::{ + AccessGroupRepository, AccessGroupRules, AccessGroupRulesRepository, LibraryRepository, + LibraryRule, NameRule, SharingTagRepository, SharingTagRule, +}; +use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; +use std::collections::HashMap; use std::sync::Arc; use uuid::Uuid; @@ -51,20 +59,25 @@ pub async fn list_access_groups( .await .map_err(|e| ApiError::Internal(format!("Failed to fetch access groups: {}", e)))?; + let group_ids: Vec = groups.iter().map(|g| g.id).collect(); + let rules = AccessGroupRulesRepository::load_for_groups(&state.db, &group_ids) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch access group rules: {}", e)))?; + let mut items = Vec::with_capacity(groups.len()); for group in groups { let member_count = AccessGroupRepository::list_members(&state.db, group.id) .await .map(|m| m.len() as u64) .unwrap_or(0); - let grant_count = AccessGroupRepository::list_grants(&state.db, group.id) - .await - .map(|g| g.len() as u64) - .unwrap_or(0); + let group_rules = rules.get(&group.id); + let grant_count = group_rules.map_or(0, |r| r.sharing_tags.len() as u64); + let rule_count = group_rules.map_or(0, rule_count); items.push(AccessGroupDto::from_model_with_counts( group, member_count, grant_count, + rule_count, )); } @@ -151,7 +164,7 @@ pub async fn create_access_group( Ok(( StatusCode::CREATED, - Json(AccessGroupDto::from_model_with_counts(group, 0, 0)), + Json(AccessGroupDto::from_model_with_counts(group, 0, 0, 0)), )) } @@ -219,15 +232,16 @@ pub async fn update_access_group( .await .map(|m| m.len() as u64) .unwrap_or(0); - let grant_count = AccessGroupRepository::list_grants(&state.db, group.id) + let rules = AccessGroupRulesRepository::get(&state.db, group.id) .await - .map(|g| g.len() as u64) - .unwrap_or(0); + .map_err(|e| ApiError::Internal(format!("Failed to fetch access group rules: {}", e)))? + .unwrap_or_default(); Ok(Json(AccessGroupDto::from_model_with_counts( group, member_count, - grant_count, + rules.sharing_tags.len() as u64, + rule_count(&rules), ))) } @@ -614,6 +628,231 @@ pub async fn remove_access_group_oidc_mapping( Ok(StatusCode::NO_CONTENT) } +// ==================== Rules ==================== + +/// Get every rule of an access group (admin only) +#[utoipa::path( + get, + path = "/api/v1/access-groups/{group_id}/rules", + params( + ("group_id" = Uuid, Path, description = "Access group ID") + ), + responses( + (status = 200, description = "The group's rules", body = AccessGroupRulesDto), + (status = 404, description = "Access group not found"), + (status = 403, description = "Forbidden - Missing permission"), + ), + security( + ("jwt_bearer" = []), + ("api_key" = []) + ), + tag = "Access Groups" +)] +pub async fn get_access_group_rules( + State(state): State>, + auth: AuthContext, + Path(group_id): Path, +) -> Result, ApiError> { + auth.require_permission(&Permission::SystemAdmin)?; + + let rules = AccessGroupRulesRepository::get(&state.db, group_id) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch access group rules: {}", e)))? + .ok_or_else(|| ApiError::NotFound("Access group not found".to_string()))?; + + Ok(Json(rules_to_dto(&state.db, rules).await?)) +} + +/// Replace every rule of an access group (admin only) +/// +/// The whole rule set is replaced atomically: anything not in the request is +/// removed. Sharing tag and library IDs must exist; tag and genre names need +/// not, so a rule can be set up before any series carries the name. +#[utoipa::path( + put, + path = "/api/v1/access-groups/{group_id}/rules", + params( + ("group_id" = Uuid, Path, description = "Access group ID") + ), + request_body = ReplaceAccessGroupRulesRequest, + responses( + (status = 200, description = "Rules replaced", body = AccessGroupRulesDto), + (status = 400, description = "Duplicate rule, blank name, or unknown sharing tag or library"), + (status = 404, description = "Access group not found"), + (status = 403, description = "Forbidden - Missing permission"), + ), + security( + ("jwt_bearer" = []), + ("api_key" = []) + ), + tag = "Access Groups" +)] +pub async fn replace_access_group_rules( + State(state): State>, + auth: AuthContext, + Path(group_id): Path, + Json(request): Json, +) -> Result, ApiError> { + auth.require_permission(&Permission::SystemAdmin)?; + + AccessGroupRepository::get_by_id(&state.db, group_id) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch access group: {}", e)))? + .ok_or_else(|| ApiError::NotFound("Access group not found".to_string()))?; + + let rules = AccessGroupRules { + sharing_tag_match: request.sharing_tag_match, + tag_match: request.tag_match, + genre_match: request.genre_match, + sharing_tags: request + .sharing_tags + .into_iter() + .map(|r| SharingTagRule { + sharing_tag_id: r.sharing_tag_id, + access_mode: r.access_mode, + }) + .collect(), + tags: request + .tags + .into_iter() + .map(|r| NameRule::new(r.name, r.access_mode)) + .collect(), + genres: request + .genres + .into_iter() + .map(|r| NameRule::new(r.name, r.access_mode)) + .collect(), + libraries: request + .libraries + .into_iter() + .map(|r| LibraryRule { + library_id: r.library_id, + access_mode: r.access_mode, + }) + .collect(), + }; + rules.validate().map_err(ApiError::BadRequest)?; + + let sharing_tag_ids: Vec = rules + .sharing_tags + .iter() + .map(|r| r.sharing_tag_id) + .collect(); + let known_tags = sharing_tag_names(&state.db, &sharing_tag_ids).await?; + if let Some(missing) = sharing_tag_ids + .iter() + .find(|id| !known_tags.contains_key(id)) + { + return Err(ApiError::BadRequest(format!( + "Sharing tag {} does not exist", + missing + ))); + } + let library_ids: Vec = rules.libraries.iter().map(|r| r.library_id).collect(); + let known_libraries = LibraryRepository::get_by_ids(&state.db, &library_ids) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch libraries: {}", e)))?; + if let Some(missing) = library_ids + .iter() + .find(|id| !known_libraries.contains_key(id)) + { + return Err(ApiError::BadRequest(format!( + "Library {} does not exist", + missing + ))); + } + + AccessGroupRulesRepository::replace(&state.db, group_id, &rules) + .await + .map_err(|e| ApiError::Internal(format!("Failed to replace access group rules: {}", e)))?; + + Ok(Json(rules_to_dto(&state.db, rules).await?)) +} + +fn rule_count(rules: &AccessGroupRules) -> u64 { + (rules.sharing_tags.len() + rules.tags.len() + rules.genres.len() + rules.libraries.len()) + as u64 +} + +async fn sharing_tag_names( + db: &sea_orm::DatabaseConnection, + ids: &[Uuid], +) -> Result, ApiError> { + if ids.is_empty() { + return Ok(HashMap::new()); + } + Ok(sharing_tags::Entity::find() + .filter(sharing_tags::Column::Id.is_in(ids.to_vec())) + .all(db) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch sharing tags: {}", e)))? + .into_iter() + .map(|t| (t.id, t.name)) + .collect()) +} + +async fn rules_to_dto( + db: &sea_orm::DatabaseConnection, + rules: AccessGroupRules, +) -> Result { + let tag_ids: Vec = rules + .sharing_tags + .iter() + .map(|r| r.sharing_tag_id) + .collect(); + let tag_names = sharing_tag_names(db, &tag_ids).await?; + let library_ids: Vec = rules.libraries.iter().map(|r| r.library_id).collect(); + let libraries = LibraryRepository::get_by_ids(db, &library_ids) + .await + .map_err(|e| ApiError::Internal(format!("Failed to fetch libraries: {}", e)))?; + + Ok(AccessGroupRulesDto { + sharing_tag_match: rules.sharing_tag_match, + tag_match: rules.tag_match, + genre_match: rules.genre_match, + sharing_tags: rules + .sharing_tags + .into_iter() + .map(|r| SharingTagRuleDto { + name: tag_names + .get(&r.sharing_tag_id) + .cloned() + .unwrap_or_default(), + sharing_tag_id: r.sharing_tag_id, + access_mode: r.access_mode, + }) + .collect(), + tags: rules + .tags + .into_iter() + .map(|r| NameRuleDto { + name: r.name, + access_mode: r.access_mode, + }) + .collect(), + genres: rules + .genres + .into_iter() + .map(|r| NameRuleDto { + name: r.name, + access_mode: r.access_mode, + }) + .collect(), + libraries: rules + .libraries + .into_iter() + .map(|r| LibraryRuleDto { + name: libraries + .get(&r.library_id) + .map(|l| l.name.clone()) + .unwrap_or_default(), + library_id: r.library_id, + access_mode: r.access_mode, + }) + .collect(), + }) +} + // ==================== Effective Grants Debug ==================== /// Get effective grants for a user with source attribution (admin only) diff --git a/crates/codex-api/src/routes/v1/routes/admin.rs b/crates/codex-api/src/routes/v1/routes/admin.rs index 8d7784c0a..03580001b 100644 --- a/crates/codex-api/src/routes/v1/routes/admin.rs +++ b/crates/codex-api/src/routes/v1/routes/admin.rs @@ -164,6 +164,11 @@ pub fn routes(_state: Arc) -> Router> { "/access-groups/{group_id}/members/{user_id}", delete(handlers::access_groups::remove_access_group_member), ) + .route( + "/access-groups/{group_id}/rules", + get(handlers::access_groups::get_access_group_rules) + .put(handlers::access_groups::replace_access_group_rules), + ) .route( "/access-groups/{group_id}/grants", post(handlers::access_groups::add_access_group_grant), diff --git a/crates/codex-db/src/entities/access_group_genre_rules.rs b/crates/codex-db/src/entities/access_group_genre_rules.rs new file mode 100644 index 000000000..82f20ac04 --- /dev/null +++ b/crates/codex-db/src/entities/access_group_genre_rules.rs @@ -0,0 +1,52 @@ +//! `SeaORM` Entity for access_group_genre_rules table +//! +//! Per-group allow/deny rules on genres, matched by normalized name rather +//! than by id: unused genres are cleaned up and recreated under new ids, and +//! a rule must outlive that. + +use chrono::{DateTime, Utc}; +use sea_orm::entity::prelude::*; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use super::user_sharing_tags::AccessMode; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel, Serialize, Deserialize)] +#[sea_orm(table_name = "access_group_genre_rules")] +pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: Uuid, + pub access_group_id: Uuid, + /// Display name, as the admin picked it + pub name: String, + /// Lowercased, trimmed name used for matching + pub normalized_name: String, + pub access_mode: String, + pub created_at: DateTime, +} + +impl Model { + pub fn get_access_mode(&self) -> AccessMode { + self.access_mode.parse().unwrap_or(AccessMode::Allow) + } +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::access_groups::Entity", + from = "Column::AccessGroupId", + to = "super::access_groups::Column::Id", + on_update = "NoAction", + on_delete = "Cascade" + )] + AccessGroup, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::AccessGroup.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/codex-db/src/entities/access_group_library_rules.rs b/crates/codex-db/src/entities/access_group_library_rules.rs new file mode 100644 index 000000000..01412172c --- /dev/null +++ b/crates/codex-db/src/entities/access_group_library_rules.rs @@ -0,0 +1,61 @@ +//! `SeaORM` Entity for access_group_library_rules table +//! +//! Per-group allow/deny rules on libraries. + +use chrono::{DateTime, Utc}; +use sea_orm::entity::prelude::*; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use super::user_sharing_tags::AccessMode; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel, Serialize, Deserialize)] +#[sea_orm(table_name = "access_group_library_rules")] +pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: Uuid, + pub access_group_id: Uuid, + pub library_id: Uuid, + pub access_mode: String, + pub created_at: DateTime, +} + +impl Model { + pub fn get_access_mode(&self) -> AccessMode { + self.access_mode.parse().unwrap_or(AccessMode::Allow) + } +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::access_groups::Entity", + from = "Column::AccessGroupId", + to = "super::access_groups::Column::Id", + on_update = "NoAction", + on_delete = "Cascade" + )] + AccessGroup, + #[sea_orm( + belongs_to = "super::libraries::Entity", + from = "Column::LibraryId", + to = "super::libraries::Column::Id", + on_update = "NoAction", + on_delete = "Cascade" + )] + Library, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::AccessGroup.def() + } +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::Library.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/codex-db/src/entities/access_group_tag_rules.rs b/crates/codex-db/src/entities/access_group_tag_rules.rs new file mode 100644 index 000000000..1a69269dc --- /dev/null +++ b/crates/codex-db/src/entities/access_group_tag_rules.rs @@ -0,0 +1,52 @@ +//! `SeaORM` Entity for access_group_tag_rules table +//! +//! Per-group allow/deny rules on tags, matched by normalized name rather +//! than by id: unused tags are cleaned up and recreated under new ids, and +//! a rule must outlive that. + +use chrono::{DateTime, Utc}; +use sea_orm::entity::prelude::*; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use super::user_sharing_tags::AccessMode; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel, Serialize, Deserialize)] +#[sea_orm(table_name = "access_group_tag_rules")] +pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: Uuid, + pub access_group_id: Uuid, + /// Display name, as the admin picked it + pub name: String, + /// Lowercased, trimmed name used for matching + pub normalized_name: String, + pub access_mode: String, + pub created_at: DateTime, +} + +impl Model { + pub fn get_access_mode(&self) -> AccessMode { + self.access_mode.parse().unwrap_or(AccessMode::Allow) + } +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::access_groups::Entity", + from = "Column::AccessGroupId", + to = "super::access_groups::Column::Id", + on_update = "NoAction", + on_delete = "Cascade" + )] + AccessGroup, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::AccessGroup.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/codex-db/src/entities/access_groups.rs b/crates/codex-db/src/entities/access_groups.rs index 6e40d03f6..dac3c8f92 100644 --- a/crates/codex-db/src/entities/access_groups.rs +++ b/crates/codex-db/src/entities/access_groups.rs @@ -1,8 +1,8 @@ //! `SeaORM` Entity for access_groups table //! -//! Access groups bundle a set of sharing-tag allow/deny rules that can be -//! assigned to multiple users. Per-user grants in `user_sharing_tags` act -//! as overrides on top of group rules. +//! Access groups bundle allow/deny rules on sharing tags, tags, genres and +//! libraries, and are assigned to many users. A user's personal grants in +//! `user_sharing_tags` count as one more group. use chrono::{DateTime, Utc}; use sea_orm::entity::prelude::*; @@ -20,6 +20,12 @@ pub struct Model { /// New users join the default group when their account is created. /// At most one group has this set (partial unique index). pub is_default: bool, + /// How allowed sharing tags combine: "any" (default) or "all" + pub sharing_tag_match: String, + /// How allowed tags combine: "any" (default) or "all" + pub tag_match: String, + /// How allowed genres combine: "any" (default) or "all" + pub genre_match: String, pub created_at: DateTime, pub updated_at: DateTime, } @@ -75,3 +81,49 @@ impl Related for Entity { } impl ActiveModelBehavior for ActiveModel {} + +/// How a group's allowed values of one rule type combine. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default, utoipa::ToSchema)] +#[serde(rename_all = "lowercase")] +pub enum RuleMatch { + /// A series matches if it carries any of the allowed values + #[default] + Any, + /// A series matches only if it carries every allowed value + All, +} + +impl RuleMatch { + pub fn as_str(&self) -> &'static str { + match self { + RuleMatch::Any => "any", + RuleMatch::All => "all", + } + } +} + +impl std::str::FromStr for RuleMatch { + type Err = String; + + fn from_str(s: &str) -> Result { + match s.to_lowercase().as_str() { + "any" => Ok(RuleMatch::Any), + "all" => Ok(RuleMatch::All), + _ => Err(format!("Unknown rule match: {}", s)), + } + } +} + +impl Model { + pub fn get_sharing_tag_match(&self) -> RuleMatch { + self.sharing_tag_match.parse().unwrap_or_default() + } + + pub fn get_tag_match(&self) -> RuleMatch { + self.tag_match.parse().unwrap_or_default() + } + + pub fn get_genre_match(&self) -> RuleMatch { + self.genre_match.parse().unwrap_or_default() + } +} diff --git a/crates/codex-db/src/entities/mod.rs b/crates/codex-db/src/entities/mod.rs index 6ea386c5c..2ade08666 100644 --- a/crates/codex-db/src/entities/mod.rs +++ b/crates/codex-db/src/entities/mod.rs @@ -83,8 +83,11 @@ pub mod sharing_tags; pub mod user_sharing_tags; // Access groups for sharing tag management +pub mod access_group_genre_rules; +pub mod access_group_library_rules; pub mod access_group_oidc_mappings; pub mod access_group_sharing_tags; +pub mod access_group_tag_rules; pub mod access_groups; pub mod user_access_groups; diff --git a/crates/codex-db/src/entities/prelude.rs b/crates/codex-db/src/entities/prelude.rs index 0b01a1d57..e72571a7b 100644 --- a/crates/codex-db/src/entities/prelude.rs +++ b/crates/codex-db/src/entities/prelude.rs @@ -72,10 +72,15 @@ pub use super::user_sharing_tags::Entity as UserSharingTags; // Access groups #[allow(unused_imports)] +pub use super::access_group_genre_rules::Entity as AccessGroupGenreRules; +#[allow(unused_imports)] +pub use super::access_group_library_rules::Entity as AccessGroupLibraryRules; pub use super::access_group_oidc_mappings::Entity as AccessGroupOidcMappings; #[allow(unused_imports)] pub use super::access_group_sharing_tags::Entity as AccessGroupSharingTags; #[allow(unused_imports)] +pub use super::access_group_tag_rules::Entity as AccessGroupTagRules; +#[allow(unused_imports)] pub use super::access_groups::Entity as AccessGroups; #[allow(unused_imports)] pub use super::user_access_groups::Entity as UserAccessGroups; diff --git a/crates/codex-db/src/repositories/access_group.rs b/crates/codex-db/src/repositories/access_group.rs index 7a5350d0d..7c79c6aeb 100644 --- a/crates/codex-db/src/repositories/access_group.rs +++ b/crates/codex-db/src/repositories/access_group.rs @@ -1,8 +1,8 @@ //! Repository for access groups, group memberships, group grants, and OIDC mappings. //! -//! Access groups bundle a set of sharing-tag allow/deny rules that can be -//! assigned to many users. Per-user grants in `user_sharing_tags` act as -//! overrides on top of group rules (see `SharingTagRepository::get_effective_grants`). +//! Access groups bundle allow/deny rules that can be assigned to many users. +//! The rules themselves live in `access_group_rules`; how they combine into +//! what a user can see is `ContentFilter` in codex-services. use anyhow::Result; use chrono::Utc; @@ -14,8 +14,8 @@ use std::collections::{HashMap, HashSet}; use uuid::Uuid; use crate::entities::{ - access_group_oidc_mappings, access_group_sharing_tags, access_groups, user_access_groups, - user_access_groups::MembershipSource, user_sharing_tags::AccessMode, users, + access_group_oidc_mappings, access_group_sharing_tags, access_groups, access_groups::RuleMatch, + user_access_groups, user_access_groups::MembershipSource, user_sharing_tags::AccessMode, users, }; pub struct AccessGroupRepository; @@ -35,6 +35,9 @@ impl AccessGroupRepository { name: Set(name.trim().to_string()), description: Set(description), is_default: Set(false), + sharing_tag_match: Set(RuleMatch::Any.as_str().to_string()), + tag_match: Set(RuleMatch::Any.as_str().to_string()), + genre_match: Set(RuleMatch::Any.as_str().to_string()), created_at: Set(now), updated_at: Set(now), }; diff --git a/crates/codex-db/src/repositories/access_group_rules.rs b/crates/codex-db/src/repositories/access_group_rules.rs new file mode 100644 index 000000000..f19426146 --- /dev/null +++ b/crates/codex-db/src/repositories/access_group_rules.rs @@ -0,0 +1,611 @@ +//! Access group rules: allow/deny on sharing tags, regular tags, genres and +//! libraries, plus the per-type "any"/"all" switches. +//! +//! This module only stores rules and resolves rule targets to series IDs. How +//! rules from several groups combine into what a user can see lives in +//! `ContentFilter` (`codex-services`). +//! +//! Tag and genre rules are keyed by normalized name (see the migration that +//! creates their tables for why), so resolving them joins on +//! `tags.normalized_name` / `genres.normalized_name` at query time. + +use anyhow::{Result, bail}; +use chrono::Utc; +use sea_orm::{ + ActiveModelTrait, ColumnTrait, ConnectionTrait, DatabaseConnection, EntityTrait, QueryFilter, + QueryOrder, QuerySelect, Set, TransactionTrait, +}; +use std::collections::{HashMap, HashSet}; +use uuid::Uuid; + +use crate::entities::{ + access_group_genre_rules, access_group_library_rules, access_group_sharing_tags, + access_group_tag_rules, access_groups, access_groups::RuleMatch, genres, series, series_genres, + series_sharing_tags, series_tags, tags, user_sharing_tags::AccessMode, +}; + +/// Normalize a tag or genre name the way the `tags` and `genres` tables do. +pub fn normalize_taxonomy_name(name: &str) -> String { + name.trim().to_lowercase() +} + +/// A rule on a sharing tag. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SharingTagRule { + pub sharing_tag_id: Uuid, + pub access_mode: AccessMode, +} + +/// A rule on a regular tag or a genre, by name. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NameRule { + /// Display name; matching uses [`normalize_taxonomy_name`] of it + pub name: String, + pub access_mode: AccessMode, +} + +impl NameRule { + pub fn new(name: impl Into, access_mode: AccessMode) -> Self { + Self { + name: name.into(), + access_mode, + } + } + + pub fn normalized_name(&self) -> String { + normalize_taxonomy_name(&self.name) + } +} + +/// A rule on a library. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LibraryRule { + pub library_id: Uuid, + pub access_mode: AccessMode, +} + +/// Every rule an access group holds. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct AccessGroupRules { + pub sharing_tag_match: RuleMatch, + pub tag_match: RuleMatch, + pub genre_match: RuleMatch, + pub sharing_tags: Vec, + pub tags: Vec, + pub genres: Vec, + pub libraries: Vec, +} + +impl AccessGroupRules { + /// Whether the group has any allow rule, of any type. + pub fn has_allows(&self) -> bool { + self.sharing_tags + .iter() + .any(|r| r.access_mode == AccessMode::Allow) + || self.tags.iter().any(|r| r.access_mode == AccessMode::Allow) + || self + .genres + .iter() + .any(|r| r.access_mode == AccessMode::Allow) + || self + .libraries + .iter() + .any(|r| r.access_mode == AccessMode::Allow) + } + + /// Check the rule set before storing it. + /// + /// Each target may appear once per group (one row per target in the + /// tables), and names must not be blank. + pub fn validate(&self) -> std::result::Result<(), String> { + let mut seen = HashSet::new(); + for rule in &self.sharing_tags { + if !seen.insert(rule.sharing_tag_id) { + return Err(format!( + "Sharing tag {} appears more than once", + rule.sharing_tag_id + )); + } + } + for (kind, rules) in [("tag", &self.tags), ("genre", &self.genres)] { + let mut seen = HashSet::new(); + for rule in rules { + let normalized = rule.normalized_name(); + if normalized.is_empty() { + return Err(format!("A {kind} rule has an empty name")); + } + if !seen.insert(normalized) { + return Err(format!( + "The {kind} '{}' appears more than once", + rule.name.trim() + )); + } + } + } + let mut seen = HashSet::new(); + for rule in &self.libraries { + if !seen.insert(rule.library_id) { + return Err(format!( + "Library {} appears more than once", + rule.library_id + )); + } + } + Ok(()) + } +} + +pub struct AccessGroupRulesRepository; + +impl AccessGroupRulesRepository { + /// All rules of one group. Returns `None` if the group does not exist. + pub async fn get(db: &DatabaseConnection, group_id: Uuid) -> Result> { + Ok(Self::load_for_groups(db, &[group_id]) + .await? + .remove(&group_id)) + } + + /// Rules for many groups in a fixed number of queries, keyed by group ID. + /// Groups that do not exist are absent from the map. + pub async fn load_for_groups( + db: &impl ConnectionTrait, + group_ids: &[Uuid], + ) -> Result> { + if group_ids.is_empty() { + return Ok(HashMap::new()); + } + let ids = group_ids.to_vec(); + + let mut by_group: HashMap = access_groups::Entity::find() + .filter(access_groups::Column::Id.is_in(ids.clone())) + .all(db) + .await? + .into_iter() + .map(|g| { + ( + g.id, + AccessGroupRules { + sharing_tag_match: g.get_sharing_tag_match(), + tag_match: g.get_tag_match(), + genre_match: g.get_genre_match(), + ..Default::default() + }, + ) + }) + .collect(); + + for row in access_group_sharing_tags::Entity::find() + .filter(access_group_sharing_tags::Column::AccessGroupId.is_in(ids.clone())) + .order_by_asc(access_group_sharing_tags::Column::CreatedAt) + .all(db) + .await? + { + if let Some(rules) = by_group.get_mut(&row.access_group_id) { + rules.sharing_tags.push(SharingTagRule { + sharing_tag_id: row.sharing_tag_id, + access_mode: row.get_access_mode(), + }); + } + } + + for row in access_group_tag_rules::Entity::find() + .filter(access_group_tag_rules::Column::AccessGroupId.is_in(ids.clone())) + .order_by_asc(access_group_tag_rules::Column::Name) + .all(db) + .await? + { + if let Some(rules) = by_group.get_mut(&row.access_group_id) { + let access_mode = row.get_access_mode(); + rules.tags.push(NameRule::new(row.name, access_mode)); + } + } + + for row in access_group_genre_rules::Entity::find() + .filter(access_group_genre_rules::Column::AccessGroupId.is_in(ids.clone())) + .order_by_asc(access_group_genre_rules::Column::Name) + .all(db) + .await? + { + if let Some(rules) = by_group.get_mut(&row.access_group_id) { + let access_mode = row.get_access_mode(); + rules.genres.push(NameRule::new(row.name, access_mode)); + } + } + + for row in access_group_library_rules::Entity::find() + .filter(access_group_library_rules::Column::AccessGroupId.is_in(ids)) + .order_by_asc(access_group_library_rules::Column::CreatedAt) + .all(db) + .await? + { + if let Some(rules) = by_group.get_mut(&row.access_group_id) { + rules.libraries.push(LibraryRule { + library_id: row.library_id, + access_mode: row.get_access_mode(), + }); + } + } + + Ok(by_group) + } + + /// Replace every rule of a group, atomically. + /// + /// Callers should [`AccessGroupRules::validate`] first; a duplicate target + /// is also refused by the tables' unique indexes. + pub async fn replace( + db: &DatabaseConnection, + group_id: Uuid, + rules: &AccessGroupRules, + ) -> Result<()> { + if let Err(message) = rules.validate() { + bail!(message); + } + + let txn = db.begin().await?; + let group = access_groups::Entity::find_by_id(group_id) + .one(&txn) + .await? + .ok_or_else(|| anyhow::anyhow!("Access group not found"))?; + + access_group_sharing_tags::Entity::delete_many() + .filter(access_group_sharing_tags::Column::AccessGroupId.eq(group_id)) + .exec(&txn) + .await?; + access_group_tag_rules::Entity::delete_many() + .filter(access_group_tag_rules::Column::AccessGroupId.eq(group_id)) + .exec(&txn) + .await?; + access_group_genre_rules::Entity::delete_many() + .filter(access_group_genre_rules::Column::AccessGroupId.eq(group_id)) + .exec(&txn) + .await?; + access_group_library_rules::Entity::delete_many() + .filter(access_group_library_rules::Column::AccessGroupId.eq(group_id)) + .exec(&txn) + .await?; + + let now = Utc::now(); + for rule in &rules.sharing_tags { + access_group_sharing_tags::ActiveModel { + id: Set(Uuid::new_v4()), + access_group_id: Set(group_id), + sharing_tag_id: Set(rule.sharing_tag_id), + access_mode: Set(rule.access_mode.as_str().to_string()), + created_at: Set(now), + } + .insert(&txn) + .await?; + } + for rule in &rules.tags { + access_group_tag_rules::ActiveModel { + id: Set(Uuid::new_v4()), + access_group_id: Set(group_id), + name: Set(rule.name.trim().to_string()), + normalized_name: Set(rule.normalized_name()), + access_mode: Set(rule.access_mode.as_str().to_string()), + created_at: Set(now), + } + .insert(&txn) + .await?; + } + for rule in &rules.genres { + access_group_genre_rules::ActiveModel { + id: Set(Uuid::new_v4()), + access_group_id: Set(group_id), + name: Set(rule.name.trim().to_string()), + normalized_name: Set(rule.normalized_name()), + access_mode: Set(rule.access_mode.as_str().to_string()), + created_at: Set(now), + } + .insert(&txn) + .await?; + } + for rule in &rules.libraries { + access_group_library_rules::ActiveModel { + id: Set(Uuid::new_v4()), + access_group_id: Set(group_id), + library_id: Set(rule.library_id), + access_mode: Set(rule.access_mode.as_str().to_string()), + created_at: Set(now), + } + .insert(&txn) + .await?; + } + + let mut active: access_groups::ActiveModel = group.into(); + active.sharing_tag_match = Set(rules.sharing_tag_match.as_str().to_string()); + active.tag_match = Set(rules.tag_match.as_str().to_string()); + active.genre_match = Set(rules.genre_match.as_str().to_string()); + active.updated_at = Set(now); + active.update(&txn).await?; + + txn.commit().await?; + Ok(()) + } + + // ==================== Resolving targets to series ==================== + + /// Series carrying each of the given sharing tags, keyed by tag ID. + pub async fn series_by_sharing_tag( + db: &impl ConnectionTrait, + sharing_tag_ids: &[Uuid], + ) -> Result>> { + let mut out: HashMap> = HashMap::new(); + if sharing_tag_ids.is_empty() { + return Ok(out); + } + for row in series_sharing_tags::Entity::find() + .filter(series_sharing_tags::Column::SharingTagId.is_in(sharing_tag_ids.to_vec())) + .all(db) + .await? + { + out.entry(row.sharing_tag_id) + .or_default() + .insert(row.series_id); + } + Ok(out) + } + + /// Series carrying each of the given tags, keyed by normalized tag name. + pub async fn series_by_tag_name( + db: &impl ConnectionTrait, + normalized_names: &[String], + ) -> Result>> { + let mut out: HashMap> = HashMap::new(); + if normalized_names.is_empty() { + return Ok(out); + } + let rows: Vec<(Uuid, String)> = series_tags::Entity::find() + .select_only() + .column(series_tags::Column::SeriesId) + .column(tags::Column::NormalizedName) + .inner_join(tags::Entity) + .filter(tags::Column::NormalizedName.is_in(normalized_names.to_vec())) + .into_tuple() + .all(db) + .await?; + for (series_id, name) in rows { + out.entry(name).or_default().insert(series_id); + } + Ok(out) + } + + /// Series carrying each of the given genres, keyed by normalized genre name. + pub async fn series_by_genre_name( + db: &impl ConnectionTrait, + normalized_names: &[String], + ) -> Result>> { + let mut out: HashMap> = HashMap::new(); + if normalized_names.is_empty() { + return Ok(out); + } + let rows: Vec<(Uuid, String)> = series_genres::Entity::find() + .select_only() + .column(series_genres::Column::SeriesId) + .column(genres::Column::NormalizedName) + .inner_join(genres::Entity) + .filter(genres::Column::NormalizedName.is_in(normalized_names.to_vec())) + .into_tuple() + .all(db) + .await?; + for (series_id, name) in rows { + out.entry(name).or_default().insert(series_id); + } + Ok(out) + } + + /// Series in each of the given libraries, keyed by library ID. + pub async fn series_by_library( + db: &impl ConnectionTrait, + library_ids: &[Uuid], + ) -> Result>> { + let mut out: HashMap> = HashMap::new(); + if library_ids.is_empty() { + return Ok(out); + } + let rows: Vec<(Uuid, Uuid)> = series::Entity::find() + .select_only() + .column(series::Column::Id) + .column(series::Column::LibraryId) + .filter(series::Column::LibraryId.is_in(library_ids.to_vec())) + .into_tuple() + .all(db) + .await?; + for (series_id, library_id) in rows { + out.entry(library_id).or_default().insert(series_id); + } + Ok(out) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ScanningStrategy; + use crate::repositories::{AccessGroupRepository, GenreRepository, LibraryRepository}; + use crate::test_helpers::setup_test_db; + + #[test] + fn validate_rejects_duplicate_names_after_normalizing() { + let rules = AccessGroupRules { + genres: vec![ + NameRule::new("Adult", AccessMode::Deny), + NameRule::new(" adult ", AccessMode::Allow), + ], + ..Default::default() + }; + assert!(rules.validate().is_err()); + } + + #[test] + fn validate_rejects_blank_names() { + let rules = AccessGroupRules { + tags: vec![NameRule::new(" ", AccessMode::Deny)], + ..Default::default() + }; + assert!(rules.validate().is_err()); + } + + #[test] + fn has_allows_ignores_denies() { + let deny_only = AccessGroupRules { + genres: vec![NameRule::new("Adult", AccessMode::Deny)], + ..Default::default() + }; + assert!(!deny_only.has_allows()); + + let with_allow = AccessGroupRules { + libraries: vec![LibraryRule { + library_id: Uuid::new_v4(), + access_mode: AccessMode::Allow, + }], + ..deny_only + }; + assert!(with_allow.has_allows()); + } + + #[tokio::test] + async fn replace_and_get_round_trip() { + let db = setup_test_db().await; + let group = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + let library = + LibraryRepository::create(&db, "Kids", "/tmp/kids", ScanningStrategy::Default) + .await + .unwrap(); + + let rules = AccessGroupRules { + genre_match: RuleMatch::All, + tags: vec![NameRule::new("Gore", AccessMode::Deny)], + genres: vec![ + NameRule::new("Adult", AccessMode::Deny), + NameRule::new("Kids", AccessMode::Allow), + ], + libraries: vec![LibraryRule { + library_id: library.id, + access_mode: AccessMode::Allow, + }], + ..Default::default() + }; + AccessGroupRulesRepository::replace(&db, group.id, &rules) + .await + .unwrap(); + + let stored = AccessGroupRulesRepository::get(&db, group.id) + .await + .unwrap() + .unwrap(); + assert_eq!(stored, rules); + + // Replacing again drops what is not in the new set + AccessGroupRulesRepository::replace(&db, group.id, &AccessGroupRules::default()) + .await + .unwrap(); + let stored = AccessGroupRulesRepository::get(&db, group.id) + .await + .unwrap() + .unwrap(); + assert_eq!(stored, AccessGroupRules::default()); + } + + #[tokio::test] + async fn replace_refuses_an_invalid_rule_set_and_keeps_the_old_one() { + let db = setup_test_db().await; + let group = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + let original = AccessGroupRules { + genres: vec![NameRule::new("Adult", AccessMode::Deny)], + ..Default::default() + }; + AccessGroupRulesRepository::replace(&db, group.id, &original) + .await + .unwrap(); + + let invalid = AccessGroupRules { + genres: vec![ + NameRule::new("Kids", AccessMode::Allow), + NameRule::new("KIDS", AccessMode::Deny), + ], + ..Default::default() + }; + assert!( + AccessGroupRulesRepository::replace(&db, group.id, &invalid) + .await + .is_err() + ); + + let stored = AccessGroupRulesRepository::get(&db, group.id) + .await + .unwrap() + .unwrap(); + assert_eq!(stored, original); + } + + /// The reason genre rules are keyed by name: cleanup deletes unused + /// genres, and the rule must still apply when the genre comes back. + #[tokio::test] + async fn genre_rule_survives_genre_cleanup() { + let db = setup_test_db().await; + let group = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + GenreRepository::create(&db, "Adult").await.unwrap(); + AccessGroupRulesRepository::replace( + &db, + group.id, + &AccessGroupRules { + genres: vec![NameRule::new("Adult", AccessMode::Deny)], + ..Default::default() + }, + ) + .await + .unwrap(); + + GenreRepository::delete_unused(&db).await.unwrap(); + + let stored = AccessGroupRulesRepository::get(&db, group.id) + .await + .unwrap() + .unwrap(); + assert_eq!( + stored.genres, + vec![NameRule::new("Adult", AccessMode::Deny)] + ); + } + + #[tokio::test] + async fn deleting_a_library_removes_its_rules() { + let db = setup_test_db().await; + let group = AccessGroupRepository::create(&db, "Kids", None) + .await + .unwrap(); + let library = + LibraryRepository::create(&db, "Adult", "/tmp/adult", ScanningStrategy::Default) + .await + .unwrap(); + AccessGroupRulesRepository::replace( + &db, + group.id, + &AccessGroupRules { + libraries: vec![LibraryRule { + library_id: library.id, + access_mode: AccessMode::Deny, + }], + ..Default::default() + }, + ) + .await + .unwrap(); + + LibraryRepository::delete(&db, library.id).await.unwrap(); + + let stored = AccessGroupRulesRepository::get(&db, group.id) + .await + .unwrap() + .unwrap(); + assert!(stored.libraries.is_empty()); + } +} diff --git a/crates/codex-db/src/repositories/genre.rs b/crates/codex-db/src/repositories/genre.rs index d769222b7..f6f962206 100644 --- a/crates/codex-db/src/repositories/genre.rs +++ b/crates/codex-db/src/repositories/genre.rs @@ -4,6 +4,7 @@ #![allow(dead_code)] +use super::access_group_rules::normalize_taxonomy_name; use anyhow::Result; use chrono::Utc; use sea_orm::{ @@ -28,7 +29,7 @@ impl GenreRepository { /// Get a genre by normalized name pub async fn get_by_name(db: &DatabaseConnection, name: &str) -> Result> { - let normalized = name.to_lowercase().trim().to_string(); + let normalized = normalize_taxonomy_name(name); let result = Genres::find() .filter(genres::Column::NormalizedName.eq(&normalized)) .one(db) @@ -47,7 +48,7 @@ impl GenreRepository { /// Create a new genre pub async fn create(db: &DatabaseConnection, name: &str) -> Result { - let normalized = name.to_lowercase().trim().to_string(); + let normalized = normalize_taxonomy_name(name); let trimmed_name = name.trim().to_string(); let active_model = genres::ActiveModel { @@ -206,7 +207,7 @@ impl GenreRepository { ) -> Result> { use crate::entities::series_genres::Entity as SeriesGenres; - let normalized = genre_name.to_lowercase().trim().to_string(); + let normalized = normalize_taxonomy_name(genre_name); // First find the genre let genre = Genres::find() diff --git a/crates/codex-db/src/repositories/mod.rs b/crates/codex-db/src/repositories/mod.rs index b76bf7570..013b26044 100644 --- a/crates/codex-db/src/repositories/mod.rs +++ b/crates/codex-db/src/repositories/mod.rs @@ -53,6 +53,7 @@ pub mod visibility; // Access groups: reusable bundles of sharing-tag grants assignable to users pub mod access_group; +pub mod access_group_rules; // OIDC authentication pub mod oidc_connection; @@ -143,6 +144,10 @@ pub use visibility::{ // Access groups pub use access_group::AccessGroupRepository; +pub use access_group_rules::{ + AccessGroupRules, AccessGroupRulesRepository, LibraryRule, NameRule, SharingTagRule, + normalize_taxonomy_name, +}; // OIDC authentication pub use oidc_connection::OidcConnectionRepository; diff --git a/crates/codex-db/src/repositories/sharing_tag.rs b/crates/codex-db/src/repositories/sharing_tag.rs index 394a05965..a7c626578 100644 --- a/crates/codex-db/src/repositories/sharing_tag.rs +++ b/crates/codex-db/src/repositories/sharing_tag.rs @@ -3,13 +3,12 @@ //! Sharing tags control content access. Series can be tagged, and users can be granted //! access (allow/deny) to content via these tags. -use anyhow::{Context, Result}; +use anyhow::Result; use chrono::Utc; use sea_orm::{ - ActiveModelTrait, ColumnTrait, ConnectionTrait, DatabaseConnection, DbBackend, EntityTrait, - PaginatorTrait, QueryFilter, QueryOrder, Set, Statement, + ActiveModelTrait, ColumnTrait, DatabaseConnection, EntityTrait, PaginatorTrait, QueryFilter, + QueryOrder, Set, }; -use std::collections::HashSet; use uuid::Uuid; use crate::entities::{ @@ -561,225 +560,10 @@ impl SharingTagRepository { Ok(user_ids) } - // ==================== Content Filtering ==================== - - /// Check if a series is visible to a user based on sharing tags - /// - /// Visibility rules: - /// 1. If series has no sharing tags -> visible to everyone - /// 2. If series has sharing tags -> user needs at least one 'allow' grant for those tags - /// 3. If user has any 'deny' grant for any of the series' tags -> not visible - #[allow(dead_code)] - pub async fn is_series_visible_to_user( - db: &DatabaseConnection, - series_id: Uuid, - user_id: Uuid, - ) -> Result { - // Get series sharing tags - let series_tags = Self::get_tags_for_series(db, series_id).await?; - - // If series has no sharing tags, it's visible to everyone - if series_tags.is_empty() { - return Ok(true); - } - - let series_tag_ids: Vec = series_tags.iter().map(|t| t.id).collect(); - - // Get user's denied tags - let denied_tag_ids = Self::get_denied_tag_ids_for_user(db, user_id).await?; - - // If user has any deny grant for this series' tags, not visible - for tag_id in &series_tag_ids { - if denied_tag_ids.contains(tag_id) { - return Ok(false); - } - } - - // Get user's allowed tags - let allowed_tag_ids = Self::get_allowed_tag_ids_for_user(db, user_id).await?; - - // User needs at least one allow grant for the series' tags - for tag_id in &series_tag_ids { - if allowed_tag_ids.contains(tag_id) { - return Ok(true); - } - } - - // No allow grants match the series' tags - Ok(false) - } - - /// Get all series IDs that are visible to a user based on sharing tags - /// - /// This is used to filter series queries. Returns None if user has no - /// sharing tag restrictions (can see all content). - #[allow(dead_code)] - pub async fn get_visible_series_ids_for_user( - db: &DatabaseConnection, - user_id: Uuid, - ) -> Result>> { - use crate::entities::series_sharing_tags::Entity as SeriesSharingTags; - - // Get user grants - let allowed_tag_ids = Self::get_allowed_tag_ids_for_user(db, user_id).await?; - let denied_tag_ids = Self::get_denied_tag_ids_for_user(db, user_id).await?; - - // If user has no grants at all, they can see all unrestricted content - if allowed_tag_ids.is_empty() && denied_tag_ids.is_empty() { - return Ok(None); - } - - // Get all series that have sharing tags - let _all_tagged_series: std::collections::HashSet = SeriesSharingTags::find() - .all(db) - .await? - .into_iter() - .map(|st| st.series_id) - .collect(); - - // Get series IDs for denied tags (these are always hidden) - let mut denied_series: std::collections::HashSet = std::collections::HashSet::new(); - for tag_id in &denied_tag_ids { - let series_ids = Self::get_series_with_tag(db, *tag_id).await?; - denied_series.extend(series_ids); - } - - // Get series IDs for allowed tags - let mut allowed_series: std::collections::HashSet = std::collections::HashSet::new(); - for tag_id in &allowed_tag_ids { - let series_ids = Self::get_series_with_tag(db, *tag_id).await?; - allowed_series.extend(series_ids); - } - - // Remove denied series from allowed - allowed_series.retain(|id| !denied_series.contains(id)); - - // Series without sharing tags are visible to everyone - // But we need to exclude series that have tags the user doesn't have grants for - - // Result: allowed_series (from grants) - denied_series - // Plus: series without any sharing tags (which are always visible) - // We return None to indicate "no special filtering needed" if: - // - User has only deny grants (hide specific content but see everything else) - - // If user has only deny grants and no allow grants - if allowed_tag_ids.is_empty() && !denied_tag_ids.is_empty() { - // User can see all series except those with denied tags - // Return None to indicate broad access, but caller should filter out denied series - return Ok(None); - } - - Ok(Some(allowed_series.into_iter().collect())) - } - - /// Get effective sharing-tag grants for a user. - /// - /// Effective grants are the union of: - /// - The user's own `user_sharing_tags` rows (per-user overrides), and - /// - Every grant on every access group the user belongs to. - /// - /// The same tag may appear in both `allows` and `denies` (e.g. a group - /// allows it while the user denies it). Callers apply the deny-wins rule - /// downstream — this method intentionally surfaces the raw union so the - /// caller can decide policy. - /// - /// Returns `(allow_tag_ids, deny_tag_ids)`. Each vec is deduplicated. - pub async fn get_effective_grants( - db: &DatabaseConnection, - user_id: Uuid, - ) -> Result<(Vec, Vec)> { - let backend = db.get_database_backend(); - let sql = match backend { - DbBackend::Postgres => { - r#" - SELECT sharing_tag_id, access_mode - FROM user_sharing_tags - WHERE user_id = $1 - UNION - SELECT gst.sharing_tag_id, gst.access_mode - FROM user_access_groups uag - JOIN access_group_sharing_tags gst - ON gst.access_group_id = uag.access_group_id - WHERE uag.user_id = $2 - "# - } - _ => { - r#" - SELECT sharing_tag_id, access_mode - FROM user_sharing_tags - WHERE user_id = ? - UNION - SELECT gst.sharing_tag_id, gst.access_mode - FROM user_access_groups uag - JOIN access_group_sharing_tags gst - ON gst.access_group_id = uag.access_group_id - WHERE uag.user_id = ? - "# - } - }; - let stmt = - Statement::from_sql_and_values(backend, sql, vec![user_id.into(), user_id.into()]); - let rows = db - .query_all(stmt) - .await - .context("Failed to query effective sharing-tag grants")?; - - let mut allows: HashSet = HashSet::new(); - let mut denies: HashSet = HashSet::new(); - for row in rows { - let tag_id: Uuid = row.try_get("", "sharing_tag_id")?; - let mode: String = row.try_get("", "access_mode")?; - match mode.parse::().unwrap_or(AccessMode::Allow) { - AccessMode::Allow => { - allows.insert(tag_id); - } - AccessMode::Deny => { - denies.insert(tag_id); - } - } - } - Ok((allows.into_iter().collect(), denies.into_iter().collect())) - } - - /// Get series IDs to exclude for a user (series with denied tags) - pub async fn get_excluded_series_ids_for_user( - db: &DatabaseConnection, - user_id: Uuid, - ) -> Result> { - let denied_tag_ids = Self::get_denied_tag_ids_for_user(db, user_id).await?; - - let mut excluded_series = std::collections::HashSet::new(); - for tag_id in denied_tag_ids { - let series_ids = Self::get_series_with_tag(db, tag_id).await?; - excluded_series.extend(series_ids); - } - - Ok(excluded_series.into_iter().collect()) - } - - /// Get all series IDs that have any of the given tags - pub async fn get_series_ids_with_any_tags( - db: &DatabaseConnection, - tag_ids: &[Uuid], - ) -> Result> { - use crate::entities::series_sharing_tags::Entity as SeriesSharingTags; - - if tag_ids.is_empty() { - return Ok(vec![]); - } - - let series_ids: Vec = SeriesSharingTags::find() - .filter(series_sharing_tags::Column::SharingTagId.is_in(tag_ids.to_vec())) - .all(db) - .await? - .into_iter() - .map(|st| st.series_id) - .collect::>() - .into_iter() - .collect(); - - Ok(series_ids) - } + // ==================== Series lookups ==================== + // + // What a user may see is decided by `ContentFilter` in codex-services, + // not here. /// Get set of all series IDs that have any sharing tags pub async fn get_tagged_series_ids( @@ -1096,355 +880,4 @@ mod tests { .unwrap(); assert_eq!(grants.len(), 1); } - - #[tokio::test] - async fn test_series_visibility() { - let (db, _temp_dir) = create_test_db().await; - - let library = LibraryRepository::create( - db.sea_orm_connection(), - "Test Library", - "/test/path", - ScanningStrategy::Default, - ) - .await - .unwrap(); - - let user = create_test_user(db.sea_orm_connection(), "viewer").await; - - // Series without sharing tags is visible to everyone - let series_public = - SeriesRepository::create(db.sea_orm_connection(), library.id, "Public Series", None) - .await - .unwrap(); - - let visible = SharingTagRepository::is_series_visible_to_user( - db.sea_orm_connection(), - series_public.id, - user.id, - ) - .await - .unwrap(); - assert!(visible); - - // Create restricted series with a sharing tag - let kids_tag = SharingTagRepository::create(db.sea_orm_connection(), "Kids", None) - .await - .unwrap(); - - let series_restricted = - SeriesRepository::create(db.sea_orm_connection(), library.id, "Kids Series", None) - .await - .unwrap(); - - SharingTagRepository::add_tag_to_series( - db.sea_orm_connection(), - series_restricted.id, - kids_tag.id, - ) - .await - .unwrap(); - - // User without grant cannot see restricted series - let visible = SharingTagRepository::is_series_visible_to_user( - db.sea_orm_connection(), - series_restricted.id, - user.id, - ) - .await - .unwrap(); - assert!(!visible); - - // Grant user access - SharingTagRepository::set_user_grant( - db.sea_orm_connection(), - user.id, - kids_tag.id, - AccessMode::Allow, - ) - .await - .unwrap(); - - // Now user can see the series - let visible = SharingTagRepository::is_series_visible_to_user( - db.sea_orm_connection(), - series_restricted.id, - user.id, - ) - .await - .unwrap(); - assert!(visible); - - // Change to deny grant - SharingTagRepository::set_user_grant( - db.sea_orm_connection(), - user.id, - kids_tag.id, - AccessMode::Deny, - ) - .await - .unwrap(); - - // User cannot see the series anymore - let visible = SharingTagRepository::is_series_visible_to_user( - db.sea_orm_connection(), - series_restricted.id, - user.id, - ) - .await - .unwrap(); - assert!(!visible); - } - - #[tokio::test] - async fn test_get_effective_grants_user_only() { - let (db, _temp_dir) = create_test_db().await; - let conn = db.sea_orm_connection(); - let user = create_test_user(conn, "u").await; - let allow_tag = SharingTagRepository::create(conn, "allow", None) - .await - .unwrap(); - let deny_tag = SharingTagRepository::create(conn, "deny", None) - .await - .unwrap(); - - SharingTagRepository::set_user_grant(conn, user.id, allow_tag.id, AccessMode::Allow) - .await - .unwrap(); - SharingTagRepository::set_user_grant(conn, user.id, deny_tag.id, AccessMode::Deny) - .await - .unwrap(); - - let (allows, denies) = SharingTagRepository::get_effective_grants(conn, user.id) - .await - .unwrap(); - assert_eq!(allows, vec![allow_tag.id]); - assert_eq!(denies, vec![deny_tag.id]); - } - - #[tokio::test] - async fn test_get_effective_grants_group_only() { - use crate::entities::user_access_groups::MembershipSource; - use crate::repositories::AccessGroupRepository; - - let (db, _temp_dir) = create_test_db().await; - let conn = db.sea_orm_connection(); - let user = create_test_user(conn, "u").await; - let group = AccessGroupRepository::create(conn, "Group", None) - .await - .unwrap(); - let allow_tag = SharingTagRepository::create(conn, "manga", None) - .await - .unwrap(); - let deny_tag = SharingTagRepository::create(conn, "adult", None) - .await - .unwrap(); - - AccessGroupRepository::set_grant(conn, group.id, allow_tag.id, AccessMode::Allow) - .await - .unwrap(); - AccessGroupRepository::set_grant(conn, group.id, deny_tag.id, AccessMode::Deny) - .await - .unwrap(); - AccessGroupRepository::add_member(conn, group.id, user.id, MembershipSource::Manual) - .await - .unwrap(); - - let (allows, denies) = SharingTagRepository::get_effective_grants(conn, user.id) - .await - .unwrap(); - assert_eq!(allows, vec![allow_tag.id]); - assert_eq!(denies, vec![deny_tag.id]); - } - - #[tokio::test] - async fn test_get_effective_grants_user_and_group_merged() { - use crate::entities::user_access_groups::MembershipSource; - use crate::repositories::AccessGroupRepository; - - let (db, _temp_dir) = create_test_db().await; - let conn = db.sea_orm_connection(); - let user = create_test_user(conn, "u").await; - let group = AccessGroupRepository::create(conn, "Manga Readers", None) - .await - .unwrap(); - let manga = SharingTagRepository::create(conn, "manga", None) - .await - .unwrap(); - let adult = SharingTagRepository::create(conn, "18+", None) - .await - .unwrap(); - let news = SharingTagRepository::create(conn, "anime-news", None) - .await - .unwrap(); - - // Group: allow manga - AccessGroupRepository::set_grant(conn, group.id, manga.id, AccessMode::Allow) - .await - .unwrap(); - AccessGroupRepository::add_member(conn, group.id, user.id, MembershipSource::Manual) - .await - .unwrap(); - - // User: deny 18+, allow anime-news (per-user extra) - SharingTagRepository::set_user_grant(conn, user.id, adult.id, AccessMode::Deny) - .await - .unwrap(); - SharingTagRepository::set_user_grant(conn, user.id, news.id, AccessMode::Allow) - .await - .unwrap(); - - let (mut allows, denies) = SharingTagRepository::get_effective_grants(conn, user.id) - .await - .unwrap(); - allows.sort(); - let mut expected_allow = vec![manga.id, news.id]; - expected_allow.sort(); - assert_eq!(allows, expected_allow); - assert_eq!(denies, vec![adult.id]); - } - - #[tokio::test] - async fn test_get_effective_grants_deduplicates_across_sources() { - use crate::entities::user_access_groups::MembershipSource; - use crate::repositories::AccessGroupRepository; - - let (db, _temp_dir) = create_test_db().await; - let conn = db.sea_orm_connection(); - let user = create_test_user(conn, "u").await; - let g1 = AccessGroupRepository::create(conn, "G1", None) - .await - .unwrap(); - let g2 = AccessGroupRepository::create(conn, "G2", None) - .await - .unwrap(); - let tag = SharingTagRepository::create(conn, "shared", None) - .await - .unwrap(); - - // Same allow grant from two groups + user - AccessGroupRepository::set_grant(conn, g1.id, tag.id, AccessMode::Allow) - .await - .unwrap(); - AccessGroupRepository::set_grant(conn, g2.id, tag.id, AccessMode::Allow) - .await - .unwrap(); - SharingTagRepository::set_user_grant(conn, user.id, tag.id, AccessMode::Allow) - .await - .unwrap(); - AccessGroupRepository::add_member(conn, g1.id, user.id, MembershipSource::Manual) - .await - .unwrap(); - AccessGroupRepository::add_member(conn, g2.id, user.id, MembershipSource::Manual) - .await - .unwrap(); - - let (allows, denies) = SharingTagRepository::get_effective_grants(conn, user.id) - .await - .unwrap(); - assert_eq!(allows, vec![tag.id]); - assert!(denies.is_empty()); - } - - #[tokio::test] - async fn test_get_effective_grants_no_grants_no_groups() { - let (db, _temp_dir) = create_test_db().await; - let conn = db.sea_orm_connection(); - let user = create_test_user(conn, "u").await; - let (allows, denies) = SharingTagRepository::get_effective_grants(conn, user.id) - .await - .unwrap(); - assert!(allows.is_empty()); - assert!(denies.is_empty()); - } - - #[tokio::test] - async fn test_get_effective_grants_surfaces_both_allow_and_deny_for_same_tag() { - use crate::entities::user_access_groups::MembershipSource; - use crate::repositories::AccessGroupRepository; - - // Group allows the tag; user denies it. Both surface; deny-wins is - // applied by the caller (ContentFilter), not here. - let (db, _temp_dir) = create_test_db().await; - let conn = db.sea_orm_connection(); - let user = create_test_user(conn, "u").await; - let group = AccessGroupRepository::create(conn, "G", None) - .await - .unwrap(); - let tag = SharingTagRepository::create(conn, "tag", None) - .await - .unwrap(); - - AccessGroupRepository::set_grant(conn, group.id, tag.id, AccessMode::Allow) - .await - .unwrap(); - AccessGroupRepository::add_member(conn, group.id, user.id, MembershipSource::Manual) - .await - .unwrap(); - SharingTagRepository::set_user_grant(conn, user.id, tag.id, AccessMode::Deny) - .await - .unwrap(); - - let (allows, denies) = SharingTagRepository::get_effective_grants(conn, user.id) - .await - .unwrap(); - assert_eq!(allows, vec![tag.id]); - assert_eq!(denies, vec![tag.id]); - } - - #[tokio::test] - async fn test_excluded_series_for_user() { - let (db, _temp_dir) = create_test_db().await; - - let library = LibraryRepository::create( - db.sea_orm_connection(), - "Test Library", - "/test/path", - ScanningStrategy::Default, - ) - .await - .unwrap(); - - let user = create_test_user(db.sea_orm_connection(), "viewer").await; - - let adults_tag = SharingTagRepository::create(db.sea_orm_connection(), "Adults", None) - .await - .unwrap(); - - let series1 = - SeriesRepository::create(db.sea_orm_connection(), library.id, "Series 1", None) - .await - .unwrap(); - let series2 = - SeriesRepository::create(db.sea_orm_connection(), library.id, "Series 2", None) - .await - .unwrap(); - - // Tag series2 as adults only - SharingTagRepository::add_tag_to_series(db.sea_orm_connection(), series2.id, adults_tag.id) - .await - .unwrap(); - - // User has deny grant for adults tag - SharingTagRepository::set_user_grant( - db.sea_orm_connection(), - user.id, - adults_tag.id, - AccessMode::Deny, - ) - .await - .unwrap(); - - // Get excluded series - let excluded = SharingTagRepository::get_excluded_series_ids_for_user( - db.sea_orm_connection(), - user.id, - ) - .await - .unwrap(); - - assert_eq!(excluded.len(), 1); - assert!(excluded.contains(&series2.id)); - assert!(!excluded.contains(&series1.id)); - } } diff --git a/crates/codex-db/src/repositories/tag.rs b/crates/codex-db/src/repositories/tag.rs index c6449bfcc..9bc6456e0 100644 --- a/crates/codex-db/src/repositories/tag.rs +++ b/crates/codex-db/src/repositories/tag.rs @@ -4,6 +4,7 @@ #![allow(dead_code)] +use super::access_group_rules::normalize_taxonomy_name; use anyhow::Result; use chrono::Utc; use sea_orm::{ @@ -28,7 +29,7 @@ impl TagRepository { /// Get a tag by normalized name pub async fn get_by_name(db: &DatabaseConnection, name: &str) -> Result> { - let normalized = name.to_lowercase().trim().to_string(); + let normalized = normalize_taxonomy_name(name); let result = Tags::find() .filter(tags::Column::NormalizedName.eq(&normalized)) .one(db) @@ -47,7 +48,7 @@ impl TagRepository { /// Create a new tag pub async fn create(db: &DatabaseConnection, name: &str) -> Result { - let normalized = name.to_lowercase().trim().to_string(); + let normalized = normalize_taxonomy_name(name); let trimmed_name = name.trim().to_string(); let active_model = tags::ActiveModel { @@ -206,7 +207,7 @@ impl TagRepository { ) -> Result> { use crate::entities::series_tags::Entity as SeriesTags; - let normalized = tag_name.to_lowercase().trim().to_string(); + let normalized = normalize_taxonomy_name(tag_name); // First find the tag let tag = Tags::find() diff --git a/crates/codex-migrate/src/registry.rs b/crates/codex-migrate/src/registry.rs index e7d49f299..f1a32744c 100644 --- a/crates/codex-migrate/src/registry.rs +++ b/crates/codex-migrate/src/registry.rs @@ -85,8 +85,11 @@ macro_rules! for_each_entity { $op!(series_sharing_tags); $op!(sharing_tags); $op!(user_sharing_tags); + $op!(access_group_genre_rules); + $op!(access_group_library_rules); $op!(access_group_oidc_mappings); $op!(access_group_sharing_tags); + $op!(access_group_tag_rules); $op!(access_groups); $op!(user_access_groups); $op!(collection_series); diff --git a/crates/codex-services/src/content_filter.rs b/crates/codex-services/src/content_filter.rs index f05498a26..b29be3b50 100644 --- a/crates/codex-services/src/content_filter.rs +++ b/crates/codex-services/src/content_filter.rs @@ -1,83 +1,278 @@ -//! Content filter for sharing tag-based access control +//! Content filter: what a user is allowed to see. //! -//! This module provides content filtering based on user sharing tag grants. -//! It allows filtering series/books based on which sharing tags the user -//! has access to (allow) or is restricted from (deny). +//! A user's visibility comes from their access groups and their personal +//! sharing-tag grants. Each access group holds allow/deny rules on four +//! types: sharing tags, regular tags, genres and libraries. //! -//! ## Access Rules (in order of precedence) +//! ## Rules //! -//! 1. **Deny always wins**: If user has `deny` grant for ANY tag on a series → hidden -//! 2. **Whitelist mode** (user has any `allow` grants): User only sees series with allowed tags -//! 3. **No grants**: User sees everything (default-open behavior) - -use codex_db::repositories::{SeriesVisibility, SharingTagRepository}; -use sea_orm::DatabaseConnection; -use std::collections::HashSet; +//! 1. **Admins and maintainers see everything.** No group or grant applies to +//! them: maintainers curate metadata, covers and scans, which needs the +//! whole library. +//! 2. **Allows, per group.** In one group, values of the same type combine as +//! "any of" (or "all of", per the group's switch for that type; libraries +//! are always "any of"), and different types combine with AND. "Allow +//! library Manga + allow genre Kids" is Kids-genre series in Manga. +//! 3. **Allows, across groups: OR.** Each group grants its own scope, and the +//! user sees the union. Personal sharing-tag allows count as one more group. +//! 4. **Denies are global and always win.** Every deny, from every group and +//! from personal grants, is collected into one set and removed at the end. +//! A deny is not scoped to the group it is written in; that is what stops +//! another group from granting it back. +//! 5. **No allow anywhere** (or only deny-only groups): the user sees +//! everything except what is denied. +//! +//! visible = (union over groups with allows of: AND over types of: match) minus denies + +use codex_db::entities::{ + access_groups::RuleMatch, user_access_groups, user_sharing_tags, user_sharing_tags::AccessMode, +}; +use codex_db::repositories::{ + AccessGroupRules, AccessGroupRulesRepository, SeriesVisibility, UserRepository, +}; +use codex_models::permissions::UserRole; +use sea_orm::{ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter}; +use std::collections::{HashMap, HashSet}; use uuid::Uuid; -/// Content filter based on user's sharing tag grants +/// Content filter based on the user's access groups and grants /// -/// This struct encapsulates the sharing tag access rules for a user -/// and can be used to filter content queries efficiently. +/// This struct encapsulates the access rules for a user and can be used to +/// filter content queries efficiently. #[derive(Debug, Clone, Default)] pub struct ContentFilter { - /// Series IDs that are explicitly excluded (user has deny grants for their tags) + /// Series IDs that are explicitly excluded (matched by a deny rule) pub excluded_series_ids: HashSet, /// Series IDs that user is allowed to see (when in whitelist mode) - /// Only populated when user has allow grants + /// Only populated when some group or grant allows something pub allowed_series_ids: Option>, - /// Whether user has any allow grants (triggers whitelist mode) + /// Whether any allow rule applies (triggers whitelist mode) pub whitelist_mode: bool, - /// Whether the user has any sharing tag restrictions at all + /// Whether the user has any restrictions at all pub has_restrictions: bool, } +/// Series IDs per rule target, resolved once for every group of a user. +#[derive(Default)] +struct ResolvedTargets { + sharing_tags: HashMap>, + tags: HashMap>, + genres: HashMap>, + libraries: HashMap>, +} + impl ContentFilter { - /// Create a content filter for a user based on their effective sharing tag grants. - /// - /// Effective grants are the union of the user's per-user `user_sharing_tags` - /// rows and grants attached to every access group the user belongs to. - /// Access groups can supply both `allow` and `deny` rules; users can - /// override either side. Downstream behavior (deny-wins, whitelist mode) - /// is unchanged — the only difference vs. the old implementation is the - /// data source feeding the rule set. + /// Build the content filter for a user. /// - /// The result can be reused for multiple queries in the same request. + /// Runs a fixed number of queries regardless of how many groups or rules + /// the user has. The result can be reused for multiple queries in the + /// same request. pub async fn for_user(db: &DatabaseConnection, user_id: Uuid) -> anyhow::Result { - let (allowed_tag_ids, denied_tag_ids) = - SharingTagRepository::get_effective_grants(db, user_id).await?; - - // Excluded series = series tagged with any effective deny tag. - let excluded_series_ids = if denied_tag_ids.is_empty() { - Vec::new() - } else { - SharingTagRepository::get_series_ids_with_any_tags(db, &denied_tag_ids).await? + let Some(user) = UserRepository::get_by_id(db, user_id).await? else { + // Fail closed: an unknown user sees nothing. + return Ok(Self::deny_all()); }; + if bypasses_content_filter(user.get_role()) { + return Ok(Self::allow_all()); + } - // Whitelist mode: any effective allow grant flips the user into - // "only-tagged" mode (untagged content disappears). - let whitelist_mode = !allowed_tag_ids.is_empty(); + // Personal sharing-tag grants act as one more group. + let mut personal = AccessGroupRules::default(); + for grant in user_sharing_tags::Entity::find() + .filter(user_sharing_tags::Column::UserId.eq(user_id)) + .all(db) + .await? + { + personal + .sharing_tags + .push(codex_db::repositories::SharingTagRule { + sharing_tag_id: grant.sharing_tag_id, + access_mode: grant.get_access_mode(), + }); + } - let allowed_series_ids = if whitelist_mode { - let ids = - SharingTagRepository::get_series_ids_with_any_tags(db, &allowed_tag_ids).await?; - Some(ids.into_iter().collect()) - } else { - None - }; + let group_ids: Vec = user_access_groups::Entity::find() + .filter(user_access_groups::Column::UserId.eq(user_id)) + .all(db) + .await? + .into_iter() + .map(|m| m.access_group_id) + .collect(); + let mut rule_sets: Vec = + AccessGroupRulesRepository::load_for_groups(db, &group_ids) + .await? + .into_values() + .collect(); + rule_sets.push(personal); + + let targets = Self::resolve_targets(db, &rule_sets).await?; + + let mut excluded_series_ids: HashSet = HashSet::new(); + let mut allowed: HashSet = HashSet::new(); + let mut whitelist_mode = false; + + for rules in &rule_sets { + excluded_series_ids.extend(Self::denied_by(rules, &targets)); + if let Some(granted) = Self::granted_by(rules, &targets) { + whitelist_mode = true; + allowed.extend(granted); + } + } let has_restrictions = !excluded_series_ids.is_empty() || whitelist_mode; - Ok(Self { - excluded_series_ids: excluded_series_ids.into_iter().collect(), - allowed_series_ids, + excluded_series_ids, + allowed_series_ids: whitelist_mode.then_some(allowed), whitelist_mode, has_restrictions, }) } + /// Resolve every target named by any rule to its series, one query per type. + async fn resolve_targets( + db: &DatabaseConnection, + rule_sets: &[AccessGroupRules], + ) -> anyhow::Result { + let mut sharing_tag_ids = HashSet::new(); + let mut tag_names = HashSet::new(); + let mut genre_names = HashSet::new(); + let mut library_ids = HashSet::new(); + for rules in rule_sets { + sharing_tag_ids.extend(rules.sharing_tags.iter().map(|r| r.sharing_tag_id)); + tag_names.extend(rules.tags.iter().map(|r| r.normalized_name())); + genre_names.extend(rules.genres.iter().map(|r| r.normalized_name())); + library_ids.extend(rules.libraries.iter().map(|r| r.library_id)); + } + + let sharing_tag_ids: Vec = sharing_tag_ids.into_iter().collect(); + let tag_names: Vec = tag_names.into_iter().collect(); + let genre_names: Vec = genre_names.into_iter().collect(); + let library_ids: Vec = library_ids.into_iter().collect(); + + Ok(ResolvedTargets { + sharing_tags: AccessGroupRulesRepository::series_by_sharing_tag(db, &sharing_tag_ids) + .await?, + tags: AccessGroupRulesRepository::series_by_tag_name(db, &tag_names).await?, + genres: AccessGroupRulesRepository::series_by_genre_name(db, &genre_names).await?, + libraries: AccessGroupRulesRepository::series_by_library(db, &library_ids).await?, + }) + } + + /// Every series matched by any deny rule of this rule set. + fn denied_by(rules: &AccessGroupRules, targets: &ResolvedTargets) -> HashSet { + let mut denied = HashSet::new(); + let empty = HashSet::new(); + for rule in rules + .sharing_tags + .iter() + .filter(|r| r.access_mode == AccessMode::Deny) + { + denied.extend( + targets + .sharing_tags + .get(&rule.sharing_tag_id) + .unwrap_or(&empty), + ); + } + for rule in rules + .tags + .iter() + .filter(|r| r.access_mode == AccessMode::Deny) + { + denied.extend(targets.tags.get(&rule.normalized_name()).unwrap_or(&empty)); + } + for rule in rules + .genres + .iter() + .filter(|r| r.access_mode == AccessMode::Deny) + { + denied.extend( + targets + .genres + .get(&rule.normalized_name()) + .unwrap_or(&empty), + ); + } + for rule in rules + .libraries + .iter() + .filter(|r| r.access_mode == AccessMode::Deny) + { + denied.extend(targets.libraries.get(&rule.library_id).unwrap_or(&empty)); + } + denied + } + + /// What this rule set's allow rules grant, or `None` if it has none. + /// + /// Each type with allows yields one set (any/all per the group's switch), + /// and the sets of different types are intersected. + fn granted_by(rules: &AccessGroupRules, targets: &ResolvedTargets) -> Option> { + let empty = HashSet::new(); + let mut per_type: Vec> = Vec::new(); + + let sharing: Vec<&HashSet> = rules + .sharing_tags + .iter() + .filter(|r| r.access_mode == AccessMode::Allow) + .map(|r| { + targets + .sharing_tags + .get(&r.sharing_tag_id) + .unwrap_or(&empty) + }) + .collect(); + if !sharing.is_empty() { + per_type.push(combine(&sharing, rules.sharing_tag_match)); + } + + let tags: Vec<&HashSet> = rules + .tags + .iter() + .filter(|r| r.access_mode == AccessMode::Allow) + .map(|r| targets.tags.get(&r.normalized_name()).unwrap_or(&empty)) + .collect(); + if !tags.is_empty() { + per_type.push(combine(&tags, rules.tag_match)); + } + + let genres: Vec<&HashSet> = rules + .genres + .iter() + .filter(|r| r.access_mode == AccessMode::Allow) + .map(|r| targets.genres.get(&r.normalized_name()).unwrap_or(&empty)) + .collect(); + if !genres.is_empty() { + per_type.push(combine(&genres, rules.genre_match)); + } + + // A series is in one library, so "all of" can never match: always any. + let libraries: Vec<&HashSet> = rules + .libraries + .iter() + .filter(|r| r.access_mode == AccessMode::Allow) + .map(|r| targets.libraries.get(&r.library_id).unwrap_or(&empty)) + .collect(); + if !libraries.is_empty() { + per_type.push(combine(&libraries, RuleMatch::Any)); + } + + let mut iter = per_type.into_iter(); + let first = iter.next()?; + Some(iter.fold(first, |acc, set| acc.intersection(&set).copied().collect())) + } + + /// A filter under which nothing is visible. + fn deny_all() -> Self { + Self { + excluded_series_ids: HashSet::new(), + allowed_series_ids: Some(HashSet::new()), + whitelist_mode: true, + has_restrictions: true, + } + } + /// Create a content filter that allows all content (no restrictions) - #[allow(dead_code)] pub fn allow_all() -> Self { Self { excluded_series_ids: HashSet::new(), @@ -151,6 +346,27 @@ impl ContentFilter { } } +/// Roles that access groups and sharing-tag grants never restrict. +pub fn bypasses_content_filter(role: UserRole) -> bool { + matches!(role, UserRole::Admin | UserRole::Maintainer) +} + +/// Combine the series sets of one rule type: union for "any", intersection for "all". +fn combine(sets: &[&HashSet], mode: RuleMatch) -> HashSet { + match mode { + RuleMatch::Any => sets.iter().flat_map(|s| s.iter().copied()).collect(), + RuleMatch::All => { + let mut iter = sets.iter(); + let Some(first) = iter.next() else { + return HashSet::new(); + }; + iter.fold((*first).clone(), |acc, set| { + acc.intersection(set).copied().collect() + }) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -312,7 +528,7 @@ mod db_tests { //! DB-backed integration tests for the group-aware `ContentFilter`. //! //! These exercise the full path from `user_sharing_tags` and - //! `user_access_groups` rows through `get_effective_grants` into the + //! `user_access_groups` rows and group rules into the //! filter's boolean output. The pure unit tests above cover the //! in-memory rules; here we cover the data wiring. @@ -640,6 +856,531 @@ mod db_tests { assert!(filter.is_series_visible(series.id)); } + // ---------- Rules on tags, genres and libraries ---------- + + use codex_db::entities::access_groups::RuleMatch; + use codex_db::repositories::{ + AccessGroupRules, AccessGroupRulesRepository, GenreRepository, LibraryRule, NameRule, + SharingTagRule, TagRepository, + }; + + struct Fixture { + db: codex_db::Database, + _temp_dir: tempfile::TempDir, + } + + impl Fixture { + async fn new() -> Self { + let (db, _temp_dir) = create_test_db().await; + Self { db, _temp_dir } + } + + fn conn(&self) -> &DatabaseConnection { + self.db.sea_orm_connection() + } + + async fn library(&self, name: &str) -> Uuid { + LibraryRepository::create( + self.conn(), + name, + &format!("/tmp/{name}"), + ScanningStrategy::Default, + ) + .await + .unwrap() + .id + } + + async fn series(&self, library_id: Uuid, name: &str, genres: &[&str]) -> Uuid { + let series = SeriesRepository::create(self.conn(), library_id, name, None) + .await + .unwrap(); + for genre in genres { + GenreRepository::add_genre_to_series(self.conn(), series.id, genre) + .await + .unwrap(); + } + series.id + } + + async fn group(&self, name: &str, user_id: Uuid, rules: AccessGroupRules) -> Uuid { + let group = AccessGroupRepository::create(self.conn(), name, None) + .await + .unwrap(); + AccessGroupRulesRepository::replace(self.conn(), group.id, &rules) + .await + .unwrap(); + AccessGroupRepository::add_member( + self.conn(), + group.id, + user_id, + MembershipSource::Manual, + ) + .await + .unwrap(); + group.id + } + + async fn filter(&self, user_id: Uuid) -> ContentFilter { + ContentFilter::for_user(self.conn(), user_id).await.unwrap() + } + } + + fn allow_library(library_id: Uuid) -> LibraryRule { + LibraryRule { + library_id, + access_mode: AccessMode::Allow, + } + } + + fn deny_library(library_id: Uuid) -> LibraryRule { + LibraryRule { + library_id, + access_mode: AccessMode::Deny, + } + } + + async fn make_with_role(db: &DatabaseConnection, username: &str, role: &str) -> Uuid { + let id = make_user(db, username).await; + let mut user = UserRepository::get_by_id(db, id).await.unwrap().unwrap(); + user.role = role.to_string(); + UserRepository::update(db, &user).await.unwrap(); + id + } + + async fn make_admin(db: &DatabaseConnection, username: &str) -> Uuid { + make_with_role(db, username, "admin").await + } + + /// Maintainers curate content (metadata, scans, covers), so they see all of it. + #[tokio::test] + async fn maintainers_bypass_every_rule() { + let f = Fixture::new().await; + let maintainer = make_with_role(f.conn(), "curator", "maintainer").await; + let lib = f.library("Adult").await; + let adult = f.series(lib, "Adult", &["Adult"]).await; + f.group( + "Locked", + maintainer, + AccessGroupRules { + genres: vec![NameRule::new("Adult", AccessMode::Deny)], + libraries: vec![deny_library(lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(maintainer).await; + assert!(!filter.has_restrictions); + assert!(filter.is_series_visible(adult)); + } + + #[tokio::test] + async fn admins_bypass_every_rule() { + let f = Fixture::new().await; + let admin = make_admin(f.conn(), "admin").await; + let lib = f.library("Adult").await; + let adult = f.series(lib, "Adult", &["Adult"]).await; + let tag = SharingTagRepository::create(f.conn(), "18+", None) + .await + .unwrap(); + SharingTagRepository::add_tag_to_series(f.conn(), adult, tag.id) + .await + .unwrap(); + SharingTagRepository::set_user_grant(f.conn(), admin, tag.id, AccessMode::Deny) + .await + .unwrap(); + f.group( + "Locked", + admin, + AccessGroupRules { + genres: vec![NameRule::new("Adult", AccessMode::Deny)], + libraries: vec![deny_library(lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(admin).await; + assert!(!filter.has_restrictions); + assert!(filter.is_series_visible(adult)); + } + + #[tokio::test] + async fn genre_deny_hides_without_starting_a_whitelist() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let lib = f.library("Manga").await; + let adult = f.series(lib, "Adult", &["Adult", "Action"]).await; + let action = f.series(lib, "Action", &["Action"]).await; + let untagged = f.series(lib, "Untagged", &[]).await; + f.group( + "No adult", + user, + AccessGroupRules { + genres: vec![NameRule::new("adult", AccessMode::Deny)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!( + !filter.whitelist_mode, + "a deny-only group must not whitelist" + ); + assert!( + !filter.is_series_visible(adult), + "matched case-insensitively" + ); + assert!(filter.is_series_visible(action)); + assert!(filter.is_series_visible(untagged)); + } + + #[tokio::test] + async fn tag_deny_matches_by_name() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let lib = f.library("Manga").await; + let gory = f.series(lib, "Gory", &[]).await; + let clean = f.series(lib, "Clean", &[]).await; + TagRepository::add_tag_to_series(f.conn(), gory, "Gore") + .await + .unwrap(); + f.group( + "No gore", + user, + AccessGroupRules { + tags: vec![NameRule::new("GORE", AccessMode::Deny)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(!filter.is_series_visible(gory)); + assert!(filter.is_series_visible(clean)); + } + + #[tokio::test] + async fn library_allow_whitelists_the_whole_library() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let kids_lib = f.library("Kids").await; + let manga_lib = f.library("Manga").await; + let in_kids = f.series(kids_lib, "Bluey", &[]).await; + let in_manga = f.series(manga_lib, "Berserk", &[]).await; + f.group( + "Kids", + user, + AccessGroupRules { + libraries: vec![allow_library(kids_lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(filter.whitelist_mode); + assert!( + filter.is_series_visible(in_kids), + "no tags needed inside the library" + ); + assert!(!filter.is_series_visible(in_manga)); + } + + #[tokio::test] + async fn library_deny_hides_the_library() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let adult_lib = f.library("Adult").await; + let manga_lib = f.library("Manga").await; + let in_adult = f.series(adult_lib, "Adult", &[]).await; + let in_manga = f.series(manga_lib, "Manga", &[]).await; + f.group( + "Hide adult library", + user, + AccessGroupRules { + libraries: vec![deny_library(adult_lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(!filter.is_series_visible(in_adult)); + assert!(filter.is_series_visible(in_manga)); + } + + /// Different rule types in one group narrow each other. + #[tokio::test] + async fn types_within_a_group_combine_with_and() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let manga_lib = f.library("Manga").await; + let comics_lib = f.library("Comics").await; + let kids_manga = f.series(manga_lib, "Kids manga", &["Kids"]).await; + let other_manga = f.series(manga_lib, "Other manga", &["Action"]).await; + let kids_comic = f.series(comics_lib, "Kids comic", &["Kids"]).await; + f.group( + "Kids manga", + user, + AccessGroupRules { + genres: vec![NameRule::new("Kids", AccessMode::Allow)], + libraries: vec![allow_library(manga_lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(filter.is_series_visible(kids_manga)); + assert!(!filter.is_series_visible(other_manga)); + assert!(!filter.is_series_visible(kids_comic)); + } + + /// Each group grants its own scope; a user in two groups gets both. + #[tokio::test] + async fn groups_combine_with_or() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "reader").await; + let manga_lib = f.library("Manga").await; + let comics_lib = f.library("Comics").await; + let action_manga = f.series(manga_lib, "Action manga", &["Action"]).await; + let kids_comic = f.series(comics_lib, "Kids comic", &["Kids"]).await; + let other_comic = f.series(comics_lib, "Other comic", &["Action"]).await; + f.group( + "Manga", + user, + AccessGroupRules { + libraries: vec![allow_library(manga_lib)], + ..Default::default() + }, + ) + .await; + f.group( + "Kids comics", + user, + AccessGroupRules { + genres: vec![NameRule::new("Kids", AccessMode::Allow)], + libraries: vec![allow_library(comics_lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!( + filter.is_series_visible(action_manga), + "the genre rule of one group must not narrow another group" + ); + assert!(filter.is_series_visible(kids_comic)); + assert!(!filter.is_series_visible(other_comic)); + } + + /// A deny written in one group applies to what another group grants. + #[tokio::test] + async fn denies_apply_across_groups() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "reader").await; + let manga_lib = f.library("Manga").await; + let comics_lib = f.library("Comics").await; + let adult_manga = f.series(manga_lib, "Adult manga", &["Adult"]).await; + let manga = f.series(manga_lib, "Manga", &["Action"]).await; + let comic = f.series(comics_lib, "Comic", &[]).await; + f.group( + "Manga", + user, + AccessGroupRules { + libraries: vec![allow_library(manga_lib)], + ..Default::default() + }, + ) + .await; + f.group( + "Comics, no adult", + user, + AccessGroupRules { + genres: vec![NameRule::new("Adult", AccessMode::Deny)], + libraries: vec![allow_library(comics_lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(!filter.is_series_visible(adult_manga)); + assert!(filter.is_series_visible(manga)); + assert!(filter.is_series_visible(comic)); + } + + #[tokio::test] + async fn genre_match_all_requires_every_allowed_genre() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let lib = f.library("Manga").await; + let both = f.series(lib, "Both", &["Kids", "Comedy"]).await; + let kids_only = f.series(lib, "Kids only", &["Kids"]).await; + f.group( + "Kids comedy", + user, + AccessGroupRules { + genre_match: RuleMatch::All, + genres: vec![ + NameRule::new("Kids", AccessMode::Allow), + NameRule::new("Comedy", AccessMode::Allow), + ], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(filter.is_series_visible(both)); + assert!(!filter.is_series_visible(kids_only)); + } + + #[tokio::test] + async fn genre_match_any_is_the_default() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let lib = f.library("Manga").await; + let kids = f.series(lib, "Kids", &["Kids"]).await; + let comedy = f.series(lib, "Comedy", &["Comedy"]).await; + let action = f.series(lib, "Action", &["Action"]).await; + f.group( + "Kids or comedy", + user, + AccessGroupRules { + genres: vec![ + NameRule::new("Kids", AccessMode::Allow), + NameRule::new("Comedy", AccessMode::Allow), + ], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(filter.is_series_visible(kids)); + assert!(filter.is_series_visible(comedy)); + assert!(!filter.is_series_visible(action)); + } + + #[tokio::test] + async fn sharing_tag_match_all_requires_every_allowed_tag() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let lib = f.library("Manga").await; + let both = f.series(lib, "Both", &[]).await; + let one = f.series(lib, "One", &[]).await; + let a = SharingTagRepository::create(f.conn(), "a", None) + .await + .unwrap(); + let b = SharingTagRepository::create(f.conn(), "b", None) + .await + .unwrap(); + for (series, tag) in [(both, a.id), (both, b.id), (one, a.id)] { + SharingTagRepository::add_tag_to_series(f.conn(), series, tag) + .await + .unwrap(); + } + f.group( + "A and B", + user, + AccessGroupRules { + sharing_tag_match: RuleMatch::All, + sharing_tags: vec![ + SharingTagRule { + sharing_tag_id: a.id, + access_mode: AccessMode::Allow, + }, + SharingTagRule { + sharing_tag_id: b.id, + access_mode: AccessMode::Allow, + }, + ], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(filter.is_series_visible(both)); + assert!(!filter.is_series_visible(one)); + } + + /// Personal sharing-tag allows act as one more group. + #[tokio::test] + async fn personal_allows_add_to_group_allows() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let kids_lib = f.library("Kids").await; + let manga_lib = f.library("Manga").await; + let in_kids = f.series(kids_lib, "Bluey", &[]).await; + let shared_manga = f.series(manga_lib, "Shared", &[]).await; + let other_manga = f.series(manga_lib, "Other", &[]).await; + let tag = SharingTagRepository::create(f.conn(), "shared", None) + .await + .unwrap(); + SharingTagRepository::add_tag_to_series(f.conn(), shared_manga, tag.id) + .await + .unwrap(); + SharingTagRepository::set_user_grant(f.conn(), user, tag.id, AccessMode::Allow) + .await + .unwrap(); + f.group( + "Kids", + user, + AccessGroupRules { + libraries: vec![allow_library(kids_lib)], + ..Default::default() + }, + ) + .await; + + let filter = f.filter(user).await; + assert!(filter.is_series_visible(in_kids)); + assert!(filter.is_series_visible(shared_manga)); + assert!(!filter.is_series_visible(other_manga)); + } + + /// A genre removed by cleanup and recreated under a new id is still denied. + #[tokio::test] + async fn genre_deny_survives_genre_recreation() { + let f = Fixture::new().await; + let user = make_user(f.conn(), "kid").await; + let lib = f.library("Manga").await; + f.group( + "No adult", + user, + AccessGroupRules { + genres: vec![NameRule::new("Adult", AccessMode::Deny)], + ..Default::default() + }, + ) + .await; + let first = f.series(lib, "First", &["Adult"]).await; + let first_genre = GenreRepository::get_by_name(f.conn(), "Adult") + .await + .unwrap() + .unwrap(); + GenreRepository::remove_genre_from_series(f.conn(), first, first_genre.id) + .await + .unwrap(); + GenreRepository::delete_unused(f.conn()).await.unwrap(); + + let second = f.series(lib, "Second", &["Adult"]).await; + let second_genre = GenreRepository::get_by_name(f.conn(), "Adult") + .await + .unwrap() + .unwrap(); + assert_ne!(first_genre.id, second_genre.id, "genre was recreated"); + + let filter = f.filter(user).await; + assert!(!filter.is_series_visible(second)); + } + /// Smoke check: building a filter for a user in 10 groups with ~50 grants /// each (~500 group grants total) should be well under 50ms on SQLite. /// Not a hard performance contract — just catches O(n*m) regressions. diff --git a/docs/api/openapi.json b/docs/api/openapi.json index 806c148d1..84fbe78c1 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -603,6 +603,112 @@ ] } }, + "/api/v1/access-groups/{group_id}/rules": { + "get": { + "tags": [ + "Access Groups" + ], + "summary": "Get every rule of an access group (admin only)", + "operationId": "get_access_group_rules", + "parameters": [ + { + "name": "group_id", + "in": "path", + "description": "Access group ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "The group's rules", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AccessGroupRulesDto" + } + } + } + }, + "403": { + "description": "Forbidden - Missing permission" + }, + "404": { + "description": "Access group not found" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + }, + "put": { + "tags": [ + "Access Groups" + ], + "summary": "Replace every rule of an access group (admin only)", + "description": "The whole rule set is replaced atomically: anything not in the request is\nremoved. Sharing tag and library IDs must exist; tag and genre names need\nnot, so a rule can be set up before any series carries the name.", + "operationId": "replace_access_group_rules", + "parameters": [ + { + "name": "group_id", + "in": "path", + "description": "Access group ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ReplaceAccessGroupRulesRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "Rules replaced", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AccessGroupRulesDto" + } + } + } + }, + "400": { + "description": "Duplicate rule, blank name, or unknown sharing tag or library" + }, + "403": { + "description": "Forbidden - Missing permission" + }, + "404": { + "description": "Access group not found" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + } + }, "/api/v1/admin/cleanup-orphans": { "post": { "tags": [ @@ -23666,6 +23772,7 @@ "isDefault", "memberCount", "grantCount", + "ruleCount", "createdAt", "updatedAt" ], @@ -23687,7 +23794,7 @@ "grantCount": { "type": "integer", "format": "int64", - "description": "Number of tag grants in the group", + "description": "Number of sharing tag grants in the group", "example": 3, "minimum": 0 }, @@ -23714,6 +23821,13 @@ "description": "Display name of the access group", "example": "Manga Readers" }, + "ruleCount": { + "type": "integer", + "format": "int64", + "description": "Number of rules in the group, across sharing tags, tags, genres and libraries", + "example": 5, + "minimum": 0 + }, "updatedAt": { "type": "string", "format": "date-time", @@ -23817,6 +23931,61 @@ } } }, + "AccessGroupRulesDto": { + "type": "object", + "description": "Every rule of an access group\n\nWithin a group, values of the same type combine as \"any of\" or \"all of\"\n(per the match field for that type; libraries are always \"any of\"), and\ndifferent types combine with AND. Across groups, what each group allows is\ncombined with OR. Denies from every group always win. Admins and\nmaintainers are never restricted.", + "required": [ + "sharingTagMatch", + "tagMatch", + "genreMatch", + "sharingTags", + "tags", + "genres", + "libraries" + ], + "properties": { + "genreMatch": { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed genres combine" + }, + "genres": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on genres, matched by name" + }, + "libraries": { + "type": "array", + "items": { + "$ref": "#/components/schemas/LibraryRuleDto" + }, + "description": "Rules on libraries" + }, + "sharingTagMatch": { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed sharing tags combine" + }, + "sharingTags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/SharingTagRuleDto" + }, + "description": "Rules on sharing tags" + }, + "tagMatch": { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed tags combine" + }, + "tags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on regular tags, matched by name" + } + } + }, "AccessGroupSummaryDto": { "type": "object", "description": "Access group summary (for user's group list)", @@ -34997,6 +35166,50 @@ } } }, + "LibraryRuleDto": { + "type": "object", + "description": "A rule on a library", + "required": [ + "libraryId", + "name", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "libraryId": { + "type": "string", + "format": "uuid", + "description": "Library ID" + }, + "name": { + "type": "string", + "description": "Library name", + "example": "Manga" + } + } + }, + "LibraryRuleInput": { + "type": "object", + "description": "A library rule in a replace request", + "required": [ + "libraryId", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "libraryId": { + "type": "string", + "format": "uuid", + "description": "Library ID" + } + } + }, "LinkProperties": { "type": "object", "description": "Additional properties that can be attached to links", @@ -36036,6 +36249,25 @@ } } }, + "NameRuleDto": { + "type": "object", + "description": "A rule on a regular tag or a genre\n\nMatched by name, case-insensitively, so the rule keeps applying when the\ntag or genre is removed by cleanup and created again later.", + "required": [ + "name", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "name": { + "type": "string", + "description": "Tag or genre name", + "example": "Adult" + } + } + }, "NewsEntryDto": { "type": "object", "description": "One news entry.", @@ -41572,6 +41804,71 @@ } } }, + "ReplaceAccessGroupRulesRequest": { + "type": "object", + "description": "Replace every rule of an access group\n\nOmitted lists are treated as empty and omitted match fields as \"any\".", + "properties": { + "genreMatch": { + "oneOf": [ + { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed genres combine" + } + ], + "default": "any" + }, + "genres": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on genres, matched by name", + "default": [] + }, + "libraries": { + "type": "array", + "items": { + "$ref": "#/components/schemas/LibraryRuleInput" + }, + "description": "Rules on libraries", + "default": [] + }, + "sharingTagMatch": { + "oneOf": [ + { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed sharing tags combine" + } + ], + "default": "any" + }, + "sharingTags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/SharingTagRuleInput" + }, + "description": "Rules on sharing tags", + "default": [] + }, + "tagMatch": { + "oneOf": [ + { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed tags combine" + } + ], + "default": "any" + }, + "tags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on regular tags, matched by name", + "default": [] + } + } + }, "ReplaceBookMetadataRequest": { "type": "object", "description": "PUT request for full replacement of book metadata\n\nAll metadata fields will be replaced with the values in this request.\nOmitting a field (or setting it to null) will clear that field.", @@ -42287,6 +42584,14 @@ } } }, + "RuleMatch": { + "type": "string", + "description": "How a group's allowed values of one rule type combine.", + "enum": [ + "any", + "all" + ] + }, "RunNowResponse": { "type": "object", "description": "Response for `POST .../run-now`.", @@ -44630,6 +44935,50 @@ } } }, + "SharingTagRuleDto": { + "type": "object", + "description": "A rule on a sharing tag", + "required": [ + "sharingTagId", + "name", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "name": { + "type": "string", + "description": "Sharing tag name", + "example": "Kids" + }, + "sharingTagId": { + "type": "string", + "format": "uuid", + "description": "Sharing tag ID" + } + } + }, + "SharingTagRuleInput": { + "type": "object", + "description": "A sharing tag rule in a replace request", + "required": [ + "sharingTagId", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "sharingTagId": { + "type": "string", + "format": "uuid", + "description": "Sharing tag ID" + } + } + }, "SharingTagSummaryDto": { "type": "object", "description": "Simplified sharing tag for lists (without counts)", diff --git a/docs/docs/users/access-groups.md b/docs/docs/users/access-groups.md index cb4706173..8815780bb 100644 --- a/docs/docs/users/access-groups.md +++ b/docs/docs/users/access-groups.md @@ -4,58 +4,93 @@ sidebar_position: 7 # Access Groups -Access groups let you bundle a set of [sharing-tag](./sharing-tags) grants and assign them to multiple users at once. Instead of configuring 50 identical per-user grants, create one group, define its rules, and add users to it. +Access groups decide what content their members can see. A group holds **rules**: allow or deny a library, a genre, a tag, or a [sharing tag](./sharing-tags). You assign the group to many users at once instead of configuring each user separately. -## Overview +:::info Admins and maintainers are never restricted +Access groups and sharing-tag grants do not apply to users with the **admin** or **maintainer** role, even when they are members of a group. Both always see the whole library: maintainers curate metadata, covers and scans, which needs every series. Only **readers** are restricted, so check a restricted setup with a reader account. +::: + +## Rule Types + +| Rule on | Matches a series when | Notes | +|---------|-----------------------|-------| +| **Library** | the series is in that library | A new library is hidden from whitelisted users until a group allows it | +| **Genre** | the series has that genre | Matched by name, ignoring case | +| **Tag** | the series has that tag | Matched by name, ignoring case | +| **Sharing tag** | the series carries that sharing tag | Sharing tags are applied by hand; see [sharing tags](./sharing-tags) | + +Each rule is either **allow** or **deny**. A value can appear once per group. + +Genre and tag rules are matched by **name**, not by the genre or tag itself. Codex removes genres and tags that no series uses anymore, and the next series that carries the name creates it again. A rule on "Adult" keeps working through that, and you can create a rule for a genre that no series has yet. -Access groups sit between sharing tags and users: +## How Rules Combine + +1. **Allow rules inside one group narrow each other.** + - Values of the **same type** combine as "any of" by default: allowing the genres Kids and Comedy shows series with either genre. For genres, tags and sharing tags you can switch a group to "all of", so only series with **every** allowed value match. Libraries are always "any of", since a series is in exactly one library. + - **Different types** must all match: "allow library Manga" plus "allow genre Kids" shows Kids-genre series in the Manga library, not all of Manga plus every Kids series. +2. **Groups add up.** Each group grants its own scope, and a member of several groups sees everything any of their groups allows. A user's personal sharing-tag grants count as one more group. +3. **Deny always wins, everywhere.** Every deny rule, from every group the user is in and from their personal grants, is collected into one list and hidden at the end. A deny is not limited to the group it is written in, so no other group can grant that content back. +4. **No allow rules at all** (no groups, or only groups with deny rules): the user sees everything that is not denied. ``` -Sharing Tags Access Groups Users - +---------+ +---------------+ +-------+ - | manga |----->| Manga Readers |---->| alice | - | 18+ |----->| allow manga |---->| bob | - +---------+ | deny 18+ | +-------+ - +---------------+ - Per-user overrides - still apply on top +visible = (what group A allows) + (what group B allows) + ... - (every deny from every group) ``` -- A group has a name, description, and a set of tag grants (allow/deny). -- Users can belong to multiple groups. -- Per-user grants in the existing `Sharing Tag Grants` panel continue to work as overrides on top of group rules. -- Deny always wins across all sources (groups + per-user). +:::warning Whitelist mode +As soon as any allow rule applies to a user, they only see what an allow rule matches. Series that match no allow rule are hidden, including series with no genres, tags or sharing tags at all. +::: -## How Grants Merge +### Examples -When a user belongs to one or more groups, their **effective grants** are the union of: +**Two groups, one deny.** A user is in: -1. All grants from all their groups -2. Their per-user grants (from Settings > Users > Edit > Sharing Tag Grants) +- *Manga*: allow library Manga +- *Comics, no adult*: allow library Comics, deny genre Adult -The same deny-wins rule from [sharing tags](./sharing-tags) applies to the merged set: +They see all of Manga and all of Comics, **minus anything in the Adult genre in either library**. The Adult deny from the second group also applies to the manga the first group grants. -| Scenario | Result | -|----------|--------| -| Group allows "manga", no user override | User sees manga-tagged content (whitelist mode) | -| Group allows "manga", user denies "18+" | User sees manga-tagged content, but not 18+-tagged content | -| Two groups: one allows "manga", other denies "manga" | Deny wins; manga content hidden | -| Group allows "manga", user also allows "manga" | Same as single allow (deduplicated) | -| No groups, no user grants | User sees all content (default open) | +**One group, two types.** *Kids manga*: allow library Manga, allow genre Kids. Members see Kids-genre series in Manga only. -:::warning Whitelist Mode -Any `allow` grant from any source (group or per-user) activates whitelist mode for that user. In whitelist mode, **untagged content is hidden**. This is the same behavior as per-user allow grants. See the [sharing tags docs](./sharing-tags#the-access-model) for details. -::: +**Deny-only group.** *No adult*: deny genre Adult. Members see the whole library except Adult-genre series. Untagged content stays visible, because a deny-only group never starts a whitelist. + +### Allow is safer than deny for metadata rules + +Genres and tags come from your files' metadata and from metadata plugins. A **deny** on a genre hides only series that actually carry it: a series whose metadata is missing the genre stays visible. An **allow** on a genre does the opposite: a series missing the genre stays hidden. For a kids account, "allow library Kids" or "allow genre Kids" is the safer setup, with deny rules as a second layer. + +## The Default Group + +You can mark **one** group as the default. Every account created afterwards joins it automatically: accounts created by an admin, by self-registration, and on a user's first OIDC login. + +- It is an ordinary membership (shown with the source `default`). You can remove a user from the default group, and they stay out. +- Making a group the default does **not** add existing users. Use **Add all existing users** on the group page to bring them in. Existing memberships are left as they are. +- Marking another group as default moves the default to it. + +A default group with an allow rule (for example "allow library Kids") gives you closed-by-default accounts: new users see only that until you add them to other groups. + +## Membership Sources + +Each membership records how the user got into the group: + +| Source | Added by | Removed by | +|--------|----------|------------| +| `manual` | an admin | an admin | +| `default` | account creation, or **Add all existing users** | an admin | +| `oidc` | an [OIDC](./oidc) group mapping on login | the next login after the user leaves the IdP group, or an admin | + +Whoever adds a user to a group first owns that membership. Adding the same user again from another source changes nothing: an admin adding someone the IdP already placed does not stop the IdP from removing them later, and an IdP group mapping to a group an admin assigned does not let the IdP remove that assignment. ## Managing Access Groups ### Via Web Interface 1. Go to **Settings** > **Access Groups** -2. Click **Create Group** -3. Enter a name and optional description -4. After creation, click the group name to open its detail page -5. Add **tag grants**, **members**, and optionally **OIDC mappings** +2. Click **Create Group** and enter a name and optional description +3. Open the group to configure it: + - **Default group for new users** switch and **Add all existing users** + - **Rules**: add libraries, genres, tags and sharing tags as allow or deny, and choose "any" or "all" per type + - **Members** and **OIDC mappings** + +The users list in **Settings** > **Users** shows each user's groups. ### Via API @@ -64,209 +99,112 @@ Any `allow` grant from any source (group or per-user) activates whitelist mode f curl -X POST http://localhost:8080/api/v1/access-groups \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d '{ - "name": "Manga Readers", - "description": "Access to all manga content" - }' -``` - -**List groups:** -```bash -curl http://localhost:8080/api/v1/access-groups \ - -H "Authorization: Bearer $ADMIN_TOKEN" -``` - -**Get group details (members, grants, OIDC mappings):** -```bash -curl http://localhost:8080/api/v1/access-groups/{id} \ - -H "Authorization: Bearer $ADMIN_TOKEN" + -d '{"name": "Kids", "description": "Kids accounts"}' ``` -**Update a group:** +**Replace a group's rules** (the whole set is replaced; omitted lists are emptied and omitted match fields are `any`): ```bash -curl -X PATCH http://localhost:8080/api/v1/access-groups/{id} \ +curl -X PUT http://localhost:8080/api/v1/access-groups/{id}/rules \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d '{"description": "Updated description"}' + -d '{ + "genreMatch": "any", + "libraries": [{"libraryId": "kids-library-uuid", "accessMode": "allow"}], + "genres": [{"name": "Adult", "accessMode": "deny"}], + "tags": [{"name": "Gore", "accessMode": "deny"}], + "sharingTags": [] + }' ``` -**Delete a group:** +**Read a group's rules:** ```bash -curl -X DELETE http://localhost:8080/api/v1/access-groups/{id} \ +curl http://localhost:8080/api/v1/access-groups/{id}/rules \ -H "Authorization: Bearer $ADMIN_TOKEN" ``` -:::warning -Deleting a group removes all its memberships and grants. Users will lose the group's grants on their next request. -::: - -## Managing Group Grants - -Grants define what content the group's members can see. - -**Add a grant:** +**Make a group the default, or stop it being the default:** ```bash -curl -X POST http://localhost:8080/api/v1/access-groups/{id}/grants \ +curl -X PATCH http://localhost:8080/api/v1/access-groups/{id} \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d '{ - "sharingTagId": "tag-uuid", - "accessMode": "allow" - }' + -d '{"isDefault": true}' ``` -**Remove a grant:** +**Add every user who is not a member yet:** ```bash -curl -X DELETE http://localhost:8080/api/v1/access-groups/{id}/grants/{tag_id} \ +curl -X POST http://localhost:8080/api/v1/access-groups/{id}/add-all-users \ -H "Authorization: Bearer $ADMIN_TOKEN" ``` -## Managing Group Members - -**Add members:** +**Add or remove members:** ```bash curl -X POST http://localhost:8080/api/v1/access-groups/{id}/members \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d '{ - "userIds": ["user-uuid-1", "user-uuid-2"] - }' -``` + -d '{"userIds": ["user-uuid-1", "user-uuid-2"]}' -**Remove a member:** -```bash curl -X DELETE http://localhost:8080/api/v1/access-groups/{id}/members/{user_id} \ -H "Authorization: Bearer $ADMIN_TOKEN" ``` -**List a user's groups:** +**List groups, get one, update, delete:** ```bash -curl http://localhost:8080/api/v1/users/{user_id}/access-groups \ - -H "Authorization: Bearer $ADMIN_TOKEN" +curl http://localhost:8080/api/v1/access-groups -H "Authorization: Bearer $ADMIN_TOKEN" +curl http://localhost:8080/api/v1/access-groups/{id} -H "Authorization: Bearer $ADMIN_TOKEN" +curl -X PATCH http://localhost:8080/api/v1/access-groups/{id} \ + -H "Authorization: Bearer $ADMIN_TOKEN" -H "Content-Type: application/json" \ + -d '{"description": "Updated description"}' +curl -X DELETE http://localhost:8080/api/v1/access-groups/{id} -H "Authorization: Bearer $ADMIN_TOKEN" ``` +The older `POST /access-groups/{id}/grants` and `DELETE /access-groups/{id}/grants/{tag_id}` endpoints still add and remove a single sharing-tag rule. + +:::warning +Deleting a group removes its memberships and rules. Members lose what the group granted on their next request. +::: + ## OIDC Auto-Assignment -If you use [OIDC authentication](./oidc), you can map IdP group names to access groups. When a user logs in via OIDC, their access group memberships are automatically reconciled: +If you use [OIDC authentication](./oidc), you can map IdP group names to access groups. On every OIDC login, the user's OIDC-sourced memberships are reconciled with their IdP groups: -- If the user's IdP groups include a mapped group name, they are added to the corresponding access group (with `source=oidc`). -- If a previously mapped group is no longer in their IdP claims, the OIDC-sourced membership is removed. -- Manually assigned memberships are never touched by the OIDC sync. +- A mapped IdP group the user is in adds them to the access group (source `oidc`), unless they are already a member. +- An `oidc` membership whose IdP group is no longer in the user's claims is removed. +- `manual` and `default` memberships are never touched by the sync. +- If the sync fails, the login fails. Codex does not let a user in with a partial or missing set of groups. -**Add an OIDC mapping:** +**Add or remove an OIDC mapping:** ```bash curl -X POST http://localhost:8080/api/v1/access-groups/{id}/oidc-mappings \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d '{ - "oidcGroupName": "library-staff" - }' -``` + -d '{"oidcGroupName": "library-staff"}' -**Remove an OIDC mapping:** -```bash curl -X DELETE http://localhost:8080/api/v1/access-groups/{id}/oidc-mappings/{mapping_id} \ -H "Authorization: Bearer $ADMIN_TOKEN" ``` :::note Case Sensitivity -OIDC group names are matched exactly as stored. If your IdP sends `Library-Staff` but the mapping says `library-staff`, it will not match. Configure the mapping to match your IdP's exact casing. +OIDC group names are matched exactly as stored. If your IdP sends `Library-Staff` but the mapping says `library-staff`, it will not match. ::: -## Debugging Effective Grants +## Debugging Sharing-Tag Grants -The **effective grants** endpoint shows exactly which grants apply to a user and where each one comes from (per-user override or group name). This is the primary tool for answering "why can't this user see series X?" +`GET /api/v1/users/{user_id}/effective-grants` (and the **Effective Grants** panel when editing a user) lists the user's sharing-tag rules and which group or personal grant each comes from. It covers sharing tags only, not library, genre or tag rules. -**Via Web Interface:** -1. Go to **Settings** > **Users** -2. Edit a user -3. Scroll to the **Effective Grants** panel +## Example: A Kids Account -**Via API:** -```bash -curl http://localhost:8080/api/v1/users/{user_id}/effective-grants \ - -H "Authorization: Bearer $ADMIN_TOKEN" -``` - -Response: -```json -{ - "userId": "user-uuid", - "grants": [ - { - "sharingTagId": "tag-uuid", - "sharingTagName": "manga", - "accessMode": "allow", - "sources": [ - { - "kind": "group", - "groupId": "group-uuid", - "groupName": "Manga Readers" - } - ] - }, - { - "sharingTagId": "tag-uuid-2", - "sharingTagName": "18+", - "accessMode": "deny", - "sources": [ - { - "kind": "user", - "groupId": null, - "groupName": null - } - ] - } - ] -} -``` - -Each grant shows `sources` with `kind: "user"` for per-user overrides or `kind: "group"` with the group name for group-inherited grants. A single tag+mode combination can have multiple sources if both a user override and a group grant contribute the same rule. - -## Example: Family + Organization Setup - -Combine access groups with per-user overrides for a multi-layered access model: - -**Step 1: Create tags and groups** -```bash -# Tags -curl -X POST .../admin/sharing-tags -d '{"name": "manga"}' -curl -X POST .../admin/sharing-tags -d '{"name": "comics"}' -curl -X POST .../admin/sharing-tags -d '{"name": "18+"}' - -# Groups -curl -X POST .../access-groups -d '{"name": "Manga Readers"}' -curl -X POST .../access-groups -d '{"name": "Comics Readers"}' -``` - -**Step 2: Configure group grants** -```bash -# Manga Readers: allow manga -curl -X POST .../access-groups/{manga-group}/grants \ - -d '{"sharingTagId": "manga-tag-id", "accessMode": "allow"}' - -# Comics Readers: allow comics -curl -X POST .../access-groups/{comics-group}/grants \ - -d '{"sharingTagId": "comics-tag-id", "accessMode": "allow"}' -``` - -**Step 3: Assign users** -```bash -# Alice gets manga access via group -curl -X POST .../access-groups/{manga-group}/members \ - -d '{"userIds": ["alice-id"]}' - -# But deny 18+ content specifically for Alice (per-user override) -curl -X PUT .../users/alice-id/sharing-tags \ - -d '{"sharingTagId": "18+-tag-id", "accessMode": "deny"}' -``` +1. Create a group **Kids** with these rules: + - allow library *Kids* + - deny genre *Adult* +2. Mark **Kids** as the default group, so every new account starts there. +3. For adult family members, remove them from **Kids** and add them to a group such as **Everyone** with no allow rules (or no group at all, which sees everything). -Result: Alice sees manga-tagged content, but never 18+-tagged content (deny wins). +A kid's account sees the Kids library and nothing else. If a kid is later added to another group by mistake, the *Adult* deny still applies to everything that group grants. ## Best Practices -1. **Use groups for organizational roles** rather than duplicating per-user grants -2. **Keep per-user grants for exceptions** (e.g., one user in a "Manga Readers" group who should not see 18+ content) -3. **Name groups by intent** (e.g., "Kids", "Staff", "Manga Readers") rather than by technical configuration -4. **Use the effective grants endpoint** to verify access before and after changes -5. **Map OIDC groups** when your IdP already has organizational groups, so membership stays in sync automatically +1. **Prefer allow rules for restricted accounts**, and use deny rules as a safety net: metadata is never perfect, and allow fails closed. +2. **Put hard limits in deny rules**: they hold whatever other groups a user joins. +3. **Use a default group** so new accounts never start unrestricted by accident. +4. **Name groups by intent** ("Kids", "Staff", "Manga Readers"). +5. **Map OIDC groups** when your IdP already has organizational groups. diff --git a/docs/docs/users/permissions.md b/docs/docs/users/permissions.md index 699d7e0dd..8e4b7530c 100644 --- a/docs/docs/users/permissions.md +++ b/docs/docs/users/permissions.md @@ -47,6 +47,7 @@ The default role for new users. Readers can: For users who manage content but not system settings. Maintainers can do everything Readers can, plus: - Create and modify libraries (but not delete them) +- See all content: [access groups](./access-groups) and [sharing tags](./sharing-tags) only restrict readers - Full series management (create, edit, delete) - Full book management (create, edit, delete) - Create, edit, and delete collections and read lists @@ -69,6 +70,7 @@ Full system access for server administrators. Admins can do everything Maintaine - Manage metadata plugins - Access system administration features - Manage sharing tags and content restrictions +- See all content: [access groups](./access-groups) and [sharing tags](./sharing-tags) never restrict an admin **Additional Permissions (6 more, 29 total):** - `libraries-delete` @@ -195,7 +197,7 @@ Custom permissions allow extending a user's access beyond their role. For exampl This user would have all Reader permissions plus `tasks-read`. :::tip -Custom permissions extend roles - they never restrict. To limit access, use [Sharing Tags](./sharing-tags) for content-level restrictions. +Custom permissions extend roles - they never restrict. To limit what content a user sees, use [access groups](./access-groups) and [sharing tags](./sharing-tags). ::: ## API Token Permissions diff --git a/docs/docs/users/sharing-tags.md b/docs/docs/users/sharing-tags.md index b8a5f6006..673bb972d 100644 --- a/docs/docs/users/sharing-tags.md +++ b/docs/docs/users/sharing-tags.md @@ -6,6 +6,12 @@ sidebar_position: 6 Sharing tags provide content-level access control, allowing admins to restrict which series specific users can see. This is useful for family sharing scenarios where you want to limit access to age-appropriate content. +Sharing tags are applied to series by hand. To restrict by library, genre or regular tag without tagging every series, use rules on an [access group](./access-groups). + +:::info Admins and maintainers are never restricted +Sharing-tag grants and access groups do not apply to users with the **admin** or **maintainer** role. +::: + ## Overview Sharing tags work independently from [permissions](./permissions). While permissions control what actions users can perform (read, write, delete), sharing tags control what content users can see. diff --git a/migration/src/lib.rs b/migration/src/lib.rs index 44268051f..499b3dab7 100644 --- a/migration/src/lib.rs +++ b/migration/src/lib.rs @@ -218,6 +218,7 @@ mod m20260828_000113_relax_renumber_series_task_dedup; mod m20260927_000114_reading_history_survives_book_delete; mod m20261018_000115_create_news; mod m20261025_000116_access_group_default; +mod m20261025_000117_access_group_rules; pub struct Migrator; @@ -424,6 +425,8 @@ impl MigratorTrait for Migrator { Box::new(m20261018_000115_create_news::Migration), // At most one default access group, which new users join. Box::new(m20261025_000116_access_group_default::Migration), + // Access group rules on regular tags, genres and libraries. + Box::new(m20261025_000117_access_group_rules::Migration), ] } } diff --git a/migration/src/m20261025_000117_access_group_rules.rs b/migration/src/m20261025_000117_access_group_rules.rs new file mode 100644 index 000000000..d44873589 --- /dev/null +++ b/migration/src/m20261025_000117_access_group_rules.rs @@ -0,0 +1,259 @@ +//! Access group rules on regular tags, genres and libraries. +//! +//! Until now an access group could only allow or deny sharing tags. These +//! tables let it allow or deny regular tags, genres and libraries as well, +//! each rule `allow` or `deny`, one rule per target per group. +//! +//! Tag and genre rules store the normalized name, not a foreign key. Unused +//! tags and genres are deleted by cleanup, and the next series to carry the +//! name recreates it under a new id. A foreign key would cascade the rule away +//! with the old row and silently drop a deny. Matching on the normalized name +//! survives that. The display name is kept alongside for the UI. +//! +//! Library rules keep a foreign key: libraries are only ever deleted by an +//! admin, and their series go with them. +//! +//! The three match columns on `access_groups` choose, per type, whether a +//! group's allowed values combine as "any of" (the default, and what sharing +//! tags have always done) or "all of". Libraries have no switch: a series is +//! in exactly one library, so "all of" two libraries could never match. + +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + for column in [ + AccessGroups::SharingTagMatch, + AccessGroups::TagMatch, + AccessGroups::GenreMatch, + ] { + manager + .alter_table( + Table::alter() + .table(AccessGroups::Table) + .add_column( + ColumnDef::new(column) + .string_len(10) + .not_null() + .default("any"), + ) + .to_owned(), + ) + .await?; + } + + create_name_rule_table( + manager, + AccessGroupTagRules::Table, + "access_group_tag_rules", + ) + .await?; + create_name_rule_table( + manager, + AccessGroupGenreRules::Table, + "access_group_genre_rules", + ) + .await?; + + manager + .create_table( + Table::create() + .table(AccessGroupLibraryRules::Table) + .if_not_exists() + .col( + ColumnDef::new(RuleColumns::Id) + .uuid() + .not_null() + .primary_key(), + ) + .col(ColumnDef::new(RuleColumns::AccessGroupId).uuid().not_null()) + .col( + ColumnDef::new(AccessGroupLibraryRules::LibraryId) + .uuid() + .not_null(), + ) + .col( + ColumnDef::new(RuleColumns::AccessMode) + .string_len(10) + .not_null(), + ) + .col( + ColumnDef::new(RuleColumns::CreatedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .foreign_key( + ForeignKey::create() + .name("fk_access_group_library_rules_group_id") + .from(AccessGroupLibraryRules::Table, RuleColumns::AccessGroupId) + .to(AccessGroups::Table, AccessGroups::Id) + .on_delete(ForeignKeyAction::Cascade) + .on_update(ForeignKeyAction::NoAction), + ) + .foreign_key( + ForeignKey::create() + .name("fk_access_group_library_rules_library_id") + .from( + AccessGroupLibraryRules::Table, + AccessGroupLibraryRules::LibraryId, + ) + .to(Libraries::Table, Libraries::Id) + .on_delete(ForeignKeyAction::Cascade) + .on_update(ForeignKeyAction::NoAction), + ) + .to_owned(), + ) + .await?; + + manager + .create_index( + Index::create() + .name("idx_access_group_library_rules_group_library") + .table(AccessGroupLibraryRules::Table) + .col(RuleColumns::AccessGroupId) + .col(AccessGroupLibraryRules::LibraryId) + .unique() + .to_owned(), + ) + .await?; + + Ok(()) + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + for table in [ + AccessGroupLibraryRules::Table.into_iden(), + AccessGroupGenreRules::Table.into_iden(), + AccessGroupTagRules::Table.into_iden(), + ] { + manager + .drop_table(Table::drop().table(table).if_exists().to_owned()) + .await?; + } + + for column in [ + AccessGroups::GenreMatch, + AccessGroups::TagMatch, + AccessGroups::SharingTagMatch, + ] { + manager + .alter_table( + Table::alter() + .table(AccessGroups::Table) + .drop_column(column) + .to_owned(), + ) + .await?; + } + + Ok(()) + } +} + +/// Create a tag-style rule table keyed by normalized name. +async fn create_name_rule_table( + manager: &SchemaManager<'_>, + table: T, + name: &str, +) -> Result<(), DbErr> +where + T: IntoIden + Copy + 'static, +{ + manager + .create_table( + Table::create() + .table(table) + .if_not_exists() + .col( + ColumnDef::new(RuleColumns::Id) + .uuid() + .not_null() + .primary_key(), + ) + .col(ColumnDef::new(RuleColumns::AccessGroupId).uuid().not_null()) + .col(ColumnDef::new(RuleColumns::Name).string().not_null()) + .col( + ColumnDef::new(RuleColumns::NormalizedName) + .string() + .not_null(), + ) + .col( + ColumnDef::new(RuleColumns::AccessMode) + .string_len(10) + .not_null(), + ) + .col( + ColumnDef::new(RuleColumns::CreatedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .foreign_key( + ForeignKey::create() + .name(format!("fk_{name}_group_id")) + .from(table, RuleColumns::AccessGroupId) + .to(AccessGroups::Table, AccessGroups::Id) + .on_delete(ForeignKeyAction::Cascade) + .on_update(ForeignKeyAction::NoAction), + ) + .to_owned(), + ) + .await?; + + manager + .create_index( + Index::create() + .name(format!("idx_{name}_group_name")) + .table(table) + .col(RuleColumns::AccessGroupId) + .col(RuleColumns::NormalizedName) + .unique() + .to_owned(), + ) + .await +} + +#[derive(DeriveIden)] +enum AccessGroups { + Table, + Id, + SharingTagMatch, + TagMatch, + GenreMatch, +} + +#[derive(DeriveIden, Clone, Copy)] +enum AccessGroupTagRules { + Table, +} + +#[derive(DeriveIden, Clone, Copy)] +enum AccessGroupGenreRules { + Table, +} + +#[derive(DeriveIden)] +enum AccessGroupLibraryRules { + Table, + LibraryId, +} + +/// Columns shared by every rule table. +#[derive(DeriveIden)] +enum RuleColumns { + Id, + AccessGroupId, + Name, + NormalizedName, + AccessMode, + CreatedAt, +} + +#[derive(DeriveIden)] +enum Libraries { + Table, + Id, +} diff --git a/tests/api/access_groups.rs b/tests/api/access_groups.rs index 4c6df5d12..ad07a7e04 100644 --- a/tests/api/access_groups.rs +++ b/tests/api/access_groups.rs @@ -1051,3 +1051,166 @@ async fn test_add_all_users_requires_admin() { make_json_request(app, request).await; assert_eq!(status, StatusCode::FORBIDDEN); } + +// ============================================================================ +// Rules Tests +// ============================================================================ + +async fn put_rules( + state: &std::sync::Arc, + token: &str, + group_id: uuid::Uuid, + body: serde_json::Value, +) -> (StatusCode, Option) { + let app = create_test_router(state.clone()).await; + let request = put_json_request_with_auth( + &format!("/api/v1/access-groups/{}/rules", group_id), + &body, + token, + ); + make_json_request(app, request).await +} + +#[tokio::test] +async fn test_replace_and_get_rules() { + use codex::db::ScanningStrategy; + use codex::db::repositories::LibraryRepository; + + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let group = create_group(&state, &token, "Kids").await; + let library = LibraryRepository::create(&db, "Kids", "/tmp/kids", ScanningStrategy::Default) + .await + .unwrap(); + let tag = SharingTagRepository::create(&db, "family", None) + .await + .unwrap(); + + let (status, response) = put_rules( + &state, + &token, + group.id, + serde_json::json!({ + "sharingTagMatch": "any", + "tagMatch": "any", + "genreMatch": "all", + "sharingTags": [{ "sharingTagId": tag.id, "accessMode": "allow" }], + "tags": [{ "name": "Gore", "accessMode": "deny" }], + "genres": [ + { "name": "Adult", "accessMode": "deny" }, + { "name": "Kids", "accessMode": "allow" } + ], + "libraries": [{ "libraryId": library.id, "accessMode": "allow" }] + }), + ) + .await; + assert_eq!(status, StatusCode::OK, "{response:?}"); + + let app = create_test_router(state.clone()).await; + let request = + get_request_with_auth(&format!("/api/v1/access-groups/{}/rules", group.id), &token); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + let rules = response.unwrap(); + assert_eq!(rules["genreMatch"], "all"); + assert_eq!(rules["sharingTags"][0]["name"], "family"); + assert_eq!(rules["tags"][0]["name"], "Gore"); + assert_eq!(rules["genres"].as_array().unwrap().len(), 2); + assert_eq!(rules["libraries"][0]["libraryId"], library.id.to_string()); + assert_eq!(rules["libraries"][0]["name"], "Kids"); + assert_eq!(rules["libraries"][0]["accessMode"], "allow"); + + // The group list counts every rule, not only sharing tags + let app = create_test_router(state.clone()).await; + let request = get_request_with_auth("/api/v1/access-groups", &token); + let (_, response): (StatusCode, Option>) = + make_json_request(app, request).await; + let listed = response.unwrap(); + assert_eq!(listed[0].rule_count, 5); + assert_eq!(listed[0].grant_count, 1); +} + +#[tokio::test] +async fn test_replace_rules_rejects_duplicates() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let group = create_group(&state, &token, "Kids").await; + + let (status, _) = put_rules( + &state, + &token, + group.id, + serde_json::json!({ + "genres": [ + { "name": "Adult", "accessMode": "deny" }, + { "name": "adult", "accessMode": "allow" } + ] + }), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); +} + +#[tokio::test] +async fn test_replace_rules_rejects_unknown_library_and_sharing_tag() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let group = create_group(&state, &token, "Kids").await; + + let (status, _) = put_rules( + &state, + &token, + group.id, + serde_json::json!({ + "libraries": [{ "libraryId": uuid::Uuid::new_v4(), "accessMode": "allow" }] + }), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); + + let (status, _) = put_rules( + &state, + &token, + group.id, + serde_json::json!({ + "sharingTags": [{ "sharingTagId": uuid::Uuid::new_v4(), "accessMode": "deny" }] + }), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); +} + +#[tokio::test] +async fn test_rules_endpoints_require_admin() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + let (_, reader_token) = create_reader_and_token(&db, &state, "alice").await; + let group = create_group(&state, &token, "Kids").await; + + let (status, _) = put_rules(&state, &reader_token, group.id, serde_json::json!({})).await; + assert_eq!(status, StatusCode::FORBIDDEN); + + let app = create_test_router(state).await; + let request = get_request_with_auth( + &format!("/api/v1/access-groups/{}/rules", group.id), + &reader_token, + ); + let (status, _): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::FORBIDDEN); +} + +#[tokio::test] +async fn test_rules_for_missing_group_is_not_found() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_, token) = create_admin_and_token(&db, &state).await; + + let (status, _) = put_rules(&state, &token, uuid::Uuid::new_v4(), serde_json::json!({})).await; + assert_eq!(status, StatusCode::NOT_FOUND); +} diff --git a/tests/api/reading_progress_transfer.rs b/tests/api/reading_progress_transfer.rs index 313fd9aa3..6bfdba92b 100644 --- a/tests/api/reading_progress_transfer.rs +++ b/tests/api/reading_progress_transfer.rs @@ -55,6 +55,26 @@ async fn admin_and_token( (created.id, token) } +async fn reader_and_token( + db: &DatabaseConnection, + state: &codex::api::extractors::AuthState, + username: &str, +) -> (Uuid, String) { + let password_hash = password::hash_password("pw123456").unwrap(); + let user = create_test_user( + username, + &format!("{username}@example.com"), + &password_hash, + false, + ); + let created = UserRepository::create(db, &user).await.unwrap(); + let token = state + .jwt_service + .generate_token(created.id, created.username.clone(), created.get_role()) + .unwrap(); + (created.id, token) +} + fn book_model( series_id: Uuid, library_id: Uuid, @@ -594,7 +614,8 @@ async fn exercise_visibility_denies_unmatched(db: &DatabaseConnection) { // Relative, because the PostgreSQL run shares one database across scenarios. let before = table_counts(db).await; let state = create_test_auth_state(db.clone()).await; - let (user_id, token) = admin_and_token(db, &state, "restricted-reader").await; + // A reader: admins are never restricted by sharing tags. + let (user_id, token) = reader_and_token(db, &state, "restricted-reader").await; let library = LibraryRepository::create(db, "Lib", "/lib", ScanningStrategy::Default) .await diff --git a/tests/db/reading_history_retention.rs b/tests/db/reading_history_retention.rs index 443ef7a8e..10f9db41c 100644 --- a/tests/db/reading_history_retention.rs +++ b/tests/db/reading_history_retention.rs @@ -23,7 +23,7 @@ use codex::db::entities::{books, read_completions, reading_sessions, users}; use codex::db::repositories::{ BookRepository, LibraryRepository, NewSession, ReadCompletionRepository, ReadProgressRepository, ReadingStatsRepository, SeriesRepository, StatsGranularity, StatsSort, - StatsWindow, UserRepository, + StatsWindow, }; use common::*; use migration::{Migrator, MigratorTrait}; @@ -39,9 +39,14 @@ fn unique() -> String { } async fn persist_user(db: &DatabaseConnection) -> Uuid { + use sea_orm::{ActiveModelTrait, IntoActiveModel}; + let handle = format!("reader-{}", unique()); let model = create_test_user(&handle, &format!("{handle}@test.test"), "hash", true); - UserRepository::create(db, &model).await.unwrap().id + // A plain row insert, not `UserRepository::create`: some scenarios run + // against the schema as it was before later migrations, and account + // creation also joins the default access group, which that schema lacks. + model.into_active_model().insert(db).await.unwrap().id } async fn persist_book(db: &DatabaseConnection) -> Uuid { diff --git a/web/openapi.json b/web/openapi.json index 806c148d1..84fbe78c1 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -603,6 +603,112 @@ ] } }, + "/api/v1/access-groups/{group_id}/rules": { + "get": { + "tags": [ + "Access Groups" + ], + "summary": "Get every rule of an access group (admin only)", + "operationId": "get_access_group_rules", + "parameters": [ + { + "name": "group_id", + "in": "path", + "description": "Access group ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "The group's rules", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AccessGroupRulesDto" + } + } + } + }, + "403": { + "description": "Forbidden - Missing permission" + }, + "404": { + "description": "Access group not found" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + }, + "put": { + "tags": [ + "Access Groups" + ], + "summary": "Replace every rule of an access group (admin only)", + "description": "The whole rule set is replaced atomically: anything not in the request is\nremoved. Sharing tag and library IDs must exist; tag and genre names need\nnot, so a rule can be set up before any series carries the name.", + "operationId": "replace_access_group_rules", + "parameters": [ + { + "name": "group_id", + "in": "path", + "description": "Access group ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ReplaceAccessGroupRulesRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "Rules replaced", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AccessGroupRulesDto" + } + } + } + }, + "400": { + "description": "Duplicate rule, blank name, or unknown sharing tag or library" + }, + "403": { + "description": "Forbidden - Missing permission" + }, + "404": { + "description": "Access group not found" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + } + }, "/api/v1/admin/cleanup-orphans": { "post": { "tags": [ @@ -23666,6 +23772,7 @@ "isDefault", "memberCount", "grantCount", + "ruleCount", "createdAt", "updatedAt" ], @@ -23687,7 +23794,7 @@ "grantCount": { "type": "integer", "format": "int64", - "description": "Number of tag grants in the group", + "description": "Number of sharing tag grants in the group", "example": 3, "minimum": 0 }, @@ -23714,6 +23821,13 @@ "description": "Display name of the access group", "example": "Manga Readers" }, + "ruleCount": { + "type": "integer", + "format": "int64", + "description": "Number of rules in the group, across sharing tags, tags, genres and libraries", + "example": 5, + "minimum": 0 + }, "updatedAt": { "type": "string", "format": "date-time", @@ -23817,6 +23931,61 @@ } } }, + "AccessGroupRulesDto": { + "type": "object", + "description": "Every rule of an access group\n\nWithin a group, values of the same type combine as \"any of\" or \"all of\"\n(per the match field for that type; libraries are always \"any of\"), and\ndifferent types combine with AND. Across groups, what each group allows is\ncombined with OR. Denies from every group always win. Admins and\nmaintainers are never restricted.", + "required": [ + "sharingTagMatch", + "tagMatch", + "genreMatch", + "sharingTags", + "tags", + "genres", + "libraries" + ], + "properties": { + "genreMatch": { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed genres combine" + }, + "genres": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on genres, matched by name" + }, + "libraries": { + "type": "array", + "items": { + "$ref": "#/components/schemas/LibraryRuleDto" + }, + "description": "Rules on libraries" + }, + "sharingTagMatch": { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed sharing tags combine" + }, + "sharingTags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/SharingTagRuleDto" + }, + "description": "Rules on sharing tags" + }, + "tagMatch": { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed tags combine" + }, + "tags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on regular tags, matched by name" + } + } + }, "AccessGroupSummaryDto": { "type": "object", "description": "Access group summary (for user's group list)", @@ -34997,6 +35166,50 @@ } } }, + "LibraryRuleDto": { + "type": "object", + "description": "A rule on a library", + "required": [ + "libraryId", + "name", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "libraryId": { + "type": "string", + "format": "uuid", + "description": "Library ID" + }, + "name": { + "type": "string", + "description": "Library name", + "example": "Manga" + } + } + }, + "LibraryRuleInput": { + "type": "object", + "description": "A library rule in a replace request", + "required": [ + "libraryId", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "libraryId": { + "type": "string", + "format": "uuid", + "description": "Library ID" + } + } + }, "LinkProperties": { "type": "object", "description": "Additional properties that can be attached to links", @@ -36036,6 +36249,25 @@ } } }, + "NameRuleDto": { + "type": "object", + "description": "A rule on a regular tag or a genre\n\nMatched by name, case-insensitively, so the rule keeps applying when the\ntag or genre is removed by cleanup and created again later.", + "required": [ + "name", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "name": { + "type": "string", + "description": "Tag or genre name", + "example": "Adult" + } + } + }, "NewsEntryDto": { "type": "object", "description": "One news entry.", @@ -41572,6 +41804,71 @@ } } }, + "ReplaceAccessGroupRulesRequest": { + "type": "object", + "description": "Replace every rule of an access group\n\nOmitted lists are treated as empty and omitted match fields as \"any\".", + "properties": { + "genreMatch": { + "oneOf": [ + { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed genres combine" + } + ], + "default": "any" + }, + "genres": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on genres, matched by name", + "default": [] + }, + "libraries": { + "type": "array", + "items": { + "$ref": "#/components/schemas/LibraryRuleInput" + }, + "description": "Rules on libraries", + "default": [] + }, + "sharingTagMatch": { + "oneOf": [ + { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed sharing tags combine" + } + ], + "default": "any" + }, + "sharingTags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/SharingTagRuleInput" + }, + "description": "Rules on sharing tags", + "default": [] + }, + "tagMatch": { + "oneOf": [ + { + "$ref": "#/components/schemas/RuleMatch", + "description": "How allowed tags combine" + } + ], + "default": "any" + }, + "tags": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NameRuleDto" + }, + "description": "Rules on regular tags, matched by name", + "default": [] + } + } + }, "ReplaceBookMetadataRequest": { "type": "object", "description": "PUT request for full replacement of book metadata\n\nAll metadata fields will be replaced with the values in this request.\nOmitting a field (or setting it to null) will clear that field.", @@ -42287,6 +42584,14 @@ } } }, + "RuleMatch": { + "type": "string", + "description": "How a group's allowed values of one rule type combine.", + "enum": [ + "any", + "all" + ] + }, "RunNowResponse": { "type": "object", "description": "Response for `POST .../run-now`.", @@ -44630,6 +44935,50 @@ } } }, + "SharingTagRuleDto": { + "type": "object", + "description": "A rule on a sharing tag", + "required": [ + "sharingTagId", + "name", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "name": { + "type": "string", + "description": "Sharing tag name", + "example": "Kids" + }, + "sharingTagId": { + "type": "string", + "format": "uuid", + "description": "Sharing tag ID" + } + } + }, + "SharingTagRuleInput": { + "type": "object", + "description": "A sharing tag rule in a replace request", + "required": [ + "sharingTagId", + "accessMode" + ], + "properties": { + "accessMode": { + "$ref": "#/components/schemas/AccessMode", + "description": "Allow or deny" + }, + "sharingTagId": { + "type": "string", + "format": "uuid", + "description": "Sharing tag ID" + } + } + }, "SharingTagSummaryDto": { "type": "object", "description": "Simplified sharing tag for lists (without counts)", diff --git a/web/src/api/accessGroups.ts b/web/src/api/accessGroups.ts index 745c87a50..ae6d2e1ee 100644 --- a/web/src/api/accessGroups.ts +++ b/web/src/api/accessGroups.ts @@ -27,6 +27,10 @@ export type EffectiveGrantsResponse = export type EffectiveGrantDto = components["schemas"]["EffectiveGrantDto"]; export type GrantSourceDto = components["schemas"]["GrantSourceDto"]; export type AddAllUsersResponse = components["schemas"]["AddAllUsersResponse"]; +export type AccessGroupRulesDto = components["schemas"]["AccessGroupRulesDto"]; +export type ReplaceAccessGroupRulesRequest = + components["schemas"]["ReplaceAccessGroupRulesRequest"]; +export type RuleMatch = components["schemas"]["RuleMatch"]; export const accessGroupsApi = { // ============================================ @@ -94,6 +98,29 @@ export const accessGroupsApi = { return response.data; }, + // ============================================ + // Rules (sharing tags, tags, genres, libraries) + // ============================================ + + getRules: async (groupId: string): Promise => { + const response = await api.get( + `/access-groups/${groupId}/rules`, + ); + return response.data; + }, + + /** Replace the group's whole rule set; anything not included is removed. */ + replaceRules: async ( + groupId: string, + request: ReplaceAccessGroupRulesRequest, + ): Promise => { + const response = await api.put( + `/access-groups/${groupId}/rules`, + request, + ); + return response.data; + }, + // ============================================ // Grants // ============================================ diff --git a/web/src/components/access-groups/AccessGroupRulesCard.test.tsx b/web/src/components/access-groups/AccessGroupRulesCard.test.tsx new file mode 100644 index 000000000..55547a792 --- /dev/null +++ b/web/src/components/access-groups/AccessGroupRulesCard.test.tsx @@ -0,0 +1,179 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { AccessGroupRulesDto } from "@/api/accessGroups"; +import { accessGroupsApi } from "@/api/accessGroups"; +import { genresApi } from "@/api/genres"; +import { librariesApi } from "@/api/libraries"; +import { sharingTagsApi } from "@/api/sharingTags"; +import { tagsApi } from "@/api/tags"; +import { renderWithProviders, screen, userEvent, waitFor } from "@/test/utils"; +import { AccessGroupRulesCard } from "./AccessGroupRulesCard"; + +vi.mock("@mantine/notifications", () => ({ + notifications: { show: vi.fn() }, +})); + +vi.mock("@/api/accessGroups", () => ({ + accessGroupsApi: { + getRules: vi.fn(), + replaceRules: vi.fn(), + }, +})); +vi.mock("@/api/genres", () => ({ genresApi: { getAll: vi.fn() } })); +vi.mock("@/api/tags", () => ({ tagsApi: { getAll: vi.fn() } })); +vi.mock("@/api/libraries", () => ({ librariesApi: { getAll: vi.fn() } })); +vi.mock("@/api/sharingTags", () => ({ sharingTagsApi: { list: vi.fn() } })); + +const rules = ( + overrides: Partial = {}, +): AccessGroupRulesDto => ({ + sharingTagMatch: "any", + tagMatch: "any", + genreMatch: "any", + sharingTags: [], + tags: [], + genres: [], + libraries: [], + ...overrides, +}); + +describe("AccessGroupRulesCard", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(genresApi.getAll).mockResolvedValue([ + { + id: "g1", + name: "Adult", + seriesCount: 3, + createdAt: "2026-01-01T00:00:00Z", + }, + ] as never); + vi.mocked(tagsApi.getAll).mockResolvedValue([]); + vi.mocked(librariesApi.getAll).mockResolvedValue([ + { id: "lib-kids", name: "Kids" }, + { id: "lib-manga", name: "Manga" }, + ] as never); + vi.mocked(sharingTagsApi.list).mockResolvedValue([]); + vi.mocked(accessGroupsApi.replaceRules).mockImplementation( + async (_id, request) => + rules({ + ...request, + sharingTags: [], + libraries: (request.libraries ?? []).map((l) => ({ + ...l, + name: l.libraryId, + })), + } as never), + ); + }); + + it("shows the existing rules and that admins are not restricted", async () => { + vi.mocked(accessGroupsApi.getRules).mockResolvedValue( + rules({ + genres: [{ name: "Adult", accessMode: "deny" }], + libraries: [ + { libraryId: "lib-kids", name: "Kids", accessMode: "allow" }, + ], + }), + ); + renderWithProviders(); + + expect(await screen.findByText("Adult")).toBeInTheDocument(); + expect(screen.getByText("Kids")).toBeInTheDocument(); + expect( + screen.getByText(/admins and maintainers are never restricted/i), + ).toBeInTheDocument(); + }); + + it("adds a genre deny on top of the existing rules", async () => { + const user = userEvent.setup(); + vi.mocked(accessGroupsApi.getRules).mockResolvedValue( + rules({ + libraries: [ + { libraryId: "lib-kids", name: "Kids", accessMode: "allow" }, + ], + }), + ); + renderWithProviders(); + + const input = await screen.findByRole("textbox", { name: /add genre/i }); + await user.type(input, "Adult"); + await user.click(screen.getByRole("button", { name: /deny genre/i })); + + await waitFor(() => + expect(accessGroupsApi.replaceRules).toHaveBeenCalledWith( + "group-1", + expect.objectContaining({ + genres: [{ name: "Adult", accessMode: "deny" }], + libraries: [{ libraryId: "lib-kids", accessMode: "allow" }], + }), + ), + ); + }); + + it("removes a rule", async () => { + const user = userEvent.setup(); + vi.mocked(accessGroupsApi.getRules).mockResolvedValue( + rules({ + genres: [ + { name: "Adult", accessMode: "deny" }, + { name: "Horror", accessMode: "deny" }, + ], + }), + ); + renderWithProviders(); + + await user.click( + await screen.findByRole("button", { name: "Remove genre Adult" }), + ); + + await waitFor(() => + expect(accessGroupsApi.replaceRules).toHaveBeenCalledWith( + "group-1", + expect.objectContaining({ + genres: [{ name: "Horror", accessMode: "deny" }], + }), + ), + ); + }); + + it("switches how allowed genres combine", async () => { + const user = userEvent.setup(); + vi.mocked(accessGroupsApi.getRules).mockResolvedValue( + rules({ + genres: [ + { name: "Kids", accessMode: "allow" }, + { name: "Comedy", accessMode: "allow" }, + ], + }), + ); + renderWithProviders(); + + await user.click( + await screen.findByRole("radio", { name: /all allowed genres/i }), + ); + + await waitFor(() => + expect(accessGroupsApi.replaceRules).toHaveBeenCalledWith( + "group-1", + expect.objectContaining({ genreMatch: "all" }), + ), + ); + }); + + it("offers no match switch for libraries", async () => { + vi.mocked(accessGroupsApi.getRules).mockResolvedValue( + rules({ + libraries: [ + { libraryId: "lib-kids", name: "Kids", accessMode: "allow" }, + { libraryId: "lib-manga", name: "Manga", accessMode: "allow" }, + ], + }), + ); + renderWithProviders(); + + await screen.findByText("Manga"); + expect( + screen.queryByRole("radio", { name: /all allowed libraries/i }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/web/src/components/access-groups/AccessGroupRulesCard.tsx b/web/src/components/access-groups/AccessGroupRulesCard.tsx new file mode 100644 index 000000000..e5b5eec54 --- /dev/null +++ b/web/src/components/access-groups/AccessGroupRulesCard.tsx @@ -0,0 +1,425 @@ +import { + ActionIcon, + Alert, + Autocomplete, + Badge, + Button, + Card, + Divider, + Group, + Loader, + SegmentedControl, + Select, + Stack, + Text, + Title, + Tooltip, +} from "@mantine/core"; +import { notifications } from "@mantine/notifications"; +import { + IconFilter, + IconInfoCircle, + IconShieldOff, + IconTrash, +} from "@tabler/icons-react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useState } from "react"; +import { + type AccessGroupRulesDto, + accessGroupsApi, + type ReplaceAccessGroupRulesRequest, + type RuleMatch, +} from "@/api/accessGroups"; +import { genresApi } from "@/api/genres"; +import { librariesApi } from "@/api/libraries"; +import { type AccessMode, sharingTagsApi } from "@/api/sharingTags"; +import { tagsApi } from "@/api/tags"; + +interface AccessGroupRulesCardProps { + groupId: string; +} + +/** Strip display-only fields so a rule set can be sent back as a replace request. */ +function toRequest(rules: AccessGroupRulesDto): ReplaceAccessGroupRulesRequest { + return { + sharingTagMatch: rules.sharingTagMatch, + tagMatch: rules.tagMatch, + genreMatch: rules.genreMatch, + sharingTags: rules.sharingTags.map(({ sharingTagId, accessMode }) => ({ + sharingTagId, + accessMode, + })), + tags: rules.tags.map(({ name, accessMode }) => ({ name, accessMode })), + genres: rules.genres.map(({ name, accessMode }) => ({ name, accessMode })), + libraries: rules.libraries.map(({ libraryId, accessMode }) => ({ + libraryId, + accessMode, + })), + }; +} + +/** + * Editor for every rule of an access group: libraries, genres, tags and + * sharing tags, each allow or deny. Every change replaces the whole rule set + * on the server, so what is shown is always what is stored. + */ +export function AccessGroupRulesCard({ groupId }: AccessGroupRulesCardProps) { + const queryClient = useQueryClient(); + const queryKey = ["access-group-rules", groupId]; + + const { data: rules, isLoading } = useQuery({ + queryKey, + queryFn: () => accessGroupsApi.getRules(groupId), + }); + const { data: libraries = [] } = useQuery({ + queryKey: ["libraries"], + queryFn: librariesApi.getAll, + }); + const { data: genres = [] } = useQuery({ + queryKey: ["genres"], + queryFn: genresApi.getAll, + }); + const { data: tags = [] } = useQuery({ + queryKey: ["tags"], + queryFn: tagsApi.getAll, + }); + const { data: sharingTags = [] } = useQuery({ + queryKey: ["sharing-tags"], + queryFn: sharingTagsApi.list, + }); + + const saveMutation = useMutation({ + mutationFn: (request: ReplaceAccessGroupRulesRequest) => + accessGroupsApi.replaceRules(groupId, request), + onSuccess: (saved) => { + queryClient.setQueryData(queryKey, saved); + queryClient.invalidateQueries({ queryKey: ["access-group", groupId] }); + queryClient.invalidateQueries({ queryKey: ["access-groups"] }); + }, + onError: (error: { message?: string }) => { + notifications.show({ + title: "Error", + message: error.message || "Failed to save rules", + color: "red", + }); + }, + }); + + if (isLoading || !rules) { + return ( + + + + + + ); + } + + const save = (change: Partial) => + saveMutation.mutate({ ...toRequest(rules), ...change }); + const current = toRequest(rules); + + const usedLibraries = new Set(rules.libraries.map((r) => r.libraryId)); + const usedSharingTags = new Set(rules.sharingTags.map((r) => r.sharingTagId)); + const usedGenres = new Set(rules.genres.map((r) => r.name.toLowerCase())); + const usedTags = new Set(rules.tags.map((r) => r.name.toLowerCase())); + + return ( + + + + + Rules + + + + Allow rules narrow what members see: different kinds of rules in this + group must all match (library and genre). A member of several + groups sees what any of their groups allows. Deny rules hide content + everywhere, whatever another group allows. With no allow rules, + members see everything that is not denied. + + + } + p="xs" + > + Admins and maintainers are never restricted by access groups, even + when they are members. + + + ({ + key: r.libraryId, + label: r.name, + accessMode: r.accessMode, + }))} + options={libraries + .filter((l) => !usedLibraries.has(l.id)) + .map((l) => ({ value: l.id, label: l.name }))} + onAdd={(libraryId, accessMode) => + save({ + libraries: [...current.libraries!, { libraryId, accessMode }], + }) + } + onRemove={(libraryId) => + save({ + libraries: current.libraries!.filter( + (r) => r.libraryId !== libraryId, + ), + }) + } + busy={saveMutation.isPending} + /> + + + + ({ + key: r.name, + label: r.name, + accessMode: r.accessMode, + }))} + options={genres + .filter((g) => !usedGenres.has(g.name.toLowerCase())) + .map((g) => ({ value: g.name, label: g.name }))} + match={rules.genreMatch} + onMatchChange={(genreMatch) => save({ genreMatch })} + onAdd={(name, accessMode) => + save({ genres: [...current.genres!, { name, accessMode }] }) + } + onRemove={(name) => + save({ genres: current.genres!.filter((r) => r.name !== name) }) + } + busy={saveMutation.isPending} + /> + + + + ({ + key: r.name, + label: r.name, + accessMode: r.accessMode, + }))} + options={tags + .filter((t) => !usedTags.has(t.name.toLowerCase())) + .map((t) => ({ value: t.name, label: t.name }))} + match={rules.tagMatch} + onMatchChange={(tagMatch) => save({ tagMatch })} + onAdd={(name, accessMode) => + save({ tags: [...current.tags!, { name, accessMode }] }) + } + onRemove={(name) => + save({ tags: current.tags!.filter((r) => r.name !== name) }) + } + busy={saveMutation.isPending} + /> + + + + ({ + key: r.sharingTagId, + label: r.name, + accessMode: r.accessMode, + }))} + options={sharingTags + .filter((t) => !usedSharingTags.has(t.id)) + .map((t) => ({ value: t.id, label: t.name }))} + match={rules.sharingTagMatch} + onMatchChange={(sharingTagMatch) => save({ sharingTagMatch })} + onAdd={(sharingTagId, accessMode) => + save({ + sharingTags: [ + ...current.sharingTags!, + { sharingTagId, accessMode }, + ], + }) + } + onRemove={(sharingTagId) => + save({ + sharingTags: current.sharingTags!.filter( + (r) => r.sharingTagId !== sharingTagId, + ), + }) + } + busy={saveMutation.isPending} + /> + + + ); +} + +// ==================== One rule type ==================== + +interface RuleRow { + key: string; + label: string; + accessMode: AccessMode; +} + +interface RuleSectionProps { + singular: string; + plural: string; + rules: RuleRow[]; + options: { value: string; label: string }[]; + /** Tags and genres accept any name, not only existing ones */ + freeText?: boolean; + hint?: string; + /** Omitted for types where "all of" cannot apply (libraries) */ + match?: RuleMatch; + onMatchChange?: (match: RuleMatch) => void; + onAdd: (value: string, accessMode: AccessMode) => void; + onRemove: (key: string) => void; + busy: boolean; +} + +function RuleSection({ + singular, + plural, + rules, + options, + freeText = false, + hint, + match, + onMatchChange, + onAdd, + onRemove, + busy, +}: RuleSectionProps) { + const [value, setValue] = useState(null); + const capitalized = plural.charAt(0).toUpperCase() + plural.slice(1); + const allowCount = rules.filter((r) => r.accessMode === "allow").length; + + const add = (accessMode: AccessMode) => { + const trimmed = value?.trim(); + if (!trimmed) return; + onAdd(trimmed, accessMode); + setValue(null); + }; + + return ( + + + + {capitalized} + + {rules.length} + + + {match && onMatchChange && allowCount >= 2 && ( + onMatchChange(v as RuleMatch)} + disabled={busy} + data={[ + { value: "any", label: `Any allowed ${singular}` }, + { value: "all", label: `All allowed ${plural}` }, + ]} + /> + )} + + + {hint && ( + + + + {hint} + + + )} + + {rules.length > 0 ? ( + + {rules.map((rule) => ( + + + + {rule.accessMode === "deny" ? "Deny" : "Allow"} + + {rule.label} + + + onRemove(rule.key)} + disabled={busy} + > + + + + + ))} + + ) : ( + + No {singular} rules. + + )} + + + {freeText ? ( + + ) : ( + - ({ value: s.id, label: s.title }))} + value={seriesId} + onChange={setSeriesId} + filter={({ options }) => options} + nothingFoundMessage={ + debouncedSearch ? "No series found" : undefined + } + clearable + /> + + {explained?.series && ( + + )} + + )} + + + ); +} + +function SeriesExplanation({ access }: { access: SeriesAccess }) { + return ( + + + + {access.visible ? "Visible" : "Hidden"} + + + {access.title} + + + {access.decision === "denied" && ( + <> + Hidden by: + {access.deniedBy.map((rule) => ( + + Deny {RULE_TYPE_LABELS[rule.ruleType]} "{rule.value}", from{" "} + {describeSource(rule.source)} + + ))} + + )} + {access.decision === "notAllowed" && ( + + Hidden: this user's groups only show what their allow rules match, and + none of them matches this series. + + )} + {access.decision === "allowed" && ( + + Shown by {access.allowedBy.map(describeSource).join(", ")}. + + )} + {access.decision === "open" && ( + + Shown: no allow rule applies to this user, and no deny rule matches. + + )} + {access.decision === "unrestricted" && ( + + Shown: this user's role is never restricted. + + )} + + ); +} diff --git a/web/src/components/users/index.ts b/web/src/components/users/index.ts index 723419ef4..95a3ceedc 100644 --- a/web/src/components/users/index.ts +++ b/web/src/components/users/index.ts @@ -1,3 +1,4 @@ export { UserAccessGroupBadges } from "./UserAccessGroupBadges"; export { UserEffectiveGrants } from "./UserEffectiveGrants"; export { UserSharingTagGrants } from "./UserSharingTagGrants"; +export { UserVisibilityPreview } from "./UserVisibilityPreview"; diff --git a/web/src/pages/settings/UsersSettings.tsx b/web/src/pages/settings/UsersSettings.tsx index b9000ddef..6ca493fec 100644 --- a/web/src/pages/settings/UsersSettings.tsx +++ b/web/src/pages/settings/UsersSettings.tsx @@ -45,6 +45,7 @@ import { UserAccessGroupBadges, UserEffectiveGrants, UserSharingTagGrants, + UserVisibilityPreview, } from "@/components/users"; import { useShowSkeleton } from "@/lib/motion/useShowSkeleton"; import { useAuthStore } from "@/store/authStore"; @@ -703,6 +704,7 @@ export function UsersSettings() { {/* Effective Grants (combined user + group view) */} {selectedUser && } + {selectedUser && } {/* Custom Permissions */} {selectedUser && selectedUser.role !== "admin" && ( diff --git a/web/src/types/api.generated.ts b/web/src/types/api.generated.ts index 141c14ba3..e8d1ec0a1 100644 --- a/web/src/types/api.generated.ts +++ b/web/src/types/api.generated.ts @@ -6647,6 +6647,30 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/users/{user_id}/visibility-preview": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Preview what a user can see (admin only) + * @description Counts the series the user can and cannot see and lists every library with + * whether they can see it. With `seriesId`, also explains that series: which + * deny rules hide it (and from which group), or which groups let it through, + * or that no allow rule matches it. Uses the same rules as enforcement, built + * fresh rather than from the cache. + */ + get: operations["get_visibility_preview"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/want-to-read": { parameters: { query?: never; @@ -8337,6 +8361,11 @@ export interface paths { export type webhooks = Record; export interface components { schemas: { + /** + * @description The outcome for one series, and the reason for it. + * @enum {string} + */ + AccessDecision: "unrestricted" | "open" | "allowed" | "notAllowed" | "denied"; /** @description Access group detail (includes members, grants, and OIDC mappings) */ AccessGroupDetailDto: { /** @@ -14662,6 +14691,13 @@ export interface components { */ libraryId: string; }; + /** @description A library and whether the previewed user can see it */ + LibraryVisibilityDto: { + /** Format: uuid */ + id: string; + name: string; + visible: boolean; + }; /** @description Additional properties that can be attached to links */ LinkProperties: { /** @@ -14798,6 +14834,13 @@ export interface components { */ message: string; }; + /** @description A deny rule that matches a series */ + MatchedRuleDto: { + ruleType: components["schemas"]["RuleTypeLabel"]; + source: components["schemas"]["GrantSourceDto"]; + /** @description The tag or genre name, or the sharing tag or library name */ + value: string; + }; /** * @description Provenance of a group membership * @enum {string} @@ -18953,6 +18996,11 @@ export interface components { * @enum {string} */ RuleMatch: "any" | "all"; + /** + * @description The kind of value a rule names. + * @enum {string} + */ + RuleTypeLabel: "sharingTag" | "tag" | "genre" | "library"; /** @description Response for `POST .../run-now`. */ RunNowResponse: { /** Format: uuid */ @@ -19122,6 +19170,18 @@ export interface components { /** @description Cover source: "default" (first book cover) or "custom" (uploaded cover) */ source: string; }; + /** @description Whether the previewed user can see one series, and why */ + SeriesAccessDto: { + /** @description Every group (or personal grant) whose allow rules let it through */ + allowedBy: components["schemas"]["GrantSourceDto"][]; + decision: components["schemas"]["AccessDecision"]; + /** @description Every deny rule that matches this series */ + deniedBy: components["schemas"]["MatchedRuleDto"][]; + /** Format: uuid */ + seriesId: string; + title: string; + visible: boolean; + }; /** * @description Title alias used by release-source plugins to match incoming releases by * title (Nyaa, MangaUpdates without an external ID, etc.). @@ -22355,6 +22415,32 @@ export interface components { /** @description User information */ user: components["schemas"]["UserInfo"]; }; + /** @description What a user can see, for an admin checking an access setup */ + VisibilityPreviewDto: { + /** + * Format: int64 + * @description Series hidden from the user + */ + hiddenSeriesCount: number; + /** @description Every library, and whether the user can see it */ + libraries: components["schemas"]["LibraryVisibilityDto"][]; + /** @description False for admins and maintainers, and for users no rule applies to */ + restricted: boolean; + /** @description The user's role exempts them from access groups (admin or maintainer) */ + roleBypass: boolean; + /** @description Present when a series was asked about */ + series?: components["schemas"]["SeriesAccessDto"]; + /** + * Format: uuid + * @description The previewed user + */ + userId: string; + /** + * Format: int64 + * @description Series the user can see + */ + visibleSeriesCount: number; + }; /** * @description A single entry in a user's want-to-read queue. Exactly one of `series_id` / * `book_id` is populated, matching `item_type`. @@ -37758,6 +37844,46 @@ export interface operations { }; }; }; + get_visibility_preview: { + parameters: { + query?: { + /** @description Explain whether the user can see this series */ + seriesId?: string | null; + }; + header?: never; + path: { + /** @description User ID */ + user_id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description What the user can see */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["VisibilityPreviewDto"]; + }; + }; + /** @description Forbidden - Missing permission */ + 403: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description User or series not found */ + 404: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; list_want_to_read: { parameters: { query?: { From 466815f76519ee581e68fab23ac153dec6a64a0e Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Sun, 25 Oct 2026 20:49:35 -0700 Subject: [PATCH 15/16] fix(api)!: make server metrics admin-only The inventory, plugin and task metrics endpoints only required libraries:read, which every reader has, while the only page that shows them (Settings > Metrics) is linked for admins alone. They report the user count, database size, every library's totals and plugin and task activity, none of which a reader needs, and the inventory totals would also tell a restricted reader how much content is hidden from them. All four read endpoints now require admin, like the metrics cleanup endpoints already did, which also replaces the special case that refused inventory metrics to accounts with content restrictions. The metrics page now sends non-admins home instead of rendering a page whose requests can only fail. BREAKING CHANGE: GET /api/v1/metrics/inventory, /metrics/plugins, /metrics/tasks and /metrics/tasks/history return 403 for non-admin users. No Codex client calls them as a non-admin. --- crates/codex-api/src/routes/v1/dto/metrics.rs | 4 +- .../src/routes/v1/handlers/metrics.rs | 29 +++------ .../src/routes/v1/handlers/task_metrics.rs | 10 ++-- docs/api/openapi.json | 10 ++-- docs/docs/users/access-groups.md | 2 +- docs/docs/users/permissions.md | 2 +- tests/api/metrics.rs | 17 +++--- tests/api/plugin_metrics.rs | 9 ++- tests/api/task_metrics.rs | 18 ++++++ web/openapi.json | 10 ++-- web/src/App.tsx | 9 ++- web/src/components/auth/AdminRoute.test.tsx | 60 +++++++++++++++++++ web/src/components/auth/AdminRoute.tsx | 16 +++++ web/src/types/api.generated.ts | 12 ++-- 14 files changed, 144 insertions(+), 64 deletions(-) create mode 100644 web/src/components/auth/AdminRoute.test.tsx create mode 100644 web/src/components/auth/AdminRoute.tsx diff --git a/crates/codex-api/src/routes/v1/dto/metrics.rs b/crates/codex-api/src/routes/v1/dto/metrics.rs index 85b030bb5..b11640ed0 100644 --- a/crates/codex-api/src/routes/v1/dto/metrics.rs +++ b/crates/codex-api/src/routes/v1/dto/metrics.rs @@ -48,8 +48,8 @@ pub struct MetricsDto { #[serde(rename_all = "camelCase")] #[into_params(rename_all = "camelCase")] pub struct InventoryMetricsQuery { - /// Count pages exactly instead of estimating. Admin only: on PostgreSQL an - /// exact count is a full scan of the largest table. + /// Count pages exactly instead of estimating. On PostgreSQL an exact count + /// is a full scan of the largest table. #[serde(default)] pub exact: bool, } diff --git a/crates/codex-api/src/routes/v1/handlers/metrics.rs b/crates/codex-api/src/routes/v1/handlers/metrics.rs index 811659412..ca3bf838e 100644 --- a/crates/codex-api/src/routes/v1/handlers/metrics.rs +++ b/crates/codex-api/src/routes/v1/handlers/metrics.rs @@ -9,7 +9,7 @@ use super::super::dto::{ InventoryMetricsQuery, LibraryMetricsDto, MetricsDto, PluginMethodMetricsDto, PluginMetricsDto, PluginMetricsResponse, PluginMetricsSummaryDto, }; -use crate::{AppState, error::ApiError, extractors::AuthContext, permissions::Permission}; +use crate::{AppState, error::ApiError, extractors::AuthContext}; use codex_db::repositories::MetricsRepository; /// Get inventory metrics (library/book counts) @@ -30,7 +30,7 @@ use codex_db::repositories::MetricsRepository; params(InventoryMetricsQuery), responses( (status = 200, description = "Inventory metrics retrieved successfully", body = MetricsDto), - (status = 403, description = "Permission denied, `exact=true` requested without admin access, or the account has content restrictions"), + (status = 403, description = "Admin only"), ), security( ("jwt_bearer" = []), @@ -43,24 +43,9 @@ pub async fn get_inventory_metrics( auth: AuthContext, Query(query): Query, ) -> Result, ApiError> { - // Check permission - metrics are considered system-level library information - auth.require_permission(&Permission::LibrariesRead)?; - // Refuse rather than fall back to the estimate, so a caller that asked for - // an exact count never receives an estimate believing it is exact. - if query.exact { - auth.require_admin()?; - } - // Library-wide totals and per-library counts would reveal how much content - // a restricted reader cannot see, so they get none. - if crate::visibility::content_filter(&state.db, auth.user_id) - .await? - .has_restrictions - { - return Err(ApiError::Forbidden( - "Library statistics are not available to accounts with content restrictions" - .to_string(), - )); - } + // Server metrics (user count, database size, every library's totals) are + // for admins, like the Settings > Metrics page that shows them. + auth.require_admin()?; // Gather all metrics in parallel let ( @@ -157,7 +142,7 @@ pub async fn get_inventory_metrics( path = "/api/v1/metrics/plugins", responses( (status = 200, description = "Plugin metrics retrieved successfully", body = PluginMetricsResponse), - (status = 403, description = "Permission denied"), + (status = 403, description = "Admin only"), ), security( ("jwt_bearer" = []), @@ -169,7 +154,7 @@ pub async fn get_plugin_metrics( State(state): State>, auth: AuthContext, ) -> Result, ApiError> { - auth.require_permission(&Permission::LibrariesRead)?; + auth.require_admin()?; let summary = state.plugin_metrics_service.get_summary().await; let plugin_snapshots = state.plugin_metrics_service.get_all_metrics().await; diff --git a/crates/codex-api/src/routes/v1/handlers/task_metrics.rs b/crates/codex-api/src/routes/v1/handlers/task_metrics.rs index 93048a014..b78b63e48 100644 --- a/crates/codex-api/src/routes/v1/handlers/task_metrics.rs +++ b/crates/codex-api/src/routes/v1/handlers/task_metrics.rs @@ -7,7 +7,7 @@ use super::super::dto::{ TaskMetricsHistoryQuery, TaskMetricsHistoryResponse, TaskMetricsResponse, TaskMetricsSummaryDto, TaskTypeMetricsDto, }; -use crate::{AppState, error::ApiError, extractors::AuthContext, permissions::Permission}; +use crate::{AppState, error::ApiError, extractors::AuthContext}; use codex_db::repositories::TaskRepository; /// Get current task metrics @@ -24,7 +24,7 @@ use codex_db::repositories::TaskRepository; path = "/api/v1/metrics/tasks", responses( (status = 200, description = "Task metrics retrieved successfully", body = TaskMetricsResponse), - (status = 403, description = "Permission denied"), + (status = 403, description = "Admin only"), (status = 503, description = "Task metrics service not available"), ), security( @@ -37,7 +37,7 @@ pub async fn get_task_metrics( State(state): State>, auth: AuthContext, ) -> Result, ApiError> { - auth.require_permission(&Permission::LibrariesRead)?; + auth.require_admin()?; let metrics_service = state .task_metrics_service @@ -118,7 +118,7 @@ pub async fn get_task_metrics( params(TaskMetricsHistoryQuery), responses( (status = 200, description = "Task metrics history retrieved successfully", body = TaskMetricsHistoryResponse), - (status = 403, description = "Permission denied"), + (status = 403, description = "Admin only"), (status = 503, description = "Task metrics service not available"), ), security( @@ -132,7 +132,7 @@ pub async fn get_task_metrics_history( auth: AuthContext, Query(query): Query, ) -> Result, ApiError> { - auth.require_permission(&Permission::LibrariesRead)?; + auth.require_admin()?; let metrics_service = state .task_metrics_service diff --git a/docs/api/openapi.json b/docs/api/openapi.json index 771ca76dd..f68fe1d27 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -9845,7 +9845,7 @@ { "name": "exact", "in": "query", - "description": "Count pages exactly instead of estimating. Admin only: on PostgreSQL an\nexact count is a full scan of the largest table.", + "description": "Count pages exactly instead of estimating. On PostgreSQL an exact count\nis a full scan of the largest table.", "required": false, "schema": { "type": "boolean" @@ -9864,7 +9864,7 @@ } }, "403": { - "description": "Permission denied, `exact=true` requested without admin access, or the account has content restrictions" + "description": "Admin only" } }, "security": [ @@ -9897,7 +9897,7 @@ } }, "403": { - "description": "Permission denied" + "description": "Admin only" } }, "security": [ @@ -9930,7 +9930,7 @@ } }, "403": { - "description": "Permission denied" + "description": "Admin only" }, "503": { "description": "Task metrics service not available" @@ -10078,7 +10078,7 @@ } }, "403": { - "description": "Permission denied" + "description": "Admin only" }, "503": { "description": "Task metrics service not available" diff --git a/docs/docs/users/access-groups.md b/docs/docs/users/access-groups.md index 353f19f59..289b519dc 100644 --- a/docs/docs/users/access-groups.md +++ b/docs/docs/users/access-groups.md @@ -208,7 +208,7 @@ curl "http://localhost:8080/api/v1/users/{user_id}/visibility-preview?seriesId={ A restricted reader gets the same view everywhere: the web app, the native API, OPDS 1.2 and 2.0 feeds, the [Komga-compatible API](../third-party-apps), KOReader sync, and live updates (new or changed series and books are not announced for content they cannot see). Hidden content answers exactly like content that does not exist: it is missing from lists, search, genre and tag lists and library counts, and opening it by id returns "not found". -Server-wide statistics (**Settings** > **Metrics**) are not available to accounts with content restrictions, since they describe the whole library. +Server-wide statistics (**Settings** > **Metrics**) are admin-only, since they describe the whole server. Restriction changes take effect on the next request. When Codex runs as several server processes, the others pick them up within 30 seconds. New series from a scan are covered by deny rules within 30 seconds of the scan, and by allow rules immediately (a new series is hidden until a rule allows it). diff --git a/docs/docs/users/permissions.md b/docs/docs/users/permissions.md index 8e4b7530c..f716ca653 100644 --- a/docs/docs/users/permissions.md +++ b/docs/docs/users/permissions.md @@ -171,7 +171,7 @@ Shared groupings of series (collections) and books (read lists). See [Collection | Permission | Description | |------------|-------------| -| `system-health` | View health status and metrics | +| `system-health` | View health status (server metrics are admin-only) | | `system-admin` | Full administrative access, server settings | ## Effective Permissions diff --git a/tests/api/metrics.rs b/tests/api/metrics.rs index 056350d20..df55d3a7e 100644 --- a/tests/api/metrics.rs +++ b/tests/api/metrics.rs @@ -169,8 +169,10 @@ async fn test_get_metrics_without_auth() { assert_eq!(error.error, "Unauthorized"); } +/// Inventory metrics describe the whole server (user count, database size, +/// every library's totals), so only admins may read them. #[tokio::test] -async fn test_get_metrics_with_readonly_user() { +async fn test_get_metrics_is_refused_to_readonly_user() { let (db, _temp_dir) = setup_test_db().await; // Create a library @@ -183,12 +185,9 @@ async fn test_get_metrics_with_readonly_user() { let app = create_test_router(state).await; let request = get_request_with_auth("/api/v1/metrics/inventory", &token); - let (status, response): (StatusCode, Option) = - make_json_request(app, request).await; + let (status, _): (StatusCode, Option) = make_json_request(app, request).await; - assert_eq!(status, StatusCode::OK); - let metrics = response.unwrap(); - assert_eq!(metrics.library_count, 1); + assert_eq!(status, StatusCode::FORBIDDEN); } #[tokio::test] @@ -414,7 +413,7 @@ async fn test_get_metrics_exact_page_count_requires_admin() { } #[tokio::test] -async fn test_get_metrics_exact_false_is_allowed_for_readonly_user() { +async fn test_get_metrics_exact_false_is_refused_to_readonly_user() { let (db, _temp_dir) = setup_test_db().await; let state = create_test_auth_state(db.clone()).await; @@ -422,7 +421,7 @@ async fn test_get_metrics_exact_false_is_allowed_for_readonly_user() { let app = create_test_router(state).await; let request = get_request_with_auth("/api/v1/metrics/inventory?exact=false", &token); - let (status, _): (StatusCode, Option) = make_json_request(app, request).await; + let (status, _): (StatusCode, Option) = make_json_request(app, request).await; - assert_eq!(status, StatusCode::OK); + assert_eq!(status, StatusCode::FORBIDDEN); } diff --git a/tests/api/plugin_metrics.rs b/tests/api/plugin_metrics.rs index 4312ef519..8401d13e3 100644 --- a/tests/api/plugin_metrics.rs +++ b/tests/api/plugin_metrics.rs @@ -141,7 +141,7 @@ async fn test_get_plugin_metrics_requires_auth() { } #[tokio::test] -async fn test_get_plugin_metrics_allowed_for_reader() { +async fn test_get_plugin_metrics_refused_to_reader() { let (db, _temp_dir) = setup_test_db().await; let state = create_test_app_state(db.clone()).await; @@ -160,10 +160,9 @@ async fn test_get_plugin_metrics_allowed_for_reader() { let app = create_test_router_with_app_state(state); let request = get_request_with_auth("/api/v1/metrics/plugins", &token); - let (status, response): (StatusCode, Option) = + let (status, _): (StatusCode, Option) = make_json_request(app, request).await; - // Reader should have libraries:read permission which is required - assert_eq!(status, StatusCode::OK); - assert!(response.is_some()); + // Server metrics are admin-only, like the Settings > Metrics page + assert_eq!(status, StatusCode::FORBIDDEN); } diff --git a/tests/api/task_metrics.rs b/tests/api/task_metrics.rs index 11669411d..63506c0da 100644 --- a/tests/api/task_metrics.rs +++ b/tests/api/task_metrics.rs @@ -278,6 +278,24 @@ async fn test_task_metrics_requires_auth() { assert_eq!(error.error, "Unauthorized"); } +#[tokio::test] +async fn test_task_metrics_reads_require_admin() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_app_state_with_metrics(db.clone()).await; + let token = create_regular_user_and_token(&db, &state).await; + + for uri in [ + "/api/v1/metrics/tasks", + "/api/v1/metrics/tasks/history?days=7&granularity=hour", + ] { + let app = create_test_router_with_app_state(state.clone()); + let request = get_request_with_auth(uri, &token); + let (status, _): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{uri}"); + } +} + #[tokio::test] async fn test_metrics_cleanup_requires_admin() { let (db, _temp_dir) = setup_test_db().await; diff --git a/web/openapi.json b/web/openapi.json index 771ca76dd..f68fe1d27 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -9845,7 +9845,7 @@ { "name": "exact", "in": "query", - "description": "Count pages exactly instead of estimating. Admin only: on PostgreSQL an\nexact count is a full scan of the largest table.", + "description": "Count pages exactly instead of estimating. On PostgreSQL an exact count\nis a full scan of the largest table.", "required": false, "schema": { "type": "boolean" @@ -9864,7 +9864,7 @@ } }, "403": { - "description": "Permission denied, `exact=true` requested without admin access, or the account has content restrictions" + "description": "Admin only" } }, "security": [ @@ -9897,7 +9897,7 @@ } }, "403": { - "description": "Permission denied" + "description": "Admin only" } }, "security": [ @@ -9930,7 +9930,7 @@ } }, "403": { - "description": "Permission denied" + "description": "Admin only" }, "503": { "description": "Task metrics service not available" @@ -10078,7 +10078,7 @@ } }, "403": { - "description": "Permission denied" + "description": "Admin only" }, "503": { "description": "Task metrics service not available" diff --git a/web/src/App.tsx b/web/src/App.tsx index 50b9efd1d..1912e78bd 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -11,6 +11,7 @@ import { } from "react-router-dom"; import { onRateLimitNotification } from "@/api/client"; import { setupApi } from "@/api/setup"; +import { AdminRoute } from "@/components/auth/AdminRoute"; import { AppLayout } from "@/components/layout/AppLayout"; import { useEntityEvents } from "@/hooks/useEntityEvents"; import { BookDetail } from "@/pages/BookDetail"; @@ -545,9 +546,11 @@ function App() { path="/settings/metrics" element={ - - - + + + + + } /> diff --git a/web/src/components/auth/AdminRoute.test.tsx b/web/src/components/auth/AdminRoute.test.tsx new file mode 100644 index 000000000..d7636d69c --- /dev/null +++ b/web/src/components/auth/AdminRoute.test.tsx @@ -0,0 +1,60 @@ +import { Route, Routes } from "react-router-dom"; +import { beforeEach, describe, expect, it } from "vitest"; +import { useAuthStore } from "@/store/authStore"; +import { renderWithProviders, screen } from "@/test/utils"; +import { AdminRoute } from "./AdminRoute"; + +function signIn(role: "admin" | "maintainer" | "reader") { + useAuthStore.setState({ + user: { + id: "u", + username: "u", + email: "u@example.com", + role, + emailVerified: true, + permissions: [], + }, + token: "token", + isAuthenticated: true, + } as never); +} + +function renderAt(path: string) { + renderWithProviders( + + +
Metrics page
+ + } + /> + Home page} /> + , + { initialEntries: [path] }, + ); +} + +describe("AdminRoute", () => { + beforeEach(() => { + useAuthStore.setState({ + user: null, + token: null, + isAuthenticated: false, + } as never); + }); + + it("shows the page to an admin", () => { + signIn("admin"); + renderAt("/settings/metrics"); + expect(screen.getByText("Metrics page")).toBeInTheDocument(); + }); + + it("sends anyone else home", () => { + signIn("maintainer"); + renderAt("/settings/metrics"); + expect(screen.queryByText("Metrics page")).not.toBeInTheDocument(); + expect(screen.getByText("Home page")).toBeInTheDocument(); + }); +}); diff --git a/web/src/components/auth/AdminRoute.tsx b/web/src/components/auth/AdminRoute.tsx new file mode 100644 index 000000000..e1f6b8665 --- /dev/null +++ b/web/src/components/auth/AdminRoute.tsx @@ -0,0 +1,16 @@ +import type { ReactNode } from "react"; +import { Navigate } from "react-router-dom"; +import { usePermissions } from "@/hooks/usePermissions"; + +/** + * Renders its page only for admins and sends anyone else home, for pages + * whose API is admin-only, so a non-admin who types the URL does not land on + * a page that can only fail. + */ +export function AdminRoute({ children }: { children: ReactNode }) { + const { isAdmin } = usePermissions(); + if (!isAdmin) { + return ; + } + return <>{children}; +} diff --git a/web/src/types/api.generated.ts b/web/src/types/api.generated.ts index e8d1ec0a1..a782162ab 100644 --- a/web/src/types/api.generated.ts +++ b/web/src/types/api.generated.ts @@ -29962,8 +29962,8 @@ export interface operations { parameters: { query?: { /** - * @description Count pages exactly instead of estimating. Admin only: on PostgreSQL an - * exact count is a full scan of the largest table. + * @description Count pages exactly instead of estimating. On PostgreSQL an exact count + * is a full scan of the largest table. */ exact?: boolean; }; @@ -29982,7 +29982,7 @@ export interface operations { "application/json": components["schemas"]["MetricsDto"]; }; }; - /** @description Permission denied, `exact=true` requested without admin access, or the account has content restrictions */ + /** @description Admin only */ 403: { headers: { [name: string]: unknown; @@ -30009,7 +30009,7 @@ export interface operations { "application/json": components["schemas"]["PluginMetricsResponse"]; }; }; - /** @description Permission denied */ + /** @description Admin only */ 403: { headers: { [name: string]: unknown; @@ -30036,7 +30036,7 @@ export interface operations { "application/json": components["schemas"]["TaskMetricsResponse"]; }; }; - /** @description Permission denied */ + /** @description Admin only */ 403: { headers: { [name: string]: unknown; @@ -30154,7 +30154,7 @@ export interface operations { "application/json": components["schemas"]["TaskMetricsHistoryResponse"]; }; }; - /** @description Permission denied */ + /** @description Admin only */ 403: { headers: { [name: string]: unknown; From 231ec3f0afbbdcbbf895ec48887020eaf0001706 Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Sun, 25 Oct 2026 21:19:10 -0700 Subject: [PATCH 16/16] fix(api): let maintainers read server metrics again The previous commit made the metrics endpoints admin-only on the claim that no Codex client calls them as a non-admin. That was wrong: the iOS app shows inventory and task metrics from its Tasks screen, which it offers to maintainers, so they would have hit a 403 there. The four metrics reads now require tasks:read, which admins and maintainers hold and readers do not. Readers are still kept out, which is what the change was for; maintainers are never restricted by access groups, so the totals reveal nothing to them. The web metrics page uses the same permission instead of an admin check. BREAKING CHANGE (narrowed): the metrics endpoints now return 403 for readers only, where they used to accept any user with libraries:read. --- .../src/routes/v1/handlers/metrics.rs | 14 +++++----- .../src/routes/v1/handlers/task_metrics.rs | 10 +++---- docs/api/openapi.json | 8 +++--- docs/docs/users/access-groups.md | 2 +- docs/docs/users/permissions.md | 4 +-- tests/api/metrics.rs | 26 ++++++++++++++++++ tests/api/task_metrics.rs | 27 ++++++++++++++++++- web/openapi.json | 8 +++--- web/src/App.tsx | 7 ++--- web/src/components/auth/AdminRoute.tsx | 16 ----------- ...te.test.tsx => RequirePermission.test.tsx} | 20 ++++++++------ web/src/components/auth/RequirePermission.tsx | 26 ++++++++++++++++++ web/src/types/api.generated.ts | 8 +++--- 13 files changed, 122 insertions(+), 54 deletions(-) delete mode 100644 web/src/components/auth/AdminRoute.tsx rename web/src/components/auth/{AdminRoute.test.tsx => RequirePermission.test.tsx} (75%) create mode 100644 web/src/components/auth/RequirePermission.tsx diff --git a/crates/codex-api/src/routes/v1/handlers/metrics.rs b/crates/codex-api/src/routes/v1/handlers/metrics.rs index ca3bf838e..5898e4eb6 100644 --- a/crates/codex-api/src/routes/v1/handlers/metrics.rs +++ b/crates/codex-api/src/routes/v1/handlers/metrics.rs @@ -9,7 +9,7 @@ use super::super::dto::{ InventoryMetricsQuery, LibraryMetricsDto, MetricsDto, PluginMethodMetricsDto, PluginMetricsDto, PluginMetricsResponse, PluginMetricsSummaryDto, }; -use crate::{AppState, error::ApiError, extractors::AuthContext}; +use crate::{AppState, error::ApiError, extractors::AuthContext, permissions::Permission}; use codex_db::repositories::MetricsRepository; /// Get inventory metrics (library/book counts) @@ -30,7 +30,7 @@ use codex_db::repositories::MetricsRepository; params(InventoryMetricsQuery), responses( (status = 200, description = "Inventory metrics retrieved successfully", body = MetricsDto), - (status = 403, description = "Admin only"), + (status = 403, description = "Requires tasks:read (admins and maintainers)"), ), security( ("jwt_bearer" = []), @@ -44,8 +44,10 @@ pub async fn get_inventory_metrics( Query(query): Query, ) -> Result, ApiError> { // Server metrics (user count, database size, every library's totals) are - // for admins, like the Settings > Metrics page that shows them. - auth.require_admin()?; + // for the people who run the server: admins and maintainers, who also see + // them from the iOS app's Tasks screen. Readers have no use for them, and + // the totals would tell a restricted reader how much is hidden from them. + auth.require_permission(&Permission::TasksRead)?; // Gather all metrics in parallel let ( @@ -142,7 +144,7 @@ pub async fn get_inventory_metrics( path = "/api/v1/metrics/plugins", responses( (status = 200, description = "Plugin metrics retrieved successfully", body = PluginMetricsResponse), - (status = 403, description = "Admin only"), + (status = 403, description = "Requires tasks:read (admins and maintainers)"), ), security( ("jwt_bearer" = []), @@ -154,7 +156,7 @@ pub async fn get_plugin_metrics( State(state): State>, auth: AuthContext, ) -> Result, ApiError> { - auth.require_admin()?; + auth.require_permission(&Permission::TasksRead)?; let summary = state.plugin_metrics_service.get_summary().await; let plugin_snapshots = state.plugin_metrics_service.get_all_metrics().await; diff --git a/crates/codex-api/src/routes/v1/handlers/task_metrics.rs b/crates/codex-api/src/routes/v1/handlers/task_metrics.rs index b78b63e48..b0e863cec 100644 --- a/crates/codex-api/src/routes/v1/handlers/task_metrics.rs +++ b/crates/codex-api/src/routes/v1/handlers/task_metrics.rs @@ -7,7 +7,7 @@ use super::super::dto::{ TaskMetricsHistoryQuery, TaskMetricsHistoryResponse, TaskMetricsResponse, TaskMetricsSummaryDto, TaskTypeMetricsDto, }; -use crate::{AppState, error::ApiError, extractors::AuthContext}; +use crate::{AppState, error::ApiError, extractors::AuthContext, permissions::Permission}; use codex_db::repositories::TaskRepository; /// Get current task metrics @@ -24,7 +24,7 @@ use codex_db::repositories::TaskRepository; path = "/api/v1/metrics/tasks", responses( (status = 200, description = "Task metrics retrieved successfully", body = TaskMetricsResponse), - (status = 403, description = "Admin only"), + (status = 403, description = "Requires tasks:read (admins and maintainers)"), (status = 503, description = "Task metrics service not available"), ), security( @@ -37,7 +37,7 @@ pub async fn get_task_metrics( State(state): State>, auth: AuthContext, ) -> Result, ApiError> { - auth.require_admin()?; + auth.require_permission(&Permission::TasksRead)?; let metrics_service = state .task_metrics_service @@ -118,7 +118,7 @@ pub async fn get_task_metrics( params(TaskMetricsHistoryQuery), responses( (status = 200, description = "Task metrics history retrieved successfully", body = TaskMetricsHistoryResponse), - (status = 403, description = "Admin only"), + (status = 403, description = "Requires tasks:read (admins and maintainers)"), (status = 503, description = "Task metrics service not available"), ), security( @@ -132,7 +132,7 @@ pub async fn get_task_metrics_history( auth: AuthContext, Query(query): Query, ) -> Result, ApiError> { - auth.require_admin()?; + auth.require_permission(&Permission::TasksRead)?; let metrics_service = state .task_metrics_service diff --git a/docs/api/openapi.json b/docs/api/openapi.json index f68fe1d27..1ca58e346 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -9864,7 +9864,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" } }, "security": [ @@ -9897,7 +9897,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" } }, "security": [ @@ -9930,7 +9930,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" }, "503": { "description": "Task metrics service not available" @@ -10078,7 +10078,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" }, "503": { "description": "Task metrics service not available" diff --git a/docs/docs/users/access-groups.md b/docs/docs/users/access-groups.md index 289b519dc..b2e8726e9 100644 --- a/docs/docs/users/access-groups.md +++ b/docs/docs/users/access-groups.md @@ -208,7 +208,7 @@ curl "http://localhost:8080/api/v1/users/{user_id}/visibility-preview?seriesId={ A restricted reader gets the same view everywhere: the web app, the native API, OPDS 1.2 and 2.0 feeds, the [Komga-compatible API](../third-party-apps), KOReader sync, and live updates (new or changed series and books are not announced for content they cannot see). Hidden content answers exactly like content that does not exist: it is missing from lists, search, genre and tag lists and library counts, and opening it by id returns "not found". -Server-wide statistics (**Settings** > **Metrics**) are admin-only, since they describe the whole server. +Server-wide statistics (**Settings** > **Metrics**, and the metrics screen in Codex Reader) are only available to admins and maintainers, since they describe the whole server. Restriction changes take effect on the next request. When Codex runs as several server processes, the others pick them up within 30 seconds. New series from a scan are covered by deny rules within 30 seconds of the scan, and by allow rules immediately (a new series is hidden until a rule allows it). diff --git a/docs/docs/users/permissions.md b/docs/docs/users/permissions.md index f716ca653..b2594b837 100644 --- a/docs/docs/users/permissions.md +++ b/docs/docs/users/permissions.md @@ -158,7 +158,7 @@ Shared groupings of series (collections) and books (read lists). See [Collection | Permission | Description | |------------|-------------| -| `tasks-read` | View background tasks and queue status | +| `tasks-read` | View background tasks, queue status and server metrics | | `tasks-write` | Cancel tasks, trigger operations | ### Plugin Permissions @@ -171,7 +171,7 @@ Shared groupings of series (collections) and books (read lists). See [Collection | Permission | Description | |------------|-------------| -| `system-health` | View health status (server metrics are admin-only) | +| `system-health` | View health status (server metrics need `tasks-read`) | | `system-admin` | Full administrative access, server settings | ## Effective Permissions diff --git a/tests/api/metrics.rs b/tests/api/metrics.rs index df55d3a7e..430a73b8d 100644 --- a/tests/api/metrics.rs +++ b/tests/api/metrics.rs @@ -425,3 +425,29 @@ async fn test_get_metrics_exact_false_is_refused_to_readonly_user() { assert_eq!(status, StatusCode::FORBIDDEN); } + +/// Maintainers run tasks and the iOS app shows them metrics from its Tasks +/// screen, so metrics reads are open to them, though not to readers. Task +/// metrics are covered in `task_metrics.rs`, which wires the metrics service. +#[tokio::test] +async fn test_metrics_reads_are_open_to_maintainers() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + + let password_hash = password::hash_password("maint123").unwrap(); + let mut user = create_test_user("maint", "maint@example.com", &password_hash, false); + user.role = "maintainer".to_string(); + let created = UserRepository::create(&db, &user).await.unwrap(); + let token = state + .jwt_service + .generate_token(created.id, created.username.clone(), created.get_role()) + .unwrap(); + + for uri in ["/api/v1/metrics/inventory", "/api/v1/metrics/plugins"] { + let app = create_test_router(state.clone()).await; + let request = get_request_with_auth(uri, &token); + let (status, _): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK, "{uri}"); + } +} diff --git a/tests/api/task_metrics.rs b/tests/api/task_metrics.rs index 63506c0da..0a334b02a 100644 --- a/tests/api/task_metrics.rs +++ b/tests/api/task_metrics.rs @@ -279,7 +279,32 @@ async fn test_task_metrics_requires_auth() { } #[tokio::test] -async fn test_task_metrics_reads_require_admin() { +async fn test_task_metrics_reads_are_open_to_maintainers() { + let (db, _temp_dir) = setup_test_db().await; + let state = create_test_app_state_with_metrics(db.clone()).await; + let password_hash = password::hash_password("maint123").unwrap(); + let mut user = create_test_user("maint", "maint@example.com", &password_hash, false); + user.role = "maintainer".to_string(); + let created = UserRepository::create(&db, &user).await.unwrap(); + let token = state + .jwt_service + .generate_token(created.id, created.username.clone(), created.get_role()) + .unwrap(); + + for uri in [ + "/api/v1/metrics/tasks", + "/api/v1/metrics/tasks/history?days=7&granularity=hour", + ] { + let app = create_test_router_with_app_state(state.clone()); + let request = get_request_with_auth(uri, &token); + let (status, _): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK, "{uri}"); + } +} + +#[tokio::test] +async fn test_task_metrics_reads_are_refused_to_readers() { let (db, _temp_dir) = setup_test_db().await; let state = create_test_app_state_with_metrics(db.clone()).await; let token = create_regular_user_and_token(&db, &state).await; diff --git a/web/openapi.json b/web/openapi.json index f68fe1d27..1ca58e346 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -9864,7 +9864,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" } }, "security": [ @@ -9897,7 +9897,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" } }, "security": [ @@ -9930,7 +9930,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" }, "503": { "description": "Task metrics service not available" @@ -10078,7 +10078,7 @@ } }, "403": { - "description": "Admin only" + "description": "Requires tasks:read (admins and maintainers)" }, "503": { "description": "Task metrics service not available" diff --git a/web/src/App.tsx b/web/src/App.tsx index 1912e78bd..f636f22d7 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -11,7 +11,7 @@ import { } from "react-router-dom"; import { onRateLimitNotification } from "@/api/client"; import { setupApi } from "@/api/setup"; -import { AdminRoute } from "@/components/auth/AdminRoute"; +import { RequirePermission } from "@/components/auth/RequirePermission"; import { AppLayout } from "@/components/layout/AppLayout"; import { useEntityEvents } from "@/hooks/useEntityEvents"; import { BookDetail } from "@/pages/BookDetail"; @@ -57,6 +57,7 @@ import { WantToRead } from "@/pages/WantToRead"; import { navigationService } from "@/services/navigation"; import { useAuthStore } from "@/store/authStore"; import { useBulkSelectionStore } from "@/store/bulkSelectionStore"; +import { PERMISSIONS } from "@/types/permissions"; import { migrateSeriesReadingDirections } from "@/utils/readingDirectionMigration"; // Protected route wrapper @@ -546,11 +547,11 @@ function App() { path="/settings/metrics" element={ - + - + } /> diff --git a/web/src/components/auth/AdminRoute.tsx b/web/src/components/auth/AdminRoute.tsx deleted file mode 100644 index e1f6b8665..000000000 --- a/web/src/components/auth/AdminRoute.tsx +++ /dev/null @@ -1,16 +0,0 @@ -import type { ReactNode } from "react"; -import { Navigate } from "react-router-dom"; -import { usePermissions } from "@/hooks/usePermissions"; - -/** - * Renders its page only for admins and sends anyone else home, for pages - * whose API is admin-only, so a non-admin who types the URL does not land on - * a page that can only fail. - */ -export function AdminRoute({ children }: { children: ReactNode }) { - const { isAdmin } = usePermissions(); - if (!isAdmin) { - return ; - } - return <>{children}; -} diff --git a/web/src/components/auth/AdminRoute.test.tsx b/web/src/components/auth/RequirePermission.test.tsx similarity index 75% rename from web/src/components/auth/AdminRoute.test.tsx rename to web/src/components/auth/RequirePermission.test.tsx index d7636d69c..c49008888 100644 --- a/web/src/components/auth/AdminRoute.test.tsx +++ b/web/src/components/auth/RequirePermission.test.tsx @@ -2,7 +2,8 @@ import { Route, Routes } from "react-router-dom"; import { beforeEach, describe, expect, it } from "vitest"; import { useAuthStore } from "@/store/authStore"; import { renderWithProviders, screen } from "@/test/utils"; -import { AdminRoute } from "./AdminRoute"; +import { PERMISSIONS } from "@/types/permissions"; +import { RequirePermission } from "./RequirePermission"; function signIn(role: "admin" | "maintainer" | "reader") { useAuthStore.setState({ @@ -25,9 +26,9 @@ function renderAt(path: string) { +
Metrics page
- + } /> Home page} /> @@ -36,7 +37,7 @@ function renderAt(path: string) { ); } -describe("AdminRoute", () => { +describe("RequirePermission", () => { beforeEach(() => { useAuthStore.setState({ user: null, @@ -45,14 +46,17 @@ describe("AdminRoute", () => { } as never); }); - it("shows the page to an admin", () => { - signIn("admin"); + it.each([ + "admin", + "maintainer", + ] as const)("shows the page to a %s", (role) => { + signIn(role); renderAt("/settings/metrics"); expect(screen.getByText("Metrics page")).toBeInTheDocument(); }); - it("sends anyone else home", () => { - signIn("maintainer"); + it("sends a reader home", () => { + signIn("reader"); renderAt("/settings/metrics"); expect(screen.queryByText("Metrics page")).not.toBeInTheDocument(); expect(screen.getByText("Home page")).toBeInTheDocument(); diff --git a/web/src/components/auth/RequirePermission.tsx b/web/src/components/auth/RequirePermission.tsx new file mode 100644 index 000000000..e1c6afc23 --- /dev/null +++ b/web/src/components/auth/RequirePermission.tsx @@ -0,0 +1,26 @@ +import type { ReactNode } from "react"; +import { Navigate } from "react-router-dom"; +import { usePermissions } from "@/hooks/usePermissions"; +import type { Permission } from "@/types/permissions"; + +interface RequirePermissionProps { + /** The permission the page's API requires */ + permission: Permission; + children: ReactNode; +} + +/** + * Renders its page only for users holding `permission` and sends anyone else + * home, so a user who types the URL does not land on a page whose requests + * can only fail. + */ +export function RequirePermission({ + permission, + children, +}: RequirePermissionProps) { + const { hasPermission } = usePermissions(); + if (!hasPermission(permission)) { + return ; + } + return <>{children}; +} diff --git a/web/src/types/api.generated.ts b/web/src/types/api.generated.ts index a782162ab..b551310fb 100644 --- a/web/src/types/api.generated.ts +++ b/web/src/types/api.generated.ts @@ -29982,7 +29982,7 @@ export interface operations { "application/json": components["schemas"]["MetricsDto"]; }; }; - /** @description Admin only */ + /** @description Requires tasks:read (admins and maintainers) */ 403: { headers: { [name: string]: unknown; @@ -30009,7 +30009,7 @@ export interface operations { "application/json": components["schemas"]["PluginMetricsResponse"]; }; }; - /** @description Admin only */ + /** @description Requires tasks:read (admins and maintainers) */ 403: { headers: { [name: string]: unknown; @@ -30036,7 +30036,7 @@ export interface operations { "application/json": components["schemas"]["TaskMetricsResponse"]; }; }; - /** @description Admin only */ + /** @description Requires tasks:read (admins and maintainers) */ 403: { headers: { [name: string]: unknown; @@ -30154,7 +30154,7 @@ export interface operations { "application/json": components["schemas"]["TaskMetricsHistoryResponse"]; }; }; - /** @description Admin only */ + /** @description Requires tasks:read (admins and maintainers) */ 403: { headers: { [name: string]: unknown;