diff --git a/.env.example b/.env.example index 10fc465..4e42d71 100644 --- a/.env.example +++ b/.env.example @@ -77,9 +77,15 @@ REDMINE_API_KEY=your_api_key # REDMINE_PER_USER_TRUST_PROXY=true # REDMINE_PER_USER_AUDIT_IDENTITY=false -# --- oauth mode (not yet implemented; only presence is validated) --- +# --- oauth mode (advanced; requires --transport http) --- +# Each request supplies its own Authorization: Bearer token, validated by RFC +# 7662 introspection against Redmine's Doorkeeper. See docs/oauth-setup.md. +# Scope enforcement and discovery documents are not live yet. # REDMINE_AUTH_MODE=oauth # REDMINE_MCP_BASE_URL=http://localhost:3040 +# REDMINE_INTROSPECT_CLIENT_ID=your-doorkeeper-application-uid +# REDMINE_INTROSPECT_CLIENT_SECRET=your-doorkeeper-application-secret +# REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS=60 # --- Attachment store (no tools use it yet; get_redmine_attachment lands in # a later sub-phase) --- diff --git a/CHANGELOG.md b/CHANGELOG.md index eeb7953..74fd8ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `REDMINE_AUTH_MODE=oauth` now works end to end for bearer-token + authentication: an axum middleware guards the whole `/mcp` route (including + `initialize`), extracting the inbound `Authorization: Bearer` token and + validating it by RFC 7662 introspection against Redmine's Doorkeeper, + cached by a SHA-256 digest of the token (never the token itself) with a TTL + capped by the token's own `exp`. A missing/malformed/invalid token gets a + `401` carrying `WWW-Authenticate: Bearer resource_metadata="..."`; a broken + or misconfigured introspection endpoint gets a `503` with `Retry-After`, + never a `401`. The validated token is forwarded to Redmine verbatim. + Requires the new `REDMINE_INTROSPECT_CLIENT_ID`/ + `REDMINE_INTROSPECT_CLIENT_SECRET`(`_FILE`) variables; `oauth` on the stdio + transport is now a startup error, matching `legacy-per-user`. Scope + enforcement, discovery documents, and `/revoke` are not implemented yet — + see `docs/oauth-setup.md`. - `REDMINE_AUTH_MODE=legacy-per-user` is now implemented: each HTTP request carries its own Redmine credential in `X-Redmine-API-Key` instead of the server holding one shared key. No ambient fallback and no cross-request diff --git a/Cargo.lock b/Cargo.lock index db98060..88ab05a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -202,6 +202,15 @@ version = "2.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" +[[package]] +name = "block-buffer" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" +dependencies = [ + "hybrid-array", +] + [[package]] name = "bumpalo" version = "3.20.3" @@ -326,6 +335,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "const-oid" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c" + [[package]] name = "core-foundation" version = "0.10.1" @@ -351,6 +366,15 @@ dependencies = [ "libc", ] +[[package]] +name = "crypto-common" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453" +dependencies = [ + "hybrid-array", +] + [[package]] name = "darling" version = "0.24.0" @@ -403,6 +427,17 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b" +[[package]] +name = "digest" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1dd6dbb5841937940781866fa1281a1ff7bd3bf827091440879f9994983d5c2" +dependencies = [ + "block-buffer", + "const-oid", + "crypto-common", +] + [[package]] name = "displaydoc" version = "0.2.7" @@ -684,6 +719,15 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" +[[package]] +name = "hybrid-array" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b" +dependencies = [ + "typenum", +] + [[package]] name = "hyper" version = "1.11.0" @@ -1370,6 +1414,7 @@ dependencies = [ "rustls-platform-verifier", "serde", "serde_json", + "serde_urlencoded", "sync_wrapper", "tokio", "tokio-rustls", @@ -1464,6 +1509,7 @@ dependencies = [ "secrecy", "serde", "serde_json", + "sha2", "thiserror", "tokio", "tokio-util", @@ -1736,6 +1782,17 @@ dependencies = [ "serde", ] +[[package]] +name = "sha2" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "446ba717509524cb3f22f17ecc096f10f4822d76ab5c0b9822c5f9c284e825f4" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -2105,6 +2162,12 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + [[package]] name = "unicode-ident" version = "1.0.24" diff --git a/Cargo.toml b/Cargo.toml index 3e5db1d..15b7fb1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ repository = "https://github.com/mimi1vx/ruprogress-mcp" [workspace.dependencies] rmcp = { version = "=3.1.1", default-features = false } tokio = { version = "1.53", default-features = false } -reqwest = { version = "0.13", default-features = false, features = ["json", "stream", "rustls"] } +reqwest = { version = "0.13", default-features = false, features = ["json", "stream", "rustls", "form"] } serde = { version = "1", features = ["derive"] } serde_json = "1" schemars = "1.2" @@ -38,6 +38,7 @@ wiremock = "0.6" futures-core = "0.3" futures-util = { version = "0.3", default-features = false, features = ["std"] } base64 = "0.22" +sha2 = "0.11" [workspace.lints.rust] unsafe_code = "forbid" diff --git a/README.md b/README.md index d15262a..86c2a2b 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,9 @@ before changing `SERVER_HOST`. - Full tool parity with the reference server (~51 tools). - Both stdio and streamable HTTP transports. - Four auth modes: `legacy` and `legacy-per-user` implemented (the latter - documented in `docs/legacy-per-user-auth.md`); `oauth`/`oauth-proxy` not + documented in `docs/legacy-per-user-auth.md`); `oauth` implemented for + bearer-token introspection (documented in `docs/oauth-setup.md`; scope + enforcement and discovery documents are not live yet); `oauth-proxy` not yet. - A reusable `redmine-client` crate, independent of MCP. diff --git a/crates/redmine-client/src/client.rs b/crates/redmine-client/src/client.rs index 0634aa4..13922ca 100644 --- a/crates/redmine-client/src/client.rs +++ b/crates/redmine-client/src/client.rs @@ -7,6 +7,7 @@ use std::time::{Duration, Instant}; use bytes::Bytes; use futures_core::Stream; use futures_util::TryStreamExt as _; +use secrecy::{ExposeSecret as _, SecretString}; use serde::Serialize; use serde::de::DeserializeOwned; use url::Url; @@ -18,9 +19,9 @@ use crate::ids::{ TimeEntryId, UserId, VersionId, WikiTitle, }; use crate::model::{ - BareCollection, Collection, attachment, custom_field, enumeration, issue, issue_category, - issue_status, journal, membership, project, query, relation, role, search, time_entry, tracker, - upload, user, version, wiki, + BareCollection, Collection, attachment, custom_field, enumeration, introspection, issue, + issue_category, issue_status, journal, membership, project, query, relation, role, search, + time_entry, tracker, upload, user, version, wiki, }; use crate::page::{Limits, Page}; use crate::retry::{self, RetryPolicy}; @@ -477,6 +478,32 @@ impl Scoped<'_> { self.read_json(resp, "response").await } + /// Send `form` as `application/x-www-form-urlencoded`, returning the raw + /// response for the caller to decode (or discard). Not covered by the + /// retry policy for a different reason than the JSON POST helpers: it + /// exists only for the OAuth introspection/revocation endpoints, which + /// this crate's retry rule (idempotent verbs only) already excludes as a + /// `POST`. + async fn post_form( + &self, + path: &str, + form: &B, + ) -> crate::Result { + let url = self.build_url(path, None)?; + let template = self.credential.apply(self.inner.http.post(url)).form(form); + self.send_with_retry(&http::Method::POST, &template).await + } + + /// Like [`Self::post_form`], decoding the response body as JSON. + async fn post_form_json( + &self, + path: &str, + form: &B, + ) -> crate::Result { + let resp = self.post_form(path, form).await?; + self.read_json(resp, "response").await + } + pub(crate) async fn put_json(&self, path: &str, body: &B) -> crate::Result<()> { let url = self.build_url(path, None)?; let template = self.credential.apply(self.inner.http.put(url)).json(body); @@ -727,6 +754,70 @@ impl Scoped<'_> { Ok(env.user) } + /// `POST /oauth/introspect` (RFC 7662). The scoping credential must be + /// `Credential::Basic { user: client_id, pass: client_secret }` for the + /// confidential OAuth client registered for token introspection — this + /// is one of the two methods on this type where the scoping credential + /// is not an end-user identity. + /// + /// # Errors + /// + /// Returns [`Error::Unauthorized`]/[`Error::Forbidden`] if the scoping + /// client credentials are rejected, [`Error::NotFound`] if the + /// introspection route is unmounted (Redmine's `allow_token_introspection` + /// defaults to `false`), or a transport/decode error otherwise. Never + /// itself an error for an inactive/expired/unknown token — that is + /// `Introspection::active`. + pub async fn introspect_token( + &self, + token: &SecretString, + ) -> crate::Result { + #[derive(Serialize)] + struct Form<'a> { + token: &'a str, + token_type_hint: &'a str, + } + self.post_form_json( + "oauth/introspect", + &Form { + token: token.expose_secret(), + token_type_hint: "access_token", + }, + ) + .await + } + + /// `POST /oauth/revoke` (RFC 7009). Same scoping-credential requirement + /// as [`Self::introspect_token`]. Per RFC 7009 a `200` is success whether + /// or not `token` was a token this client ever issued — revoking an + /// unknown token is not an error. + /// + /// # Errors + /// + /// Returns [`Error::Unauthorized`]/[`Error::Forbidden`] if the scoping + /// client credentials are rejected, or a transport error otherwise. + pub async fn revoke_token( + &self, + token: &SecretString, + hint: Option<&str>, + ) -> crate::Result<()> { + #[derive(Serialize)] + struct Form<'a> { + token: &'a str, + #[serde(skip_serializing_if = "Option::is_none")] + token_type_hint: Option<&'a str>, + } + self.post_form( + "oauth/revoke", + &Form { + token: token.expose_secret(), + token_type_hint: hint, + }, + ) + .await?; + Ok(()) + } + /// `GET /projects.json`. /// /// # Errors diff --git a/crates/redmine-client/src/model/introspection.rs b/crates/redmine-client/src/model/introspection.rs new file mode 100644 index 0000000..50cbf32 --- /dev/null +++ b/crates/redmine-client/src/model/introspection.rs @@ -0,0 +1,137 @@ +//! `POST /oauth/introspect` (RFC 7662). + +use serde::Deserialize; + +/// An RFC 7662 introspection response. Permissive by design (every field but +/// `active` is `Option`, no `deny_unknown_fields`): Doorkeeper's exact field +/// set is not part of any contract this crate can rely on. +/// +/// Holds no secret, so a derived `Debug` is fine — see the `debug_has_no_manual_impl` +/// test, which exists precisely so a future field addition has to think about +/// that before assuming it. +#[non_exhaustive] +#[derive(Debug, Clone, Deserialize)] +pub struct Introspection { + /// Whether the token is currently active. + pub active: bool, + /// Space-delimited scope string, if the token carries one. + #[serde(default)] + pub scope: Option, + /// The OAuth client the token was issued to. + #[serde(default)] + pub client_id: Option, + /// The resource owner's username, if the token is user-bound. + #[serde(default)] + pub username: Option, + /// The subject identifier (resource owner), if present. + #[serde(default)] + pub sub: Option, + /// Expiry, as Unix seconds. + #[serde(default)] + pub exp: Option, + /// Issued-at, as Unix seconds. + #[serde(default)] + pub iat: Option, + /// The token type, e.g. `"Bearer"`. + #[serde(default)] + pub token_type: Option, +} + +impl Introspection { + /// Split [`Self::scope`] on ASCII whitespace, per RFC 7662's + /// space-delimited scope string. Empty when `scope` is `None` or empty. + #[must_use] + pub fn scopes(&self) -> Vec<&str> { + self.scope + .as_deref() + .map(|s| s.split_ascii_whitespace().collect()) + .unwrap_or_default() + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + + #[test] + fn debug_has_no_manual_impl_and_holds_no_secret() { + // A canary, not a security control: `Introspection` carries no + // secret today, so the derived `Debug` is safe. If a future field + // addition changes that, this test's assertion (not its existence) + // must be revisited alongside a manual `Debug`. + let value = Introspection { + active: true, + scope: Some("view_issues edit_issues".to_string()), + client_id: Some("client-1".to_string()), + username: Some("alice".to_string()), + sub: Some("5".to_string()), + exp: Some(1_000_000), + iat: Some(999_000), + token_type: Some("Bearer".to_string()), + }; + let rendered = format!("{value:?}"); + assert!(rendered.contains("active")); + } + + #[test] + fn scopes_splits_on_whitespace() { + let value = Introspection { + active: true, + scope: Some("view_issues edit_issues\tadd_issue_notes".to_string()), + client_id: None, + username: None, + sub: None, + exp: None, + iat: None, + token_type: None, + }; + assert_eq!( + value.scopes(), + vec!["view_issues", "edit_issues", "add_issue_notes"] + ); + } + + #[test] + fn scopes_empty_when_scope_is_none() { + let value = Introspection { + active: false, + scope: None, + client_id: None, + username: None, + sub: None, + exp: None, + iat: None, + token_type: None, + }; + assert!(value.scopes().is_empty()); + } + + #[test] + fn deserializes_a_minimal_inactive_response() { + let value: Introspection = serde_json::from_str(r#"{"active":false}"#).unwrap(); + assert!(!value.active); + assert!(value.scope.is_none()); + } + + #[test] + fn deserializes_a_full_active_response_and_ignores_unknown_fields() { + let value: Introspection = serde_json::from_str( + r#"{ + "active": true, + "scope": "view_issues edit_issues", + "client_id": "abc", + "username": "alice", + "sub": "5", + "exp": 1999999999, + "iat": 1999999000, + "token_type": "Bearer", + "some_future_field": "ignored" + }"#, + ) + .unwrap(); + assert!(value.active); + assert_eq!(value.username.as_deref(), Some("alice")); + assert_eq!(value.exp, Some(1_999_999_999)); + } +} diff --git a/crates/redmine-client/src/model/mod.rs b/crates/redmine-client/src/model/mod.rs index 9e23374..de5be94 100644 --- a/crates/redmine-client/src/model/mod.rs +++ b/crates/redmine-client/src/model/mod.rs @@ -8,6 +8,7 @@ pub mod attachment; pub mod custom_field; pub mod enumeration; +pub mod introspection; pub mod issue; pub mod issue_category; pub mod issue_status; diff --git a/crates/redmine-client/tests/oauth.rs b/crates/redmine-client/tests/oauth.rs new file mode 100644 index 0000000..52a6a3f --- /dev/null +++ b/crates/redmine-client/tests/oauth.rs @@ -0,0 +1,169 @@ +//! `Scoped::introspect_token`/`revoke_token`: RFC 7662/7009 wire shape, +//! Basic-auth client credentials, and error mapping. +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::items_after_statements +)] + +mod support; + +use redmine_client::{Credential, Error}; +use secrecy::SecretString; +use wiremock::matchers::{basic_auth, body_string_contains, method, path}; +use wiremock::{Mock, ResponseTemplate}; + +fn client_credential() -> Credential { + Credential::Basic { + user: "introspect-client".to_string(), + pass: SecretString::from("introspect-secret"), + } +} + +#[tokio::test] +async fn introspect_token_sends_basic_auth_and_the_exact_form_body() { + let (server, client) = support::mock_redmine().await; + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .and(basic_auth("introspect-client", "introspect-secret")) + .and(body_string_contains("token=the-token")) + .and(body_string_contains("token_type_hint=access_token")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "active": true, + "scope": "view_issues edit_issues", + "username": "alice", + }))) + .mount(&server) + .await; + + let cred = client_credential(); + let result = client + .as_user(&cred) + .introspect_token(&SecretString::from("the-token")) + .await + .expect("introspection should succeed"); + + assert!(result.active); + assert_eq!(result.username.as_deref(), Some("alice")); + assert_eq!(result.scopes(), vec!["view_issues", "edit_issues"]); +} + +#[tokio::test] +async fn introspect_token_parses_an_inactive_response() { + let (server, client) = support::mock_redmine().await; + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({ "active": false })), + ) + .mount(&server) + .await; + + let cred = client_credential(); + let result = client + .as_user(&cred) + .introspect_token(&SecretString::from("unknown-token")) + .await + .expect("an inactive token is a 200, not an error"); + assert!(!result.active); +} + +#[tokio::test] +async fn introspect_token_maps_401_to_unauthorized() { + let (server, client) = support::mock_redmine().await; + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + + let cred = client_credential(); + let err = client + .as_user(&cred) + .introspect_token(&SecretString::from("the-token")) + .await + .expect_err("401 should be an error"); + assert!(matches!(err, Error::Unauthorized)); +} + +#[tokio::test] +async fn introspect_token_maps_404_to_not_found() { + let (server, client) = support::mock_redmine().await; + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .respond_with(ResponseTemplate::new(404)) + .mount(&server) + .await; + + let cred = client_credential(); + let err = client + .as_user(&cred) + .introspect_token(&SecretString::from("the-token")) + .await + .expect_err("404 should be an error"); + assert!(matches!(err, Error::NotFound)); +} + +#[tokio::test] +async fn introspect_token_error_never_contains_the_token() { + let (server, client) = support::mock_redmine().await; + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .respond_with(ResponseTemplate::new(500).set_body_string("not json")) + .mount(&server) + .await; + + let cred = client_credential(); + const TOKEN: &str = "super-secret-access-token-xyz"; + let err = client + .as_user(&cred) + .introspect_token(&SecretString::from(TOKEN)) + .await + .expect_err("500 should be an error"); + let display = format!("{err}"); + let debug = format!("{err:?}"); + assert!( + !display.contains(TOKEN), + "Display leaked the token: {display}" + ); + assert!(!debug.contains(TOKEN), "Debug leaked the token: {debug}"); +} + +#[tokio::test] +async fn revoke_token_sends_basic_auth_and_the_form_body() { + let (server, client) = support::mock_redmine().await; + Mock::given(method("POST")) + .and(path("/oauth/revoke")) + .and(basic_auth("introspect-client", "introspect-secret")) + .and(body_string_contains("token=the-token")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + let cred = client_credential(); + client + .as_user(&cred) + .revoke_token(&SecretString::from("the-token"), Some("access_token")) + .await + .expect("revocation should succeed"); +} + +#[tokio::test] +async fn revoke_token_of_an_unknown_token_is_still_success() { + // RFC 7009: revoking a token the server does not recognise is a 200, not + // an error. + let (server, client) = support::mock_redmine().await; + Mock::given(method("POST")) + .and(path("/oauth/revoke")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + let cred = client_credential(); + client + .as_user(&cred) + .revoke_token(&SecretString::from("never-issued"), None) + .await + .expect("revoking an unknown token must not be an error"); +} diff --git a/crates/ruprogress-mcp/Cargo.toml b/crates/ruprogress-mcp/Cargo.toml index 0949456..4ac516d 100644 --- a/crates/ruprogress-mcp/Cargo.toml +++ b/crates/ruprogress-mcp/Cargo.toml @@ -37,6 +37,7 @@ dotenvy = { workspace = true } secrecy = { workspace = true } url = { workspace = true } rand = { workspace = true } +sha2 = { workspace = true } [dev-dependencies] rmcp = { workspace = true, features = ["client", "transport-streamable-http-client-reqwest"] } diff --git a/crates/ruprogress-mcp/src/auth/mod.rs b/crates/ruprogress-mcp/src/auth/mod.rs index 1837f9c..bd6aa40 100644 --- a/crates/ruprogress-mcp/src/auth/mod.rs +++ b/crates/ruprogress-mcp/src/auth/mod.rs @@ -3,4 +3,5 @@ //! `legacy_per_user`/`oauth` siblings alongside `legacy`. pub(crate) mod legacy; +pub(crate) mod oauth; pub(crate) mod per_user; diff --git a/crates/ruprogress-mcp/src/auth/oauth.rs b/crates/ruprogress-mcp/src/auth/oauth.rs new file mode 100644 index 0000000..178ae26 --- /dev/null +++ b/crates/ruprogress-mcp/src/auth/oauth.rs @@ -0,0 +1,651 @@ +//! `AuthMode::OAuth`: bearer-token extraction, RFC 7662 introspection with a +//! digest-keyed cache, and the axum middleware that issues the `401`/`503` +//! challenge before any MCP request reaches [`crate::server::RedmineMcp::scoped`]. +//! +//! See `plans/phase-6b1-bearer-introspection.md` decisions O1–O2, O4, O7, +//! B6–B10. + +use std::collections::{BTreeSet, HashMap}; +use std::sync::{Arc, Mutex, PoisonError}; +use std::time::{Duration, Instant}; + +use axum::extract::{Request, State}; +use axum::middleware::Next; +use axum::response::{IntoResponse, Response}; +use http::request::Parts; +use http::{HeaderValue, StatusCode, header}; +use redmine_client::model::introspection::Introspection; +use redmine_client::{Credential, RedmineClient}; +use rmcp::ErrorData as McpError; +use rmcp::RoleServer; +use rmcp::service::RequestContext; +use secrecy::{ExposeSecret as _, SecretString}; +use sha2::{Digest as _, Sha256}; +use url::Url; + +use crate::config::OAuthConfig; + +/// Clock-skew allowance for `exp` comparisons (B6). Not configurable: a knob +/// nobody can set correctly, guarding a 5-second window. +const CLOCK_SKEW_SECS: i64 = 5; + +/// How long an `active:false` (or expired) introspection result is cached +/// (O4). +const NEGATIVE_CACHE_TTL: Duration = Duration::from_secs(5); + +/// Bounded cache capacity (O4). A new entry is simply not cached once full, +/// rather than evicting something a concurrent request might still need. +const CACHE_CAPACITY: usize = 1024; + +/// A bearer token's maximum accepted length (B9): far above any real +/// Doorkeeper token, far below anything worth forwarding. +const MAX_TOKEN_LEN: usize = 4096; + +/// Validated per-request identity. Rides inside `http::request::Parts`, +/// which rmcp moves whole into the JSON-RPC request's extensions — so this +/// type's `Debug` must never print the token (B7). +#[derive(Clone)] +pub(crate) struct AuthContext { + pub(crate) token: SecretString, + pub(crate) subject: Option, + pub(crate) scopes: Arc>, + pub(crate) expires_at: Option, +} + +impl std::fmt::Debug for AuthContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("AuthContext") + .field("subject", &self.subject) + .field("scopes", &self.scopes) + .field("expires_at", &self.expires_at) + .field("token", &"") + .finish() + } +} + +/// Why [`TokenVerifier::verify`] rejected a token, distinct from a bearer +/// *header* parsing failure (see [`BearerError`]). +#[derive(Debug, Clone, Copy)] +pub(crate) enum AuthError { + /// The token introspected as inactive, or active with an `exp` inside + /// the clock-skew window. + InvalidToken(&'static str), + /// Introspection could not be reached, or answered with a transport/5xx + /// failure. Never the caller's fault (O7). + Unavailable, + /// Introspection rejected *this server's* client credentials, or the + /// route is unmounted. Also never the caller's fault (O7). + Misconfigured, +} + +#[derive(Clone)] +enum CacheValue { + Valid(AuthContext), + Invalid, +} + +struct CachedEntry { + value: CacheValue, + /// Monotonic cache expiry: `min(configured ttl, exp - now - skew)` for a + /// positive entry, a fixed 5s for a negative one. + expires_at: Instant, +} + +/// Introspects and caches bearer tokens against Redmine's Doorkeeper +/// endpoint, scoped to the confidential introspection client (O3): this +/// reuses `RedmineClient`'s configured TLS/CA/timeout settings rather than a +/// second `reqwest::Client`. +pub(crate) struct TokenVerifier { + client: RedmineClient, + credential: Credential, + ttl: Duration, + cache: Mutex>, +} + +impl std::fmt::Debug for TokenVerifier { + // Manual: never print a cache key (a token digest) or entry. + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let len = self + .cache + .lock() + .unwrap_or_else(PoisonError::into_inner) + .len(); + f.debug_struct("TokenVerifier") + .field("ttl", &self.ttl) + .field("cache_len", &len) + .finish_non_exhaustive() + } +} + +impl TokenVerifier { + pub(crate) fn new(client: RedmineClient, oauth: &OAuthConfig) -> Self { + Self { + client, + credential: Credential::Basic { + user: oauth.introspect_client_id.clone(), + pass: oauth.introspect_client_secret.clone(), + }, + ttl: oauth.token_cache_ttl, + cache: Mutex::new(HashMap::new()), + } + } + + fn digest(token: &str) -> [u8; 32] { + let mut hasher = Sha256::new(); + hasher.update(token.as_bytes()); + hasher.finalize().into() + } + + /// `exp` is necessary but not sufficient (B6): a token within + /// [`CLOCK_SKEW_SECS`] of its expiry is treated as already expired. + fn is_expired(exp: Option) -> bool { + let Some(exp) = exp else { + return false; + }; + let now = chrono::Utc::now().timestamp(); + exp <= now.saturating_add(CLOCK_SKEW_SECS) + } + + /// Verify `token`: a cache hit, or a fresh RFC 7662 introspection on a + /// miss. Never caches a transport/misconfiguration failure (O4). + pub(crate) async fn verify(&self, token: SecretString) -> Result { + let digest = Self::digest(token.expose_secret()); + + if !self.ttl.is_zero() + && let Some(cached) = self.cache_get(digest) + { + return cached; + } + + let introspection = self.introspect(&token).await?; + let result = Self::context_from(&introspection, token); + if !self.ttl.is_zero() { + self.cache_put(digest, &introspection, &result); + } + result + } + + /// Remove a cached entry for `token`, if any. Unused until 6b2's + /// `POST /revoke` purges the entry it revokes. + #[allow( + dead_code, + reason = "consumed by 6b2's POST /revoke; wired here so the whole oauth/* surface lands in one review" + )] + pub(crate) fn purge(&self, token: &SecretString) { + let digest = Self::digest(token.expose_secret()); + self.cache + .lock() + .unwrap_or_else(PoisonError::into_inner) + .remove(&digest); + } + + async fn introspect(&self, token: &SecretString) -> Result { + let scoped = self.client.as_user(&self.credential); + match scoped.introspect_token(token).await { + Ok(introspection) => Ok(introspection), + Err(redmine_client::Error::Unauthorized | redmine_client::Error::Forbidden) => { + tracing::error!( + "introspection rejected this server's own client credentials; check \ + REDMINE_INTROSPECT_CLIENT_ID and REDMINE_INTROSPECT_CLIENT_SECRET" + ); + Err(AuthError::Misconfigured) + } + Err(redmine_client::Error::NotFound) => { + tracing::error!( + "introspection endpoint not found; Redmine's allow_token_introspection \ + setting must be enabled for this OAuth application" + ); + Err(AuthError::Misconfigured) + } + Err(error) => { + tracing::warn!(%error, "introspection request failed"); + Err(AuthError::Unavailable) + } + } + } + + fn context_from( + introspection: &Introspection, + token: SecretString, + ) -> Result { + if !introspection.active { + return Err(AuthError::InvalidToken("token is not active")); + } + if Self::is_expired(introspection.exp) { + return Err(AuthError::InvalidToken("token is expired")); + } + let scopes = introspection + .scopes() + .into_iter() + .map(ToString::to_string) + .collect(); + let subject = introspection + .sub + .clone() + .or_else(|| introspection.username.clone()); + Ok(AuthContext { + token, + subject, + scopes: Arc::new(scopes), + expires_at: introspection.exp, + }) + } + + fn cache_get(&self, digest: [u8; 32]) -> Option> { + let mut cache = self.cache.lock().unwrap_or_else(PoisonError::into_inner); + let entry = cache.get(&digest)?; + if entry.expires_at <= Instant::now() { + cache.remove(&digest); + return None; + } + match &entry.value { + CacheValue::Valid(context) => { + // Re-check `exp` on every hit (B6): the cache's own TTL is + // already capped by it, but this is the belt to that + // suspenders. + if Self::is_expired(context.expires_at) { + cache.remove(&digest); + return None; + } + Some(Ok(context.clone())) + } + CacheValue::Invalid => Some(Err(AuthError::InvalidToken("token is not active"))), + } + } + + fn cache_put( + &self, + digest: [u8; 32], + introspection: &Introspection, + result: &Result, + ) { + let ttl = match result { + Ok(_) => { + let mut ttl = self.ttl; + if let Some(exp) = introspection.exp { + let now = chrono::Utc::now().timestamp(); + let remaining_secs = exp.saturating_sub(now).saturating_sub(CLOCK_SKEW_SECS); + // Already rejected by `context_from` when this would be + // non-positive, but a defensive `return` costs nothing. + if remaining_secs <= 0 { + return; + } + let remaining = + Duration::from_secs(u64::try_from(remaining_secs).unwrap_or(u64::MAX)); + ttl = ttl.min(remaining); + } + ttl + } + Err(AuthError::InvalidToken(_)) => NEGATIVE_CACHE_TTL, + // Transport/misconfiguration failures are never cached (O4): the + // next request should get a fresh chance to see the outage end. + Err(AuthError::Unavailable | AuthError::Misconfigured) => return, + }; + if ttl.is_zero() { + return; + } + + let mut cache = self.cache.lock().unwrap_or_else(PoisonError::into_inner); + let now = Instant::now(); + cache.retain(|_, entry| entry.expires_at > now); + if cache.len() >= CACHE_CAPACITY && !cache.contains_key(&digest) { + // Degrade gracefully: skip caching this one rather than evict a + // still-live entry another concurrent request may need. + return; + } + let value = match result { + Ok(context) => CacheValue::Valid(context.clone()), + Err(_) => CacheValue::Invalid, + }; + cache.insert( + digest, + CachedEntry { + value, + expires_at: now.checked_add(ttl).unwrap_or(now), + }, + ); + } +} + +/// The `AuthMode::OAuth` arm of `RedmineMcp::scoped` (O2). Reads the +/// `AuthContext` the middleware inserted into `Parts.extensions` — mirroring +/// how `auth::per_user::credential` reads inbound headers from the same +/// `Parts` — and fails closed (an internal error, never a fallback) if it is +/// absent, which can only happen if a future refactor mounts the MCP route +/// without [`layer`]. +/// +/// # Errors +/// +/// Returns an [`McpError`] if the request did not arrive over HTTP, or if +/// the bearer-auth middleware did not run for it. +pub(crate) fn scoped<'c>( + client: &'c RedmineClient, + ctx: &RequestContext, +) -> Result, McpError> { + let parts = ctx.extensions.get::().ok_or_else(|| { + McpError::internal_error( + "oauth auth mode requires the HTTP transport (no request headers available)", + None, + ) + })?; + let auth = parts.extensions.get::().ok_or_else(|| { + McpError::internal_error( + "oauth auth mode requires the bearer-auth middleware, which did not run for this \ + request", + None, + ) + })?; + tracing::debug!(subject = ?auth.subject, "oauth request"); + Ok(client.as_user_owned(Credential::Bearer(auth.token.clone()))) +} + +/// The pre-built `WWW-Authenticate` challenge (B10): a pure function of +/// config, computed once at startup so every `401` response carries a +/// byte-identical value. +#[derive(Debug, Clone)] +pub(crate) struct Challenge { + /// The full challenge with no `error` parameter, e.g. + /// `Bearer resource_metadata="https://host/.well-known/oauth-protected-resource/mcp"`. + base: String, +} + +impl Challenge { + pub(crate) fn build(base_url: &Url, mcp_path: &str) -> Self { + let raw = base_url.as_str(); + let trimmed = raw.strip_suffix('/').unwrap_or(raw); + Self { + base: format!( + r#"Bearer resource_metadata="{trimmed}/.well-known/oauth-protected-resource{mcp_path}""# + ), + } + } + + fn header_value(&self, error: Option<&str>) -> HeaderValue { + let value = match error { + Some(error) => format!(r#"{}, error="{error}""#, self.base), + None => self.base.clone(), + }; + HeaderValue::from_str(&value).unwrap_or_else(|_| HeaderValue::from_static("Bearer")) + } +} + +/// Why [`extract_bearer`] rejected the `Authorization` header, distinct from +/// [`AuthError`] (which only ever applies once a token has been extracted). +#[derive(Debug)] +enum BearerError { + /// No `Authorization` header at all: the ordinary "client has no token + /// yet" case, so the challenge carries no `error` parameter. + Missing, + /// A header was present but did not parse as a well-formed bearer + /// token (B9): duplicated, wrong scheme, malformed, oversized, or + /// non-visible-ASCII. + Malformed(&'static str), +} + +/// Extracts and validates the inbound bearer token per B9. Never falls back +/// to a different scheme or a second header value: header smuggling through +/// a misconfigured proxy is the realistic attack this guards against. +fn extract_bearer(headers: &http::HeaderMap) -> Result { + let mut values = headers.get_all(header::AUTHORIZATION).iter(); + let Some(value) = values.next() else { + return Err(BearerError::Missing); + }; + if values.next().is_some() { + return Err(BearerError::Malformed( + "Authorization header must be sent exactly once", + )); + } + let text = value.to_str().map_err(|_| { + BearerError::Malformed( + "Authorization header contains invalid (non-visible-ASCII) characters", + ) + })?; + let Some((scheme, token)) = text.split_once(' ') else { + return Err(BearerError::Malformed( + "Authorization header must be \"Bearer \"", + )); + }; + if !scheme.eq_ignore_ascii_case("bearer") { + return Err(BearerError::Malformed( + "Authorization header must use the Bearer scheme", + )); + } + if token.trim() != token { + return Err(BearerError::Malformed( + "Authorization header must have exactly one space between the scheme and the token", + )); + } + if token.is_empty() { + return Err(BearerError::Malformed( + "Authorization header token must not be empty", + )); + } + if token.len() > MAX_TOKEN_LEN { + return Err(BearerError::Malformed( + "Authorization header token exceeds the maximum accepted length", + )); + } + if !token.bytes().all(|b| (0x21..=0x7e).contains(&b)) { + return Err(BearerError::Malformed( + "Authorization header token contains invalid (non-visible-ASCII) characters", + )); + } + Ok(SecretString::from(token.to_string())) +} + +/// Shared state for [`require_bearer`]: cheap to clone (two `Arc` bumps), +/// which is what `axum::middleware::from_fn_with_state` requires. +pub(crate) type AuthState = (Arc, Arc); + +/// The `oauth` mode's authentication middleware (O1): every request must +/// carry a valid bearer token, including `initialize`. Never mounted outside +/// this auth mode, and only ever on the MCP route (O8) — see the `SECURITY:` +/// comment in `transport::http::router`. +pub(crate) async fn require_bearer( + State((verifier, challenge)): State, + mut req: Request, + next: Next, +) -> Response { + let token = match extract_bearer(req.headers()) { + Ok(token) => token, + Err(BearerError::Missing) => return challenge_response(&challenge, None), + Err(BearerError::Malformed(reason)) => { + tracing::warn!(reason, "rejected a malformed Authorization header"); + return challenge_response(&challenge, Some("invalid_request")); + } + }; + + match verifier.verify(token).await { + Ok(context) => { + req.extensions_mut().insert(context); + next.run(req).await + } + Err(AuthError::InvalidToken(reason)) => { + tracing::warn!(reason, "rejected an invalid bearer token"); + challenge_response(&challenge, Some("invalid_token")) + } + Err(AuthError::Unavailable) => { + tracing::warn!("introspection is unavailable; rejecting with 503, not 401"); + unavailable_response() + } + Err(AuthError::Misconfigured) => { + // The specific misconfiguration was already logged at ERROR by + // `TokenVerifier::introspect`. + unavailable_response() + } + } +} + +fn challenge_response(challenge: &Challenge, error: Option<&str>) -> Response { + let mut response = (StatusCode::UNAUTHORIZED, "unauthorized").into_response(); + response + .headers_mut() + .insert(header::WWW_AUTHENTICATE, challenge.header_value(error)); + response +} + +/// `503` + `Retry-After` (O7): introspection being broken is never the +/// caller's fault, so this must never look like an invalid-token `401`. +fn unavailable_response() -> Response { + let mut response = (StatusCode::SERVICE_UNAVAILABLE, "service unavailable").into_response(); + response + .headers_mut() + .insert(header::RETRY_AFTER, HeaderValue::from_static("5")); + response +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + + fn headers(pairs: &[(&str, &str)]) -> http::HeaderMap { + let mut builder = http::Request::builder(); + for (name, value) in pairs { + builder = builder.header(*name, *value); + } + let (parts, ()) = builder.body(()).expect("request should build").into_parts(); + parts.headers + } + + fn token_of(result: &Result) -> &str { + result.as_ref().expect("expected Ok").expose_secret() + } + + #[test] + fn missing_header_is_missing() { + let h = headers(&[]); + assert!(matches!(extract_bearer(&h), Err(BearerError::Missing))); + } + + #[test] + fn duplicated_header_is_malformed() { + let h = headers(&[ + ("authorization", "Bearer one"), + ("authorization", "Bearer two"), + ]); + assert!(matches!(extract_bearer(&h), Err(BearerError::Malformed(_)))); + } + + #[test] + fn non_bearer_scheme_is_malformed() { + let h = headers(&[("authorization", "Basic dXNlcjpwYXNz")]); + assert!(matches!(extract_bearer(&h), Err(BearerError::Malformed(_)))); + } + + #[test] + fn empty_token_is_malformed() { + let h = headers(&[("authorization", "Bearer ")]); + assert!(matches!(extract_bearer(&h), Err(BearerError::Malformed(_)))); + } + + #[test] + fn oversized_token_is_malformed() { + let value = format!("Bearer {}", "a".repeat(MAX_TOKEN_LEN + 1)); + let h = headers(&[("authorization", &value)]); + assert!(matches!(extract_bearer(&h), Err(BearerError::Malformed(_)))); + } + + #[test] + fn extra_whitespace_is_malformed() { + let h = headers(&[("authorization", "Bearer leading-double-space")]); + assert!(matches!(extract_bearer(&h), Err(BearerError::Malformed(_)))); + } + + #[test] + fn non_ascii_token_is_malformed() { + let mut map = http::HeaderMap::new(); + map.insert( + header::AUTHORIZATION, + HeaderValue::from_bytes(b"Bearer abc\xC3\xA9def").expect("opaque bytes construct"), + ); + assert!(matches!( + extract_bearer(&map), + Err(BearerError::Malformed(_)) + )); + } + + #[test] + fn happy_path_is_case_insensitive_on_scheme() { + let h = headers(&[("authorization", "bearer the-token")]); + let result = extract_bearer(&h); + assert_eq!(token_of(&result), "the-token"); + } + + #[test] + fn is_expired_treats_none_as_not_expired() { + assert!(!TokenVerifier::is_expired(None)); + } + + #[test] + fn is_expired_respects_the_clock_skew_window() { + let now = chrono::Utc::now().timestamp(); + assert!(TokenVerifier::is_expired(Some(now + CLOCK_SKEW_SECS - 1))); + assert!(!TokenVerifier::is_expired(Some(now + CLOCK_SKEW_SECS + 60))); + } + + #[test] + fn challenge_has_no_error_param_by_default() { + let base = "https://mcp.example.com".parse().expect("valid url"); + let challenge = Challenge::build(&base, "/mcp"); + let value = challenge.header_value(None); + assert_eq!( + value.to_str().unwrap(), + r#"Bearer resource_metadata="https://mcp.example.com/.well-known/oauth-protected-resource/mcp""# + ); + } + + #[test] + fn challenge_strips_a_trailing_slash_from_the_base() { + let base = "https://mcp.example.com/".parse().expect("valid url"); + let challenge = Challenge::build(&base, "/mcp"); + let value = challenge.header_value(None); + assert!(!value.to_str().unwrap().contains("com//")); + } + + #[test] + fn challenge_appends_the_error_parameter() { + let base = "https://mcp.example.com".parse().expect("valid url"); + let challenge = Challenge::build(&base, "/mcp"); + let value = challenge.header_value(Some("invalid_token")); + assert!( + value + .to_str() + .unwrap() + .ends_with(r#", error="invalid_token""#) + ); + } + + #[test] + fn debug_of_auth_context_never_contains_the_token() { + const TOKEN: &str = "super-secret-access-token"; + let context = AuthContext { + token: SecretString::from(TOKEN), + subject: Some("alice".to_string()), + scopes: Arc::new(BTreeSet::new()), + expires_at: None, + }; + let rendered = format!("{context:?}"); + assert!(!rendered.contains(TOKEN)); + assert!(rendered.contains("alice")); + } + + #[test] + fn debug_of_token_verifier_never_contains_a_key() { + let client = redmine_client::RedmineClientBuilder::new( + "https://example.com".parse().expect("valid url"), + ) + .build() + .expect("client should build"); + let oauth = OAuthConfig { + base_url: "https://example.com".parse().expect("valid url"), + introspect_client_id: "client-id".to_string(), + introspect_client_secret: SecretString::from("client-secret"), + token_cache_ttl: Duration::from_mins(1), + }; + let verifier = TokenVerifier::new(client, &oauth); + let rendered = format!("{verifier:?}"); + assert!(!rendered.contains("client-secret")); + assert!(rendered.contains("cache_len")); + } +} diff --git a/crates/ruprogress-mcp/src/config.rs b/crates/ruprogress-mcp/src/config.rs index 19b929a..b9e191a 100644 --- a/crates/ruprogress-mcp/src/config.rs +++ b/crates/ruprogress-mcp/src/config.rs @@ -72,14 +72,27 @@ pub enum AuthMode { trust: ProxyTrust, audit_identity: bool, }, - /// Not yet implemented. + /// Each request carries its own Redmine credential as an + /// `Authorization: Bearer` token, validated by RFC 7662 introspection — + /// see `auth::oauth`. OAuth(OAuthConfig), } -/// OAuth settings; not yet implemented, only the base URL is validated so far. +/// OAuth settings. #[derive(Debug, Clone)] pub struct OAuthConfig { + /// `REDMINE_MCP_BASE_URL`: this server's own public base URL, embedded in + /// the `WWW-Authenticate` challenge and OAuth discovery documents. pub base_url: Url, + /// `REDMINE_INTROSPECT_CLIENT_ID`: the confidential OAuth client id used + /// to authenticate introspection/revocation requests to Redmine. + pub introspect_client_id: String, + /// `REDMINE_INTROSPECT_CLIENT_SECRET`/`_FILE`. + pub introspect_client_secret: SecretString, + /// `REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS`: how long a positive + /// introspection result is cached, capped further by the token's own + /// `exp`. `0` disables caching. + pub token_cache_ttl: Duration, } /// Zero-sized proof that `REDMINE_PER_USER_TRUST_PROXY=true` was set. Only @@ -678,6 +691,97 @@ fn parse_http(vars: &EnvMap) -> Result { }) } +/// Validates `REDMINE_MCP_BASE_URL`: absolute, scheme `http`/`https`, no +/// userinfo, no query, no fragment. A trailing slash is not rejected — Url +/// forces one on a root path regardless — but every consumer that appends a +/// path to this value (the `WWW-Authenticate` challenge, discovery documents) +/// must strip it first; see `auth::oauth`'s challenge builder. +/// +/// Non-`https` on a non-loopback host is a startup `WARN`, not an error: +/// local development over loopback (and Docker-internal hostnames) is +/// legitimate, but this value is embedded in a challenge every OAuth client +/// receives, so a production deployment serving it over plain HTTP is worth +/// flagging. +fn parse_oauth_base_url(vars: &EnvMap) -> Result { + const VAR: &str = "REDMINE_MCP_BASE_URL"; + let raw = required( + vars, + VAR, + "oauth auth mode requires the server's own public base URL", + )?; + let url: Url = raw.parse().map_err(|_| ConfigError::Invalid { + var: VAR, + expected: "an absolute http(s) URL with no userinfo, query, or fragment", + because: "the value could not be parsed as a URL".to_string(), + })?; + if url.scheme() != "http" && url.scheme() != "https" { + return Err(ConfigError::Invalid { + var: VAR, + expected: "an http or https URL", + because: format!("scheme {:?} is not http/https", url.scheme()), + }); + } + if !url.username().is_empty() || url.password().is_some() { + return Err(ConfigError::Invalid { + var: VAR, + expected: "a URL without embedded credentials", + because: "the URL contains userinfo".to_string(), + }); + } + if url.query().is_some() { + return Err(ConfigError::Invalid { + var: VAR, + expected: "a URL without a query string", + because: "the URL contains a query string".to_string(), + }); + } + if url.fragment().is_some() { + return Err(ConfigError::Invalid { + var: VAR, + expected: "a URL without a fragment", + because: "the URL contains a fragment".to_string(), + }); + } + let is_loopback = url.host_str().is_some_and(|host| { + host.eq_ignore_ascii_case("localhost") + || host.parse::().is_ok_and(|ip| ip.is_loopback()) + }); + if url.scheme() == "http" && !is_loopback { + tracing::warn!( + base_url = %url, + "REDMINE_MCP_BASE_URL uses http on a non-loopback host: this value is embedded in \ + the WWW-Authenticate challenge and every OAuth discovery document, so it should be \ + https in production" + ); + } + Ok(url) +} + +const DEFAULT_OAUTH_TOKEN_CACHE_TTL_SECONDS: u64 = 60; +const MAX_OAUTH_TOKEN_CACHE_TTL_SECONDS: u64 = 3600; + +/// `REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS`: `0..=3600`, default `60`. `0` +/// disables caching entirely — unlike [`positive_u64`], `0` is an accepted +/// value here. +fn parse_token_cache_ttl(vars: &EnvMap) -> Result { + const VAR: &str = "REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS"; + let Some(raw) = optional(vars, VAR) else { + return Ok(Duration::from_secs(DEFAULT_OAUTH_TOKEN_CACHE_TTL_SECONDS)); + }; + let invalid = |because: String| ConfigError::Invalid { + var: VAR, + expected: "a number of seconds between 0 and 3600", + because, + }; + let seconds: u64 = raw + .parse() + .map_err(|_| invalid("the value could not be parsed as a number".to_string()))?; + if seconds > MAX_OAUTH_TOKEN_CACHE_TTL_SECONDS { + return Err(invalid(format!("{seconds} is longer than an hour"))); + } + Ok(Duration::from_secs(seconds)) +} + fn parse_auth(vars: &EnvMap, transport: TransportKind) -> Result { let mode = optional(vars, "REDMINE_AUTH_MODE").unwrap_or_else(|| "legacy".to_string()); match mode.as_str() { @@ -709,17 +813,34 @@ fn parse_auth(vars: &EnvMap, transport: TransportKind) -> Result { - let raw = required( + let base_url = parse_oauth_base_url(vars)?; + let introspect_client_id = required( vars, - "REDMINE_MCP_BASE_URL", - "oauth auth mode requires the server's own public base URL", + "REDMINE_INTROSPECT_CLIENT_ID", + "oauth auth mode requires the OAuth client credentials used to introspect \ + bearer tokens", )?; - let base_url = raw.parse().map_err(|_| ConfigError::Invalid { - var: "REDMINE_MCP_BASE_URL", - expected: "a valid URL", - because: "the value could not be parsed as a URL".to_string(), - })?; - Ok(AuthMode::OAuth(OAuthConfig { base_url })) + let introspect_client_secret = secret(vars, "REDMINE_INTROSPECT_CLIENT_SECRET")? + .ok_or(ConfigError::Missing { + var: "REDMINE_INTROSPECT_CLIENT_SECRET", + because: "oauth auth mode requires the OAuth client credentials used to \ + introspect bearer tokens (REDMINE_INTROSPECT_CLIENT_SECRET or \ + REDMINE_INTROSPECT_CLIENT_SECRET_FILE)", + })?; + if transport == TransportKind::Stdio { + return Err(ConfigError::Conflict { + because: "oauth auth requires per-request bearer tokens and a 401 challenge \ + to discover them, neither of which the stdio transport has" + .to_string(), + }); + } + let token_cache_ttl = parse_token_cache_ttl(vars)?; + Ok(AuthMode::OAuth(OAuthConfig { + base_url, + introspect_client_id, + introspect_client_secret, + token_cache_ttl, + })) } other => Err(ConfigError::Invalid { var: "REDMINE_AUTH_MODE", @@ -1015,12 +1136,21 @@ impl Config { "mcp_path": http.mcp_path, }), }; + let oauth = match &self.auth { + AuthMode::OAuth(cfg) => Some(json!({ + "base_url": cfg.base_url.as_str(), + "introspect_client_id": cfg.introspect_client_id, + "token_cache_ttl_seconds": cfg.token_cache_ttl.as_secs(), + })), + AuthMode::Legacy { .. } | AuthMode::LegacyPerUser { .. } => None, + }; json!({ "redmine": { "url_host": self.redmine.url.host_str(), "ssl_verify": self.redmine.ssl_verify, }, "auth_mode": self.auth_mode_label(), + "oauth": oauth, "transport": transport, "read_only_mode": self.read_only, "plugin_flags": self.plugin_flags_json(), @@ -1164,12 +1294,24 @@ mod tests { assert!(matches!(err, ConfigError::Conflict { .. })); } + fn valid_oauth() -> EnvMap { + map(&[ + ("REDMINE_URL", "https://redmine.example.com"), + ("REDMINE_AUTH_MODE", "oauth"), + ("REDMINE_MCP_BASE_URL", "http://localhost:3040"), + ("REDMINE_INTROSPECT_CLIENT_ID", "introspect-client"), + ("REDMINE_INTROSPECT_CLIENT_SECRET", "introspect-secret"), + ]) + } + #[test] fn oauth_without_base_url_is_missing() { let vars = map(&[ ("REDMINE_URL", "https://redmine.example.com"), ("REDMINE_AUTH_MODE", "oauth"), ]); + // Base-url is checked before the introspection credentials and the + // transport conflict, so this reports Missing even on stdio. let err = Config::from_map(&vars, TransportKind::Stdio).unwrap_err(); assert!(matches!( err, @@ -1181,16 +1323,194 @@ mod tests { } #[test] - fn oauth_with_base_url_succeeds() { - let vars = map(&[ - ("REDMINE_URL", "https://redmine.example.com"), - ("REDMINE_AUTH_MODE", "oauth"), - ("REDMINE_MCP_BASE_URL", "http://localhost:3040"), - ]); - let config = Config::from_map(&vars, TransportKind::Stdio).expect("should be valid"); + fn oauth_with_valid_config_succeeds_on_http() { + let config = + Config::from_map(&valid_oauth(), TransportKind::Http).expect("should be valid"); assert!(matches!(config.auth, AuthMode::OAuth(_))); } + #[test] + fn oauth_without_introspect_client_id_is_missing() { + let mut vars = valid_oauth(); + vars.remove("REDMINE_INTROSPECT_CLIENT_ID"); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Missing { + var: "REDMINE_INTROSPECT_CLIENT_ID", + .. + } + )); + } + + #[test] + fn oauth_without_introspect_client_secret_is_missing() { + let mut vars = valid_oauth(); + vars.remove("REDMINE_INTROSPECT_CLIENT_SECRET"); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Missing { + var: "REDMINE_INTROSPECT_CLIENT_SECRET", + .. + } + )); + } + + #[test] + fn oauth_with_both_client_secret_and_secret_file_is_conflict() { + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_INTROSPECT_CLIENT_SECRET_FILE".to_string(), + "/tmp/whatever".to_string(), + ); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!(err, ConfigError::Conflict { .. })); + } + + #[test] + fn oauth_on_stdio_transport_is_conflict() { + let err = Config::from_map(&valid_oauth(), TransportKind::Stdio).unwrap_err(); + assert!(matches!(err, ConfigError::Conflict { .. })); + } + + #[test] + fn oauth_base_url_with_userinfo_is_invalid() { + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_MCP_BASE_URL".to_string(), + "http://user:pass@localhost:3040".to_string(), + ); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Invalid { + var: "REDMINE_MCP_BASE_URL", + .. + } + )); + } + + #[test] + fn oauth_base_url_with_query_is_invalid() { + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_MCP_BASE_URL".to_string(), + "http://localhost:3040/?x=1".to_string(), + ); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Invalid { + var: "REDMINE_MCP_BASE_URL", + .. + } + )); + } + + #[test] + fn oauth_base_url_with_fragment_is_invalid() { + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_MCP_BASE_URL".to_string(), + "http://localhost:3040/#frag".to_string(), + ); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Invalid { + var: "REDMINE_MCP_BASE_URL", + .. + } + )); + } + + #[test] + fn oauth_base_url_relative_is_invalid() { + let mut vars = valid_oauth(); + vars.insert("REDMINE_MCP_BASE_URL".to_string(), "not a url".to_string()); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Invalid { + var: "REDMINE_MCP_BASE_URL", + .. + } + )); + } + + #[test] + fn oauth_base_url_with_non_http_scheme_is_invalid() { + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_MCP_BASE_URL".to_string(), + "ftp://localhost:3040".to_string(), + ); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Invalid { + var: "REDMINE_MCP_BASE_URL", + .. + } + )); + } + + #[test] + fn oauth_token_cache_ttl_defaults_to_60() { + let config = + Config::from_map(&valid_oauth(), TransportKind::Http).expect("should be valid"); + let AuthMode::OAuth(oauth) = &config.auth else { + panic!("expected oauth mode"); + }; + assert_eq!(oauth.token_cache_ttl, Duration::from_mins(1)); + } + + #[test] + fn oauth_token_cache_ttl_zero_is_accepted() { + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS".to_string(), + "0".to_string(), + ); + let config = Config::from_map(&vars, TransportKind::Http).expect("should be valid"); + let AuthMode::OAuth(oauth) = &config.auth else { + panic!("expected oauth mode"); + }; + assert_eq!(oauth.token_cache_ttl, Duration::ZERO); + } + + #[test] + fn oauth_token_cache_ttl_out_of_range_is_invalid() { + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS".to_string(), + "3601".to_string(), + ); + let err = Config::from_map(&vars, TransportKind::Http).unwrap_err(); + assert!(matches!( + err, + ConfigError::Invalid { + var: "REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS", + .. + } + )); + } + + #[test] + fn oauth_redacted_summary_omits_the_client_secret() { + const SECRET: &str = "super-secret-introspect-value"; + let mut vars = valid_oauth(); + vars.insert( + "REDMINE_INTROSPECT_CLIENT_SECRET".to_string(), + SECRET.to_string(), + ); + let config = Config::from_map(&vars, TransportKind::Http).expect("should be valid"); + let summary = config.redacted_summary().to_string(); + assert!(!summary.contains(SECRET)); + assert!(summary.contains("introspect-client")); + } + #[test] fn ssl_verify_false_is_accepted() { let mut vars = valid_legacy(); diff --git a/crates/ruprogress-mcp/src/server.rs b/crates/ruprogress-mcp/src/server.rs index 65ad14f..3e74a82 100644 --- a/crates/ruprogress-mcp/src/server.rs +++ b/crates/ruprogress-mcp/src/server.rs @@ -10,6 +10,7 @@ use rmcp::service::RequestContext; use rmcp::{ErrorData as McpError, RoleServer, ServerHandler, tool_handler}; use crate::attachments::AttachmentStore; +use crate::auth::oauth::TokenVerifier; use crate::config::{AuthMode, Config, SchemaDialect}; use crate::readonly::write_tools; use crate::tools::schema; @@ -25,6 +26,10 @@ pub(crate) struct ServerInner { pub(crate) client: RedmineClient, pub(crate) config: Config, pub(crate) attachments: Arc, + /// `Some` only in `AuthMode::OAuth`, built once here so + /// `transport::http::router`'s middleware and `health`'s future + /// introspection probe share one verifier — and therefore one cache. + pub(crate) oauth_verifier: Option>, } impl RedmineMcp { @@ -56,16 +61,29 @@ impl RedmineMcp { route.attr.input_schema = Arc::new(schema::to_portable(&route.attr.input_schema)); } } + let oauth_verifier = match &config.auth { + AuthMode::OAuth(oauth) => Some(Arc::new(TokenVerifier::new(client.clone(), oauth))), + AuthMode::Legacy { .. } | AuthMode::LegacyPerUser { .. } => None, + }; Self { inner: Arc::new(ServerInner { client, config, attachments, + oauth_verifier, }), tool_router: router, } } + /// The `oauth`-mode token verifier, for `transport::http::router` to + /// mount its bearer-auth middleware with. `None` in every other auth + /// mode. + #[must_use] + pub(crate) fn verifier(&self) -> Option> { + self.inner.oauth_verifier.clone() + } + /// The attachment store handle, for `transport::http::router` to hand to /// the `/files/{uuid}` route as its `axum` state. #[must_use] @@ -82,10 +100,7 @@ impl RedmineMcp { AuthMode::LegacyPerUser { audit_identity, .. } => { crate::auth::per_user::scoped(&self.inner.client, ctx, *audit_identity) } - AuthMode::OAuth(_) => Err(McpError::internal_error( - "oauth auth is not yet implemented", - None, - )), + AuthMode::OAuth(_) => crate::auth::oauth::scoped(&self.inner.client, ctx), } } diff --git a/crates/ruprogress-mcp/src/transport/http.rs b/crates/ruprogress-mcp/src/transport/http.rs index f306042..316a31f 100644 --- a/crates/ruprogress-mcp/src/transport/http.rs +++ b/crates/ruprogress-mcp/src/transport/http.rs @@ -27,7 +27,8 @@ use tower_http::trace::TraceLayer; use uuid::Uuid; use crate::attachments::AttachmentStore; -use crate::config::HttpConfig; +use crate::auth::oauth; +use crate::config::{AuthMode, HttpConfig}; use crate::health::{self, HealthState}; use crate::server::RedmineMcp; @@ -62,6 +63,16 @@ pub fn router(server: RedmineMcp, cfg: &HttpConfig, service_ct: CancellationToke let attachments = server.attachments(); let health_state = HealthState::new(server.clone(), cfg.health_ttl); + let oauth_state = server.verifier().map(|verifier| { + let AuthMode::OAuth(oauth_config) = &server.inner.config.auth else { + unreachable!("verifier() is Some only in AuthMode::OAuth"); + }; + let challenge = Arc::new(oauth::Challenge::build( + &oauth_config.base_url, + &cfg.mcp_path, + )); + (verifier, challenge) + }); let mcp_service: StreamableHttpService = StreamableHttpService::new( move || Ok(server.clone()), Arc::new(SessionManager::default()), @@ -90,12 +101,23 @@ pub fn router(server: RedmineMcp, cfg: &HttpConfig, service_ct: CancellationToke // on a timer, and `TraceLayer` emits an ERROR for every 5xx — so a Redmine // outage would turn a `/readyz` poll into a flood. Suppressing only the // span (`Span::none()`) is not enough; the response events fire anyway. - let mcp_route = Router::new() + let mut mcp_route = Router::new() // `nest_service`, not `nest`: `nest` can drop the `Host` header hyper // synthesises from an HTTP/2 `:authority`, which is the input rmcp's // rebinding check reads. - .nest_service(&cfg.mcp_path, mcp_service) - .layer(TraceLayer::new_for_http()); + .nest_service(&cfg.mcp_path, mcp_service); + + // SECURITY: mounted on the MCP route only, and only in `oauth` mode. + // Every other route this router serves — `/livez`, `/readyz`, `/health`, + // `/files/{uuid}` (6a's L8 capability URL), and every future + // `/.well-known/*` discovery document — must stay reachable with no + // bearer token: RFC 9728 metadata has to be fetchable *before* a client + // has a token, and probes must not need a credential (O8). + if let Some(state) = oauth_state { + mcp_route = mcp_route.layer(middleware::from_fn_with_state(state, oauth::require_bearer)); + } + + let mcp_route = mcp_route.layer(TraceLayer::new_for_http()); let mut router = Router::new() .merge(mcp_route) diff --git a/crates/ruprogress-mcp/tests/auth_oauth.rs b/crates/ruprogress-mcp/tests/auth_oauth.rs new file mode 100644 index 0000000..ad22911 --- /dev/null +++ b/crates/ruprogress-mcp/tests/auth_oauth.rs @@ -0,0 +1,638 @@ +//! End-to-end `AuthMode::OAuth`: bearer extraction, RFC 7662 introspection, +//! the `401`/`503` challenge, and the token cache — over the real HTTP +//! router, against a wiremock Redmine that also stands in for Doorkeeper. +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::indexing_slicing, + clippy::items_after_statements +)] + +mod support; + +use reqwest::StatusCode; +use rmcp::ServiceExt as _; +use rmcp::model::CallToolRequestParams; +use rmcp::transport::StreamableHttpClientTransport; +use rmcp::transport::streamable_http_client::StreamableHttpClientTransportConfig; +use wiremock::matchers::{basic_auth, body_string_contains, header, method, path}; +use wiremock::{Mock, ResponseTemplate}; + +const CLIENT_ID: &str = "introspect-client"; +const CLIENT_SECRET: &str = "introspect-secret"; + +fn oauth_env(extra: &[(&'static str, &'static str)]) -> Vec<(&'static str, &'static str)> { + let mut env = vec![ + ("REDMINE_AUTH_MODE", "oauth"), + ("REDMINE_MCP_BASE_URL", "http://localhost:3040"), + ("REDMINE_INTROSPECT_CLIENT_ID", CLIENT_ID), + ("REDMINE_INTROSPECT_CLIENT_SECRET", CLIENT_SECRET), + ]; + env.extend_from_slice(extra); + env +} + +async fn mock_introspect( + redmine: &wiremock::MockServer, + token: &str, + body: serde_json::Value, + times: Option, +) { + let mut mock = Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .and(basic_auth(CLIENT_ID, CLIENT_SECRET)) + .and(body_string_contains(format!("token={token}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(body)); + if let Some(times) = times { + mock = mock.expect(times); + } + mock.mount(redmine).await; +} + +async fn mock_introspect_status(redmine: &wiremock::MockServer, token: &str, status: u16) { + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .and(body_string_contains(format!("token={token}"))) + .respond_with(ResponseTemplate::new(status)) + .mount(redmine) + .await; +} + +async fn mock_current_user_for( + redmine: &wiremock::MockServer, + token: &str, + id: u64, + login: &str, + times: Option, +) { + let mut mock = Mock::given(method("GET")) + .and(path("/my/account.json")) + .and(header("authorization", format!("Bearer {token}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "user": { + "id": id, + "login": login, + "firstname": "First", + "lastname": "Last", + "mail": format!("{login}@example.com"), + "created_on": "2024-01-01T00:00:00Z", + } + }))); + if let Some(times) = times { + mock = mock.expect(times); + } + mock.mount(redmine).await; +} + +fn initialize_body() -> serde_json::Value { + serde_json::json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2025-11-25", + "capabilities": {}, + "clientInfo": { "name": "oauth-test", "version": "0" } + } + }) +} + +fn raw_client() -> reqwest::Client { + reqwest::Client::builder() + .build() + .expect("build a test HTTP client") +} + +/// A raw `POST /mcp` `initialize` call, optionally with headers applied by +/// `configure`. Used for every test that asserts on the `401`/`503` +/// response itself rather than on tool-call behaviour. +async fn raw_initialize( + harness: &support::HttpHarness, + configure: impl FnOnce(reqwest::RequestBuilder) -> reqwest::RequestBuilder, +) -> reqwest::Response { + let request = raw_client() + .post(harness.mcp_url()) + .header("content-type", "application/json") + .header("accept", "application/json, text/event-stream") + .json(&initialize_body()); + configure(request) + .send() + .await + .expect("request should complete") +} + +async fn connect_with_token( + harness: &support::HttpHarness, + token: &str, +) -> rmcp::service::RunningService { + let config = StreamableHttpClientTransportConfig::with_uri(harness.mcp_url()) + .auth_header(token.to_string()); + let transport = StreamableHttpClientTransport::from_config(config); + ().serve(transport) + .await + .expect("client with a valid bearer token should connect") +} + +#[tokio::test] +async fn valid_token_reaches_redmine_verbatim_as_authorization_bearer() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN: &str = "the-users-access-token"; + mock_introspect( + &harness.redmine, + TOKEN, + serde_json::json!({ "active": true, "sub": "5", "username": "alice" }), + None, + ) + .await; + mock_current_user_for(&harness.redmine, TOKEN, 5, "alice", None).await; + + let client = connect_with_token(&harness, TOKEN).await; + let result = client + .call_tool(CallToolRequestParams::new("get_current_user")) + .await + .expect("get_current_user should succeed with a valid token"); + let text = result + .content + .iter() + .filter_map(rmcp::model::ContentBlock::as_text) + .map(|t| t.text.clone()) + .collect::>() + .join("\n"); + let body: serde_json::Value = + serde_json::from_str(text.lines().last().unwrap()).expect("last block is the JSON body"); + assert_eq!(body["login"], "alice"); + client.cancel().await.ok(); +} + +#[tokio::test] +async fn no_token_is_401_with_the_resource_metadata_challenge_and_zero_upstream_hits() { + let harness = support::http_harness(&oauth_env(&[])).await; + // Any hit at all — introspection included — is a bug: no request should + // leave this server before the header is checked. + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&harness.redmine) + .await; + + let response = raw_initialize(&harness, |r| r).await; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let challenge = response + .headers() + .get("www-authenticate") + .and_then(|v| v.to_str().ok()) + .expect("401 must carry WWW-Authenticate") + .to_string(); + assert!(challenge.starts_with("Bearer resource_metadata=")); + assert!(challenge.contains("/.well-known/oauth-protected-resource/mcp")); + assert!(!challenge.contains("error=")); +} + +#[tokio::test] +async fn unauthenticated_routes_are_never_401d_in_oauth_mode() { + // O8: the middleware is mounted on the MCP route only. Every one of + // these must stay reachable with no bearer token, even though `/mcp` + // itself requires one in this mode. `REDMINE_MCP_ALLOWED_ORIGINS` is set + // so the CORS preflight check below actually exercises the CORS layer + // (which answers `OPTIONS` itself, outside the auth middleware) rather + // than the no-CORS-configured default. + let harness = support::http_harness(&oauth_env(&[( + "REDMINE_MCP_ALLOWED_ORIGINS", + "https://app.example.com", + )])) + .await; + let client = raw_client(); + + for path in ["/livez", "/readyz", "/health"] { + let response = client + .get(harness.url(path)) + .send() + .await + .expect("request should complete"); + assert_ne!( + response.status(), + StatusCode::UNAUTHORIZED, + "{path} must not require a bearer token" + ); + } + + let files_response = client + .get(harness.url("/files/00000000-0000-0000-0000-000000000000")) + .send() + .await + .expect("request should complete"); + assert_ne!(files_response.status(), StatusCode::UNAUTHORIZED); + + let well_known_response = client + .get(harness.url("/.well-known/oauth-protected-resource/mcp")) + .send() + .await + .expect("request should complete"); + assert_ne!(well_known_response.status(), StatusCode::UNAUTHORIZED); + + let preflight_response = client + .request(reqwest::Method::OPTIONS, harness.mcp_url()) + .header("origin", "https://app.example.com") + .header("access-control-request-method", "POST") + .send() + .await + .expect("request should complete"); + assert_ne!(preflight_response.status(), StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn initialize_itself_requires_a_token() { + // Pinned deliberately (O1's consequence): unlike every other auth mode, + // `initialize` is not exempt in `oauth` mode. + let harness = support::http_harness(&oauth_env(&[])).await; + let response = raw_initialize(&harness, |r| r).await; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn a_duplicated_authorization_header_is_401_invalid_request() { + let harness = support::http_harness(&oauth_env(&[])).await; + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&harness.redmine) + .await; + + let response = raw_initialize(&harness, |r| { + r.header("authorization", "Bearer one") + .header("authorization", "Bearer two") + }) + .await; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let challenge = response + .headers() + .get("www-authenticate") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!(challenge.contains(r#"error="invalid_request""#)); +} + +#[tokio::test] +async fn a_non_bearer_scheme_is_401_invalid_request() { + let harness = support::http_harness(&oauth_env(&[])).await; + let response = raw_initialize(&harness, |r| { + r.header("authorization", "Basic dXNlcjpwYXNz") + }) + .await; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let challenge = response + .headers() + .get("www-authenticate") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!(challenge.contains(r#"error="invalid_request""#)); +} + +#[tokio::test] +async fn an_oversized_token_is_401_invalid_request_with_zero_upstream_hits() { + let harness = support::http_harness(&oauth_env(&[])).await; + Mock::given(method("POST")) + .and(path("/oauth/introspect")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&harness.redmine) + .await; + + let value = format!("Bearer {}", "a".repeat(5000)); + let response = raw_initialize(&harness, |r| r.header("authorization", value)).await; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn an_inactive_token_is_401_invalid_token() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN: &str = "revoked-or-unknown-token"; + mock_introspect( + &harness.redmine, + TOKEN, + serde_json::json!({ "active": false }), + None, + ) + .await; + + let response = raw_initialize(&harness, |r| { + r.header("authorization", format!("Bearer {TOKEN}")) + }) + .await; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let challenge = response + .headers() + .get("www-authenticate") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!(challenge.contains(r#"error="invalid_token""#)); +} + +#[tokio::test] +async fn an_active_but_expired_token_is_401_invalid_token() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN: &str = "expired-token"; + let past = chrono::Utc::now().timestamp() - 3600; + mock_introspect( + &harness.redmine, + TOKEN, + serde_json::json!({ "active": true, "exp": past }), + None, + ) + .await; + + let response = raw_initialize(&harness, |r| { + r.header("authorization", format!("Bearer {TOKEN}")) + }) + .await; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let challenge = response + .headers() + .get("www-authenticate") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!(challenge.contains(r#"error="invalid_token""#)); +} + +#[tokio::test] +async fn introspection_5xx_is_503_with_retry_after_never_401() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN: &str = "some-token"; + mock_introspect_status(&harness.redmine, TOKEN, 500).await; + + let response = raw_initialize(&harness, |r| { + r.header("authorization", format!("Bearer {TOKEN}")) + }) + .await; + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + assert_eq!( + response + .headers() + .get("retry-after") + .and_then(|v| v.to_str().ok()), + Some("5") + ); +} + +#[tokio::test] +async fn introspection_rejecting_our_own_client_credentials_is_503_never_401() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN: &str = "some-token"; + mock_introspect_status(&harness.redmine, TOKEN, 401).await; + + let response = raw_initialize(&harness, |r| { + r.header("authorization", format!("Bearer {TOKEN}")) + }) + .await; + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); +} + +#[tokio::test] +async fn introspection_route_not_found_is_503_never_401() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN: &str = "some-token"; + mock_introspect_status(&harness.redmine, TOKEN, 404).await; + + let response = raw_initialize(&harness, |r| { + r.header("authorization", format!("Bearer {TOKEN}")) + }) + .await; + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); +} + +#[tokio::test] +async fn two_tool_calls_with_the_same_token_perform_one_introspection() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN: &str = "cached-token"; + mock_introspect( + &harness.redmine, + TOKEN, + serde_json::json!({ "active": true, "sub": "5", "username": "alice" }), + Some(1), + ) + .await; + mock_current_user_for(&harness.redmine, TOKEN, 5, "alice", Some(2)).await; + + let client = connect_with_token(&harness, TOKEN).await; + for _ in 0..2 { + client + .call_tool(CallToolRequestParams::new("get_current_user")) + .await + .expect("get_current_user should succeed"); + } + client.cancel().await.ok(); + // Dropping the harness verifies wiremock's `expect(1)` on introspection. +} + +#[tokio::test] +async fn a_zero_cache_ttl_introspects_more_than_once_across_two_calls() { + // Unlike the cached case above (exactly one introspection for the whole + // session), `ttl=0` must re-introspect on every request that reaches the + // middleware. This only asserts "more than once", not an exact count, + // since the transport itself may issue more than one HTTP request per + // logical tool call (e.g. opening its event stream). + let harness = support::http_harness(&oauth_env(&[( + "REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS", + "0", + )])) + .await; + const TOKEN: &str = "uncached-token"; + mock_introspect( + &harness.redmine, + TOKEN, + serde_json::json!({ "active": true, "sub": "5", "username": "alice" }), + None, + ) + .await; + mock_current_user_for(&harness.redmine, TOKEN, 5, "alice", Some(2)).await; + + let client = connect_with_token(&harness, TOKEN).await; + for _ in 0..2 { + client + .call_tool(CallToolRequestParams::new("get_current_user")) + .await + .expect("get_current_user should succeed"); + } + client.cancel().await.ok(); + + let requests = harness + .redmine + .received_requests() + .await + .expect("request recording should be enabled"); + let introspections = requests + .iter() + .filter(|r| r.url.path() == "/oauth/introspect") + .count(); + assert!( + introspections >= 2, + "expected at least one introspection per tool call, got {introspections}" + ); +} + +#[tokio::test] +async fn two_concurrent_tokens_never_cross_contaminate_identity() { + let harness = support::http_harness(&oauth_env(&[])).await; + const TOKEN_ALICE: &str = "alice-token"; + const TOKEN_BOB: &str = "bob-token"; + mock_introspect( + &harness.redmine, + TOKEN_ALICE, + serde_json::json!({ "active": true, "sub": "1", "username": "alice" }), + None, + ) + .await; + mock_introspect( + &harness.redmine, + TOKEN_BOB, + serde_json::json!({ "active": true, "sub": "2", "username": "bob" }), + None, + ) + .await; + mock_current_user_for(&harness.redmine, TOKEN_ALICE, 1, "alice", None).await; + mock_current_user_for(&harness.redmine, TOKEN_BOB, 2, "bob", None).await; + + let alice = connect_with_token(&harness, TOKEN_ALICE).await; + let bob = connect_with_token(&harness, TOKEN_BOB).await; + + let mut calls = tokio::task::JoinSet::new(); + for _ in 0..5 { + let alice_result = alice.call_tool(CallToolRequestParams::new("get_current_user")); + let bob_result = bob.call_tool(CallToolRequestParams::new("get_current_user")); + let (a, b) = tokio::join!(alice_result, bob_result); + calls.spawn(async move { (a, b) }); + } + while let Some(joined) = calls.join_next().await { + let (a, b) = joined.expect("task should not panic"); + let a = a.expect("alice's call should succeed"); + let b = b.expect("bob's call should succeed"); + let a_text = a + .content + .iter() + .filter_map(rmcp::model::ContentBlock::as_text) + .map(|t| t.text.clone()) + .collect::>() + .join("\n"); + let b_text = b + .content + .iter() + .filter_map(rmcp::model::ContentBlock::as_text) + .map(|t| t.text.clone()) + .collect::>() + .join("\n"); + let a_body: serde_json::Value = + serde_json::from_str(a_text.lines().last().unwrap()).unwrap(); + let b_body: serde_json::Value = + serde_json::from_str(b_text.lines().last().unwrap()).unwrap(); + assert_eq!(a_body["login"], "alice"); + assert_eq!(b_body["login"], "bob"); + } + alice.cancel().await.ok(); + bob.cancel().await.ok(); +} + +#[derive(Clone, Default)] +struct SharedBuf(std::sync::Arc>>); + +impl std::io::Write for SharedBuf { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +impl<'a> tracing_subscriber::fmt::MakeWriter<'a> for SharedBuf { + type Writer = Self; + + fn make_writer(&'a self) -> Self::Writer { + self.clone() + } +} + +/// The access token, the introspection client secret, and the form body's +/// `token=` field must never appear in captured `TRACE` output, across a +/// success, a `401` (invalid token), and a `503` (introspection down). +/// Mirrors `auth_per_user.rs`'s equivalent test for the `X-Redmine-API-Key` +/// header. (Manually verified this assertion fails if a +/// `tracing::debug!(?parts)` is added to the bearer-auth middleware; removed +/// after confirming.) +#[tokio::test] +async fn no_secret_appears_in_captured_trace_logs() { + let buf = SharedBuf::default(); + let subscriber = tracing_subscriber::fmt() + .with_writer(buf.clone()) + .with_max_level(tracing::Level::TRACE) + .without_time() + .finish(); + let guard = tracing::subscriber::set_default(subscriber); + + let harness = support::http_harness(&oauth_env(&[])).await; + const SUCCESS_TOKEN: &str = "super-secret-success-path-token-0123456789"; + const INVALID_TOKEN: &str = "super-secret-invalid-path-token-abcdefghijk"; + const UNAVAILABLE_TOKEN: &str = "super-secret-unavailable-path-token-zyxwvu"; + + mock_introspect( + &harness.redmine, + SUCCESS_TOKEN, + serde_json::json!({ "active": true, "sub": "5", "username": "alice" }), + None, + ) + .await; + mock_current_user_for(&harness.redmine, SUCCESS_TOKEN, 5, "alice", None).await; + mock_introspect( + &harness.redmine, + INVALID_TOKEN, + serde_json::json!({ "active": false }), + None, + ) + .await; + mock_introspect_status(&harness.redmine, UNAVAILABLE_TOKEN, 500).await; + + let success_client = connect_with_token(&harness, SUCCESS_TOKEN).await; + success_client + .call_tool(CallToolRequestParams::new("get_current_user")) + .await + .expect("success path should succeed"); + success_client.cancel().await.ok(); + + let invalid_response = raw_initialize(&harness, |r| { + r.header("authorization", format!("Bearer {INVALID_TOKEN}")) + }) + .await; + assert_eq!(invalid_response.status(), StatusCode::UNAUTHORIZED); + + let unavailable_response = raw_initialize(&harness, |r| { + r.header("authorization", format!("Bearer {UNAVAILABLE_TOKEN}")) + }) + .await; + assert_eq!( + unavailable_response.status(), + StatusCode::SERVICE_UNAVAILABLE + ); + + drop(guard); + + let captured = String::from_utf8(buf.0.lock().unwrap().clone()).expect("logs are valid UTF-8"); + for secret in [ + SUCCESS_TOKEN, + INVALID_TOKEN, + UNAVAILABLE_TOKEN, + CLIENT_SECRET, + ] { + assert!( + !captured.contains(secret), + "captured TRACE log leaked a secret {secret:?}: {captured}" + ); + } + assert!( + !captured.contains("token="), + "captured TRACE log leaked the introspection form body: {captured}" + ); +} diff --git a/crates/ruprogress-mcp/tests/health.rs b/crates/ruprogress-mcp/tests/health.rs index 05e3661..07ec399 100644 --- a/crates/ruprogress-mcp/tests/health.rs +++ b/crates/ruprogress-mcp/tests/health.rs @@ -159,6 +159,8 @@ async fn readyz_reports_not_probed_when_the_server_owns_no_credential() { let harness = support::http_harness(&[ ("REDMINE_AUTH_MODE", "oauth"), ("REDMINE_MCP_BASE_URL", "http://localhost:3040"), + ("REDMINE_INTROSPECT_CLIENT_ID", "introspect-client"), + ("REDMINE_INTROSPECT_CLIENT_SECRET", "introspect-secret"), ]) .await; diff --git a/docs/adr/0008-oauth-resource-server.md b/docs/adr/0008-oauth-resource-server.md new file mode 100644 index 0000000..8d8c2ca --- /dev/null +++ b/docs/adr/0008-oauth-resource-server.md @@ -0,0 +1,83 @@ +# ADR 0008: `oauth` as a resource server, not an authorization server + +## Context + +`REDMINE_AUTH_MODE=oauth` needs an MCP client to authenticate its user against +Redmine's Doorkeeper OAuth2 provider and present the resulting access token to +this server. The design space has three shapes: (1) hand-roll bearer +validation inside the existing per-request choke point (`RedmineMcp::scoped`, +the shape `legacy-per-user` uses), (2) mint our own tokens as a second +authorization server (`oauth-proxy`, explicitly out of scope for v1.0), or (3) +be a pure OAuth *resource server*: validate a token someone else issued, by +asking its issuer. + +Two findings, verified against the vendored dependencies (`rmcp` 3.1.1, +`reqwest` 0.13.4), decided this ADR: + +- **rmcp ships no server-side OAuth support.** Its `auth` feature and + `transport::auth` module are an OAuth *client* (this process authenticating + *to* another MCP server) — `AuthClient`/`OAuthState`/`AuthorizationManager`. + Nothing in `streamable_http_server/` ever emits a `WWW-Authenticate` header. + Enabling that feature would pull in `oauth2` for zero server-side benefit. +- **Doorkeeper issues opaque tokens with no published JWKS.** There is nothing + to validate offline as a JWT. RFC 7662 introspection — a network call to + Redmine per (uncached) token — is the only validation Redmine supports. + +## Decisions + +- **O1 — an axum middleware in front of the MCP route, not a check inside + `scoped()`.** An MCP client discovers *that* it needs a token, and *where* + to get one, from an HTTP `401` carrying `WWW-Authenticate: Bearer + resource_metadata="…"`. A JSON-RPC-level error inside a `200` response (the + `legacy-per-user` shape) is invisible to that machinery — the client never + starts the OAuth flow. This means every MCP request, `initialize` included, + needs a token in this mode; `legacy-per-user`'s decision to leave + `initialize`/`tools/list` open does not carry over. +- **O2 — fail closed on a missing `AuthContext`.** The middleware inserts a + validated `AuthContext` into the request extensions; `scoped()`'s OAuth arm + reads it back out and returns `client.as_user_owned(Credential::Bearer(..))`, + or an internal error — never a fallback — if the extension is absent. A + future refactor that mounts the route without the middleware breaks loudly + on every tool call instead of running unauthenticated. +- **O3 — introspection and revocation reuse `RedmineClient`, scoped to + `Credential::Basic`, not a second `reqwest::Client`.** Doorkeeper is part of + the Redmine deployment: same origin, same TLS/custom-CA/mTLS configuration, + same timeouts, same connection pool. A second client would silently ignore + `REDMINE_SSL_VERIFY` and the CA settings — the classic way a TLS bypass + sneaks in. +- **O4 — the introspection cache is keyed by SHA-256 of the token, never the + token itself**, with a positive TTL capped by the token's own `exp` and a + short fixed TTL for a negative (`active:false`) result. A digest key means + the cache cannot leak a plaintext token in a core dump or a stray `Debug`. A + fingerprint collision in a log line (the shape `legacy-per-user`'s + `KeyFingerprint` uses) is a cosmetic annoyance; a collision in this cache + would serve one user's session to another, which is why that fingerprint is + not reused here. +- **O7 — introspection *unavailability* is `503`, never `401`.** `401` tells + the client "your token is bad, re-authorize"; sending it when this server + (or Redmine) is broken would send every connected client through a + pointless browser flow and hide an outage as a fleet-wide auth failure. + +## Consequences + +- **No audience binding.** Doorkeeper introspection returns no `aud`, so this + server cannot verify a presented token was issued *for it* rather than for + another OAuth client of the same Redmine. Any holder of any valid Redmine + access token can drive this server as that user — the confused-deputy shape + the MCP authorization spec warns about. Accepted: it is also the reference + server's behaviour, unfixable without upstream Redmine changes, and bounded + by the token granting no *more* than it already grants directly against + Redmine's own REST API. Documented in `docs/oauth-setup.md`. +- **Cache staleness on revocation.** A token revoked at Redmine stays usable + here for up to the cache's positive TTL (default 60s, capped further by the + token's own `exp`). Mitigated by the low default and, once phase 6b2 lands + `POST /revoke`, by that route purging the cache entry it revokes. +- **A new dependency, `sha2`,** on the auth path, for the sake of a cache key. + Small, audited, RustCrypto-maintained; the alternative (no cache) doubles + Redmine traffic per tool call, and a 64-bit keyed hash (as used for + `legacy-per-user`'s audit fingerprint) is the wrong trade-off for a cache + key rather than a log breadcrumb. +- **`legacy` and `legacy-per-user` are untouched.** The middleware is mounted + only when `RedmineMcp::verifier()` returns `Some` (i.e. only in `oauth` + mode), so neither existing mode's request shape, error shape, or tests + change. diff --git a/docs/configuration.md b/docs/configuration.md index c29a2d9..c0094de 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -22,7 +22,11 @@ real process environment. | `REDMINE_API_KEY_FILE` | yes, in `legacy` mode (alternative) | — | Path to a file containing the key (Docker/K8s secret mount). Exactly one trailing newline is trimmed. Setting both this and `REDMINE_API_KEY` is a `Conflict`. | | `REDMINE_PER_USER_TRUST_PROXY` | yes, in `legacy-per-user` mode | — | Must be exactly `true`. Absent/false refuses to start (`Missing`) — this is the operator's explicit attestation that a TLS-terminating proxy sits in front and does not forward client `X-Forwarded-Proto`. `legacy-per-user` is additionally rejected outright on the `stdio` transport (`Conflict`): there is no per-request header to carry a credential over stdio. Startup logs a `WARN` naming this assumption every time the mode is enabled. See `docs/legacy-per-user-auth.md`. | | `REDMINE_PER_USER_AUDIT_IDENTITY` | no | `false` | In `legacy-per-user` mode, logs one line per tool call naming a per-process, non-reversible fingerprint of the inbound `X-Redmine-API-Key` (never the key or a resolved Redmine identity) — see `docs/legacy-per-user-auth.md`. | -| `REDMINE_MCP_BASE_URL` | yes, in `oauth` mode | — | Public base URL of this MCP server. Full OAuth wiring is not implemented yet; only presence is validated here. | +| `REDMINE_MCP_BASE_URL` | yes, in `oauth` mode | — | This server's own public base URL, embedded in the `WWW-Authenticate` challenge and OAuth discovery documents. Must be absolute `http`/`https` with no userinfo, query, or fragment. `http` on a non-loopback host logs a `WARN` rather than erroring. | +| `REDMINE_INTROSPECT_CLIENT_ID` | yes, in `oauth` mode | — | The confidential OAuth client id used to authenticate RFC 7662 introspection requests to Redmine's Doorkeeper. | +| `REDMINE_INTROSPECT_CLIENT_SECRET` | yes, in `oauth` mode | — | Mutually exclusive with `REDMINE_INTROSPECT_CLIENT_SECRET_FILE`. | +| `REDMINE_INTROSPECT_CLIENT_SECRET_FILE` | yes, in `oauth` mode (alternative) | — | Path to a file containing the secret (Docker/K8s secret mount). Setting both this and `REDMINE_INTROSPECT_CLIENT_SECRET` is a `Conflict`. | +| `REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS` | no | `60` | 0–3600. How long a positive introspection result is cached, further capped by the token's own `exp`. `0` disables caching entirely. Has no upstream counterpart — a `ruprogress-mcp` addition, see `docs/oauth-setup.md`. | | `REDMINE_SSL_VERIFY` | no | `true` | `false` is accepted but logs a `WARN` — never silently downgrades without a trace. | | `REDMINE_MCP_READ_ONLY` | no | `false` | Removes every tool in `readonly::write_tools::ALL` from the router (hides from `tools/list` **and** rejects `tools/call`). | | `REDMINE_MCP_SCHEMA_DIALECT` | no | `strict` | One of `strict`, `portable`. `portable` inlines every `inputSchema`'s `$ref`/`$defs` and collapses `{"type":["T","null"]}` to `{"type":"T"}`, for clients (Google Vertex/Gemini) whose function-calling schema validator rejects the rich JSON Schema 2020-12 form. `outputSchema` is unaffected either way — see ADR 0007. | @@ -91,7 +95,6 @@ These are read by the upstream reference server but not by `REDMINE_USERNAME`, `REDMINE_PASSWORD`, `REDMINE_SSL_CERT`, `REDMINE_SSL_CLIENT_CERT`, -`REDMINE_INTROSPECT_CLIENT_ID`, `REDMINE_INTROSPECT_CLIENT_SECRET`(`_FILE`), `REDMINE_OAUTH_SCOPE_ENFORCEMENT`, `REDMINE_OAUTH_DISCOVERY_AS`, `REDMINE_MCP_SCOPES`, `REDMINE_MCP_JWT_SIGNING_KEY`(`_FILE`), `FASTMCP_HOME`, `REDMINE_MCP_ALLOWED_CLIENT_REDIRECT_URIS`, `REDMINE_OAUTH_CLIENT_ID`, diff --git a/docs/oauth-setup.md b/docs/oauth-setup.md new file mode 100644 index 0000000..8dd41f3 --- /dev/null +++ b/docs/oauth-setup.md @@ -0,0 +1,184 @@ +# OAuth (bearer-token) setup + +`REDMINE_AUTH_MODE=oauth` — each MCP client presents its own Redmine +Doorkeeper access token as `Authorization: Bearer`, and this server validates +it by RFC 7662 introspection before forwarding it upstream verbatim. Every +user acts as themselves, with their own token's permissions. + +Ported from the upstream reference server's `docs/oauth-setup.md` +(`jztan/redmine-mcp-server`, branch `develop`, captured 2026-08-09), trimmed +to what `ruprogress-mcp` actually implements today. + +## Status + +**Live now (phase 6b1):** bearer extraction, RFC 7662 introspection with a +digest-keyed cache, the `401 WWW-Authenticate: Bearer` / +`503 Retry-After` challenge, and forwarding the validated token to Redmine. + +**Not live yet:** + +- RFC 9728/RFC 8414 discovery documents (`/.well-known/oauth-protected-resource`, + `/.well-known/oauth-authorization-server`) — phase 6b2. +- `POST /revoke` and the introspection readiness probe behind `/readyz` — phase + 6b2. +- Per-tool scope enforcement (`REDMINE_OAUTH_SCOPE_ENFORCEMENT`, + `REDMINE_MCP_SCOPES`) and `tools/list` filtering — phase 6b3. Every token + that introspects as active and unexpired can call every tool today, + regardless of its `scope`. +- `oauth-proxy` mode (this server acting as an authorization server with + Dynamic Client Registration) — out of scope for v1.0 entirely. + +`/readyz` reports `redmine: "not_probed"` in `oauth` mode until 6b2 wires the +introspection readiness probe — this is expected, not a bug. + +## Step 1: Register an OAuth app for your users + +1. Log in to Redmine as admin → **Administration → Applications** → **New + Application**. +2. Fill in: + - **Name:** anything recognisable, e.g. `MCP Server`. + - **Redirect URI:** whatever your MCP client's own OAuth flow uses (this + server does not participate in that flow — it only validates the + resulting token). + - **Confidential:** Yes. +3. Save and note the **Client ID** and **Client Secret** — your MCP client + needs these to obtain tokens; this server never sees them. + +## Step 2: Register a Doorkeeper introspection client + +This server validates incoming bearer tokens by calling Doorkeeper's RFC 7662 +introspection endpoint (`POST /oauth/introspect`), authenticating itself with +its own confidential OAuth application. This can be the same application as +Step 1, or a separate one for independent credential rotation. + +### 2a. Register the application + +1. **Administration → Applications → New Application.** +2. Fill in: + - **Name:** `Redmine MCP Server (introspection)`. + - **Redirect URI:** `urn:ietf:wg:oauth:2.0:oob` (unused, but required by + the form — this client never performs an authorization-code flow). + - **Confidential:** Yes. + - **Scopes:** leave empty. +3. Save and note the **Client ID** and **Client Secret**. + +> If **Administration → Applications** 403s: enable **Administration → +> Settings → API → "Enable REST web service"** first (or, from a Rails +> console, `Setting.rest_api_enabled = "1"`). + +### 2b. Enable cross-app token introspection in Doorkeeper + +Redmine ships with `allow_token_introspection false` hard-coded, so even an +authenticated introspection client cannot introspect a token issued to a +*different* OAuth app — exactly the case here, since the introspection client +(Step 2a) must introspect tokens issued to the user-flow app (Step 1). + +**Edit Redmine's own initializer in place, on the Redmine server:** +`config/initializers/30-redmine.rb`. Find: + +```ruby + allow_token_introspection false +``` + +Replace it with: + +```ruby + allow_token_introspection do |_token, authorized_client, _resource_owner| + !authorized_client.nil? && authorized_client.confidential? + end +``` + +This grants introspection rights to any confidential OAuth client. Restart +Redmine after the change. + +> **Why edit `30-redmine.rb` directly instead of a new initializer?** Redmine +> wraps its Doorkeeper configuration in a `Rails.application.config.to_prepare +> do ... end` block, and `Doorkeeper.configure do ... end` **rebuilds the +> entire config from scratch** on every call rather than merging. A second +> `Doorkeeper.configure` block in your own initializer would silently wipe +> Redmine's `admin_authenticator`, `resource_owner_authenticator`, +> `grant_flows`, and everything else it sets — the visible symptom is +> **Administration → Applications** 403ing with a log line about +> `admin_authenticator being unconfigured`. Track this edit as a deployment +> patch (e.g. a Dockerfile `RUN sed -i ...` step) so it survives Redmine +> upgrades. + +### 2c. Verify + +```bash +curl -X POST "$REDMINE_URL/oauth/introspect" \ + -u "$REDMINE_INTROSPECT_CLIENT_ID:$REDMINE_INTROSPECT_CLIENT_SECRET" \ + -d "token=any-test-token&token_type_hint=access_token" +``` + +Expect `200 {"active":false}` — the `false` is correct (the token is made +up); what matters is the `200`. + +| Response | Meaning | +|---|---| +| `404 Page not found` | The route isn't mounted. Confirm step 2b was applied and Redmine was restarted. | +| `401 invalid_client` | The introspection client's id/secret are wrong, or it isn't confidential. | +| `200 {"active": false}` for a *known-valid* token | `allow_token_introspection` is returning falsy for this client — recheck 2b. | + +## Step 3: Configure ruprogress-mcp + +```bash +REDMINE_AUTH_MODE=oauth +REDMINE_URL=https://redmine.example.com +REDMINE_MCP_BASE_URL=https://mcp.example.com # this server's own public URL + +# Introspection client (Step 2) +REDMINE_INTROSPECT_CLIENT_ID= +REDMINE_INTROSPECT_CLIENT_SECRET= +# Or: REDMINE_INTROSPECT_CLIENT_SECRET_FILE=/run/secrets/redmine_introspect_client_secret + +# Optional: positive-introspection cache TTL, 0..3600 seconds (default 60). +# 0 disables caching. No upstream counterpart — a ruprogress-mcp addition. +# REDMINE_OAUTH_TOKEN_CACHE_TTL_SECONDS=60 +``` + +`oauth` mode requires `--transport http` (or `FASTMCP_TRANSPORT=http`): there +is no per-request header, and nothing to challenge with, on stdio. Missing +`REDMINE_INTROSPECT_CLIENT_ID`/`REDMINE_INTROSPECT_CLIENT_SECRET`, or `oauth` +on stdio, both refuse to start with a message naming the variable. + +## Step 4: Start and verify + +```bash +cargo run -- --transport http +``` + +```bash +# No token: 401 with a WWW-Authenticate challenge (the resource-metadata +# document it names is not served yet — that is 6b2). +curl -i http://127.0.0.1:8000/mcp + +# A real token, obtained however your MCP client obtains one: +curl -i http://127.0.0.1:8000/mcp \ + -H "Authorization: Bearer $ACCESS_TOKEN" \ + -H 'content-type: application/json' \ + -H 'accept: application/json, text/event-stream' \ + -d '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"curl","version":"0"}}}' +``` + +## Troubleshooting + +| Symptom | Cause | Fix | +|---|---|---| +| Every request is `401` with no `Authorization` header sent | Client isn't attaching a bearer token | Confirm the client actually obtained one from Redmine and is sending `Authorization: Bearer ` | +| `401` with `error="invalid_token"` | Token is inactive, revoked, or expired per introspection | Re-authorize; test directly with the Step 2c `curl` command against the real token | +| Every request is `503` | Introspection endpoint unreachable, misconfigured, or unmounted | Re-run Step 2c's verification `curl`; check `REDMINE_URL` and the introspection credentials | +| Server refuses to start naming `REDMINE_INTROSPECT_CLIENT_ID`/`_SECRET` | `oauth` mode requires them | Register the introspection client per Step 2 | +| Server refuses to start with a `Conflict` naming the transport | `oauth` mode was requested on `stdio` | Use `--transport http` | +| Token works directly against Redmine but not through this server | Wrong `REDMINE_URL`, or a proxy stripping the `Authorization` header | In Docker, use the internal hostname (e.g. `http://redmine:3000`); check for a proxy that drops `Authorization` | + +## Known limitation: no audience binding + +Doorkeeper introspection returns no `aud` claim, so this server cannot verify +that a presented token was issued *for it* specifically rather than for +another OAuth client of the same Redmine instance. Any holder of any valid +Redmine access token can drive this server as that user. This is also the +reference server's behaviour and is not fixable without changes upstream in +Redmine/Doorkeeper; it is bounded by the fact that the token grants no *more* +against this server than it already grants directly against Redmine's own +REST API. See `docs/adr/0008-oauth-resource-server.md`.