From a05c3a926dfa08ff292227924136cda3fa781d2e Mon Sep 17 00:00:00 2001 From: maltesander Date: Fri, 17 Jul 2026 13:27:37 +0200 Subject: [PATCH] fix: Respect the next link in UIF Entra backend requests (#859) * fix: respect next link in uif entra backend requests * fix: bound Entra group pagination, keep all memberships, add HTTP timeouts Follow-ups to the @odata.nextLink pagination fix in #859: - Add a 60s request / 15s connect timeout to every user-info-fetcher outbound HTTP backend (Keycloak, Entra, XFSC AAS) via a shared utils::http::client_builder(); reqwest otherwise waits indefinitely. - Keep every directory membership by requesting /memberOf instead of restricting to microsoft.graph.group, so directory roles and administrative units are still returned. - Cap @odata.nextLink following at 100 pages as a backstop against a cyclic or runaway link, warning if the group list may be truncated. - Bump the yanked spin 0.9.8 to 0.9.9 (matching #857, which only landed on release-26.7) so cargo-deny passes, and regenerate Cargo.nix for the new wiremock dev-dependency. * fix: address review feedback - Restructure the Entra group pagination loop guard-first: check the page cap and warn/break before doing the request, per review. - Reword the changelog entry for the pagination fix. --------- Co-authored-by: Lars Francke --- CHANGELOG.md | 9 + Cargo.lock | 68 +++++ Cargo.nix | 215 ++++++++++++++++ Cargo.toml | 1 + rust/user-info-fetcher/Cargo.toml | 3 + rust/user-info-fetcher/src/backend/entra.rs | 235 ++++++++++++++++-- .../user-info-fetcher/src/backend/keycloak.rs | 3 +- .../user-info-fetcher/src/backend/xfsc_aas.rs | 3 +- rust/user-info-fetcher/src/utils/http.rs | 19 +- 9 files changed, 535 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be649d77..0c6e599b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,14 @@ All notable changes to this project will be documented in this file. - BREAKING: Removed product-config machinery. This is a breaking change in terms of configuration. Users relying on the product-config `properties.yaml` file have to set these properties via the CRD. JsonPatches has been renamed to JsonPatch ([#842]). +- Apply a 60s request and 15s connect timeout to all user-info-fetcher outbound HTTP backends + (Keycloak, Entra, XFSC AAS), which previously waited indefinitely ([#859]). + +### Fixed + +- The user-info-fetcher Entra backend now follows Microsoft Graph's `@odata.nextLink` pagination + links - up to 100 times - to fetch 999 group memberships per result page, resulting in a maximum + of 99900 fetched memberships for a single user ([#859]). ### Removed @@ -38,6 +46,7 @@ All notable changes to this project will be documented in this file. [#842]: https://github.com/stackabletech/opa-operator/pull/842 [#843]: https://github.com/stackabletech/opa-operator/pull/843 [#851]: https://github.com/stackabletech/opa-operator/pull/851 +[#859]: https://github.com/stackabletech/opa-operator/pull/859 ## [26.3.0] - 2026-03-16 diff --git a/Cargo.lock b/Cargo.lock index 59158ebf..7aa2891f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -127,6 +127,16 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d902e3d592a523def97af8f317b08ce16b7ab854c1985a0c671e6f15cebc236" +[[package]] +name = "assert-json-diff" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47e4f2b81832e72834d7518d8487a0396a28cc408186a2e8854c0f98011faf12" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "async-broadcast" version = "0.7.2" @@ -678,6 +688,24 @@ dependencies = [ "syn 2.0.118", ] +[[package]] +name = "deadpool" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0be2b1d1d6ec8d846f05e137292d0b89133caf95ef33695424c09568bdd39b1b" +dependencies = [ + "deadpool-runtime", + "lazy_static", + "num_cpus", + "tokio", +] + +[[package]] +name = "deadpool-runtime" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b" + [[package]] name = "defmt" version = "1.1.1" @@ -1291,6 +1319,12 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" +[[package]] +name = "hermit-abi" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" + [[package]] name = "hmac" version = "0.12.1" @@ -2233,6 +2267,16 @@ dependencies = [ "libm", ] +[[package]] +name = "num_cpus" +version = "1.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91df4bbde75afed763b708b7eee1e8e7651e02d97f6d5dd763e89367e957b23b" +dependencies = [ + "hermit-abi", + "libc", +] + [[package]] name = "once_cell" version = "1.21.4" @@ -3467,6 +3511,7 @@ dependencies = [ "tracing", "url", "uuid", + "wiremock", ] [[package]] @@ -4615,6 +4660,29 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "wiremock" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08db1edfb05d9b3c1542e521aea074442088292f00b5f28e435c714a98f85031" +dependencies = [ + "assert-json-diff", + "base64", + "deadpool", + "futures", + "http", + "http-body-util", + "hyper", + "hyper-util", + "log", + "once_cell", + "regex", + "serde", + "serde_json", + "tokio", + "url", +] + [[package]] name = "wit-bindgen" version = "0.57.1" diff --git a/Cargo.nix b/Cargo.nix index 8c3eaad2..79cb1878 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -422,6 +422,34 @@ rec { "default" = [ "std" ]; }; }; + "assert-json-diff" = rec { + crateName = "assert-json-diff"; + version = "2.0.2"; + edition = "2018"; + sha256 = "04mg3w0rh3schpla51l18362hsirl23q93aisws2irrj32wg5r27"; + libName = "assert_json_diff"; + authors = [ + "David Pedersen " + ]; + dependencies = [ + { + name = "serde"; + packageId = "serde"; + } + { + name = "serde_json"; + packageId = "serde_json"; + } + ]; + devDependencies = [ + { + name = "serde"; + packageId = "serde"; + features = [ "derive" ]; + } + ]; + + }; "async-broadcast" = rec { crateName = "async-broadcast"; version = "0.7.2"; @@ -2102,6 +2130,62 @@ rec { ]; }; + "deadpool" = rec { + crateName = "deadpool"; + version = "0.12.3"; + edition = "2021"; + sha256 = "06wvsfyni5f04ia6jczgjnpkq4w91cnjjdz10mpq93gcsv8v3qhb"; + authors = [ + "Michael P. Jung " + ]; + dependencies = [ + { + name = "deadpool-runtime"; + packageId = "deadpool-runtime"; + } + { + name = "lazy_static"; + packageId = "lazy_static"; + } + { + name = "num_cpus"; + packageId = "num_cpus"; + } + { + name = "tokio"; + packageId = "tokio"; + features = [ "sync" ]; + } + ]; + devDependencies = [ + { + name = "tokio"; + packageId = "tokio"; + features = [ "macros" "rt" "rt-multi-thread" "time" ]; + } + ]; + features = { + "default" = [ "managed" "unmanaged" ]; + "rt_async-std_1" = [ "deadpool-runtime/async-std_1" ]; + "rt_tokio_1" = [ "deadpool-runtime/tokio_1" ]; + "serde" = [ "dep:serde" ]; + }; + resolvedDefaultFeatures = [ "default" "managed" "unmanaged" ]; + }; + "deadpool-runtime" = rec { + crateName = "deadpool-runtime"; + version = "0.1.4"; + edition = "2021"; + sha256 = "0arbchl5j887hcfvjy4gq38d32055s5cf7pkpmwn0lfw3ss6ca89"; + libName = "deadpool_runtime"; + authors = [ + "Michael P. Jung " + ]; + features = { + "async-std_1" = [ "dep:async-std_1" ]; + "tokio_1" = [ "dep:tokio_1" ]; + }; + }; "defmt" = rec { crateName = "defmt"; version = "1.1.1"; @@ -4048,6 +4132,22 @@ rec { sha256 = "1sjmpsdl8czyh9ywl3qcsfsq9a307dg4ni2vnlwgnzzqhc4y0113"; }; + "hermit-abi" = rec { + crateName = "hermit-abi"; + version = "0.5.2"; + edition = "2021"; + sha256 = "1744vaqkczpwncfy960j2hxrbjl1q01csm84jpd9dajbdr2yy3zw"; + libName = "hermit_abi"; + authors = [ + "Stefan Lankes" + ]; + features = { + "alloc" = [ "dep:alloc" ]; + "core" = [ "dep:core" ]; + "rustc-dep-of-std" = [ "core" "alloc" ]; + }; + resolvedDefaultFeatures = [ "default" ]; + }; "hmac" = rec { crateName = "hmac"; version = "0.12.1"; @@ -7269,6 +7369,28 @@ rec { }; resolvedDefaultFeatures = [ "default" "i128" "libm" "std" ]; }; + "num_cpus" = rec { + crateName = "num_cpus"; + version = "1.17.0"; + edition = "2015"; + sha256 = "0fxjazlng4z8cgbmsvbzv411wrg7x3hyxdq8nxixgzjswyylppwi"; + authors = [ + "Sean McArthur " + ]; + dependencies = [ + { + name = "hermit-abi"; + packageId = "hermit-abi"; + target = { target, features }: ("hermit" == target."os" or null); + } + { + name = "libc"; + packageId = "libc"; + target = { target, features }: (!(target."windows" or false)); + } + ]; + + }; "once_cell" = rec { crateName = "once_cell"; version = "1.21.4"; @@ -11533,6 +11655,12 @@ rec { features = [ "chrono" "git2" ]; } ]; + devDependencies = [ + { + name = "wiremock"; + packageId = "wiremock"; + } + ]; }; "stackable-operator" = rec { @@ -16875,6 +17003,93 @@ rec { "Microsoft" ]; + }; + "wiremock" = rec { + crateName = "wiremock"; + version = "0.6.5"; + edition = "2024"; + sha256 = "0cahz2c4lwaw8f7g5d805wlqh824fjhaw8g588akr6sxn3gixnq8"; + authors = [ + "Luca Palmieri " + ]; + dependencies = [ + { + name = "assert-json-diff"; + packageId = "assert-json-diff"; + } + { + name = "base64"; + packageId = "base64"; + } + { + name = "deadpool"; + packageId = "deadpool"; + } + { + name = "futures"; + packageId = "futures"; + } + { + name = "http"; + packageId = "http"; + } + { + name = "http-body-util"; + packageId = "http-body-util"; + } + { + name = "hyper"; + packageId = "hyper"; + features = [ "full" ]; + } + { + name = "hyper-util"; + packageId = "hyper-util"; + features = [ "tokio" "server" "http1" "http2" ]; + } + { + name = "log"; + packageId = "log"; + } + { + name = "once_cell"; + packageId = "once_cell"; + } + { + name = "regex"; + packageId = "regex"; + } + { + name = "serde"; + packageId = "serde"; + } + { + name = "serde_json"; + packageId = "serde_json"; + } + { + name = "tokio"; + packageId = "tokio"; + features = [ "rt" "macros" "net" ]; + } + { + name = "url"; + packageId = "url"; + } + ]; + devDependencies = [ + { + name = "serde"; + packageId = "serde"; + features = [ "derive" ]; + } + { + name = "tokio"; + packageId = "tokio"; + features = [ "macros" "rt-multi-thread" ]; + } + ]; + }; "wit-bindgen" = rec { crateName = "wit-bindgen"; diff --git a/Cargo.toml b/Cargo.toml index c7557a50..79cd785a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ tokio = { version = "1.52", features = ["full"] } tracing = "0.1" url = "2.5" uuid = "1.10" +wiremock = "0.6" [patch."https://github.com/stackabletech/operator-rs.git"] # stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } diff --git a/rust/user-info-fetcher/Cargo.toml b/rust/user-info-fetcher/Cargo.toml index a8260c66..a591ac29 100644 --- a/rust/user-info-fetcher/Cargo.toml +++ b/rust/user-info-fetcher/Cargo.toml @@ -34,5 +34,8 @@ tracing.workspace = true url.workspace = true uuid.workspace = true +[dev-dependencies] +wiremock.workspace = true + [build-dependencies] built.workspace = true diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 721e92ee..bc02da04 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -1,7 +1,6 @@ use std::{collections::HashMap, path::Path}; use hyper::StatusCode; -use reqwest::ClientBuilder; use serde::Deserialize; use snafu::{ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha2; @@ -98,6 +97,10 @@ struct UserMetadata { #[serde(rename_all = "camelCase")] struct GroupMembershipResponse { value: Vec, + + /// Set by Graph when further pages of group memberships are available. + #[serde(rename = "@odata.nextLink")] + next_link: Option, } #[derive(Deserialize)] @@ -106,6 +109,13 @@ struct GroupMembership { display_name: Option, } +/// Upper bound on the number of `@odata.nextLink` pages followed for a single group lookup. +/// +/// At Graph's maximum page size of 999 this covers ~99,900 memberships, hopefully well beyond any realistic +/// account. +/// It exists purely as a final protection against an infinite loop for whatever reason. +const MAX_GROUP_PAGES: usize = 100; + /// Entra backend with resolved credentials. /// /// This struct combines the CRD configuration with credentials loaded from the filesystem. @@ -141,7 +151,7 @@ impl ResolvedEntraBackend { path: client_secret_path.display().to_string(), })?; - let mut client_builder = ClientBuilder::new(); + let mut client_builder = utils::http::client_builder(); client_builder = utils::tls::configure_reqwest( &TlsClientDetails { tls: config.tls.clone(), @@ -215,22 +225,45 @@ impl ResolvedEntraBackend { } }; - let groups = send_json_request::( - self.http_client - .get(entra_backend.group_info(&user_info.id)) - .bearer_auth(&authn.access_token), - ) - .await - .with_context(|_| RequestUserGroupsSnafu { - username: user_info.user_principal_name.clone(), - user_id: user_info.id.clone(), - })? - .value; + let mut groups = Vec::new(); + let mut next_url = Some(entra_backend.group_info(&user_info.id)); + let mut pages_remaining = MAX_GROUP_PAGES; + + while let Some(url) = next_url { + // Bound the loop so we can't run into an infinite loop for any reason + if pages_remaining == 0 { + tracing::warn!( + user.id = %user_info.id, + max_pages = MAX_GROUP_PAGES, + "reached the maximum number of Entra group membership pages; \ + the resolved group list may be incomplete" + ); + + break; + } + pages_remaining -= 1; + + let response = send_json_request::( + self.http_client.get(url).bearer_auth(&authn.access_token), + ) + .await + .with_context(|_| RequestUserGroupsSnafu { + username: user_info.user_principal_name.clone(), + user_id: user_info.id.clone(), + })?; + + groups.extend(response.value.into_iter().filter_map(|g| g.display_name)); + + next_url = response + .next_link + .map(|next_link| entra_backend.next_page(&next_link)) + .transpose()?; + } Ok(UserInfo { id: Some(user_info.id), username: Some(user_info.user_principal_name), - groups: groups.into_iter().filter_map(|g| g.display_name).collect(), + groups, custom_attributes: user_info.attributes, }) } @@ -282,18 +315,175 @@ impl EntraBackend { user_info_url } + /// Requests the first page of the user's directory memberships. + /// + /// `memberOf` returns every directory object the user belongs to: groups as well as directory + /// roles and administrative units. pub fn group_info(&self, user: &str) -> Url { let mut user_info_url = self.user_info_endpoint_url.clone(); user_info_url.set_path(&format!("/v1.0/users/{user}/memberOf")); + // 999 is the largest page size Graph accepts, and keeps the number of round-trips down. + user_info_url.set_query(Some("$select=displayName&$top=999")); user_info_url } + + /// Rebases an `@odata.nextLink` onto the configured user info endpoint. + /// + /// Graph reports the link against its public `graph.microsoft.com` host, so following it + /// verbatim would ignore a configured endpoint and send the access token to whichever host + /// the response names. Only the path and query are taken from the link itself. + pub fn next_page(&self, next_link: &str) -> Result { + let next_link_url = Url::parse(next_link).context(BuildEntraEndpointFailedSnafu { + endpoint: next_link, + })?; + + let mut next_page_url = self.user_info_endpoint_url.clone(); + next_page_url.set_path(next_link_url.path()); + next_page_url.set_query(next_link_url.query()); + Ok(next_page_url) + } } #[cfg(test)] mod tests { use std::str::FromStr; + use stackable_operator::v2::types::kubernetes::SecretName; + use wiremock::{ + Mock, MockServer, ResponseTemplate, + matchers::{method, path, query_param}, + }; + use super::*; + use crate::UserInfoRequestById; + + const TENANT_ID: &str = "1234-5678-1234-5678"; + const USER_ID: &str = "8765-4321-8765-4321"; + + /// Builds a backend pointing at `mock_server`, bypassing [`ResolvedEntraBackend::resolve`] + /// so that no credentials have to be read from disk. + fn backend_for(mock_server: &MockServer) -> ResolvedEntraBackend { + let host = HostName::from_str(&mock_server.address().ip().to_string()).unwrap(); + + ResolvedEntraBackend { + config: v1alpha2::EntraBackend { + token_hostname: host.clone(), + user_info_hostname: host, + port: Some(mock_server.address().port()), + tenant_id: TENANT_ID.to_owned(), + tls: None, + client_credentials_secret: SecretName::from_str("entra-credentials").unwrap(), + }, + client_id: "client-id".to_owned(), + client_secret: "client-secret".to_owned(), + http_client: reqwest::Client::new(), + } + } + + /// Mocks the OAuth2 token and user metadata endpoints, which every `get_user_info` call hits + /// before it gets to the group memberships we actually care about. + async fn mock_token_and_user(mock_server: &MockServer) { + Mock::given(method("POST")) + .and(path(format!("/{TENANT_ID}/oauth2/v2.0/token"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "access_token": "access-token", + }))) + .mount(mock_server) + .await; + + Mock::given(method("GET")) + .and(path(format!("/v1.0/users/{USER_ID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": USER_ID, + "userPrincipalName": "alice@example.com", + }))) + .mount(mock_server) + .await; + } + + async fn get_user_info_by_id(backend: &ResolvedEntraBackend) -> UserInfo { + backend + .get_user_info(&UserInfoRequest::UserInfoRequestById(UserInfoRequestById { + id: USER_ID.to_owned(), + })) + .await + .unwrap() + } + + #[tokio::test] + async fn test_entra_follows_paginated_group_memberships() { + let mock_server = MockServer::start().await; + mock_token_and_user(&mock_server).await; + + let group_path = format!("/v1.0/users/{USER_ID}/memberOf"); + let next_link_path = "/v1.0/paged-groups"; + + // First page: two groups, plus a link to the second page. + Mock::given(method("GET")) + .and(path(group_path)) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "value": [ + {"displayName": "group-1"}, + {"displayName": "group-2"}, + ], + "@odata.nextLink": format!("https://graph.microsoft.com{next_link_path}?$skiptoken=abc"), + })), + ) + .mount(&mock_server) + .await; + + // Second (final) page: one more group and no further link. + Mock::given(method("GET")) + .and(path(next_link_path)) + .and(query_param("$skiptoken", "abc")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "value": [ + {"displayName": "group-3"}, + ], + }))) + .mount(&mock_server) + .await; + + let user_info = get_user_info_by_id(&backend_for(&mock_server)).await; + + assert_eq!(user_info.groups, vec!["group-1", "group-2", "group-3"]); + } + + #[tokio::test] + async fn test_entra_group_pagination_is_bounded() { + let mock_server = MockServer::start().await; + mock_token_and_user(&mock_server).await; + + // Both the first page and every subsequent page link onwards, so without the page cap this + // would page forever. `next_page` rebases the link onto the mock host, so the loop path is + // what actually gets requested after the first page. + let loop_path = "/v1.0/looping-groups"; + let next_link = format!("https://graph.microsoft.com{loop_path}"); + + Mock::given(method("GET")) + .and(path(format!("/v1.0/users/{USER_ID}/memberOf"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "value": [{"displayName": "group"}], + "@odata.nextLink": next_link, + }))) + .mount(&mock_server) + .await; + + Mock::given(method("GET")) + .and(path(loop_path)) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "value": [{"displayName": "group"}], + "@odata.nextLink": next_link, + }))) + .mount(&mock_server) + .await; + + let user_info = get_user_info_by_id(&backend_for(&mock_server)).await; + + // One group per page, stopping once the cap is hit instead of looping forever. + assert_eq!(user_info.groups.len(), MAX_GROUP_PAGES); + } #[test] fn test_entra_defaults_id() { @@ -323,10 +513,16 @@ mod tests { assert_eq!( entra.group_info(user), Url::parse(&format!( - "https://graph.microsoft.com/v1.0/users/{user}/memberOf" + "https://graph.microsoft.com/v1.0/users/{user}/memberOf?$select=displayName&$top=999" )) .unwrap() ); + assert_eq!( + entra + .next_page("https://graph.microsoft.com/v1.0/paged-groups?$skiptoken=abc") + .unwrap(), + Url::parse("https://graph.microsoft.com/v1.0/paged-groups?$skiptoken=abc").unwrap() + ); } #[test] @@ -357,9 +553,16 @@ mod tests { assert_eq!( entra.group_info(user), Url::parse(&format!( - "http://graph.mock.com:8080/v1.0/users/{user}/memberOf" + "http://graph.mock.com:8080/v1.0/users/{user}/memberOf?$select=displayName&$top=999" )) .unwrap() ); + // The link Graph returns names its own public host, but the configured endpoint wins. + assert_eq!( + entra + .next_page("https://graph.microsoft.com/v1.0/paged-groups?$skiptoken=abc") + .unwrap(), + Url::parse("http://graph.mock.com:8080/v1.0/paged-groups?$skiptoken=abc").unwrap() + ); } } diff --git a/rust/user-info-fetcher/src/backend/keycloak.rs b/rust/user-info-fetcher/src/backend/keycloak.rs index 49c19822..6af2a334 100644 --- a/rust/user-info-fetcher/src/backend/keycloak.rs +++ b/rust/user-info-fetcher/src/backend/keycloak.rs @@ -1,7 +1,6 @@ use std::{collections::HashMap, path::Path}; use hyper::StatusCode; -use reqwest::ClientBuilder; use serde::Deserialize; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha2; @@ -147,7 +146,7 @@ impl ResolvedKeycloakBackend { path: client_secret_path.display().to_string(), })?; - let mut client_builder = ClientBuilder::new(); + let mut client_builder = utils::http::client_builder(); client_builder = utils::tls::configure_reqwest(&config.tls, client_builder) .await .context(ConfigureTlsSnafu)?; diff --git a/rust/user-info-fetcher/src/backend/xfsc_aas.rs b/rust/user-info-fetcher/src/backend/xfsc_aas.rs index 0b4df775..9a5aa121 100644 --- a/rust/user-info-fetcher/src/backend/xfsc_aas.rs +++ b/rust/user-info-fetcher/src/backend/xfsc_aas.rs @@ -13,7 +13,6 @@ use std::collections::HashMap; use hyper::StatusCode; -use reqwest::ClientBuilder; use serde::Deserialize; use snafu::{ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha2; @@ -91,7 +90,7 @@ pub struct ResolvedXfscAasBackend { impl ResolvedXfscAasBackend { /// Resolves an XFSC AAS backend by initializing the HTTP client. pub fn resolve(config: v1alpha2::AasBackend) -> Result { - let http_client = ClientBuilder::new() + let http_client = crate::utils::http::client_builder() .build() .context(ConstructHttpClientSnafu)?; diff --git a/rust/user-info-fetcher/src/utils/http.rs b/rust/user-info-fetcher/src/utils/http.rs index a59ddc43..eb171d55 100644 --- a/rust/user-info-fetcher/src/utils/http.rs +++ b/rust/user-info-fetcher/src/utils/http.rs @@ -1,8 +1,25 @@ +use std::time::Duration; + use hyper::StatusCode; -use reqwest::{RequestBuilder, Response}; +use reqwest::{ClientBuilder, RequestBuilder, Response}; use serde::de::DeserializeOwned; use snafu::{ResultExt, Snafu}; +/// Overall deadline for a single outbound HTTP request. +/// Backends can issue several requests per lookup, so this bounds each one, not the lookup +/// as a whole. +const REQUEST_TIMEOUT: Duration = Duration::from_secs(60); + +/// Deadline for establishing the connection of an outbound request. +const CONNECT_TIMEOUT: Duration = Duration::from_secs(15); + +/// A [`reqwest::ClientBuilder`] preconfigured with our outbound timeouts. +pub fn client_builder() -> ClientBuilder { + ClientBuilder::new() + .timeout(REQUEST_TIMEOUT) + .connect_timeout(CONNECT_TIMEOUT) +} + #[derive(Snafu, Debug)] pub enum Error { #[snafu(display("failed to execute request"))]