Skip to content

Commit be0ac6a

Browse files
committed
Add OAuth2 HTTP request timeouts
Motivation: OAuth2 issuer discovery and token requests previously used libcurl without application-level connection or total request deadlines. An unavailable or stalled issuer could therefore delay client startup and recovery for several minutes. Modification: Add a distinct CurlWrapper connection timeout and configurable OAuth2 connect_timeout_seconds and request_timeout_seconds parameters, defaulting to 10 and 30 seconds. Validate both parameters as positive integers, apply them to discovery, token acquisition, and refresh for both OAuth2 flows, document the public configuration, and add black-box regression coverage. Testing: Built the modified production and AuthPluginTest objects with -Werror. Ran three new timeout and validation tests plus four existing OAuth TLS tests; all 7 passed. The complete pulsar-tests target remains blocked by the pre-existing DagWatchSession incompatibility with the installed Boost.Asio API. Usage: Set connect_timeout_seconds and request_timeout_seconds as positive integer strings in the AuthOauth2 parameter map or JSON passed to AuthOauth2::create. If omitted, the client uses 10-second connection and 30-second total-request timeouts.
1 parent 1d08b2b commit be0ac6a

5 files changed

Lines changed: 195 additions & 18 deletions

File tree

‎include/pulsar/Authentication.h‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,9 @@ typedef std::shared_ptr<CachedToken> CachedTokenPtr;
519519
* "issuer_url": "https://accounts.google.com",
520520
* "client_id": "d9ZyX97q1ef8Cr81WHVC4hFQ64vSlDK3",
521521
* "client_secret": "on1uJ...k6F6R",
522-
* "audience": "https://broker.example.com"
522+
* "audience": "https://broker.example.com",
523+
* "connect_timeout_seconds": "10",
524+
* "request_timeout_seconds": "30"
523525
* ```
524526
*
525527
* For `tokenEndpointAuthMethod = "tls_client_auth"`:
@@ -543,12 +545,17 @@ class PULSAR_PUBLIC AuthOauth2 : public Authentication {
543545
*
544546
* For `tokenEndpointAuthMethod = "client_secret_post"` (default), the required parameter
545547
* keys are “issuer_url”, “private_key”, and “audience”.
546-
* Optional keys: `scope`, `tls_cert_file`, `tls_key_file`.
548+
* Optional keys: `scope`, `tls_cert_file`, `tls_key_file`, `connect_timeout_seconds`,
549+
* and `request_timeout_seconds`.
547550
*
548551
* For `tokenEndpointAuthMethod = "tls_client_auth"`, the required parameter keys are
549552
* `issuer_url`, `tls_cert_file`, and `tls_key_file`.
550-
* Optional keys: `client_id`, `audience`, `scope`. If `client_id` is omitted, the client
551-
* uses `pulsar-client`.
553+
* Optional keys: `client_id`, `audience`, `scope`, `connect_timeout_seconds`, and
554+
* `request_timeout_seconds`. If `client_id` is omitted, the client uses `pulsar-client`.
555+
*
556+
* `connect_timeout_seconds` controls the OAuth HTTP connection timeout and defaults to 10 seconds.
557+
* `request_timeout_seconds` controls the total OAuth HTTP request timeout and defaults to 30 seconds.
558+
* Both values must be positive integers and apply to issuer discovery and token requests.
552559
*
553560
* @param parameters the key-value to create OAuth 2.0 client credentials
554561
* @see http://pulsar.apache.org/docs/en/security-oauth2/#client-credentials

‎lib/CurlWrapper.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class CurlWrapper {
5252
std::string method;
5353
std::string postFields;
5454
std::string userAgent;
55+
int connectTimeoutInSeconds{0};
5556
int timeoutInSeconds{0};
5657
int maxLookupRedirects{-1};
5758
bool authAllowRedirect{false};
@@ -120,7 +121,8 @@ inline CurlWrapper::Result CurlWrapper::get(const std::string& url, const std::s
120121
// Without this config, Curl_resolv_timeout might crash in multi-threads environment
121122
curl_easy_setopt(handle_, CURLOPT_NOSIGNAL, 1L);
122123

123-
curl_easy_setopt(handle_, CURLOPT_TIMEOUT, options.timeoutInSeconds);
124+
curl_easy_setopt(handle_, CURLOPT_CONNECTTIMEOUT, static_cast<long>(options.connectTimeoutInSeconds));
125+
curl_easy_setopt(handle_, CURLOPT_TIMEOUT, static_cast<long>(options.timeoutInSeconds));
124126
if (!options.userAgent.empty()) {
125127
curl_easy_setopt(handle_, CURLOPT_USERAGENT, options.userAgent.c_str());
126128
}

‎lib/auth/AuthOauth2.cc‎

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020

2121
#include <boost/property_tree/json_parser.hpp>
2222
#include <boost/property_tree/ptree.hpp>
23+
#include <charconv>
2324
#include <cstdint>
2425
#include <sstream>
2526
#include <stdexcept>
27+
#include <system_error>
2628

2729
#include "InitialAuthData.h"
2830
#include "lib/Base64Utils.h"
@@ -34,6 +36,11 @@ namespace pulsar {
3436

3537
const std::string TlsClientAuthFlow::DEFAULT_CLIENT_ID = "pulsar-client";
3638
namespace {
39+
constexpr int DEFAULT_OAUTH2_CONNECT_TIMEOUT_SECONDS = 10;
40+
constexpr int DEFAULT_OAUTH2_REQUEST_TIMEOUT_SECONDS = 30;
41+
constexpr char CONNECT_TIMEOUT_PARAM[] = "connect_timeout_seconds";
42+
constexpr char REQUEST_TIMEOUT_PARAM[] = "request_timeout_seconds";
43+
3744
enum class OAuth2TokenEndpointAuthMethod : std::uint8_t
3845
{
3946
ClientSecretPost,
@@ -60,8 +67,37 @@ std::string toFlowName(OAuth2TokenEndpointAuthMethod authMethod) {
6067
return "ClientCredentialFlow";
6168
}
6269
}
70+
71+
int parsePositiveTimeout(const ParamMap& params, const char* name, int defaultValue) {
72+
const auto it = params.find(name);
73+
if (it == params.end()) {
74+
return defaultValue;
75+
}
76+
77+
const auto& rawValue = it->second;
78+
int value = 0;
79+
const auto result = std::from_chars(rawValue.data(), rawValue.data() + rawValue.size(), value);
80+
if (rawValue.empty() || result.ec != std::errc() || result.ptr != rawValue.data() + rawValue.size() ||
81+
value <= 0) {
82+
throw std::invalid_argument(std::string("OAuth2 parameter ") + name + " must be a positive integer");
83+
}
84+
return value;
85+
}
86+
87+
CurlWrapper::Options createHttpOptions(const Oauth2HttpTimeouts& timeouts) {
88+
CurlWrapper::Options options;
89+
options.connectTimeoutInSeconds = timeouts.connectTimeoutInSeconds;
90+
options.timeoutInSeconds = timeouts.requestTimeoutInSeconds;
91+
return options;
92+
}
6393
} // namespace
6494

95+
Oauth2HttpTimeouts::Oauth2HttpTimeouts(const ParamMap& params)
96+
: connectTimeoutInSeconds(
97+
parsePositiveTimeout(params, CONNECT_TIMEOUT_PARAM, DEFAULT_OAUTH2_CONNECT_TIMEOUT_SECONDS)),
98+
requestTimeoutInSeconds(
99+
parsePositiveTimeout(params, REQUEST_TIMEOUT_PARAM, DEFAULT_OAUTH2_REQUEST_TIMEOUT_SECONDS)) {}
100+
65101
// AuthDataOauth2
66102

67103
AuthDataOauth2::AuthDataOauth2(const std::string& accessToken) { accessToken_ = accessToken; }
@@ -265,16 +301,17 @@ static std::unique_ptr<CurlWrapper::TlsContext> createTlsContext(const std::stri
265301
return tlsContext;
266302
}
267303

268-
static std::string fetchTokenEndpoint(const std::string& issuerUrl,
269-
const CurlWrapper::TlsContext* tlsContext) {
304+
static std::string fetchTokenEndpoint(const std::string& issuerUrl, const CurlWrapper::TlsContext* tlsContext,
305+
const Oauth2HttpTimeouts& timeouts) {
270306
const auto wellKnownUrl = getWellKnownUrl(issuerUrl);
271307
CurlWrapper curl;
272308
if (!curl.init()) {
273309
LOG_ERROR("Failed to initialize curl");
274310
return "";
275311
}
276312

277-
auto result = curl.get(wellKnownUrl, "Accept: application/json", {}, tlsContext);
313+
const auto options = createHttpOptions(timeouts);
314+
auto result = curl.get(wellKnownUrl, "Accept: application/json", options, tlsContext);
278315
if (!result.error.empty()) {
279316
LOG_ERROR("Failed to get the well-known configuration " << issuerUrl << ": " << result.error);
280317
return "";
@@ -305,6 +342,11 @@ static std::string fetchTokenEndpoint(const std::string& issuerUrl,
305342
<< issuerUrl << ". response Code " << responseCode);
306343
}
307344
break;
345+
case CURLE_OPERATION_TIMEDOUT:
346+
LOG_ERROR("Timed out retrieving OAuth2 issuer metadata from "
347+
<< issuerUrl << " (connect timeout: " << timeouts.connectTimeoutInSeconds
348+
<< " seconds, request timeout: " << timeouts.requestTimeoutInSeconds << " seconds)");
349+
break;
308350
default:
309351
LOG_ERROR("Response failed for getting the well-known configuration "
310352
<< issuerUrl << ". Error Code " << res << ": " << errorBuffer);
@@ -315,7 +357,8 @@ static std::string fetchTokenEndpoint(const std::string& issuerUrl,
315357

316358
static Oauth2TokenResultPtr fetchOauth2Token(const std::string& tokenEndpoint, const ParamMap& params,
317359
const CurlWrapper::TlsContext* tlsContext,
318-
OAuth2TokenEndpointAuthMethod authMethod) {
360+
OAuth2TokenEndpointAuthMethod authMethod,
361+
const Oauth2HttpTimeouts& timeouts) {
319362
Oauth2TokenResultPtr resultPtr = Oauth2TokenResultPtr(new Oauth2TokenResult());
320363
if (tokenEndpoint.empty()) {
321364
return resultPtr;
@@ -333,7 +376,7 @@ static Oauth2TokenResultPtr fetchOauth2Token(const std::string& tokenEndpoint, c
333376
}
334377
LOG_DEBUG("Generate URL encoded body for " << toFlowName(authMethod) << ": " << postData);
335378

336-
CurlWrapper::Options options;
379+
auto options = createHttpOptions(timeouts);
337380
options.postFields = std::move(postData);
338381
auto result =
339382
curl.get(tokenEndpoint, "Content-Type: application/x-www-form-urlencoded", options, tlsContext);
@@ -379,6 +422,11 @@ static Oauth2TokenResultPtr fetchOauth2Token(const std::string& tokenEndpoint, c
379422
<< responseCode);
380423
}
381424
break;
425+
case CURLE_OPERATION_TIMEDOUT:
426+
LOG_ERROR("Timed out fetching OAuth2 token from "
427+
<< tokenEndpoint << " (connect timeout: " << timeouts.connectTimeoutInSeconds
428+
<< " seconds, request timeout: " << timeouts.requestTimeoutInSeconds << " seconds)");
429+
break;
382430
default:
383431
LOG_ERROR("Response failed for token endpoint " << tokenEndpoint << ". ErrorCode " << res << ": "
384432
<< errorBuffer);
@@ -389,7 +437,8 @@ static Oauth2TokenResultPtr fetchOauth2Token(const std::string& tokenEndpoint, c
389437
}
390438

391439
ClientCredentialFlow::ClientCredentialFlow(ParamMap& params)
392-
: issuerUrl_(params["issuer_url"]),
440+
: httpTimeouts_(params),
441+
issuerUrl_(params["issuer_url"]),
393442
keyFile_(KeyFile::fromParamMap(params)),
394443
audience_(params["audience"]),
395444
scope_(params["scope"]),
@@ -408,7 +457,7 @@ void ClientCredentialFlow::initialize() {
408457
}
409458

410459
const auto tlsContext = createTlsContext(tlsTrustCertsFilePath_, tlsCertFilePath_, tlsKeyFilePath_);
411-
this->tokenEndPoint_ = fetchTokenEndpoint(issuerUrl_, tlsContext.get());
460+
this->tokenEndPoint_ = fetchTokenEndpoint(issuerUrl_, tlsContext.get(), httpTimeouts_);
412461
if (!this->tokenEndPoint_.empty()) {
413462
LOG_DEBUG("Get token endpoint: " << this->tokenEndPoint_);
414463
}
@@ -466,11 +515,12 @@ Oauth2TokenResultPtr ClientCredentialFlow::authenticate() {
466515
const auto params = generateParamMap();
467516
const auto tlsContext = createTlsContext(tlsTrustCertsFilePath_, tlsCertFilePath_, tlsKeyFilePath_);
468517
return fetchOauth2Token(tokenEndPoint_, params, tlsContext.get(),
469-
OAuth2TokenEndpointAuthMethod::ClientSecretPost);
518+
OAuth2TokenEndpointAuthMethod::ClientSecretPost, httpTimeouts_);
470519
}
471520

472521
TlsClientAuthFlow::TlsClientAuthFlow(ParamMap& params)
473-
: issuerUrl_(params["issuer_url"]),
522+
: httpTimeouts_(params),
523+
issuerUrl_(params["issuer_url"]),
474524
clientId_(params["client_id"].empty() ? DEFAULT_CLIENT_ID : params["client_id"]),
475525
audience_(params["audience"]),
476526
scope_(params["scope"]),
@@ -494,7 +544,7 @@ void TlsClientAuthFlow::initialize() {
494544
LOG_ERROR("Failed to initialize TlsClientAuthFlow: tls_cert_file or tls_key_file is not set");
495545
return;
496546
}
497-
this->tokenEndPoint_ = fetchTokenEndpoint(issuerUrl_, tlsContext.get());
547+
this->tokenEndPoint_ = fetchTokenEndpoint(issuerUrl_, tlsContext.get(), httpTimeouts_);
498548
if (!this->tokenEndPoint_.empty()) {
499549
LOG_DEBUG("Get token endpoint: " << this->tokenEndPoint_);
500550
}
@@ -523,7 +573,7 @@ Oauth2TokenResultPtr TlsClientAuthFlow::authenticate() {
523573
return resultPtr;
524574
}
525575
return fetchOauth2Token(tokenEndPoint_, params, tlsContext.get(),
526-
OAuth2TokenEndpointAuthMethod::TlsClientAuth);
576+
OAuth2TokenEndpointAuthMethod::TlsClientAuth, httpTimeouts_);
527577
}
528578

529579
// AuthOauth2

‎lib/auth/AuthOauth2.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ class KeyFile {
5151
static KeyFile fromBase64(const std::string& encoded);
5252
};
5353

54+
struct Oauth2HttpTimeouts {
55+
explicit Oauth2HttpTimeouts(const ParamMap& params);
56+
57+
int connectTimeoutInSeconds;
58+
int requestTimeoutInSeconds;
59+
};
60+
5461
class ClientCredentialFlow : public Oauth2Flow {
5562
public:
5663
ClientCredentialFlow(ParamMap& params);
@@ -66,6 +73,7 @@ class ClientCredentialFlow : public Oauth2Flow {
6673
}
6774

6875
private:
76+
const Oauth2HttpTimeouts httpTimeouts_;
6977
std::string tokenEndPoint_;
7078
const std::string issuerUrl_;
7179
const KeyFile keyFile_;
@@ -94,6 +102,7 @@ class TlsClientAuthFlow : public Oauth2Flow {
94102
}
95103

96104
private:
105+
const Oauth2HttpTimeouts httpTimeouts_;
97106
std::string tokenEndPoint_;
98107
const std::string issuerUrl_;
99108
const std::string clientId_;

0 commit comments

Comments
 (0)