diff --git a/.github/agents/committer.agent.md b/.github/agents/committer.agent.md index a497f834a..27a7d7582 100644 --- a/.github/agents/committer.agent.md +++ b/.github/agents/committer.agent.md @@ -22,6 +22,10 @@ Treat every commit request as a review-and-verify workflow, not as a blind reque and retry with `./contrib/dev-tools/git/hooks/pre-commit.sh --format=text --verbosity=verbose` when deeper diagnostics are needed. - Create GPG-signed Conventional Commits (`git commit -S`). + **GPG timeout handling**: If a `git commit -S` invocation fails because the GPG passphrase + prompt timed out, stop immediately and notify the user. Do not retry with `--no-gpg-sign`, + do not amend the commit without a signature, and do not proceed with the work. The user + must manually retry the commit to provide the passphrase. ## Required Workflow diff --git a/.github/skills/dev/git-workflow/commit-changes/SKILL.md b/.github/skills/dev/git-workflow/commit-changes/SKILL.md index 49e3c975c..e76609e30 100644 --- a/.github/skills/dev/git-workflow/commit-changes/SKILL.md +++ b/.github/skills/dev/git-workflow/commit-changes/SKILL.md @@ -62,6 +62,17 @@ Scope should reflect the affected package or area (e.g., `tracker-core`, `udp-pr git commit -S -m "your commit message" ``` +### GPG Timeout Handling + +If the GPG passphrase prompt times out (`gpg: signing failed: Timeout`), the agent **must**: + +1. **Stop immediately.** Do not retry, do not use `--no-gpg-sign`, do not skip signing. +2. **Notify the user.** Tell them the GPG passphrase prompt timed out and they need to retry + the commit manually by running the same `git commit -S` command themselves. +3. **Wait for the user** to confirm the commit was completed before proceeding. + +This rule is absolute. Never bypass GPG signing for any reason. + ## Pre-commit Verification (MANDATORY) ### Git Hook diff --git a/AGENTS.md b/AGENTS.md index cafb1968e..f9f770fab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -269,21 +269,23 @@ Implementation workflow references: ## πŸ”§ Essential Rules 1. **Linting gate**: `linter all` must exit `0` before every commit. No exceptions. -2. **GPG commit signing**: All commits **must** be signed with GPG (`git commit -S`). -3. **Never commit `storage/` or `target/`**: These directories contain runtime data and build +2. **GPG commit signing**: All commits **must** be signed with GPG (`git commit -S`). **GPG timeout handling**: If the GPG passphrase prompt times out during a commit, the agent + **must stop immediately**, notify the user, and ask them to retry the commit manually. + Never bypass GPG signing with `--no-gpg-sign` or skip the signing step under any + circumstances. This rule is absolute.3. **Never commit `storage/` or `target/`**: These directories contain runtime data and build artifacts. They are git-ignored; never force-add them. -4. **Unused dependencies**: Run `cargo machete` before committing. Remove any unused +3. **Unused dependencies**: Run `cargo machete` before committing. Remove any unused dependencies immediately. -5. **Rust imports**: All imports at the top of the file, grouped (std β†’ external crates β†’ +4. **Rust imports**: All imports at the top of the file, grouped (std β†’ external crates β†’ internal crate). Prefer short imported names over fully-qualified paths. -6. **Continuous self-review**: Review your own work against project quality standards. Apply +5. **Continuous self-review**: Review your own work against project quality standards. Apply self-review at three levels: - **Mandatory** β€” before opening a pull request - **Strongly recommended** β€” before each commit - **Recommended** β€” after completing each small, independent, deployable change -7. **Security**: Do not report security vulnerabilities through public GitHub issues. Send an +6. **Security**: Do not report security vulnerabilities through public GitHub issues. Send an email to `info@nautilus-cyberneering.de` instead. See [SECURITY.md](SECURITY.md). -8. **Skill-link synchronization**: When modifying any artifact containing a `skill-link:` marker, +7. **Skill-link synchronization**: When modifying any artifact containing a `skill-link:` marker, also review and update the linked skill instructions in `.github/skills/` so behavior, commands, and references remain aligned. If the linked skill has a validation script, run it before finishing. diff --git a/Cargo.lock b/Cargo.lock index 8715e39f1..c8a70d316 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4962,7 +4962,6 @@ dependencies = [ "reqwest", "serde", "serde_json", - "serde_with", "thiserror 2.0.18", "tokio", "torrust-clock", @@ -5250,6 +5249,7 @@ name = "torrust-tracker-rest-api-protocol" version = "3.0.0-develop" dependencies = [ "serde", + "serde_with", ] [[package]] diff --git a/docs/issues/open/1941-1938-si-3-migrate-auth-key-context.md b/docs/issues/open/1941-1938-si-3-migrate-auth-key-context.md index 54f9b185e..5b530eb7c 100644 --- a/docs/issues/open/1941-1938-si-3-migrate-auth-key-context.md +++ b/docs/issues/open/1941-1938-si-3-migrate-auth-key-context.md @@ -55,7 +55,7 @@ The context has locally-defined DTOs (`AuthKey`, `AddKeyForm`, `KeyParam`) and 7 - Move `AuthKey`, `AddKeyForm`, `KeyParam` DTOs to `rest-api-protocol/src/v1/context/auth_key/resources/auth_key.rs`. - Add auth-key-specific response/error DTOs to protocol (or reuse `ActionStatus` where applicable). -- Define `AuthKeyCommandPort` trait in `rest-api-application/src/ports/`. +- Define `AuthKeyPort` trait in `rest-api-application/src/ports/`. - Implement `AuthKeyApiService` use-case in `rest-api-application/src/use_cases/`. - Implement `TrackerAuthKeyAdapter` in `rest-api-runtime-adapter/src/adapters/`. - Add conversion functions for domainβ†’protocol types. @@ -71,14 +71,18 @@ The context has locally-defined DTOs (`AuthKey`, `AddKeyForm`, `KeyParam`) and 7 The auth key context has both command and query operations, and includes form validation (duration parsing via `clock`). The 7 response functions produce 4 distinct error types plus a success response. Some can be consolidated into protocol-level error codes. -All protocol DTOs follow the normalized context-based module structure under `packages/rest-api-protocol/src/v1/context/`: +All protocol types follow the normalized context-based module structure under `packages/rest-api-protocol/src/v1/context/`. +Each context can have a `forms/` subdirectory alongside `resources/` for input DTOs: ```text context/auth_key/ -β”œβ”€β”€ mod.rs # pub mod resources; +β”œβ”€β”€ mod.rs # pub mod forms; pub mod resources; +β”œβ”€β”€ forms/ +β”‚ β”œβ”€β”€ mod.rs # pub mod add_key_form; +β”‚ └── add_key_form.rs # AddKeyForm input DTO └── resources/ β”œβ”€β”€ mod.rs # pub mod auth_key; - └── auth_key.rs # AuthKey, AddKeyForm, DTOs + └── auth_key.rs # AuthKey, AuthKeyError ``` Ports, use-cases, and adapters are flat files named after the context: @@ -99,30 +103,31 @@ See the `torrent` and `health_check` contexts for the reference pattern. ## Implementation Plan -| ID | Status | Task | Notes | -| --- | ------ | ---------------------------------------------------------------------------------------------------------- | ----------------------------- | -| T1 | TODO | Add `auth_key` context module to `rest-api-protocol/src/v1/context/` with `AuthKey` DTO (resources subdir) | | -| T2 | TODO | Add `AddKeyRequest` DTO to protocol (or reuse from domain with wrapper) | | -| T3 | TODO | Add auth-key error response types to protocol | | -| T4 | TODO | Define `AuthKeyCommandPort` in `rest-api-application/src/ports/` | Methods for CRUD + reload | -| T5 | TODO | Implement `AuthKeyApiService` in `rest-api-application/src/use_cases/` | | -| T6 | TODO | Implement `TrackerAuthKeyAdapter` in `rest-api-runtime-adapter/src/adapters/` | Wraps `KeysHandler` + `clock` | -| T7 | TODO | Update Axum handlers to use `AuthKeyApiService` | | -| T8 | TODO | Update Axum state/routes to wire the new adapter | | -| T9 | TODO | Verify pre-commit and pre-push checks pass | | +| ID | Status | Task | Notes | +| --- | ------ | ---------------------------------------------------------------------------------------------------------- | ----------------------------------------------- | +| T1 | DONE | Add `auth_key` context module to `rest-api-protocol/src/v1/context/` with `AuthKey` DTO (resources subdir) | | +| T2 | DONE | Add `AddKeyForm` input DTO to protocol (forms/ subdir) | `AddKeyForm` moved to protocol `forms/` | +| T3 | DONE | Add `AuthKeyError` response types to protocol | 3-variant enum matching `PeerKeyError` | +| T4 | DONE | Define `AuthKeyPort` in `rest-api-application/src/ports/` | Methods for add, generate, delete, reload | +| T5 | DONE | Implement `AuthKeyApiService` in `rest-api-application/src/use_cases/` | | +| T6 | DONE | Implement `TrackerAuthKeyAdapter` in `rest-api-runtime-adapter/src/adapters/` | Wraps `KeysHandler` + `peer_key_to_auth_key` fn | +| T7 | DONE | Update Axum handlers to use `AuthKeyApiService` | | +| T8 | DONE | Update Axum state/routes to wire the new adapter | In `v1/routes.rs` | +| T9 | DONE | Verify pre-commit and pre-push checks pass | Pre-commit passed | ## Verification / Progress -- [ ] Protocol DTOs created and exported -- [ ] `AuthKeyCommandPort` trait defined in `rest-api-application` -- [ ] `AuthKeyApiService` use-case implemented -- [ ] `TrackerAuthKeyAdapter` implemented in `rest-api-runtime-adapter` -- [ ] Axum handlers dispatch through use-case +- [x] Protocol DTOs created and exported (resources + forms) +- [x] `AuthKeyPort` trait defined in `rest-api-application` +- [x] `AuthKeyApiService` use-case implemented +- [x] `TrackerAuthKeyAdapter` implemented in `rest-api-runtime-adapter` +- [x] Axum handlers dispatch through use-case - [ ] Pre-commit checks pass - [ ] Pre-push checks pass ### Progress Log -| Date | Event | -| ---------- | ------------------ | -| 2026-06-24 | Draft spec created | +| Date | Event | +| ---------- | -------------------------------------------------------- | +| 2026-06-24 | Draft spec created | +| 2026-06-26 | Auth key context migrated to contract-first architecture | diff --git a/packages/axum-rest-api-server/Cargo.toml b/packages/axum-rest-api-server/Cargo.toml index e8f129eaf..d3e31d003 100644 --- a/packages/axum-rest-api-server/Cargo.toml +++ b/packages/axum-rest-api-server/Cargo.toml @@ -27,7 +27,6 @@ hyper = "1" reqwest = { version = "0", features = [ "json" ] } serde = { version = "1", features = [ "derive" ] } serde_json = { version = "1", features = [ "preserve_order" ] } -serde_with = { version = "3", features = [ "json" ] } thiserror = "2" tokio = { version = "1", features = [ "macros", "net", "rt-multi-thread", "signal", "sync" ] } torrust-tracker-axum-server = { version = "3.0.0-develop", path = "../axum-server" } diff --git a/packages/axum-rest-api-server/src/v1/context/auth_key/handlers.rs b/packages/axum-rest-api-server/src/v1/context/auth_key/handlers.rs index 68c4283d0..aa5d3ae4b 100644 --- a/packages/axum-rest-api-server/src/v1/context/auth_key/handlers.rs +++ b/packages/axum-rest-api-server/src/v1/context/auth_key/handlers.rs @@ -1,20 +1,16 @@ //! API handlers for the [`auth_key`](crate::v1::context::auth_key) API context. -use std::str::FromStr; use std::sync::Arc; -use std::time::Duration; use axum::extract::{self, Path, State}; use axum::response::Response; use serde::Deserialize; -use torrust_tracker_core::authentication::Key; -use torrust_tracker_core::authentication::handler::{AddKeyRequest, KeysHandler}; +use torrust_tracker_rest_api_application::use_cases::auth_key::AuthKeyApiService; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::forms::add_key_form::AddKeyForm; -use super::forms::AddKeyForm; use super::responses::{ - auth_key_response, failed_to_delete_key_response, failed_to_generate_key_response, failed_to_reload_keys_response, - invalid_auth_key_duration_response, invalid_auth_key_response, + auth_key_response, failed_to_add_key_response, failed_to_delete_key_response, failed_to_generate_key_response, + failed_to_reload_keys_response, invalid_auth_key_duration_response, invalid_auth_key_response, }; -use crate::v1::context::auth_key::resources::AuthKey; use crate::v1::responses::{invalid_auth_key_param_response, ok_response}; /// It handles the request to add a new authentication key. @@ -31,23 +27,22 @@ use crate::v1::responses::{invalid_auth_key_param_response, ok_response}; /// Refer to the [API endpoint documentation](crate::v1::context::auth_key#generate-a-new-authentication-key) /// for more information about this endpoint. pub async fn add_auth_key_handler( - State(keys_handler): State>, + State(auth_key_service): State>, extract::Json(add_key_form): extract::Json, ) -> Response { - match keys_handler - .add_peer_key(AddKeyRequest { - opt_key: add_key_form.opt_key.clone(), - opt_seconds_valid: add_key_form.opt_seconds_valid, - }) - .await - { - Ok(auth_key) => auth_key_response(&AuthKey::from(auth_key)), - Err(err) => match err { - torrust_tracker_core::error::PeerKeyError::DurationOverflow { seconds_valid } => { - invalid_auth_key_duration_response(seconds_valid) + match auth_key_service.add_key(&add_key_form).await { + Ok(auth_key) => auth_key_response(&auth_key), + Err(err) => match &err { + torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::AuthKeyError::DurationOverflow { + seconds_valid, + } => invalid_auth_key_duration_response(*seconds_valid), + torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::AuthKeyError::InvalidKey { + key, + reason, + } => invalid_auth_key_response(key, reason), + torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::AuthKeyError::Database(_) => { + failed_to_add_key_response(AuthKeyErrorDisplay(&err)) } - torrust_tracker_core::error::PeerKeyError::InvalidKey { key, source } => invalid_auth_key_response(&key, source), - torrust_tracker_core::error::PeerKeyError::DatabaseError { source } => failed_to_generate_key_response(source), }, } } @@ -66,34 +61,19 @@ pub async fn add_auth_key_handler( /// /// This endpoint has been deprecated. Use [`add_auth_key_handler`]. pub async fn generate_auth_key_handler( - State(keys_handler): State>, + State(auth_key_service): State>, Path(seconds_valid_or_key): Path, ) -> Response { let seconds_valid = seconds_valid_or_key; - match keys_handler - .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) - .await - { - Ok(auth_key) => auth_key_response(&AuthKey::from(auth_key)), - Err(e) => failed_to_generate_key_response(e), + match auth_key_service.generate_key(seconds_valid).await { + Ok(auth_key) => auth_key_response(&auth_key), + Err(e) => failed_to_generate_key_response(AuthKeyErrorDisplay(&e)), } } /// A container for the `key` parameter extracted from the URL PATH. /// /// It does not perform any validation, it just stores the value. -/// -/// In the current API version, the `key` parameter can be either a valid key -/// like `xqD6NWH9TcKrOCwDmqcdH5hF5RrbL0A6` or the number of seconds the -/// key will be valid, for example two minutes `120`. -/// -/// For example, the `key` is used in the following requests: -/// -/// - `POST /api/v1/key/120`. It will generate a new key valid for two minutes. -/// - `DELETE /api/v1/key/xqD6NWH9TcKrOCwDmqcdH5hF5RrbL0A6`. It will delete the -/// key `xqD6NWH9TcKrOCwDmqcdH5hF5RrbL0A6`. -/// -/// > **NOTICE**: this may change in the future, in the [API v2](https://github.com/torrust/torrust-tracker/issues/144). #[derive(Deserialize)] pub struct KeyParam(String); @@ -109,15 +89,16 @@ pub struct KeyParam(String); /// Refer to the [API endpoint documentation](crate::v1::context::auth_key#delete-an-authentication-key) /// for more information about this endpoint. pub async fn delete_auth_key_handler( - State(keys_handler): State>, + State(auth_key_service): State>, Path(seconds_valid_or_key): Path, ) -> Response { - match Key::from_str(&seconds_valid_or_key.0) { - Err(_) => invalid_auth_key_param_response(&seconds_valid_or_key.0), - Ok(key) => match keys_handler.remove_peer_key(&key).await { - Ok(()) => ok_response(), - Err(e) => failed_to_delete_key_response(e), - }, + match auth_key_service.delete_key(&seconds_valid_or_key.0).await { + Ok(()) => ok_response(), + Err(torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::AuthKeyError::InvalidKey { + key: _, + reason: _, + }) => invalid_auth_key_param_response(&seconds_valid_or_key.0), + Err(e) => failed_to_delete_key_response(AuthKeyErrorDisplay(&e)), } } @@ -133,9 +114,27 @@ pub async fn delete_auth_key_handler( /// /// Refer to the [API endpoint documentation](crate::v1::context::auth_key#reload-authentication-keys) /// for more information about this endpoint. -pub async fn reload_keys_handler(State(keys_handler): State>) -> Response { - match keys_handler.load_peer_keys_from_database().await { +pub async fn reload_keys_handler(State(auth_key_service): State>) -> Response { + match auth_key_service.reload_keys().await { Ok(()) => ok_response(), - Err(e) => failed_to_reload_keys_response(e), + Err(e) => failed_to_reload_keys_response(AuthKeyErrorDisplay(&e)), + } +} + +/// Wrapper to allow passing an [`AuthKeyError`] reference to response +/// functions that expect `E: std::error::Error`. +struct AuthKeyErrorDisplay<'a>(&'a torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::AuthKeyError); + +impl std::fmt::Display for AuthKeyErrorDisplay<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(self.0, f) + } +} + +impl std::error::Error for AuthKeyErrorDisplay<'_> {} + +impl std::fmt::Debug for AuthKeyErrorDisplay<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(self.0, f) } } diff --git a/packages/axum-rest-api-server/src/v1/context/auth_key/mod.rs b/packages/axum-rest-api-server/src/v1/context/auth_key/mod.rs index 0a3937ef2..744e4d4cc 100644 --- a/packages/axum-rest-api-server/src/v1/context/auth_key/mod.rs +++ b/packages/axum-rest-api-server/src/v1/context/auth_key/mod.rs @@ -126,8 +126,6 @@ //! "status": "ok" //! } //! ``` -pub mod forms; pub mod handlers; -pub mod resources; pub mod responses; pub mod routes; diff --git a/packages/axum-rest-api-server/src/v1/context/auth_key/resources.rs b/packages/axum-rest-api-server/src/v1/context/auth_key/resources.rs deleted file mode 100644 index d297d2c43..000000000 --- a/packages/axum-rest-api-server/src/v1/context/auth_key/resources.rs +++ /dev/null @@ -1,129 +0,0 @@ -//! API resources for the [`auth_key`](crate::v1::context::auth_key) API context. - -use serde::{Deserialize, Serialize}; -use torrust_clock::conv::convert_from_iso_8601_to_timestamp; -use torrust_tracker_core::authentication::{self, Key}; - -/// A resource that represents an authentication key. -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] -pub struct AuthKey { - /// The authentication key. - pub key: String, - /// The timestamp when the key will expire. - #[deprecated(since = "3.0.0", note = "please use `expiry_time` instead")] - pub valid_until: Option, // todo: remove when the torrust-index-backend starts using the `expiry_time` attribute. - /// The ISO 8601 timestamp when the key will expire. - pub expiry_time: Option, -} - -impl From for authentication::PeerKey { - fn from(auth_key_resource: AuthKey) -> Self { - authentication::PeerKey { - key: auth_key_resource.key.parse::().unwrap(), - valid_until: auth_key_resource - .expiry_time - .map(|expiry_time| convert_from_iso_8601_to_timestamp(&expiry_time)), - } - } -} - -#[allow(deprecated)] -impl From for AuthKey { - fn from(auth_key: authentication::PeerKey) -> Self { - match (auth_key.valid_until, auth_key.expiry_time()) { - (Some(valid_until), Some(expiry_time)) => AuthKey { - key: auth_key.key.to_string(), - valid_until: Some(valid_until.as_secs()), - expiry_time: Some(expiry_time.to_string()), - }, - _ => AuthKey { - key: auth_key.key.to_string(), - valid_until: None, - expiry_time: None, - }, - } - } -} - -#[cfg(test)] -mod tests { - use std::time::Duration; - - use torrust_clock::clock::stopped::Stopped as _; - use torrust_clock::clock::{self, Time}; - use torrust_tracker_core::authentication::{self, Key}; - - use super::AuthKey; - use crate::CurrentClock; - - struct TestTime { - pub timestamp: u64, - pub iso_8601_v1: String, - pub iso_8601_v2: String, - } - - fn one_hour_after_unix_epoch() -> TestTime { - let timestamp = 60_u64; - let iso_8601_v1 = "1970-01-01T00:01:00.000Z".to_string(); - let iso_8601_v2 = "1970-01-01 00:01:00 UTC".to_string(); - TestTime { - timestamp, - iso_8601_v1, - iso_8601_v2, - } - } - - #[test] - #[allow(deprecated)] - fn it_should_be_convertible_into_an_auth_key() { - clock::Stopped::local_set_to_unix_epoch(); - - let auth_key_resource = AuthKey { - key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".to_string(), // cspell:disable-line - valid_until: Some(one_hour_after_unix_epoch().timestamp), - expiry_time: Some(one_hour_after_unix_epoch().iso_8601_v1), - }; - - assert_eq!( - authentication::PeerKey::from(auth_key_resource), - authentication::PeerKey { - key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line - valid_until: Some(CurrentClock::now_add(&Duration::new(one_hour_after_unix_epoch().timestamp, 0)).unwrap()) - } - ); - } - - #[test] - #[allow(deprecated)] - fn it_should_be_convertible_from_an_auth_key() { - clock::Stopped::local_set_to_unix_epoch(); - - let auth_key = authentication::PeerKey { - key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line - valid_until: Some(CurrentClock::now_add(&Duration::new(one_hour_after_unix_epoch().timestamp, 0)).unwrap()), - }; - - assert_eq!( - AuthKey::from(auth_key), - AuthKey { - key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".to_string(), // cspell:disable-line - valid_until: Some(one_hour_after_unix_epoch().timestamp), - expiry_time: Some(one_hour_after_unix_epoch().iso_8601_v2), - } - ); - } - - #[test] - #[allow(deprecated)] - fn it_should_be_convertible_into_json() { - assert_eq!( - serde_json::to_string(&AuthKey { - key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".to_string(), // cspell:disable-line - valid_until: Some(one_hour_after_unix_epoch().timestamp), - expiry_time: Some(one_hour_after_unix_epoch().iso_8601_v1), - }) - .unwrap(), - "{\"key\":\"IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM\",\"valid_until\":60,\"expiry_time\":\"1970-01-01T00:01:00.000Z\"}" // cspell:disable-line - ); - } -} diff --git a/packages/axum-rest-api-server/src/v1/context/auth_key/responses.rs b/packages/axum-rest-api-server/src/v1/context/auth_key/responses.rs index 41fbad874..5621b0a5d 100644 --- a/packages/axum-rest-api-server/src/v1/context/auth_key/responses.rs +++ b/packages/axum-rest-api-server/src/v1/context/auth_key/responses.rs @@ -3,8 +3,8 @@ use std::error::Error; use axum::http::{StatusCode, header}; use axum::response::{IntoResponse, Response}; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::AuthKey; -use crate::v1::context::auth_key::resources::AuthKey; use crate::v1::responses::{bad_request_response, unhandled_rejection_response}; /// `200` response that contains the `AuthKey` resource as json. @@ -50,8 +50,8 @@ pub fn failed_to_reload_keys_response(e: E) -> Response { } #[must_use] -pub fn invalid_auth_key_response(auth_key: &str, e: E) -> Response { - bad_request_response(&format!("Invalid URL: invalid auth key: string \"{auth_key}\", {e}")) +pub fn invalid_auth_key_response(auth_key: &str, reason: &str) -> Response { + bad_request_response(&format!("Invalid URL: invalid auth key: string \"{auth_key}\", {reason}")) } #[must_use] diff --git a/packages/axum-rest-api-server/src/v1/context/auth_key/routes.rs b/packages/axum-rest-api-server/src/v1/context/auth_key/routes.rs index 9f0f2387c..3fa0d0a11 100644 --- a/packages/axum-rest-api-server/src/v1/context/auth_key/routes.rs +++ b/packages/axum-rest-api-server/src/v1/context/auth_key/routes.rs @@ -10,36 +10,28 @@ use std::sync::Arc; use axum::Router; use axum::routing::{get, post}; -use torrust_tracker_core::authentication::handler::KeysHandler; +use torrust_tracker_rest_api_application::use_cases::auth_key::AuthKeyApiService; use super::handlers::{add_auth_key_handler, delete_auth_key_handler, generate_auth_key_handler, reload_keys_handler}; /// It adds the routes to the router for the [`auth_key`](crate::v1::context::auth_key) API context. -pub fn add(prefix: &str, router: Router, keys_handler: &Arc) -> Router { +pub fn add(prefix: &str, router: Router, auth_key_service: &Arc) -> Router { // Keys router .route( - // code-review: Axum does not allow two routes with the same path but different path variable name. - // In the new major API version, `seconds_valid` should be a POST form field so that we will have two paths: - // - // POST /keys - // DELETE /keys/:key - // - // The POST /key/:seconds_valid has been deprecated and it will removed in the future. - // Use POST /keys &format!("{prefix}/key/{{seconds_valid_or_key}}"), post(generate_auth_key_handler) - .with_state(keys_handler.clone()) + .with_state(auth_key_service.clone()) .delete(delete_auth_key_handler) - .with_state(keys_handler.clone()), + .with_state(auth_key_service.clone()), ) // Keys command .route( &format!("{prefix}/keys/reload"), - get(reload_keys_handler).with_state(keys_handler.clone()), + get(reload_keys_handler).with_state(auth_key_service.clone()), ) .route( &format!("{prefix}/keys"), - post(add_auth_key_handler).with_state(keys_handler.clone()), + post(add_auth_key_handler).with_state(auth_key_service.clone()), ) } diff --git a/packages/axum-rest-api-server/src/v1/routes.rs b/packages/axum-rest-api-server/src/v1/routes.rs index 809d86d2c..37aca9e09 100644 --- a/packages/axum-rest-api-server/src/v1/routes.rs +++ b/packages/axum-rest-api-server/src/v1/routes.rs @@ -2,9 +2,11 @@ use std::sync::Arc; use axum::Router; +use torrust_tracker_rest_api_application::use_cases::auth_key::AuthKeyApiService; use torrust_tracker_rest_api_application::use_cases::torrent::TorrentApiService; use torrust_tracker_rest_api_application::use_cases::whitelist::WhitelistApiService; use torrust_tracker_rest_api_core::container::TrackerHttpApiCoreContainer; +use torrust_tracker_rest_api_runtime_adapter::adapters::auth_key::TrackerAuthKeyAdapter; use torrust_tracker_rest_api_runtime_adapter::adapters::torrent::TrackerTorrentQueryAdapter; use torrust_tracker_rest_api_runtime_adapter::adapters::whitelist::TrackerWhitelistAdapter; @@ -14,11 +16,10 @@ use super::context::{auth_key, stats, torrent, whitelist}; pub fn add(prefix: &str, router: Router, http_api_container: &Arc) -> Router { let v1_prefix = format!("{prefix}/v1"); - let router = auth_key::routes::add( - &v1_prefix, - router, - &http_api_container.tracker_core_container.keys_handler.clone(), - ); + let auth_key_adapter = TrackerAuthKeyAdapter::new(&http_api_container.tracker_core_container.keys_handler); + let auth_key_service = Arc::new(AuthKeyApiService::new(Box::new(auth_key_adapter))); + let router = auth_key::routes::add(&v1_prefix, router, &auth_key_service); + let router = stats::routes::add(&v1_prefix, router, http_api_container); let whitelist_adapter = TrackerWhitelistAdapter::new(&http_api_container.tracker_core_container.whitelist_manager); diff --git a/packages/axum-rest-api-server/tests/server/v1/asserts.rs b/packages/axum-rest-api-server/tests/server/v1/asserts.rs index 2be8d356c..7d0d624d6 100644 --- a/packages/axum-rest-api-server/tests/server/v1/asserts.rs +++ b/packages/axum-rest-api-server/tests/server/v1/asserts.rs @@ -1,8 +1,8 @@ // code-review: should we use macros to return the exact line where the assert fails? use reqwest::Response; -use torrust_tracker_axum_rest_api_server::v1::context::auth_key::resources::AuthKey; use torrust_tracker_axum_rest_api_server::v1::context::stats::resources::Stats; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::AuthKey; use torrust_tracker_rest_api_protocol::v1::context::torrent::resources::torrent::{ListItem, Torrent}; // Resource responses @@ -146,6 +146,10 @@ pub async fn assert_failed_to_generate_key(response: Response) { assert_unhandled_rejection(response, "failed to generate key").await; } +pub async fn assert_failed_to_add_key(response: Response) { + assert_unhandled_rejection(response, "failed to add key").await; +} + pub async fn assert_failed_to_delete_key(response: Response) { assert_unhandled_rejection(response, "failed to delete key").await; } diff --git a/packages/axum-rest-api-server/tests/server/v1/contract/context/auth_key.rs b/packages/axum-rest-api-server/tests/server/v1/contract/context/auth_key.rs index 693dab82a..c9dba6bfb 100644 --- a/packages/axum-rest-api-server/tests/server/v1/contract/context/auth_key.rs +++ b/packages/axum-rest-api-server/tests/server/v1/contract/context/auth_key.rs @@ -11,7 +11,7 @@ use uuid::Uuid; use crate::server::connection_info::{connection_with_invalid_token, connection_with_no_token}; use crate::server::force_database_error; use crate::server::v1::asserts::{ - assert_auth_key_utf8, assert_failed_to_delete_key, assert_failed_to_generate_key, assert_failed_to_reload_keys, + assert_auth_key_utf8, assert_failed_to_add_key, assert_failed_to_delete_key, assert_failed_to_reload_keys, assert_invalid_auth_key_get_param, assert_invalid_auth_key_post_param, assert_ok, assert_token_not_valid, assert_unauthorized, assert_unprocessable_auth_key_duration_param, }; @@ -152,7 +152,7 @@ async fn should_fail_when_the_auth_key_cannot_be_generated() { ) .await; - assert_failed_to_generate_key(response).await; + assert_failed_to_add_key(response).await; assert!( logs_contains_a_line_with(&["ERROR", "API", &format!("{request_id}")]), diff --git a/packages/rest-api-application/src/ports/auth_key.rs b/packages/rest-api-application/src/ports/auth_key.rs new file mode 100644 index 000000000..1a8ba47db --- /dev/null +++ b/packages/rest-api-application/src/ports/auth_key.rs @@ -0,0 +1,27 @@ +//! Port trait for authentication key operations. +//! +//! Defines the boundary between the application layer and the +//! tracker-internal key management implementation. Implementations +//! live in the runtime adapter package. +use async_trait::async_trait; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::forms::add_key_form::AddKeyForm; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::{AuthKey, AuthKeyError}; + +/// Port for authentication key operations. +/// +/// Covers both command and query operations: adding/generating/deleting +/// keys, and reloading them from the database. +#[async_trait] +pub trait AuthKeyPort: Send + Sync { + /// Adds a new peer key (pre-generated or generated on-the-fly). + async fn add_key(&self, form: &AddKeyForm) -> Result; + + /// Generates a new expiring peer key with the given lifetime in seconds. + async fn generate_key(&self, seconds_valid: u64) -> Result; + + /// Deletes an authentication key. + async fn delete_key(&self, key: &str) -> Result<(), AuthKeyError>; + + /// Reloads authentication keys from the database into memory. + async fn reload_keys(&self) -> Result<(), AuthKeyError>; +} diff --git a/packages/rest-api-application/src/ports/mod.rs b/packages/rest-api-application/src/ports/mod.rs index 9c45c4125..c7c7512b2 100644 --- a/packages/rest-api-application/src/ports/mod.rs +++ b/packages/rest-api-application/src/ports/mod.rs @@ -3,5 +3,6 @@ //! These traits define the boundary between the application layer and //! the tracker-internal implementation. Implementations live in the //! runtime adapter package. +pub mod auth_key; pub mod torrent; pub mod whitelist; diff --git a/packages/rest-api-application/src/use_cases/auth_key.rs b/packages/rest-api-application/src/use_cases/auth_key.rs new file mode 100644 index 000000000..eb1bde71c --- /dev/null +++ b/packages/rest-api-application/src/use_cases/auth_key.rs @@ -0,0 +1,60 @@ +//! Use-case service for authentication key API operations. +//! +//! Orchestrates calls to the [`AuthKeyPort`] and adds business logic +//! such as validation, error mapping, or caching as needed. +use torrust_tracker_rest_api_protocol::v1::context::auth_key::forms::add_key_form::AddKeyForm; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::{AuthKey, AuthKeyError}; + +use crate::ports::auth_key::AuthKeyPort; + +/// Use-case service for auth-key-related API operations. +/// +/// Delegates to an [`AuthKeyPort`] implementation (tracker adapter) +/// and maps domain errors to protocol error types. +pub struct AuthKeyApiService { + port: Box, +} + +impl AuthKeyApiService { + /// Creates a new service backed by the given port implementation. + #[must_use] + pub fn new(port: Box) -> Self { + Self { port } + } + + /// Adds a new peer key. + /// + /// # Errors + /// + /// Returns an [`AuthKeyError`] if the operation fails. + pub async fn add_key(&self, form: &AddKeyForm) -> Result { + self.port.add_key(form).await + } + + /// Generates a new expiring peer key. + /// + /// # Errors + /// + /// Returns an [`AuthKeyError`] if the operation fails. + pub async fn generate_key(&self, seconds_valid: u64) -> Result { + self.port.generate_key(seconds_valid).await + } + + /// Deletes an authentication key. + /// + /// # Errors + /// + /// Returns an [`AuthKeyError`] if the operation fails. + pub async fn delete_key(&self, key: &str) -> Result<(), AuthKeyError> { + self.port.delete_key(key).await + } + + /// Reloads authentication keys from the database. + /// + /// # Errors + /// + /// Returns an [`AuthKeyError`] if the operation fails. + pub async fn reload_keys(&self) -> Result<(), AuthKeyError> { + self.port.reload_keys().await + } +} diff --git a/packages/rest-api-application/src/use_cases/mod.rs b/packages/rest-api-application/src/use_cases/mod.rs index 73bdf84bc..fd969c6ec 100644 --- a/packages/rest-api-application/src/use_cases/mod.rs +++ b/packages/rest-api-application/src/use_cases/mod.rs @@ -1,5 +1,6 @@ //! Use-case services for the REST API. //! //! Each service orchestrates business logic by calling port traits. +pub mod auth_key; pub mod torrent; pub mod whitelist; diff --git a/packages/rest-api-protocol/Cargo.toml b/packages/rest-api-protocol/Cargo.toml index 97bb7b828..4f9564b8d 100644 --- a/packages/rest-api-protocol/Cargo.toml +++ b/packages/rest-api-protocol/Cargo.toml @@ -15,3 +15,4 @@ version.workspace = true [dependencies] serde = { version = "1", features = [ "derive" ] } +serde_with = { version = "3", features = [ "json" ] } diff --git a/packages/axum-rest-api-server/src/v1/context/auth_key/forms.rs b/packages/rest-api-protocol/src/v1/context/auth_key/forms/add_key_form.rs similarity index 83% rename from packages/axum-rest-api-server/src/v1/context/auth_key/forms.rs rename to packages/rest-api-protocol/src/v1/context/auth_key/forms/add_key_form.rs index 2905579d9..e08b45abb 100644 --- a/packages/axum-rest-api-server/src/v1/context/auth_key/forms.rs +++ b/packages/rest-api-protocol/src/v1/context/auth_key/forms/add_key_form.rs @@ -1,3 +1,6 @@ +//! Form for adding a new authentication key. +//! +//! This is the input DTO for the `POST /api/v1/keys` endpoint. use serde::{Deserialize, Serialize}; use serde_with::{DefaultOnNull, serde_as}; @@ -5,7 +8,7 @@ use serde_with::{DefaultOnNull, serde_as}; /// /// You can upload a pre-generated key or let the app to generate a new one. /// You can also set an expiration date or leave it empty (`None`) if you want -/// to create permanent key that does not expire. +/// to create a permanent key that does not expire. #[serde_as] #[derive(Serialize, Deserialize, Debug)] pub struct AddKeyForm { diff --git a/packages/rest-api-protocol/src/v1/context/auth_key/forms/mod.rs b/packages/rest-api-protocol/src/v1/context/auth_key/forms/mod.rs new file mode 100644 index 000000000..56c87e8bc --- /dev/null +++ b/packages/rest-api-protocol/src/v1/context/auth_key/forms/mod.rs @@ -0,0 +1,2 @@ +//! Forms (input DTOs) for the [`auth_key`](super) context. +pub mod add_key_form; diff --git a/packages/rest-api-protocol/src/v1/context/auth_key/mod.rs b/packages/rest-api-protocol/src/v1/context/auth_key/mod.rs new file mode 100644 index 000000000..7045d266f --- /dev/null +++ b/packages/rest-api-protocol/src/v1/context/auth_key/mod.rs @@ -0,0 +1,6 @@ +//! Authentication key context β€” `/api/v1/keys` endpoints. +//! +//! Refer to the [`axum-rest-api-server`] counterpart at +//! `v1::context::auth_key` for the HTTP routing and handler layer. +pub mod forms; +pub mod resources; diff --git a/packages/rest-api-protocol/src/v1/context/auth_key/resources/auth_key.rs b/packages/rest-api-protocol/src/v1/context/auth_key/resources/auth_key.rs new file mode 100644 index 000000000..b1f29c8fd --- /dev/null +++ b/packages/rest-api-protocol/src/v1/context/auth_key/resources/auth_key.rs @@ -0,0 +1,49 @@ +//! API resources for the authentication key context. +//! +//! These types define the serialization contract for the `/api/v1/keys` +//! endpoint responses. +use std::fmt; + +use serde::{Deserialize, Serialize}; + +/// A resource that represents an authentication key. +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] +pub struct AuthKey { + /// The authentication key. + pub key: String, + /// The timestamp when the key will expire. + #[deprecated(since = "3.0.0", note = "please use `expiry_time` instead")] + pub valid_until: Option, + /// The ISO 8601 timestamp when the key will expire. + pub expiry_time: Option, +} + +/// Errors that can occur during auth key operations. +/// +/// These correspond to the variants of `tracker_core::error::PeerKeyError` +/// but are protocol-level types without tracker-core dependencies. +#[derive(Debug)] +pub enum AuthKeyError { + /// The provided duration overflows. + DurationOverflow { seconds_valid: u64 }, + /// The provided key is invalid. + InvalidKey { key: String, reason: String }, + /// A database error occurred during the auth key operation. + Database(String), +} + +impl fmt::Display for AuthKeyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AuthKeyError::DurationOverflow { seconds_valid } => { + write!(f, "duration overflow: {seconds_valid}") + } + AuthKeyError::InvalidKey { key, reason } => { + write!(f, "invalid key: \"{key}\", {reason}") + } + AuthKeyError::Database(msg) => write!(f, "{msg}"), + } + } +} + +impl std::error::Error for AuthKeyError {} diff --git a/packages/rest-api-protocol/src/v1/context/auth_key/resources/mod.rs b/packages/rest-api-protocol/src/v1/context/auth_key/resources/mod.rs new file mode 100644 index 000000000..ad0ae78e3 --- /dev/null +++ b/packages/rest-api-protocol/src/v1/context/auth_key/resources/mod.rs @@ -0,0 +1,2 @@ +//! Resources for the [`auth_key`](super) context. +pub mod auth_key; diff --git a/packages/rest-api-protocol/src/v1/context/mod.rs b/packages/rest-api-protocol/src/v1/context/mod.rs index a542e5ebc..60029ce00 100644 --- a/packages/rest-api-protocol/src/v1/context/mod.rs +++ b/packages/rest-api-protocol/src/v1/context/mod.rs @@ -1,7 +1,8 @@ //! API resources (DTOs) for the v1 REST API contract, organized by context. //! //! Each submodule corresponds to an API context. Resources for each context -//! live under its `resources/` subdirectory. +//! live under its `resources/` subdirectory. Input forms live under `forms/`. +pub mod auth_key; pub mod health_check; pub mod torrent; pub mod whitelist; diff --git a/packages/rest-api-runtime-adapter/src/adapters/auth_key.rs b/packages/rest-api-runtime-adapter/src/adapters/auth_key.rs new file mode 100644 index 000000000..dbd8300de --- /dev/null +++ b/packages/rest-api-runtime-adapter/src/adapters/auth_key.rs @@ -0,0 +1,102 @@ +//! Tracker-specific implementation of [`AuthKeyPort`]. +use std::str::FromStr; +use std::sync::Arc; + +use async_trait::async_trait; +use torrust_tracker_core::authentication::handler::{AddKeyRequest, KeysHandler}; +use torrust_tracker_core::authentication::{Key, PeerKey}; +use torrust_tracker_rest_api_application::ports::auth_key::AuthKeyPort; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::forms::add_key_form::AddKeyForm; +use torrust_tracker_rest_api_protocol::v1::context::auth_key::resources::auth_key::{AuthKey, AuthKeyError}; + +/// Adapter that wraps [`KeysHandler`] and implements the [`AuthKeyPort`] trait. +pub struct TrackerAuthKeyAdapter { + keys_handler: Arc, +} + +impl TrackerAuthKeyAdapter { + /// Creates a new adapter wrapping the given keys handler. + #[must_use] + pub fn new(keys_handler: &Arc) -> Self { + Self { + keys_handler: keys_handler.clone(), + } + } +} + +#[async_trait] +impl AuthKeyPort for TrackerAuthKeyAdapter { + async fn add_key(&self, form: &AddKeyForm) -> Result { + let result = self + .keys_handler + .add_peer_key(AddKeyRequest { + opt_key: form.opt_key.clone(), + opt_seconds_valid: form.opt_seconds_valid, + }) + .await; + + result.map(peer_key_to_auth_key).map_err(map_peer_key_error) + } + + async fn generate_key(&self, seconds_valid: u64) -> Result { + let result = self + .keys_handler + .generate_expiring_peer_key(Some(std::time::Duration::from_secs(seconds_valid))) + .await; + + result + .map(peer_key_to_auth_key) + .map_err(|e| AuthKeyError::Database(e.to_string())) + } + + async fn delete_key(&self, key: &str) -> Result<(), AuthKeyError> { + match Key::from_str(key) { + Err(_) => Err(AuthKeyError::InvalidKey { + key: key.to_string(), + reason: "invalid key format".to_string(), + }), + Ok(key) => self + .keys_handler + .remove_peer_key(&key) + .await + .map_err(|e| AuthKeyError::Database(e.to_string())), + } + } + + async fn reload_keys(&self) -> Result<(), AuthKeyError> { + self.keys_handler + .load_peer_keys_from_database() + .await + .map_err(|e| AuthKeyError::Database(e.to_string())) + } +} + +fn map_peer_key_error(err: torrust_tracker_core::error::PeerKeyError) -> AuthKeyError { + use torrust_tracker_core::error::PeerKeyError; + + match err { + PeerKeyError::DurationOverflow { seconds_valid } => AuthKeyError::DurationOverflow { seconds_valid }, + PeerKeyError::InvalidKey { key, source } => AuthKeyError::InvalidKey { + key, + reason: source.to_string(), + }, + PeerKeyError::DatabaseError { source } => AuthKeyError::Database(source.to_string()), + } +} + +#[allow(clippy::needless_pass_by_value)] +#[allow(deprecated)] +fn peer_key_to_auth_key(peer_key: PeerKey) -> AuthKey { + match (peer_key.valid_until, peer_key.expiry_time()) { + (Some(valid_until), Some(expiry_time)) => AuthKey { + key: peer_key.key.to_string(), + valid_until: Some(valid_until.as_secs()), + expiry_time: Some(expiry_time.to_string()), + }, + _ => AuthKey { + key: peer_key.key.to_string(), + valid_until: None, + expiry_time: None, + }, + } +} diff --git a/packages/rest-api-runtime-adapter/src/adapters/mod.rs b/packages/rest-api-runtime-adapter/src/adapters/mod.rs index ad46e8f45..432d5eec9 100644 --- a/packages/rest-api-runtime-adapter/src/adapters/mod.rs +++ b/packages/rest-api-runtime-adapter/src/adapters/mod.rs @@ -1,3 +1,4 @@ //! Adapter implementations for REST API port traits. +pub mod auth_key; pub mod torrent; pub mod whitelist;