From 3c011abbc424f5ef904e7dab09ca2f58b2ca8f51 Mon Sep 17 00:00:00 2001 From: kshcherban Date: Mon, 7 Sep 2026 22:28:59 +0200 Subject: [PATCH] Tidy up more --- Cargo.lock | 9 -- TODO.md | 33 ------- charts/pkgly/templates/configmap.yaml | 1 - charts/pkgly/values.yaml | 1 - crates/core/Cargo.toml | 6 +- .../src/database/entities/project/members.rs | 3 +- .../core/src/database/entities/project/mod.rs | 3 +- .../src/database/entities/project/utils.rs | 14 --- .../core/src/database/entities/repository.rs | 20 +---- crates/core/src/database/mod.rs | 3 +- crates/core/src/user/permissions.rs | 13 --- crates/core/src/utils.rs | 3 +- crates/core/src/utils/time.rs | 30 ------- crates/core/src/utils/time/iso_8601/tests.rs | 7 -- crates/storage/Cargo.toml | 1 - crates/storage/src/fs/file_meta.rs | 19 +++-- crates/storage/src/fs/mod.rs | 4 +- crates/storage/src/fs/path.rs | 15 +--- crates/storage/src/fs/utils.rs | 79 +---------------- crates/storage/src/local/error.rs | 6 +- crates/storage/src/s3/mod.rs | 7 +- docker/config.dev.toml | 1 - docker/config.toml | 1 - docs/docs/sso/index.md | 2 - examples/config.toml | 1 - pkgly/Cargo.toml | 2 - pkgly/build.rs | 9 +- pkgly/src/app/api/repository/packages.rs | 57 +++---------- .../src/app/api/repository/packages/tests.rs | 33 ++----- pkgly/src/app/api/user/password_reset.rs | 42 ++++----- .../src/app/api/user/password_reset/tests.rs | 8 ++ pkgly/src/app/authentication/session.rs | 4 +- .../src/app/authentication/session/storage.rs | 85 ------------------- pkgly/src/app/authentication/ws.rs | 21 +++-- pkgly/src/app/config/security.rs | 4 +- pkgly/src/app/email_service.rs | 50 +++++------ pkgly/src/repository/proxy/base_proxy.rs | 22 +---- .../src/repository/proxy/base_proxy/tests.rs | 21 +---- pkgly/src/repository/proxy/mod.rs | 4 +- pkgly/src/repository/staging.rs | 37 +------- pkgly/src/utils/header.rs | 26 +----- tests/docker/config/pkgly.test.toml | 1 - 42 files changed, 146 insertions(+), 562 deletions(-) delete mode 100644 TODO.md delete mode 100644 crates/core/src/database/entities/project/utils.rs delete mode 100644 crates/core/src/utils/time.rs delete mode 100644 crates/core/src/utils/time/iso_8601/tests.rs delete mode 100644 pkgly/src/app/authentication/session/storage.rs diff --git a/Cargo.lock b/Cargo.lock index f9b81d4..1f5500e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -896,12 +896,6 @@ dependencies = [ "pkg-config", ] -[[package]] -name = "camino" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "276a59bf2b2c967788139340c9f0c5b12d7fd6630315c15c217e559de85d2609" - [[package]] name = "casbin" version = "2.17.0" @@ -3190,7 +3184,6 @@ dependencies = [ "digest 0.10.7", "fs2", "futures", - "hex", "http-body 1.0.1", "http-body-util", "hyper 1.8.1", @@ -3700,7 +3693,6 @@ dependencies = [ "base64 0.22.1", "bytes", "bzip2", - "camino", "casbin", "chrono", "clap", @@ -3760,7 +3752,6 @@ dependencies = [ "serde_yaml", "sha1 0.10.6", "sha2 0.10.9", - "sha2 0.11.0", "sha3", "sqlx", "strum", diff --git a/TODO.md b/TODO.md deleted file mode 100644 index f5a1f32..0000000 --- a/TODO.md +++ /dev/null @@ -1,33 +0,0 @@ -# S3 performance and robustness plan - -This checklist records the S3 performance review and its implementation status. - -## Engineering work - -- [x] Eliminate duplicate Docker blob downloads. Preserve verified digests and deliver verified - content from the same read, including large objects that use bounded temporary files. -- [x] Replace quadratic S3 append read/rewrite behavior for Docker uploads with bounded local - staging and one guarded upload during finalization. Preserve the generic storage append - contract for other repository types. -- [x] Bound total memory and temporary storage consumption. Stream large S3 writes and cache - reads, apply shared body and temporary-file budgets, and spool incoming upload chunks in - bounded buffers. -- [x] Guard cache publication against concurrent mutations. Use per-object generations and - per-key miss coordination so stale in-flight reads cannot replace newer cache entries. -- [x] Reduce metadata round trips and listing contention. Memoize safe ancestor probes, coordinate - cold manifest loads per repository, and paginate manifest listings directly from S3. -- [x] Add Docker accounting reconciliation. Reconcile PostgreSQL rows with storage inventory, - backfill missing manifest graphs, correct size drift, remove missing rows, and use revisions - to avoid overwriting concurrent writes. -- [x] Document S3 cache integrity behavior, Docker upload staging, resource limits, and the - one-writer-per-S3-repository deployment constraint. - -## Validation work - -- [x] Add regression coverage for cache publication races, concurrent cache misses, staged - appends, and accounting backfill detection. -- [x] Run the workspace Rust test suite, S3 storage tests, Clippy, formatting checks, and the - real MinIO S3 integration suite. -- [ ] Benchmark cold and warm pulls plus chunked pushes at 1, 16, and 64 concurrent clients, - using 1 MiB, 64 MiB, and 1 GiB artifacts. Record p95 time to first byte, throughput, peak - memory, S3 request counts, and transferred bytes per delivered byte. diff --git a/charts/pkgly/templates/configmap.yaml b/charts/pkgly/templates/configmap.yaml index f935ab9..5cff42d 100644 --- a/charts/pkgly/templates/configmap.yaml +++ b/charts/pkgly/templates/configmap.yaml @@ -55,7 +55,6 @@ data: # Security Configuration # ========================================================================== [security] - allow_basic_without_tokens = {{ .Values.security.allowBasicWithoutTokens }} [security.password_rules] min_length = {{ .Values.security.passwordRules.minLength }} diff --git a/charts/pkgly/values.yaml b/charts/pkgly/values.yaml index 1f93a2e..d620b03 100644 --- a/charts/pkgly/values.yaml +++ b/charts/pkgly/values.yaml @@ -120,7 +120,6 @@ sessions: # Security configuration security: - allowBasicWithoutTokens: true passwordRules: minLength: 12 requireUppercase: false diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index aa21ae7..ca81bd4 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -40,10 +40,6 @@ clap.workspace = true pretty_assertions = "1" tokio.workspace = true [features] -default = ["migrations"] -migrations = [] -testing = [ - "migrations", -] +testing = [] [lints] workspace = true diff --git a/crates/core/src/database/entities/project/members.rs b/crates/core/src/database/entities/project/members.rs index 85df63a..e85b582 100644 --- a/crates/core/src/database/entities/project/members.rs +++ b/crates/core/src/database/entities/project/members.rs @@ -1,3 +1,5 @@ +// ABOUTME: Defines project membership rows and their database mapping. +// ABOUTME: Carries user access flags for project-level authorization. use crate::database::prelude::*; use serde::Serialize; use utoipa::ToSchema; @@ -14,4 +16,3 @@ pub struct DBProjectMember { pub can_manage: bool, pub added: chrono::DateTime, } -impl DBProjectMember {} diff --git a/crates/core/src/database/entities/project/mod.rs b/crates/core/src/database/entities/project/mod.rs index 7b4e63c..42b2f25 100644 --- a/crates/core/src/database/entities/project/mod.rs +++ b/crates/core/src/database/entities/project/mod.rs @@ -1,3 +1,5 @@ +// ABOUTME: Defines project entities and reusable project/version queries. +// ABOUTME: Exposes database models used by repository catalog operations. use serde::Serialize; use sqlx::{FromRow, PgPool, postgres::PgRow}; use tracing::instrument; @@ -5,7 +7,6 @@ use utoipa::ToSchema; use uuid::Uuid; use versions::{DBProjectVersion, DBProjectVersionColumn, ProjectVersionType}; mod new; -pub mod utils; pub use new::*; use crate::{database::prelude::*, repository::project::ReleaseType}; diff --git a/crates/core/src/database/entities/project/utils.rs b/crates/core/src/database/entities/project/utils.rs deleted file mode 100644 index 0889c61..0000000 --- a/crates/core/src/database/entities/project/utils.rs +++ /dev/null @@ -1,14 +0,0 @@ -use pg_extended_sqlx_queries::prelude::*; -use sqlx::PgPool; -use uuid::Uuid; - -use super::{DBProject, DBProjectColumn}; - -pub async fn does_project_id_exist(id: Uuid, database: &PgPool) -> Result { - let result = SelectExists::new(DBProject::table_name()) - .filter(DBProjectColumn::Id.equals(id)) - .execute(database) - .await?; - - Ok(result) -} diff --git a/crates/core/src/database/entities/repository.rs b/crates/core/src/database/entities/repository.rs index 7c9300c..3f4ce7d 100644 --- a/crates/core/src/database/entities/repository.rs +++ b/crates/core/src/database/entities/repository.rs @@ -1,8 +1,8 @@ -use std::fmt::Debug; - +// ABOUTME: Defines repository database rows and configuration persistence APIs. +// ABOUTME: Provides typed lookups for repositories and storage associations. use serde::{Deserialize, Serialize}; use serde_json::Value; -use sqlx::{PgPool, Row, postgres::PgRow, prelude::FromRow, types::Json}; +use sqlx::{PgPool, Row, prelude::FromRow, types::Json}; use tracing::info; use utoipa::ToSchema; use uuid::Uuid; @@ -15,20 +15,6 @@ use crate::{ storage::StorageName, }; -pub trait RepositoryDBType: for<'r> FromRow<'r, PgRow> + Unpin + Send + Sync { - fn columns() -> Vec<&'static str>; - fn format_columns(prefix: Option<&str>) -> String { - if let Some(prefix) = prefix { - Self::columns() - .iter() - .map(|column| format!("{}.`{}`", prefix, column)) - .collect::>() - .join(", ") - } else { - Self::columns().join(", ") - } - } -} #[derive(Debug, Clone, Serialize, FromRow, ToSchema, Deserialize)] pub struct DBRepositoryWithStorageName { diff --git a/crates/core/src/database/mod.rs b/crates/core/src/database/mod.rs index db1fe6e..9f3bba8 100644 --- a/crates/core/src/database/mod.rs +++ b/crates/core/src/database/mod.rs @@ -1,5 +1,6 @@ +// ABOUTME: Groups database entities, configuration, and migration helpers. +// ABOUTME: Exposes the shared database result types used across the workspace. pub mod entities; -#[cfg(feature = "migrations")] pub mod migration; pub type DateTime = chrono::DateTime; mod config; diff --git a/crates/core/src/user/permissions.rs b/crates/core/src/user/permissions.rs index 40ce7b4..347d168 100644 --- a/crates/core/src/user/permissions.rs +++ b/crates/core/src/user/permissions.rs @@ -14,7 +14,6 @@ use uuid::Uuid; use super::scopes::NRScope; use crate::database::entities::user::{ - UserType, auth_token::AuthToken, permissions::{NewUserRepositoryPermissions, UserRepositoryPermissions}, }; @@ -51,18 +50,6 @@ impl HasPermissions for Option { self.as_ref().and_then(HasPermissions::user_id) } } -pub trait HasUserType { - type UserType: UserType; - - fn user(&self) -> Option<&Self::UserType>; -} -impl HasUserType for Option { - type UserType = HS::UserType; - - fn user(&self) -> Option<&Self::UserType> { - self.as_ref().and_then(HasUserType::user) - } -} pub trait HasPermissions { fn user_id(&self) -> Option; /// Get the permissions of the user. If the user or not logged in, return None diff --git a/crates/core/src/utils.rs b/crates/core/src/utils.rs index dafe0a5..8854b61 100644 --- a/crates/core/src/utils.rs +++ b/crates/core/src/utils.rs @@ -1,4 +1,5 @@ -pub mod time; +// ABOUTME: Provides shared encoding, hashing, duration, and URL utilities. +// ABOUTME: Keeps serialization helpers consistent across core and application crates. pub mod utopia; pub mod base64_utils { use base64::{DecodeError, Engine, engine::general_purpose::STANDARD}; diff --git a/crates/core/src/utils/time.rs b/crates/core/src/utils/time.rs deleted file mode 100644 index c80645c..0000000 --- a/crates/core/src/utils/time.rs +++ /dev/null @@ -1,30 +0,0 @@ -pub mod iso_8601 { - use chrono::{DateTime, FixedOffset}; - use serde::{Deserialize, Serialize}; - - pub static ISO_8601: &str = "%Y-%m-%dT%H:%M:%S.%f"; - pub fn serialize(time: &DateTime, serializer: S) -> Result - where - S: serde::ser::Serializer, - { - to_string(time).serialize(serializer) - } - - pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> - where - D: serde::de::Deserializer<'de>, - { - let s = String::deserialize(deserializer)?; - - DateTime::parse_from_str(&s, ISO_8601).map_err(serde::de::Error::custom) - } - pub fn to_string(time: &DateTime) -> String { - time.format(ISO_8601).to_string() - } - pub fn from_string(s: &str) -> Result, chrono::ParseError> { - DateTime::::parse_from_rfc3339(s) - } - - #[cfg(test)] - mod tests; -} diff --git a/crates/core/src/utils/time/iso_8601/tests.rs b/crates/core/src/utils/time/iso_8601/tests.rs deleted file mode 100644 index c301691..0000000 --- a/crates/core/src/utils/time/iso_8601/tests.rs +++ /dev/null @@ -1,7 +0,0 @@ -#![allow(clippy::expect_used, clippy::panic, clippy::todo, clippy::unwrap_used)] - -#[test] -pub fn test() { - let from = super::from_string("2024-08-28T00:09:11.230Z").unwrap(); - println!("{:?}", from); -} diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index 4e3652a..1c0c797 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -49,7 +49,6 @@ aws-types = "1" aws-smithy-types = { version = "1", features = ["rt-tokio"] } aws-smithy-runtime-api = "1" lru = "0.16" -hex = "0.4" url = { version = "2.4", features = ["serde"] } ipnet.workspace = true aws-smithy-http-client = { version = "1", features = ["rustls-ring"] } diff --git a/crates/storage/src/fs/file_meta.rs b/crates/storage/src/fs/file_meta.rs index 581f902..42f6c56 100644 --- a/crates/storage/src/fs/file_meta.rs +++ b/crates/storage/src/fs/file_meta.rs @@ -1,3 +1,5 @@ +// ABOUTME: Reads, writes, and updates repository metadata sidecar files. +// ABOUTME: Computes content hashes and maintains metadata during local storage changes. use std::{ fs::File, io::{self, BufReader, Read, Write}, @@ -16,10 +18,7 @@ use tracing::{ instrument, trace, warn, }; -use crate::{ - fs::utils::MetadataUtils, local::error::LocalStorageError, meta::RepositoryMeta, - path::PathUtils, -}; +use crate::{local::error::LocalStorageError, meta::RepositoryMeta, path::PathUtils}; use uuid::Uuid; pub static HIDDEN_FILE_EXTENSIONS: &[&str] = &["nr-meta"]; pub static PKGLY_REPO_META_EXTENSION: &str = "nr-meta"; @@ -271,8 +270,16 @@ impl LocationMeta { let (created, modified) = { let file = File::open(path_ref)?; let metadata = file.metadata()?; - let modified = metadata.modified_as_chrono_or_now()?; - let created = metadata.created_as_chrono_or_now()?; + let modified = metadata + .modified() + .ok() + .map(|time| DateTime::::from(time).fixed_offset()) + .unwrap_or_else(|| Local::now().fixed_offset()); + let created = metadata + .created() + .ok() + .map(|time| DateTime::::from(time).fixed_offset()) + .unwrap_or_else(|| Local::now().fixed_offset()); (created, modified) }; let location_meta = if path_ref.is_dir() { diff --git a/crates/storage/src/fs/mod.rs b/crates/storage/src/fs/mod.rs index de5db5f..63121f0 100644 --- a/crates/storage/src/fs/mod.rs +++ b/crates/storage/src/fs/mod.rs @@ -1,3 +1,5 @@ +// ABOUTME: Groups local filesystem storage primitives and metadata helpers. +// ABOUTME: Re-exports the file, stream, path, and content interfaces. mod content; mod file; mod file_meta; @@ -7,7 +9,7 @@ pub(crate) mod utils; pub use content::*; pub use file::*; pub use file_meta::*; -pub use path::{ExtensionError, ParentDirectoryDoesNotExist}; +pub use path::ExtensionError; mod file_reader; pub use file_reader::*; use nr_core::storage::StoragePath; diff --git a/crates/storage/src/fs/path.rs b/crates/storage/src/fs/path.rs index 7967af7..3e3e393 100644 --- a/crates/storage/src/fs/path.rs +++ b/crates/storage/src/fs/path.rs @@ -1,12 +1,10 @@ -use std::path::{Path, PathBuf}; +// ABOUTME: Provides small path transformations used by local storage metadata. +// ABOUTME: Reports non-UTF-8 extensions without hiding filesystem errors. +use std::path::PathBuf; use thiserror::Error; use tracing::instrument; -#[derive(Debug, Error)] -#[error("Parent directory for {0} does not exist")] -pub struct ParentDirectoryDoesNotExist(pub PathBuf); - #[derive(Debug, Error)] pub enum ExtensionError { #[error("The extension of path {0} is not UTF-8")] @@ -14,19 +12,12 @@ pub enum ExtensionError { } pub trait PathUtils { - /// Gets the parent directory of the path or returns an error if it does not exist. - #[allow(unused)] - fn parent_or_err(&self) -> Result<&Path, ParentDirectoryDoesNotExist>; /// Appends an extension to the path. fn add_extension(&self, extension: &str) -> Result; /// Gets the current extension and attempts to convert it to a string. fn extension_to_string(&self) -> Result, ExtensionError>; } impl PathUtils for PathBuf { - fn parent_or_err(&self) -> Result<&Path, ParentDirectoryDoesNotExist> { - self.parent() - .ok_or_else(|| ParentDirectoryDoesNotExist(self.clone())) - } fn extension_to_string(&self) -> Result, ExtensionError> { self.extension() .map(|v| { diff --git a/crates/storage/src/fs/utils.rs b/crates/storage/src/fs/utils.rs index 70ae3a4..e1c9b2b 100644 --- a/crates/storage/src/fs/utils.rs +++ b/crates/storage/src/fs/utils.rs @@ -1,44 +1,9 @@ -use std::{fs::File, io, path::PathBuf}; +// ABOUTME: Provides filesystem MIME detection shared by local storage readers. +// ABOUTME: Keeps path-based content classification in one small utility. +use std::{fs::File, path::PathBuf}; -use chrono::{DateTime, FixedOffset, Local, TimeZone, offset::LocalResult}; use nr_core::storage::SerdeMime; -use tracing::{error, instrument, warn}; - -/// Converts a SystemTime to a DateTime. -/// -/// The offset is based on the local timezone. -/// -/// This function will return an error if the SystemTime is before the Unix Epoch. -/// This should not be possible, but it is handled just in case. -/// -/// If the conversion is ambiguous, the earliest time is used. -pub fn system_time_to_date_time(time: std::time::SystemTime) -> io::Result> { - let time = time - .duration_since(std::time::UNIX_EPOCH) - .map_err(|v| { - io::Error::new( - io::ErrorKind::InvalidData, - format!("SystemTime is before the Unix Epoch: {}", v), - ) - })? - .as_millis(); - // If this program is running when the unix epoch overflows the i64. I will be very impressed. - - match Local.timestamp_millis_opt(time as i64) { - LocalResult::Single(ok) => Ok(ok.fixed_offset()), - LocalResult::Ambiguous(earliest, latest) => { - warn!(earliest= ?earliest, latest = ?latest,"Ambiguous time conversion. Using the earliest time"); - Ok(earliest.fixed_offset()) - } - LocalResult::None => { - error!("Could not convert SystemTime to DateTime. Duration {time}"); - Err(io::Error::new( - io::ErrorKind::InvalidData, - "Could not convert SystemTime to DateTime", - )) - } - } -} +use tracing::instrument; #[instrument] pub fn mime_type_for_file(file: &File, path: PathBuf) -> Option { @@ -48,39 +13,3 @@ pub fn mime_type_for_file(file: &File, path: PathBuf) -> Option { let mime = mime_guess::from_path(&path).first_or_octet_stream(); Some(SerdeMime(mime)) } - -pub trait MetadataUtils { - /// Get the creation time of the file as a DateTime. - fn created_as_chrono(&self) -> Result>, io::Error>; - - fn created_as_chrono_or_now(&self) -> Result, io::Error> { - let time = self - .created_as_chrono()? - .unwrap_or_else(|| Local::now().into()); - Ok(time) - } - - /// Get the modification time of the file as a DateTime. - fn modified_as_chrono(&self) -> Result>, io::Error>; - - fn modified_as_chrono_or_now(&self) -> Result, io::Error> { - let time = self - .modified_as_chrono()? - .unwrap_or_else(|| Local::now().into()); - Ok(time) - } -} -impl MetadataUtils for std::fs::Metadata { - fn created_as_chrono(&self) -> Result>, io::Error> { - self.created() - .ok() - .map(system_time_to_date_time) - .transpose() - } - fn modified_as_chrono(&self) -> Result>, io::Error> { - self.modified() - .ok() - .map(system_time_to_date_time) - .transpose() - } -} diff --git a/crates/storage/src/local/error.rs b/crates/storage/src/local/error.rs index e56893c..e594322 100644 --- a/crates/storage/src/local/error.rs +++ b/crates/storage/src/local/error.rs @@ -1,4 +1,6 @@ -use super::{ExtensionError, ParentDirectoryDoesNotExist, PathCollisionError}; +// ABOUTME: Defines errors raised by local filesystem storage operations. +// ABOUTME: Converts path, metadata, and serialization failures into one type. +use super::{ExtensionError, PathCollisionError}; use crate::error::WrongFileType; use nr_core::storage::InvalidStoragePath; @@ -9,8 +11,6 @@ pub enum LocalStorageError { #[error(transparent)] ExtensionError(#[from] ExtensionError), #[error(transparent)] - ParentDirectoryDoesNotExist(#[from] ParentDirectoryDoesNotExist), - #[error(transparent)] PathCollision(#[from] PathCollisionError), #[error("Metadata Error {0}")] Postcard(#[from] postcard::Error), diff --git a/crates/storage/src/s3/mod.rs b/crates/storage/src/s3/mod.rs index 52d27b6..443cd87 100644 --- a/crates/storage/src/s3/mod.rs +++ b/crates/storage/src/s3/mod.rs @@ -27,7 +27,6 @@ use aws_types::{SdkConfig, region::Region}; use bytes::{Bytes, BytesMut}; use chrono::{DateTime as ChronoDateTime, FixedOffset, Local, Utc}; use futures::future::BoxFuture; -use hex::encode; use lru::LruCache; use mime::Mime; use nr_core::storage::{FileHashes, FileTypeCheck, SerdeMime, StoragePath}; @@ -818,7 +817,7 @@ impl S3DiskCache { fn hashed_filename(key: &str) -> PathBuf { let digest = Sha256::digest(key.as_bytes()); - let hex = encode(digest); + let hex = format!("{digest:x}"); let (prefix, rest) = hex.split_at(2); PathBuf::from(prefix).join(rest) } @@ -909,7 +908,7 @@ impl S3DiskCache { size = size.saturating_add(read as u64); hasher.update(&buffer[..read]); } - size == expected_size && encode(hasher.finalize()) == expected_digest + size == expected_size && format!("{:x}", hasher.finalize()) == expected_digest } async fn recover_entries( @@ -1209,7 +1208,7 @@ impl S3DiskCache { } let temp_path = path.with_extension(format!("tmp-{}", Uuid::new_v4().simple())); fs::write(&temp_path, data.as_ref()).await?; - let digest = hex::encode(Sha256::digest(data.as_ref())); + let digest = format!("{:x}", Sha256::digest(data.as_ref())); let cached_at_ms = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default() diff --git a/docker/config.dev.toml b/docker/config.dev.toml index c1ec69a..3ec8f6d 100644 --- a/docker/config.dev.toml +++ b/docker/config.dev.toml @@ -14,7 +14,6 @@ cleanup_interval = 3600 database_location = "/data/sessions.redb" [security] -allow_basic_without_tokens = true [security.password_rules] min_length = 12 diff --git a/docker/config.toml b/docker/config.toml index 6ddc9a8..85e5fe0 100644 --- a/docker/config.toml +++ b/docker/config.toml @@ -14,7 +14,6 @@ cleanup_interval = 3600 database_location = "/data/sessions.redb" [security] -allow_basic_without_tokens = true [security.password_rules] min_length = 12 diff --git a/docs/docs/sso/index.md b/docs/docs/sso/index.md index 5b132b5..4f2434a 100644 --- a/docs/docs/sso/index.md +++ b/docs/docs/sso/index.md @@ -148,7 +148,6 @@ Configure Pkgly to validate the Cloudflare Access token via JWKS: ```toml [security] -allow_basic_without_tokens = false [security.sso] enabled = true @@ -285,4 +284,3 @@ server { } } ``` - diff --git a/examples/config.toml b/examples/config.toml index 1e164f4..36cfe27 100644 --- a/examples/config.toml +++ b/examples/config.toml @@ -58,7 +58,6 @@ is_https = true # Security Configuration # ============================================================================ [security] -allow_basic_without_tokens = true # Outbound requests may use public addresses by default. Add exact hostnames # or CIDRs only for private proxy, webhook, OIDC, or S3 destinations. diff --git a/pkgly/Cargo.toml b/pkgly/Cargo.toml index dfe2382..1832cae 100644 --- a/pkgly/Cargo.toml +++ b/pkgly/Cargo.toml @@ -66,7 +66,6 @@ multer = "3.1" uuid.workspace = true flume = "0.11" sha2.workspace = true -sha2_0_11 = { package = "sha2", version = "0.11" } sha1.workspace = true sha3.workspace = true md-5.workspace = true @@ -151,4 +150,3 @@ zip = { version = "5", default-features = false, features = ["deflate-flate2-zli walkdir = "2" anyhow = "1.0" -camino = "1.1" diff --git a/pkgly/build.rs b/pkgly/build.rs index 0322e4b..c7dabb2 100644 --- a/pkgly/build.rs +++ b/pkgly/build.rs @@ -162,20 +162,21 @@ where for entry in it { let absolute_path = entry.path(); let stripped_path = entry.path().strip_prefix(prefix)?; - let name = camino::Utf8Path::from_path(stripped_path) + let name = stripped_path + .to_str() .with_context(|| format!("{stripped_path:?} Could not be converted to UTF-8"))?; // Write file or directory explicitly // Some unzip tools unzip files with directory paths correctly, some do not! if absolute_path.is_file() { - zip.start_file(name.as_str(), options)?; + zip.start_file(name, options)?; let mut f = File::open(absolute_path)?; f.read_to_end(&mut buffer)?; zip.write_all(&buffer)?; buffer.clear(); - } else if !name.as_str().is_empty() { - zip.add_directory(name.to_string(), options)?; + } else if !name.is_empty() { + zip.add_directory(name, options)?; } } zip.finish()?; diff --git a/pkgly/src/app/api/repository/packages.rs b/pkgly/src/app/api/repository/packages.rs index c3f795d..409a356 100644 --- a/pkgly/src/app/api/repository/packages.rs +++ b/pkgly/src/app/api/repository/packages.rs @@ -7,7 +7,6 @@ use std::{ collections::BinaryHeap, }; -use futures::future::BoxFuture; #[cfg(test)] use futures::{StreamExt, stream}; @@ -492,33 +491,6 @@ enum CatalogDeletionMode { StripLastSegment, } -#[cfg_attr(test, mockall::automock)] -trait CatalogDeletionExecutor { - fn delete_paths<'a>( - &'a self, - repository_id: Uuid, - normalized_paths: Vec, - ) -> BoxFuture<'a, Result>; -} - -struct SqlCatalogDeletionExecutor<'a> { - database: &'a PgPool, -} - -impl<'a> CatalogDeletionExecutor for SqlCatalogDeletionExecutor<'a> { - fn delete_paths<'b>( - &'b self, - repository_id: Uuid, - normalized_paths: Vec, - ) -> BoxFuture<'b, Result> { - Box::pin(sql_delete_project_versions( - self.database, - repository_id, - normalized_paths, - )) - } -} - fn package_strategy(repository: &DynRepository) -> PackageStrategy { match repository { DynRepository::Maven(maven_repo) => match maven_repo { @@ -622,27 +594,29 @@ fn normalize_catalog_path(path: &str) -> Option { Some(trimmed.trim_end_matches('/').to_lowercase()) } -async fn delete_version_records_by_path( - executor: &E, - repository_id: Uuid, - version_paths: &HashSet, -) -> Result { - if version_paths.is_empty() { - return Ok(0); - } +fn normalize_catalog_paths(version_paths: &HashSet) -> Vec { let mut normalized = Vec::with_capacity(version_paths.len()); for path in version_paths { if let Some(value) = normalize_catalog_path(path) { normalized.push(value); } } + normalized.sort(); + normalized.dedup(); + normalized +} + +async fn delete_version_records_by_path( + database: &PgPool, + repository_id: Uuid, + version_paths: &HashSet, +) -> Result { + let normalized = normalize_catalog_paths(version_paths); if normalized.is_empty() { return Ok(0); } - normalized.sort(); - normalized.dedup(); - executor.delete_paths(repository_id, normalized).await + sql_delete_project_versions(database, repository_id, normalized).await } async fn sql_delete_project_versions( @@ -2405,10 +2379,7 @@ pub async fn delete_cached_package_paths( } if catalog_mode != CatalogDeletionMode::None && !catalog_targets.is_empty() { - let executor = SqlCatalogDeletionExecutor { - database: &site.database, - }; - delete_version_records_by_path(&executor, repository.id(), &catalog_targets) + delete_version_records_by_path(&site.database, repository.id(), &catalog_targets) .await .map_err(|err| InternalError::from(OtherInternalError::new(err)))?; } diff --git a/pkgly/src/app/api/repository/packages/tests.rs b/pkgly/src/app/api/repository/packages/tests.rs index f29afc1..4b49beb 100644 --- a/pkgly/src/app/api/repository/packages/tests.rs +++ b/pkgly/src/app/api/repository/packages/tests.rs @@ -1331,39 +1331,24 @@ async fn load_php_version_entries_uses_proxy_metadata_for_cached_dist() -> Resul Ok(()) } -#[tokio::test] -async fn delete_version_records_by_path_normalizes_and_deletes() { - let repository_id = Uuid::new_v4(); +#[test] +fn normalize_catalog_paths_normalizes_and_deduplicates() { let mut targets = ahash::HashSet::new(); targets.insert("Crates/Demo/1.0.0/".to_string()); targets.insert("crates/demo/1.0.0".to_string()); targets.insert(" ".to_string()); - let mut mock = super::MockCatalogDeletionExecutor::new(); - mock.expect_delete_paths() - .times(1) - .withf(move |repo, paths| { - repo == &repository_id && paths == &vec!["crates/demo/1.0.0".to_string()] - }) - .returning(|_, _| Box::pin(async { Ok(1) })); - - let deleted = super::delete_version_records_by_path(&mock, repository_id, &targets) - .await - .expect("deletion succeeds"); - assert_eq!(deleted, 1); + assert_eq!( + super::normalize_catalog_paths(&targets), + vec!["crates/demo/1.0.0".to_string()] + ); } -#[tokio::test] -async fn delete_version_records_by_path_skips_executor_when_empty() { - let repository_id = Uuid::new_v4(); - let mut mock = super::MockCatalogDeletionExecutor::new(); - mock.expect_delete_paths().never(); +#[test] +fn normalize_catalog_paths_skips_empty_paths() { let targets: ahash::HashSet = ahash::HashSet::new(); - let deleted = super::delete_version_records_by_path(&mock, repository_id, &targets) - .await - .expect("skip is ok"); - assert_eq!(deleted, 0); + assert!(super::normalize_catalog_paths(&targets).is_empty()); } mod catalog_db_tests { diff --git a/pkgly/src/app/api/user/password_reset.rs b/pkgly/src/app/api/user/password_reset.rs index ef9edab..6f14814 100644 --- a/pkgly/src/app/api/user/password_reset.rs +++ b/pkgly/src/app/api/user/password_reset.rs @@ -1,3 +1,5 @@ +// ABOUTME: Handles password reset requests, token checks, and password changes. +// ABOUTME: Builds trusted reset links and queues reset notifications. use std::{io, net::SocketAddr, str::FromStr}; use axum::{ @@ -19,12 +21,7 @@ use url::Url; use utoipa::ToSchema; use crate::{ - app::{ - Pkgly, - authentication::password, - config::normalize_app_url, - email_service::{Email, EmailDebug, template}, - }, + app::{Pkgly, authentication::password, config::normalize_app_url, email_service::EmailDebug}, error::{InternalError, OtherInternalError}, utils::{ResponseBuilder, request_logging::access_log::AccessLogContext}, }; @@ -50,6 +47,16 @@ pub struct PasswordResetEmail { pub required: bool, } +const PASSWORD_RESET_TEMPLATE_HTML: &str = "password_reset.html"; +const PASSWORD_RESET_TEMPLATE_TXT: &str = "password_reset.txt"; + +fn password_reset_debug_info(username: &str) -> EmailDebug { + EmailDebug { + to: username.to_owned(), + subject: "Password Reset", + } +} + fn build_reset_url(panel_url: &str, token: &str) -> Result { let mut reset_url = Url::parse(panel_url).map_err(OtherInternalError::new)?; { @@ -62,20 +69,6 @@ fn build_reset_url(panel_url: &str, token: &str) -> Result &'static str { - "Password Reset" - } - - fn debug_info(self) -> EmailDebug { - EmailDebug { - to: self.username, - subject: Self::subject(), - } - } -} #[utoipa::path( post, path = "/password-reset/request", @@ -127,7 +120,14 @@ async fn request_password_reset( username: user.username.into(), required: false, }; - site.email_access.send_one_fn(address, email) + let username = email.username.clone(); + site.email_access.send_one( + address, + &email, + PASSWORD_RESET_TEMPLATE_TXT, + PASSWORD_RESET_TEMPLATE_HTML, + || password_reset_debug_info(&username), + ) } Ok(ResponseBuilder::ok().empty()) } diff --git a/pkgly/src/app/api/user/password_reset/tests.rs b/pkgly/src/app/api/user/password_reset/tests.rs index 52f0ff6..4c75b4b 100644 --- a/pkgly/src/app/api/user/password_reset/tests.rs +++ b/pkgly/src/app/api/user/password_reset/tests.rs @@ -25,3 +25,11 @@ fn reset_url_encodes_token_and_preserves_panel_path() { "a+/=?&" ); } + +#[test] +fn password_reset_debug_info_contains_recipient_and_subject() { + let debug = password_reset_debug_info("alice"); + + assert_eq!(debug.to, "alice"); + assert_eq!(debug.subject, "Password Reset"); +} diff --git a/pkgly/src/app/authentication/session.rs b/pkgly/src/app/authentication/session.rs index e68cd94..4da10cd 100644 --- a/pkgly/src/app/authentication/session.rs +++ b/pkgly/src/app/authentication/session.rs @@ -1,3 +1,5 @@ +// ABOUTME: Manages persisted browser sessions and their cleanup lifecycle. +// ABOUTME: Provides session creation, lookup, deletion, and expiration handling. use std::{ fmt::Debug, fs, io, @@ -31,8 +33,6 @@ use crate::{ utils::{IntoErrorResponse, ResponseBuilder}, }; -mod storage; -pub use storage::SessionStorage; #[derive(Debug, Error)] pub enum SessionError { #[error("Session not found")] diff --git a/pkgly/src/app/authentication/session/storage.rs b/pkgly/src/app/authentication/session/storage.rs deleted file mode 100644 index 35ce5e8..0000000 --- a/pkgly/src/app/authentication/session/storage.rs +++ /dev/null @@ -1,85 +0,0 @@ -use chrono::Duration; - -use super::{Session, SessionError, SessionManager}; - -/// Abstract session storage backend. -/// -/// This trait allows `SessionManager` to work with different storage -/// implementations (e.g. Redb, in-memory, or external services) without -/// changing its public API. -pub trait SessionStorage { - /// Create a new session with a custom lifetime. - fn create_session( - &self, - user_id: i32, - user_agent: String, - ip_address: String, - life: Duration, - ) -> Result; - - /// Get a session by id. - fn get_session(&self, session_id: &str) -> Result, SessionError>; - - /// Delete a session by id and return it if it existed. - fn delete_session(&self, session_id: &str) -> Result, SessionError>; - - /// Delete all sessions for the given user and return the number removed. - fn delete_sessions_for_user(&self, user_id: i32) -> Result; - - /// Return the number of active sessions. - fn number_of_sessions(&self) -> Result; -} - -#[cfg(test)] -mod tests { - use super::{SessionManager, SessionStorage}; - use crate::app::{authentication::session::SessionManagerConfig, config::Mode}; - use chrono::Duration; - use tempfile::tempdir; - - #[test] - fn session_manager_implements_session_storage_trait() { - let tmp_dir = tempdir().expect("create temp dir"); - let db_path = tmp_dir.path().join("sessions.redb"); - let config = SessionManagerConfig { - lifespan: Duration::seconds(60), - cleanup_interval: Duration::seconds(60), - database_location: db_path, - }; - - let manager = - SessionManager::new(config, Mode::Debug).expect("session manager should build"); - - // This is a compile-time check that SessionManager implements SessionStorage. - fn assert_storage(_value: &T) {} - assert_storage(&manager); - } -} - -impl SessionStorage for SessionManager { - fn create_session( - &self, - user_id: i32, - user_agent: String, - ip_address: String, - life: Duration, - ) -> Result { - SessionManager::create_session(self, user_id, user_agent, ip_address, life) - } - - fn get_session(&self, session_id: &str) -> Result, SessionError> { - SessionManager::get_session(self, session_id) - } - - fn delete_session(&self, session_id: &str) -> Result, SessionError> { - SessionManager::delete_session(self, session_id) - } - - fn delete_sessions_for_user(&self, user_id: i32) -> Result { - SessionManager::delete_sessions_for_user(self, user_id) - } - - fn number_of_sessions(&self) -> Result { - SessionManager::number_of_sessions(self) - } -} diff --git a/pkgly/src/app/authentication/ws.rs b/pkgly/src/app/authentication/ws.rs index 1d36445..55195f7 100644 --- a/pkgly/src/app/authentication/ws.rs +++ b/pkgly/src/app/authentication/ws.rs @@ -1,6 +1,8 @@ +// ABOUTME: Authenticates WebSocket clients with sessions or bearer tokens. +// ABOUTME: Exposes authenticated user identity and permission access for sockets. use nr_core::{ database::entities::user::{UserSafeData, UserType, auth_token::AuthToken}, - user::permissions::{HasPermissions, HasUserType, UserPermissions}, + user::permissions::{HasPermissions, UserPermissions}, }; use serde::{Deserialize, Serialize}; use tracing::{Span, debug, instrument}; @@ -65,6 +67,13 @@ pub enum WebSocketAuthentication { user: UserSafeData, }, } +impl WebSocketAuthentication { + pub fn user(&self) -> &UserSafeData { + match self { + Self::AuthToken { user, .. } | Self::Session { user, .. } => user, + } + } +} impl HasPermissions for WebSocketAuthentication { fn user_id(&self) -> Option { match self { @@ -80,13 +89,3 @@ impl HasPermissions for WebSocketAuthentication { } } } -impl HasUserType for WebSocketAuthentication { - type UserType = UserSafeData; - - fn user(&self) -> Option<&Self::UserType> { - match self { - WebSocketAuthentication::AuthToken { user, .. } - | WebSocketAuthentication::Session { user, .. } => Some(user), - } - } -} diff --git a/pkgly/src/app/config/security.rs b/pkgly/src/app/config/security.rs index adaf3f7..328501f 100644 --- a/pkgly/src/app/config/security.rs +++ b/pkgly/src/app/config/security.rs @@ -1,3 +1,5 @@ +// ABOUTME: Defines security, password, SSO, OAuth, and egress configuration. +// ABOUTME: Supplies safe defaults and validation for authentication settings. use std::{fmt, path::PathBuf, str::FromStr}; use serde::{Deserialize, Serialize}; @@ -8,7 +10,6 @@ const DEFAULT_CASBIN_POLICY: &str = include_str!("../../../resources/rbac/policy #[derive(Debug, Deserialize, Serialize, Clone)] pub struct SecuritySettings { - pub allow_basic_without_tokens: bool, pub password_rules: Option, pub sso: Option, pub oauth2: Option, @@ -18,7 +19,6 @@ pub struct SecuritySettings { impl Default for SecuritySettings { fn default() -> Self { Self { - allow_basic_without_tokens: false, password_rules: Some(PasswordRules::default()), sso: None, oauth2: None, diff --git a/pkgly/src/app/email_service.rs b/pkgly/src/app/email_service.rs index 2098fcb..341bc9b 100644 --- a/pkgly/src/app/email_service.rs +++ b/pkgly/src/app/email_service.rs @@ -46,29 +46,7 @@ impl EmailRequest { } } } -macro_rules! template { - ($template:expr) => { - fn template_html() -> &'static str { - concat!($template, ".html") - } - fn template_txt() -> &'static str { - concat!($template, ".txt") - } - }; -} -pub(crate) use template; - use super::email::{EmailEncryption, EmailSetting}; -pub trait Email: Serialize + Debug { - /// template().html and template().txt must exist in the resources/emails folder - fn template_html() -> &'static str; - - fn template_txt() -> &'static str; - - fn subject() -> &'static str; - - fn debug_info(self) -> EmailDebug; -} #[derive(Debug)] pub struct EmailAccess { @@ -91,10 +69,15 @@ impl EmailAccess { }; } #[inline] - #[instrument()] - pub fn build_body(&self, data: &E) -> MultiPart { + #[instrument(skip(data))] + fn build_body( + &self, + data: &S, + template_txt: &str, + template_html: &str, + ) -> MultiPart { let multipart = MultiPart::alternative(); - let mut multipart = match self.email_handlebars.render(E::template_txt(), &data) { + let mut multipart = match self.email_handlebars.render(template_txt, data) { Ok(ok) => multipart.singlepart( SinglePart::builder() .header(header::ContentType::TEXT_PLAIN) @@ -105,7 +88,7 @@ impl EmailAccess { multipart.build() } }; - match self.email_handlebars.render(E::template_html(), &data) { + match self.email_handlebars.render(template_html, data) { Ok(ok) => { multipart = multipart.singlepart( SinglePart::builder() @@ -125,9 +108,16 @@ impl EmailAccess { pub fn prep_builder(&self) -> MessageBuilder { self.message_builder.clone() } - #[instrument()] - pub fn send_one_fn(&self, to: Address, data: impl Email) { - let body = self.build_body(&data); + #[instrument(skip(data, debug_info))] + pub fn send_one EmailDebug>( + &self, + to: Address, + data: &S, + template_txt: &str, + template_html: &str, + debug_info: F, + ) { + let body = self.build_body(data, template_txt, template_html); let message = match self.prep_builder().to(to.into()).multipart(body) { Ok(ok) => ok, @@ -137,7 +127,7 @@ impl EmailAccess { } }; let debug = if log_enabled!(tracing::log::Level::Debug) { - Some(data.debug_info()) + Some(debug_info()) } else { None }; diff --git a/pkgly/src/repository/proxy/base_proxy.rs b/pkgly/src/repository/proxy/base_proxy.rs index 7fcd207..c4e1989 100644 --- a/pkgly/src/repository/proxy/base_proxy.rs +++ b/pkgly/src/repository/proxy/base_proxy.rs @@ -1,3 +1,5 @@ +// ABOUTME: Provides shared cache-hit and eviction helpers for proxy repositories. +// ABOUTME: Delegates catalog updates to the common proxy indexer. //! Common proxy utilities and traits for proxy repositories. //! //! Concrete proxy implementations (Docker, Go, Maven, NPM, Python, etc.) @@ -14,16 +16,6 @@ use nr_core::repository::project::{ProxyArtifactKey, ProxyArtifactMeta}; use crate::repository::proxy_indexing::{ProxyIndexing, ProxyIndexingError}; -/// Marker trait for proxy repositories. -/// -/// Implemented by format-specific proxy repository types such as: -/// - `go::proxy::GoProxy` -/// - `maven::proxy::MavenProxy` -/// - `npm::proxy::NpmProxyRegistry` -/// - `python::proxy::PythonProxy` -/// - `docker::proxy::DockerProxy` -pub trait ProxyRepository {} - /// Record a cached proxy artifact if metadata is available. /// /// This helper encapsulates the common pattern: @@ -54,15 +46,5 @@ pub async fn evict_proxy_cache_entry( Ok(()) } -// Implement the marker trait for the known proxy repository types. This keeps -// behavior unchanged while giving the type system a way to talk about "any -// proxy repository" when needed. -impl ProxyRepository for crate::repository::go::proxy::GoProxy {} -impl ProxyRepository for crate::repository::maven::proxy::MavenProxy {} -impl ProxyRepository for crate::repository::npm::proxy::NpmProxyRegistry {} -impl ProxyRepository for crate::repository::python::proxy::PythonProxy {} -impl ProxyRepository for crate::repository::docker::proxy::DockerProxy {} -impl ProxyRepository for crate::repository::php::proxy::PhpProxy {} - #[cfg(test)] mod tests; diff --git a/pkgly/src/repository/proxy/base_proxy/tests.rs b/pkgly/src/repository/proxy/base_proxy/tests.rs index e825734..b6b47c1 100644 --- a/pkgly/src/repository/proxy/base_proxy/tests.rs +++ b/pkgly/src/repository/proxy/base_proxy/tests.rs @@ -1,3 +1,5 @@ +// ABOUTME: Tests proxy cache helper behavior with an in-memory recording indexer. +// ABOUTME: Verifies optional metadata and eviction keys are dispatched correctly. use std::sync::Arc; use async_trait::async_trait; @@ -5,27 +7,10 @@ use nr_core::repository::project::{ProxyArtifactKey, ProxyArtifactMeta}; use tokio::sync::Mutex; use crate::repository::{ - docker::proxy::DockerProxy, - go::proxy::GoProxy, - maven::proxy::MavenProxy, - npm::proxy::NpmProxyRegistry, - proxy::base_proxy::{ProxyRepository, evict_proxy_cache_entry, record_proxy_cache_hit}, + proxy::base_proxy::{evict_proxy_cache_entry, record_proxy_cache_hit}, proxy_indexing::{ProxyIndexing, ProxyIndexingError}, - python::proxy::PythonProxy, }; -// Compile-time assertion that the main proxy types implement the marker trait. -fn assert_is_proxy() {} - -#[test] -fn proxy_repositories_implement_marker_trait() { - assert_is_proxy::(); - assert_is_proxy::(); - assert_is_proxy::(); - assert_is_proxy::(); - assert_is_proxy::(); -} - #[derive(Clone, Default)] struct RecordingIndexer { recorded: Arc>>, diff --git a/pkgly/src/repository/proxy/mod.rs b/pkgly/src/repository/proxy/mod.rs index 6bd6f1c..ee49808 100644 --- a/pkgly/src/repository/proxy/mod.rs +++ b/pkgly/src/repository/proxy/mod.rs @@ -1,3 +1,5 @@ +// ABOUTME: Groups proxy repository modules and their shared utilities. +// ABOUTME: Re-exports format-specific proxy implementations for callers. //! Grouping module for proxy repository implementations. //! //! This module provides a single place to find proxy-related types while @@ -5,8 +7,6 @@ pub mod base_proxy; -pub use base_proxy::ProxyRepository; - // Re-export the main proxy repository types so callers can opt into // format-specific proxies from a single place without depending on // the individual format modules directly. diff --git a/pkgly/src/repository/staging.rs b/pkgly/src/repository/staging.rs index 88b7790..270c99b 100644 --- a/pkgly/src/repository/staging.rs +++ b/pkgly/src/repository/staging.rs @@ -1,27 +1,9 @@ -use std::{env, fmt::Debug, path::PathBuf, sync::Arc}; +// ABOUTME: Defines staging directory configuration for incoming artifacts. +// ABOUTME: Supplies the default location and cleanup interval. +use std::{env, path::PathBuf}; -use axum::response::IntoResponse; use chrono::Duration; -use derive_more::derive::Deref; use serde::{Deserialize, Serialize}; -use thiserror::Error; -use tracing::error; -use uuid::Uuid; - -#[derive(Debug, Error)] -pub enum StagingManagerError { - #[error("Database Error")] - DBError(#[from] sqlx::Error), - #[error("IO Error")] - IOError(#[from] std::io::Error), -} -impl IntoResponse for StagingManagerError { - fn into_response(self) -> axum::response::Response { - error!("{}", self); - let message = format!("Staging Manager Error {:?}. ", self); - crate::utils::ResponseBuilder::internal_server_error().body(message) - } -} /// Stages are stored locally before being moved to the storage #[derive(Debug, Deserialize, Serialize, Clone)] pub struct StagingConfig { @@ -43,16 +25,3 @@ fn default_staging_directory() -> PathBuf { .unwrap_or_else(|_| PathBuf::new()) .join("staging") } -pub struct StagingManagerInner { - repository: Uuid, -} -#[derive(Deref, Clone)] -pub struct StagingManager(Arc); - -impl Debug for StagingManager { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("StagingManager") - .field("repository_id", &self.repository) - .finish() - } -} diff --git a/pkgly/src/utils/header.rs b/pkgly/src/utils/header.rs index ecf53cb..1b7fc14 100644 --- a/pkgly/src/utils/header.rs +++ b/pkgly/src/utils/header.rs @@ -1,12 +1,12 @@ +// ABOUTME: Provides typed accessors for HTTP header values and maps. +// ABOUTME: Preserves empty-header handling while reducing duplicate conversions. use http::{HeaderName, HeaderValue, header::ToStrError}; -use tracing::{error, warn}; +use tracing::warn; pub mod date_time; /// Extension trait for [http::HeaderValue] pub trait HeaderValueExt { /// Converts the header value to a string fn to_string(&self) -> Result; - /// Converts the header value to a string - fn to_string_as_option(&self) -> Option; /// Parses the header value into a type Over the [TryFrom] trait /// /// Error must be convertible from [ToStrError] @@ -20,15 +20,6 @@ impl HeaderValueExt for HeaderValue { self.to_str().map(|x| x.to_string()) } - fn to_string_as_option(&self) -> Option { - self.to_str() - .map(|x| x.to_string()) - .inspect_err(|error| { - error!("Failed to convert header value to string: {}", error); - }) - .ok() - } - fn parsed(&self) -> Result where T: TryFrom, @@ -46,16 +37,7 @@ pub trait HeaderMapExt { impl HeaderMapExt for http::HeaderMap { fn get_string_ignore_empty(&self, header: &HeaderName) -> Option { - self.get(header) - .and_then(|v| v.to_str().ok()) - .and_then(|v| { - if v.is_empty() { - warn!(?header, "Empty header Value",); - None - } else { - Some(v.to_owned()) - } - }) + self.get_str_ignore_empty(header).map(str::to_owned) } fn get_str_ignore_empty<'headers>(&'headers self, key: &HeaderName) -> Option<&'headers str> { diff --git a/tests/docker/config/pkgly.test.toml b/tests/docker/config/pkgly.test.toml index e14c235..6060fd0 100644 --- a/tests/docker/config/pkgly.test.toml +++ b/tests/docker/config/pkgly.test.toml @@ -30,7 +30,6 @@ encryption = "NONE" from = "pkgly@test.local" [security] -allow_basic_without_tokens = false [security.egress] # Proxy fixtures intentionally point back to the in-stack Pkgly service.