From a7df0d1127fa942b567200cd6d9bc919d70f4af8 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Wed, 11 Dec 2024 12:16:46 +0800 Subject: [PATCH] Fix: oauth2 state encoding (#37473) Commit Message: * Changes the state pramater to a json object to store the user state before redirecting the user request to the auth server. Currently, the json object has two fields: the original url and a nonce for csrf prevention. * Changes the state econding to [Base64URL](https://datatracker.ietf.org/doc/html/rfc4648#section-5) to fix [ AWS cognito doesn't support url encoded state value](https://docs.aws.amazon.com/cognito/latest/developerguide/authorization-endpoint.html#get-authorize-request-parameters) * Change the nonce from a plain timestamp to a random number to enhance its security robustness. This addresses the "1. Generation" in this issue https://github.com/envoyproxy/envoy/issues/36276 * Small refacor: rename variables related to token_secret to hmac_secret to improve clarity and consistency. Example: original state: `{"url":"https://localhost:8080/login","nonce":"IPOom6PfIoFS+MmiV04aTJai8vUYlzyO5zUgT2G8mZA="}` base64url encoded state: `eyJ1cmwiOiJodHRwczovL2xvY2FsaG9zdDo4MDgwL2xvZ2luIiwibm9uY2UiOiJJUE9vbTZQZklvRlMrTW1pVjA0YVRKYWk4dlVZbHp5TzV6VWdUMkc4bVpBPSJ9` Additional Description: The nonce in the [state parameter ](https://datatracker.ietf.org/doc/html/rfc6749#section-10.12)is used for csrf prevention and is applicable for both oauth2 and oidc. Please note that the OIDC spec defines a seperate [nonce parameter](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest) ,which is specifically designed to prevent replay attacks and is unique to OIDC. More discussion about state and nonce parameters and be found in this comment: https://github.com/envoyproxy/envoy/pull/37050#issuecomment-2510418618 Risk Level: Testing: Unit test and integration test Docs Changes: Release Notes: Yes Platform Specific Features: [Optional Runtime guard:] A runtime gurad "envoy.reloadable_features.oauth2_enable_state_nonce" has been added for the new nonce in the state parameter. [Optional Fixes #37049 #36871] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] Related Envoy Gateway issue: https://github.com/envoyproxy/gateway/issues/4625 I've tested this PR against AWS cognito using Envoy Gateway SecurityPolicy, and it worked. cc @missBerg @arkodg --------- Signed-off-by: Huabing Zhao Signed-off-by: code Co-authored-by: code Co-authored-by: phlax (cherry picked from commit 0c6a1d2b847bd5608db1b603c9517341758836f9) Signed-off-by: Huabing Zhao --- changelogs/current.yaml | 75 ++++ source/extensions/filters/http/oauth2/BUILD | 1 + .../extensions/filters/http/oauth2/config.cc | 13 +- .../extensions/filters/http/oauth2/filter.cc | 128 ++++--- .../extensions/filters/http/oauth2/filter.h | 29 +- .../filters/http/oauth2/filter_test.cc | 321 +++++++++--------- .../http/oauth2/oauth_integration_test.cc | 12 +- 7 files changed, 337 insertions(+), 242 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 9ecf0d6e48ce..d0f2d801195a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -6,6 +6,81 @@ behavior_changes: minor_behavior_changes: # *Changes that may cause incompatibilities for some users, but should not for most* +- area: access_log + change: | + New implementation of the JSON formatter will be enabled by default. + The :ref:`sort_properties ` field will + be ignored in the new implementation because the new implementation always sorts properties. And the new implementation + will always keep the value type in the JSON output. For example, the duration field will always be rendered as a number + instead of a string. + This behavior change could be disabled temporarily by setting the runtime + ``envoy.reloadable_features.logging_with_fast_json_formatter`` to false. +- area: xds + change: | + A minor delta-xDS optimization that avoids copying resources when ingesting them was introduced. + No impact to the behavior is expected, but a runtime flag was added as this may impact config-ingestion + related extensions (e.g., custom-config-validators, config-tracker), as the order of the elements passed + to the callback functions may be different. This change can be temporarily reverted + by setting ``envoy.reloadable_features.xds_prevent_resource_copy`` to ``false``. +- area: formatter + change: | + The NaN and Infinity values of float will be serialized to ``null`` and ``"inf"`` respectively in the + metadata (``DYNAMIC_METADATA``, ``CLUSTER_METADATA``, etc.) formatter. +- area: sds + change: | + Relaxed the backing cluster validation for Secret Discovery Service(SDS). Currently, the cluster that supports SDS, + needs to be a primary cluster i.e. a non-EDS cluster defined in bootstrap configuration. This change relaxes that + restriction i.e. SDS cluster can be a dynamic cluster. This change is enabled by default, and can be reverted by setting + the runtime flag ``envoy.restart_features.skip_backing_cluster_check_for_sds`` to ``false``. +- area: http + change: | + If the :ref:`pack_trace_reason ` + is set to false, Envoy will not parse the trace reason from the ``x-request-id`` header to ensure reads and writes of + trace reason be consistant. + If the :ref:`pack_trace_reason ` + is set to true and external ``x-request-id`` value is used, the trace reason in the external request id will not + be trusted and will be cleared. +- area: oauth2 + change: | + :ref:`use_refresh_token ` + is now enabled by default. This behavioral change can be temporarily reverted by setting runtime guard + ``envoy.reloadable_features.oauth2_use_refresh_token`` to false. +- area: oauth2 + change: | + The ``state`` parameter in the OAuth2 authorization request has been changed to a base64url-encoded JSON object. + The JSON object contains the original request URL and a nonce for CSRF prevention. +- area: quic + change: | + Enable UDP GRO in QUIC client connections by default. This behavior can be reverted by setting + the runtime guard ``envoy.reloadable_features.prefer_quic_client_udp_gro`` to false. +- area: scoped_rds + change: | + The :ref:`route_configuration ` field + is supported when the ``ScopedRouteConfiguration`` resource is delivered via SRDS. +- area: http + change: | + Local replies now traverse the filter chain if 1xx headers have been sent to the client. This change can be reverted + by setting the runtime guard ``envoy.reloadable_features.local_reply_traverses_filter_chain_after_1xx`` to false. +- area: dns + change: | + Patched c-ares to address CVE-2024-25629. +- area: csrf + change: | + Increase only the statistics counter ``missing_source_origin`` for requests with a missing source origin. + Previously, the ``request_invalid`` counter was also increased for such requests. +- area: rate_limit + change: | + add ``WEEK`` to the unit of time for rate limit. +- area: rds + change: | + When a new RDS provider config is pushed via xDS and the only difference is change to + :ref:`initial_fetch_timeout `, + the already existing provider will be reused. Envoy will not ask RDS server for routes + config because existing provider already has up to date routes config. + This behavioral change can be temporarily reverted by setting runtime guard + ``envoy.reloadable_features.normalize_rds_provider_config`` to false. +>>>>>>> 0c6a1d2b84 (Fix: oauth2 state encoding (#37473)) + bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* diff --git a/source/extensions/filters/http/oauth2/BUILD b/source/extensions/filters/http/oauth2/BUILD index 5874d69036c6..c7cff9c1dc71 100644 --- a/source/extensions/filters/http/oauth2/BUILD +++ b/source/extensions/filters/http/oauth2/BUILD @@ -48,6 +48,7 @@ envoy_cc_library( ":oauth_client", "//envoy/server:filter_config_interface", "//source/common/common:assert_lib", + "//source/common/common:base64_lib", "//source/common/common:empty_string", "//source/common/common:matchers_lib", "//source/common/crypto:utility_lib", diff --git a/source/extensions/filters/http/oauth2/config.cc b/source/extensions/filters/http/oauth2/config.cc index e55a17e65b9d..e73e4861da54 100644 --- a/source/extensions/filters/http/oauth2/config.cc +++ b/source/extensions/filters/http/oauth2/config.cc @@ -47,15 +47,15 @@ Http::FilterFactoryCb OAuth2Config::createFilterFactoryFromProtoTyped( const auto& proto_config = proto.config(); const auto& credentials = proto_config.credentials(); - const auto& token_secret = credentials.token_secret(); + const auto& client_secret = credentials.token_secret(); const auto& hmac_secret = credentials.hmac_secret(); auto& cluster_manager = context.serverFactoryContext().clusterManager(); auto& secret_manager = cluster_manager.clusterManagerFactory().secretManager(); auto& transport_socket_factory = context.getTransportSocketFactoryContext(); - auto secret_provider_token_secret = secretsProvider( - token_secret, secret_manager, transport_socket_factory, context.initManager()); - if (secret_provider_token_secret == nullptr) { + auto secret_provider_client_secret = secretsProvider( + client_secret, secret_manager, transport_socket_factory, context.initManager()); + if (secret_provider_client_secret == nullptr) { throw EnvoyException("invalid token secret configuration"); } auto secret_provider_hmac_secret = @@ -72,7 +72,7 @@ Http::FilterFactoryCb OAuth2Config::createFilterFactoryFromProtoTyped( } auto secret_reader = std::make_shared( - std::move(secret_provider_token_secret), std::move(secret_provider_hmac_secret), + std::move(secret_provider_client_secret), std::move(secret_provider_hmac_secret), context.serverFactoryContext().threadLocal(), context.serverFactoryContext().api()); auto config = std::make_shared(proto_config, context.serverFactoryContext(), secret_reader, context.scope(), stats_prefix); @@ -83,7 +83,8 @@ Http::FilterFactoryCb OAuth2Config::createFilterFactoryFromProtoTyped( std::make_unique(cluster_manager, config->oauthTokenEndpoint(), config->retryPolicy(), config->defaultExpiresIn()); callbacks.addStreamFilter(std::make_shared( - config, std::move(oauth_client), context.serverFactoryContext().timeSource())); + config, std::move(oauth_client), context.serverFactoryContext().timeSource(), + context.serverFactoryContext().api().randomGenerator())); }; } diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 4c8bd1da6b62..e9bb45dcb077 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -7,6 +7,7 @@ #include #include "source/common/common/assert.h" +#include "source/common/common/base64.h" #include "source/common/common/empty_string.h" #include "source/common/common/enum_to_int.h" #include "source/common/common/fmt.h" @@ -152,19 +153,24 @@ std::string encodeHmacHexBase64(const std::vector& secret, absl::string return encoded_hmac; } +// Generates a SHA256 HMAC from a secret and a message and returns the result as a base64 encoded +// string. +std::string generateHmacBase64(const std::vector& secret, std::string& message) { + auto& crypto_util = Envoy::Common::Crypto::UtilitySingleton::get(); + std::vector hmac_result = crypto_util.getSha256Hmac(secret, message); + std::string hmac_string(hmac_result.begin(), hmac_result.end()); + std::string base64_encoded_hmac; + absl::Base64Escape(hmac_string, &base64_encoded_hmac); + return base64_encoded_hmac; +} + std::string encodeHmacBase64(const std::vector& secret, absl::string_view domain, absl::string_view expires, absl::string_view token = "", absl::string_view id_token = "", absl::string_view refresh_token = "") { - auto& crypto_util = Envoy::Common::Crypto::UtilitySingleton::get(); - const auto hmac_payload = + std::string hmac_payload = absl::StrJoin({domain, expires, token, id_token, refresh_token}, HmacPayloadSeparator); - - std::string base64_encoded_hmac; - std::vector hmac_result = crypto_util.getSha256Hmac(secret, hmac_payload); - std::string hmac_string(hmac_result.begin(), hmac_result.end()); - absl::Base64Escape(hmac_string, &base64_encoded_hmac); - return base64_encoded_hmac; + return generateHmacBase64(secret, hmac_payload); } std::string encodeHmac(const std::vector& secret, absl::string_view domain, @@ -173,19 +179,21 @@ std::string encodeHmac(const std::vector& secret, absl::string_view dom return encodeHmacBase64(secret, domain, expires, token, id_token, refresh_token); } -// Generates a nonce based on the current time -std::string generateFixedLengthNonce(TimeSource& time_source) { - constexpr size_t length = 16; - - std::string nonce = fmt::format("{}", time_source.systemTime().time_since_epoch().count()); - - if (nonce.length() < length) { - nonce.append(length - nonce.length(), '0'); - } else if (nonce.length() > length) { - nonce = nonce.substr(0, length); - } +// Generates a non-guessable nonce that can be used to prevent CSRF attacks. +std::string generateNonce(Random::RandomGenerator& random) { + return Hex::uint64ToHex(random.random()); +} - return nonce; +/** + * Encodes the state parameter for the OAuth2 flow. + * The state parameter is a base64Url encoded JSON object containing the original request URL and a + * nonce for CSRF protection. + */ +std::string encodeState(const std::string& original_request_url, const std::string& nonce) { + std::string buffer; + absl::string_view sanitized_url = Json::sanitize(buffer, original_request_url); + std::string json = fmt::format(R"({{"url":"{}","nonce":"{}"}})", sanitized_url, nonce); + return Base64Url::encode(json.data(), json.size()); } } // namespace @@ -284,11 +292,12 @@ bool OAuth2CookieValidator::timestampIsValid() const { bool OAuth2CookieValidator::isValid() const { return hmacIsValid() && timestampIsValid(); } OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, - std::unique_ptr&& oauth_client, TimeSource& time_source) + std::unique_ptr&& oauth_client, TimeSource& time_source, + Random::RandomGenerator& random) : validator_(std::make_shared(time_source, config->cookieNames(), config->cookieDomain())), - oauth_client_(std::move(oauth_client)), config_(std::move(config)), - time_source_(time_source) { + oauth_client_(std::move(oauth_client)), config_(std::move(config)), time_source_(time_source), + random_(random) { oauth_client_->setCallbacks(*this); } @@ -361,7 +370,6 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he if (config_->redirectPathMatcher().match(original_request_url.pathAndQueryParams())) { ENVOY_LOG(debug, "state url query params {} matches the redirect path matcher", original_request_url.pathAndQueryParams()); - // TODO(zhaohuabing): Should return 401 unauthorized or 400 bad request? sendUnauthorizedResponse(); return Http::FilterHeadersStatus::StopIteration; } @@ -447,7 +455,7 @@ Http::FilterHeadersStatus OAuth2Filter::encodeHeaders(Http::ResponseHeaderMap& h bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const { // We can skip OAuth if the supplied HMAC cookie is valid. Apply the OAuth details as headers // if we successfully validate the cookie. - validator_->setParams(headers, config_->tokenSecret()); + validator_->setParams(headers, config_->hmacSecret()); if (validator_->isValid()) { config_->stats().oauth_success_.inc(); if (config_->forwardBearerToken() && !validator_->token().empty()) { @@ -473,7 +481,6 @@ bool OAuth2Filter::canRedirectToOAuthServer(Http::RequestHeaderMap& headers) con void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const { Http::ResponseHeaderMapPtr response_headers{Http::createHeaderMap( {{Http::Headers::get().Status, std::to_string(enumToInt(Http::Code::Found))}})}; - // Construct the correct scheme. We default to https since this is a requirement for OAuth to // succeed. However, if a downstream client explicitly declares the "http" scheme for whatever // reason, we also use "http" when constructing our redirect uri to the authorization server. @@ -482,12 +489,11 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const if (Http::Utility::schemeIsHttp(headers.getSchemeValue())) { scheme = Http::Headers::get().SchemeValues.Http; } - const std::string base_path = absl::StrCat(scheme, "://", host_); - const std::string full_url = absl::StrCat(base_path, headers.Path()->value().getStringView()); - const std::string escaped_url = Http::Utility::PercentEncoding::urlEncodeQueryParameter(full_url); + const std::string original_url = absl::StrCat(base_path, headers.Path()->value().getStringView()); - // Generate a nonce to prevent CSRF attacks + // Encode the original request URL and the nonce to the state parameter. + // Generate a nonce to prevent CSRF attacks. std::string nonce; bool nonce_cookie_exists = false; const auto nonce_cookie = Http::Utility::parseCookies(headers, [this](absl::string_view key) { @@ -497,7 +503,7 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const nonce = nonce_cookie.at(config_->cookieNames().oauth_nonce_); nonce_cookie_exists = true; } else { - nonce = generateFixedLengthNonce(time_source_); + nonce = generateNonce(random_); } // Set the nonce cookie if it does not exist. @@ -514,11 +520,10 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const Http::Headers::get().SetCookie, absl::StrCat(config_->cookieNames().oauth_nonce_, "=", nonce, cookie_tail_http_only)); } + const std::string state = encodeState(original_url, nonce); - // Encode the original request URL and the nonce to the state parameter - const std::string state = - absl::StrCat(stateParamsUrl, "=", escaped_url, "&", stateParamsNonce, "=", nonce); - const std::string escaped_state = Http::Utility::PercentEncoding::urlEncodeQueryParameter(state); + auto query_params = config_->authorizationQueryParams(); + query_params.overwrite(queryParamsState, state); Formatter::FormatterPtr formatter = THROW_OR_RETURN_VALUE( Formatter::FormatterImpl::create(config_->redirectUri()), Formatter::FormatterPtr); @@ -526,17 +531,14 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const formatter->formatWithContext({&headers}, decoder_callbacks_->streamInfo()); const std::string escaped_redirect_uri = Http::Utility::PercentEncoding::urlEncodeQueryParameter(redirect_uri); - - auto query_params = config_->authorizationQueryParams(); query_params.overwrite(queryParamsRedirectUri, escaped_redirect_uri); - query_params.overwrite(queryParamsState, escaped_state); + // Copy the authorization endpoint URL to replace its query params. auto authorization_endpoint_url = config_->authorizationEndpointUrl(); const std::string path_and_query_params = query_params.replaceQueryString( Http::HeaderString(authorization_endpoint_url.pathAndQueryParams())); authorization_endpoint_url.setPathAndQueryParams(path_and_query_params); const std::string new_url = authorization_endpoint_url.toString(); - response_headers->setLocation(new_url + config_->encodedResourceQueryParams()); decoder_callbacks_->encodeHeaders(std::move(response_headers), true, REDIRECT_FOR_CREDENTIALS); @@ -574,6 +576,10 @@ Http::FilterHeadersStatus OAuth2Filter::signOutUser(const Http::RequestHeaderMap Http::Headers::get().SetCookie, absl::StrCat(fmt::format(CookieDeleteFormatString, config_->cookieNames().refresh_token_), cookie_domain)); + response_headers->addReferenceKey( + Http::Headers::get().SetCookie, + absl::StrCat(fmt::format(CookieDeleteFormatString, config_->cookieNames().oauth_nonce_), + cookie_domain)); response_headers->setLocation(new_path); decoder_callbacks_->encodeHeaders(std::move(response_headers), true, SIGN_OUT); @@ -612,7 +618,7 @@ void OAuth2Filter::updateTokens(const std::string& access_token, const std::stri } std::string OAuth2Filter::getEncodedToken() const { - auto token_secret = config_->tokenSecret(); + auto token_secret = config_->hmacSecret(); std::vector token_secret_vec(token_secret.begin(), token_secret.end()); std::string encoded_token; @@ -829,22 +835,21 @@ CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::Request } // Return 401 unauthorized if the state query parameter does not contain the original request URL - // and nonce. state is an HTTP URL encoded string that contains the url and nonce, for example: - // state=url%3Dhttp%253A%252F%252Ftraffic.example.com%252Fnot%252F_oauth%26nonce%3D1234567890000000". - std::string state = Http::Utility::PercentEncoding::urlDecodeQueryParameter(stateVal.value()); - const auto state_parameters = Http::Utility::QueryParamsMulti::parseParameters(state, 0, true); - auto urlVal = state_parameters.getFirstValue(stateParamsUrl); - auto nonceVal = state_parameters.getFirstValue(stateParamsNonce); - if (!urlVal.has_value() || !nonceVal.has_value()) { - ENVOY_LOG(error, "state query param does not contain url or nonce: \n{}", state); + // or nonce. + // Decode the state parameter to get the original request URL and nonce. + const std::string state = Base64Url::decode(stateVal.value()); + bool has_unknown_field; + ProtobufWkt::Struct message; + + auto status = MessageUtil::loadFromJsonNoThrow(state, message, has_unknown_field); + if (!status.ok()) { + ENVOY_LOG(error, "state query param is not a valid JSON: \n{}", state); return {false, "", ""}; } - // Return 401 unauthorized if the URL in the state is not valid. - std::string original_request_url = urlVal.value(); - Http::Utility::Url url; - if (!url.initialize(original_request_url, false)) { - ENVOY_LOG(error, "state url {} can not be initialized", original_request_url); + const auto& filed_value_pair = message.fields(); + if (!filed_value_pair.contains(stateParamsUrl) || !filed_value_pair.contains(stateParamsNonce)) { + ENVOY_LOG(error, "state query param does not contain url or nonce: \n{}", state); return {false, "", ""}; } @@ -854,15 +859,26 @@ CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::Request // sessions via CSRF attack. The attack can result in victims saving their sensitive data // in the attacker's account. // More information can be found at https://datatracker.ietf.org/doc/html/rfc6819#section-5.3.5 - if (!validateNonce(headers, nonceVal.value())) { - ENVOY_LOG(error, "nonce cookie does not match nonce query param: \n{}", nonceVal.value()); + std::string nonce = filed_value_pair.at(stateParamsNonce).string_value(); + if (!validateNonce(headers, nonce)) { + ENVOY_LOG(error, "nonce cookie does not match nonce in the state: \n{}", nonce); + return {false, "", ""}; + } + const std::string original_request_url = filed_value_pair.at(stateParamsUrl).string_value(); + + // Return 401 unauthorized if the URL in the state is not valid. + Http::Utility::Url url; + if (!url.initialize(original_request_url, false)) { + ENVOY_LOG(error, "state url {} can not be initialized", original_request_url); return {false, "", ""}; } return {true, codeVal.value(), original_request_url}; } -bool OAuth2Filter::validateNonce(const Http::RequestHeaderMap& headers, const std::string& nonce) { +// Validates the nonce in the state parameter against the nonce in the cookie. +bool OAuth2Filter::validateNonce(const Http::RequestHeaderMap& headers, + const std::string& nonce) const { const auto nonce_cookie = Http::Utility::parseCookies(headers, [this](absl::string_view key) { return key == config_->cookieNames().oauth_nonce_; }); diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 0fcad488f9a4..85a2cdd0b496 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -40,22 +40,28 @@ class SecretReader { public: virtual ~SecretReader() = default; virtual const std::string& clientSecret() const PURE; - virtual const std::string& tokenSecret() const PURE; + virtual const std::string& hmacSecret() const PURE; }; class SDSSecretReader : public SecretReader { public: SDSSecretReader(Secret::GenericSecretConfigProviderSharedPtr&& client_secret_provider, - Secret::GenericSecretConfigProviderSharedPtr&& token_secret_provider, + Secret::GenericSecretConfigProviderSharedPtr&& hmac_secret_provider, ThreadLocal::SlotAllocator& tls, Api::Api& api) - : client_secret_(std::move(client_secret_provider), tls, api), - token_secret_(std::move(token_secret_provider), tls, api) {} - const std::string& clientSecret() const override { return client_secret_.secret(); } - const std::string& tokenSecret() const override { return token_secret_.secret(); } + : client_secret_( + THROW_OR_RETURN_VALUE(Secret::ThreadLocalGenericSecretProvider::create( + std::move(client_secret_provider), tls, api), + std::unique_ptr)), + hmac_secret_( + THROW_OR_RETURN_VALUE(Secret::ThreadLocalGenericSecretProvider::create( + std::move(hmac_secret_provider), tls, api), + std::unique_ptr)) {} + const std::string& clientSecret() const override { return client_secret_->secret(); } + const std::string& hmacSecret() const override { return hmac_secret_->secret(); } private: - Secret::ThreadLocalGenericSecretProvider client_secret_; - Secret::ThreadLocalGenericSecretProvider token_secret_; + std::unique_ptr client_secret_; + std::unique_ptr hmac_secret_; }; /** @@ -140,7 +146,7 @@ class FilterConfig { const Matchers::PathMatcher& redirectPathMatcher() const { return redirect_matcher_; } const Matchers::PathMatcher& signoutPath() const { return signout_path_; } std::string clientSecret() const { return secret_reader_->clientSecret(); } - std::string tokenSecret() const { return secret_reader_->tokenSecret(); } + std::string hmacSecret() const { return secret_reader_->hmacSecret(); } FilterStats& stats() { return stats_; } const std::string& encodedResourceQueryParams() const { return encoded_resource_query_params_; } const CookieNames& cookieNames() const { return cookie_names_; } @@ -260,7 +266,7 @@ class OAuth2Filter : public Http::PassThroughFilter, Logger::Loggable { public: OAuth2Filter(FilterConfigSharedPtr config, std::unique_ptr&& oauth_client, - TimeSource& time_source); + TimeSource& time_source, Random::RandomGenerator& random); // Http::PassThroughFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override; @@ -285,7 +291,6 @@ class OAuth2Filter : public Http::PassThroughFilter, void finishRefreshAccessTokenFlow(); void updateTokens(const std::string& access_token, const std::string& id_token, const std::string& refresh_token, std::chrono::seconds expires_in); - bool validateNonce(const Http::RequestHeaderMap& headers, const std::string& nonce); private: friend class OAuth2Test; @@ -309,6 +314,7 @@ class OAuth2Filter : public Http::PassThroughFilter, std::unique_ptr oauth_client_; FilterConfigSharedPtr config_; TimeSource& time_source_; + Random::RandomGenerator& random_; // Determines whether or not the current request can skip the entire OAuth flow (HMAC is valid, // connection is mTLS, etc.) @@ -327,6 +333,7 @@ class OAuth2Filter : public Http::PassThroughFilter, const std::string& bearerPrefix() const; CallbackValidationResult validateOAuthCallback(const Http::RequestHeaderMap& headers, const absl::string_view path_str); + bool validateNonce(const Http::RequestHeaderMap& headers, const std::string& nonce) const; }; } // namespace Oauth2 diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 58c638e3a56e..75b963e35cf4 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -39,6 +39,11 @@ static const std::string TEST_CLIENT_SECRET_ID = "MyClientSecretKnoxID"; static const std::string TEST_TOKEN_SECRET_ID = "MyTokenSecretKnoxID"; static const std::string TEST_DEFAULT_SCOPE = "user"; static const std::string TEST_ENCODED_AUTH_SCOPES = "user%20openid%20email"; +static const std::string TEST_STATE_NONCE = "00000000075bcd15"; +// {"url":"https://traffic.example.com/original_path?var1=1&var2=2","nonce":"00000000075bcd15"} +static const std::string TEST_ENCODED_STATE = + "eyJ1cmwiOiJodHRwczovL3RyYWZmaWMuZXhhbXBsZS5jb20vb3JpZ2luYWxfcGF0aD92YXIxPTEmdmFyMj0yIiwibm9uY2" + "UiOiIwMDAwMDAwMDA3NWJjZDE1In0"; namespace { Http::RegisterCustomInlineHeader @@ -50,7 +55,7 @@ class MockSecretReader : public SecretReader { const std::string& clientSecret() const override { CONSTRUCT_ON_FIRST_USE(std::string, "asdf_client_secret_fdsa"); } - const std::string& tokenSecret() const override { + const std::string& hmacSecret() const override { CONSTRUCT_ON_FIRST_USE(std::string, "asdf_token_secret_fdsa"); } }; @@ -101,7 +106,9 @@ class OAuth2Test : public testing::TestWithParam { std::unique_ptr oauth_client_ptr{oauth_client_}; config_ = config; - filter_ = std::make_shared(config_, std::move(oauth_client_ptr), test_time_); + ON_CALL(test_random_, random()).WillByDefault(Return(123456789)); + filter_ = std::make_shared(config_, std::move(oauth_client_ptr), test_time_, + test_random_); filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); validator_ = std::make_shared(); @@ -224,6 +231,7 @@ class OAuth2Test : public testing::TestWithParam { Stats::IsolatedStoreImpl store_; Stats::Scope& scope_{*store_.rootScope()}; Event::SimulatedTimeSystem test_time_; + NiceMock test_random_; }; // Verifies that the OAuth SDSSecretReader correctly updates dynamic generic secret. @@ -259,7 +267,7 @@ TEST_F(OAuth2Test, SdsDynamicGenericSecret) { SDSSecretReader secret_reader(std::move(client_secret_provider), std::move(token_secret_provider), tls, *api); EXPECT_TRUE(secret_reader.clientSecret().empty()); - EXPECT_TRUE(secret_reader.tokenSecret().empty()); + EXPECT_TRUE(secret_reader.hmacSecret().empty()); const std::string yaml_client = R"EOF( name: client @@ -274,7 +282,7 @@ name: client EXPECT_TRUE(client_callback->onConfigUpdate(decoded_resources_client.refvec_, "").ok()); EXPECT_EQ(secret_reader.clientSecret(), "client_test"); - EXPECT_EQ(secret_reader.tokenSecret(), ""); + EXPECT_EQ(secret_reader.hmacSecret(), ""); const std::string yaml_token = R"EOF( name: token @@ -287,7 +295,7 @@ name: token EXPECT_TRUE(token_callback->onConfigUpdate(decoded_resources_token.refvec_, "").ok()); EXPECT_EQ(secret_reader.clientSecret(), "client_test"); - EXPECT_EQ(secret_reader.tokenSecret(), "token_test"); + EXPECT_EQ(secret_reader.hmacSecret(), "token_test"); const std::string yaml_client_recheck = R"EOF( name: client @@ -300,7 +308,7 @@ name: client EXPECT_TRUE(client_callback->onConfigUpdate(decoded_resources_client_recheck.refvec_, "").ok()); EXPECT_EQ(secret_reader.clientSecret(), "client_test_recheck"); - EXPECT_EQ(secret_reader.tokenSecret(), "token_test"); + EXPECT_EQ(secret_reader.hmacSecret(), "token_test"); } // Verifies that we fail constructing the filter if the configured cluster doesn't exist. TEST_F(OAuth2Test, InvalidCluster) { @@ -365,29 +373,25 @@ TEST_F(OAuth2Test, DefaultAuthScope) { // as a query parameter in response headers. init(test_config_); Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/not/_oauth"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "http"}, + {Http::Headers::get().Scheme.get(), "https"}, }; - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); - Http::TestResponseHeaderMapImpl response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + TEST_CLIENT_ID + - "&redirect_uri=http%3A%2F%2Ftraffic.example.com%2F_oauth" + "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_DEFAULT_SCOPE + - "&state=url%3Dhttp%253A%252F%252Ftraffic.example.com%252Fnot%252F_oauth%26nonce%" - "3D1234567890000000"}, + TEST_DEFAULT_SCOPE + "&state=" + TEST_ENCODED_STATE}, }; // explicitly tell the validator to fail the validation. @@ -424,34 +428,30 @@ TEST_F(OAuth2Test, PreservesQueryParametersInAuthorizationEndpoint) { secret_reader, scope_, "test."); init(test_config_); Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/not/_oauth"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "http"}, + {Http::Headers::get().Scheme.get(), "https"}, }; // Explicitly tell the validator to fail the validation. EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); // Verify that the foo=bar query parameter is preserved in the redirect. Http::TestResponseHeaderMapImpl response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + TEST_CLIENT_ID + "&foo=bar" - "&redirect_uri=http%3A%2F%2Ftraffic.example.com%2F_oauth" + "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_DEFAULT_SCOPE + - "&state=url%3Dhttp%253A%252F%252Ftraffic.example.com%252Fnot%252F_oauth%26nonce%" - "3D1234567890000000"}, + TEST_DEFAULT_SCOPE + "&state=" + TEST_ENCODED_STATE}, }; EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -482,35 +482,30 @@ TEST_F(OAuth2Test, PreservesQueryParametersInAuthorizationEndpointWithUrlEncodin secret_reader, scope_, "test."); init(test_config_); Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/not/_oauth"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "http"}, + {Http::Headers::get().Scheme.get(), "https"}, }; // Explicitly tell the validator to fail the validation. EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); - // Verify that the foo=bar query parameter is preserved in the redirect. Http::TestResponseHeaderMapImpl response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + TEST_CLIENT_ID + "&foo=bar" - "&redirect_uri=http%3A%2F%2Ftraffic.example.com%2F_oauth" + "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_DEFAULT_SCOPE + - "&state=url%3Dhttp%253A%252F%252Ftraffic.example.com%252Fnot%252F_oauth%26nonce%" - "3D1234567890000000"}, + TEST_DEFAULT_SCOPE + "&state=" + TEST_ENCODED_STATE}, }; EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -541,6 +536,8 @@ TEST_F(OAuth2Test, RequestSignout) { "IdToken=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"}, {Http::Headers::get().SetCookie.get(), "RefreshToken=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"}, + {Http::Headers::get().SetCookie.get(), + "OauthNonce=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"}, {Http::Headers::get().Location.get(), "https://traffic.example.com/"}, }; EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -691,10 +688,9 @@ TEST_F(OAuth2Test, SetBearerToken) { test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Fasdf%26nonce%3D1234567890000000"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, @@ -729,7 +725,8 @@ TEST_F(OAuth2Test, SetBearerToken) { "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), "RefreshToken=some-refresh-token;path=/;Max-Age=600;secure;HttpOnly"}, - {Http::Headers::get().Location.get(), "https://asdf"}, + {Http::Headers::get().Location.get(), + "https://traffic.example.com/original_path?var1=1&var2=2"}, }; EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -750,27 +747,19 @@ TEST_F(OAuth2Test, SetBearerToken) { * in the query parameters. */ TEST_F(OAuth2Test, OAuthErrorNonOAuthHttpCallback) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - init(); // First construct the initial request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, }; - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); - // This is the immediate response - a redirect to the auth cluster. Http::TestResponseHeaderMapImpl first_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + @@ -778,10 +767,8 @@ TEST_F(OAuth2Test, OAuthErrorNonOAuthHttpCallback) { "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%253Dadmin%" - "2526level%253Dtrace%26nonce%3D1234567890000000" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" + TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE + "&resource=oauth2-resource" + + "&resource=http%3A%2F%2Fexample.com" "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%3Dbar%" "3Fvar1%3D1%26var2%3D2"}, }; @@ -799,11 +786,9 @@ TEST_F(OAuth2Test, OAuthErrorNonOAuthHttpCallback) { // This represents the callback request from the authorization server. Http::TestRequestHeaderMapImpl second_request_headers{ - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%" - "253Dadmin%2526level%253Dtrace%26nonce%3D1234567890000000"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Scheme.get(), "https"}, @@ -833,7 +818,7 @@ TEST_F(OAuth2Test, OAuthErrorNonOAuthHttpCallback) { "path=/;Max-Age=;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, {Http::Headers::get().Location.get(), - "https://traffic.example.com/test?name=admin&level=trace"}, + "https://traffic.example.com/original_path?var1=1&var2=2"}, }; EXPECT_CALL(decoder_callbacks_, @@ -894,10 +879,9 @@ TEST_F(OAuth2Test, OAuthErrorQueryString) { */ TEST_F(OAuth2Test, OAuthCallbackStartsAuthentication) { Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Fasdf%26nonce%3D1234567890000000"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, @@ -923,10 +907,13 @@ TEST_F(OAuth2Test, OAuthCallbackStartsAuthentication) { * Expected behavior: the filter should fail the request and return a 401 Unauthorized response. */ TEST_F(OAuth2Test, OAuthCallbackStartsAuthenticationNoNonce) { + // {"url":"https://traffic.example.com/original_path?var1=1&var2=2"} + static const std::string state_without_nonce = + "eyJ1cmwiOiJodHRwczovL3RyYWZmaWMuZXhhbXBsZS5jb20vb3JpZ2luYWxfcGF0aD92YXIxPTEmdmFyMj0yIn0"; Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Fasdf"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + state_without_nonce}, {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, @@ -950,11 +937,14 @@ TEST_F(OAuth2Test, OAuthCallbackStartsAuthenticationNoNonce) { * Expected behavior: the filter should fail the request and return a 401 Unauthorized response. */ TEST_F(OAuth2Test, OAuthCallbackStartsAuthenticationInvalidNonce) { + // {"url":"https://traffic.example.com/original_path?var1=1&var2=2","nonce":"0"} + static const std::string state_with_invalid_nonce = + "eyJ1cmwiOiJodHRwczovL3RyYWZmaWMuZXhhbXBsZS5jb20vb3JpZ2luYWxfcGF0aD92YXIxPTEmdmFyMj0yIiwibm9u" + "Y2UiOiIwIn0"; Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Fasdf%26nonce%3D123456788000000"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + state_with_invalid_nonce}, {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, @@ -981,10 +971,15 @@ TEST_F(OAuth2Test, OAuthCallbackStartsAuthenticationMalformedState) { // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + // {"url":"https://traffic.example.com/original_path?var1=1&var2=2","nonce":"} + static const std::string state_with_invalid_nonce_json = + "eyJ1cmwiOiJodHRwczovL3RyYWZmaWMuZXhhbXBsZS5jb20vb3JpZ2luYWxfcGF0aD92YXIxPTEmdmFyMj0yIiwibm9u" + "Y2UiOiJ9"; + Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=https%253A%252F%252Fasdf"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + state_with_invalid_nonce_json}, {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, @@ -1208,17 +1203,22 @@ TEST_F(OAuth2Test, CookieValidatorCanUpdateToken) { // Verify that we 401 the request if the state query param doesn't contain a valid URL. TEST_F(OAuth2Test, OAuthTestInvalidUrlInStateQueryParam) { + // {"url":"blah","nonce":"00000000075bcd15"} + static const std::string state_with_invalid_url = + "eyJ1cmwiOiJibGFoIiwibm9uY2UiOiIwMDAwMDAwMDA3NWJjZDE1In0"; Http::TestRequestHeaderMapImpl request_headers{ {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Path.get(), - "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + "&state=blah"}, + "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + + "&state=" + state_with_invalid_url}, {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, {Http::Headers::get().Cookie.get(), "BearerToken=legit_token"}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_STATE_NONCE}, }; Http::TestRequestHeaderMapImpl expected_headers{ @@ -1242,12 +1242,17 @@ TEST_F(OAuth2Test, OAuthTestInvalidUrlInStateQueryParam) { // Verify that we 401 the request if the state query param contains the callback URL. TEST_F(OAuth2Test, OAuthTestCallbackUrlInStateQueryParam) { + // {"url":"https://traffic.example.com/_oauth","nonce":"00000000075bcd15"} + static const std::string state_with_callback_url = + "eyJ1cmwiOiJodHRwczovL3RyYWZmaWMuZXhhbXBsZS5jb20vX29hdXRoIiwibm9uY2UiOiIwMDAwMDAwMDA3NWJjZDE1" + "In0"; + Http::TestRequestHeaderMapImpl request_headers{ {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Path.get(), "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + - "&state=https%3A%2F%2Ftraffic.example.com%2F_oauth"}, + "&state=" + state_with_callback_url}, {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, {Http::Headers::get().Cookie.get(), "BearerToken=legit_token"}, @@ -1255,6 +1260,7 @@ TEST_F(OAuth2Test, OAuthTestCallbackUrlInStateQueryParam) { "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_STATE_NONCE}, }; Http::TestRequestHeaderMapImpl expected_response_headers{ @@ -1280,13 +1286,14 @@ TEST_F(OAuth2Test, OAuthTestCallbackUrlInStateQueryParam) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Path.get(), "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + - "&state=https%3A%2F%2Ftraffic.example.com%2F_oauth"}, + "&state=" + state_with_callback_url}, {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, {Http::Headers::get().Cookie.get(), "BearerToken=legit_token"}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_STATE_NONCE}, {Http::CustomHeaders::get().Authorization.get(), "Bearer legit_token"}, }; @@ -1296,22 +1303,19 @@ TEST_F(OAuth2Test, OAuthTestCallbackUrlInStateQueryParam) { TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); - - init(); Http::TestRequestHeaderMapImpl request_headers{ {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Path.get(), "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + - "&state=url%3Dhttps%3A%2F%2Ftraffic.example.com%2Foriginal_path%3Fvar1%3D1%2526var2%3D2%" - "26nonce%3D1234567890000000"}, + "&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, {Http::Headers::get().Cookie.get(), "BearerToken=legit_token"}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "OauthNonce=1234567890000000"}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_STATE_NONCE}, }; Http::TestRequestHeaderMapImpl expected_response_headers{ @@ -1337,15 +1341,14 @@ TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Path.get(), "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + - "&state=url%3Dhttps%3A%2F%2Ftraffic.example.com%2Foriginal_path%3Fvar1%3D1%2526var2%3D2%" - "26nonce%3D1234567890000000"}, + "&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, {Http::Headers::get().Cookie.get(), "BearerToken=legit_token"}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "OauthNonce=1234567890000000"}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_STATE_NONCE}, {Http::CustomHeaders::get().Authorization.get(), "Bearer legit_token"}, }; @@ -1353,22 +1356,18 @@ TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { } /** - * Testing oauth state with query string parameters. + * Testing oauth state with cookie domain. * - * Expected behavior: HTTP Utility should not strip the parameters of the original request. + * Expected behavior: Cookie domain should be set to the domain in the config. */ -TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); - - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - init(); +TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { + init(getConfig(true, false, + ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_URL_ENCODED_BODY, + 0, false, false, true /* set_cookie_domain */)); // First construct the initial request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -1378,7 +1377,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { Http::TestResponseHeaderMapImpl first_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";domain=example.com;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + @@ -1386,12 +1385,9 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%253Dadmin%" - "2526level%253Dtrace%26nonce%3D1234567890000000" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" - "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%" - "3Dbar%" + TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE + "&resource=oauth2-resource" + + "&resource=http%3A%2F%2Fexample.com" + "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%3Dbar%" "3Fvar1%3D1%26var2%3D2"}, }; @@ -1409,10 +1405,8 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { // This represents the callback request from the authorization server. Http::TestRequestHeaderMapImpl second_request_headers{ {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%" - "253Dadmin%2526level%253Dtrace%26nonce%3D1234567890000000"}, + "OauthNonce=" + TEST_STATE_NONCE + ";domain=example.com;path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Scheme.get(), "https"}, @@ -1432,16 +1426,28 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { EXPECT_EQ(1, config_->stats().oauth_unauthorized_rq_.value()); EXPECT_EQ(config_->clusterName(), "auth.example.com"); + // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + const std::chrono::seconds expiredTime(10); + filter_->updateTokens("accessToken", "idToken", "refreshToken", expiredTime); + // Expected response after the callback & validation is complete - verifying we kept the // state and method of the original request, including the query string parameters. Http::TestRequestHeaderMapImpl second_response_headers{ {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().SetCookie.get(), "OauthHMAC=" - "fV62OgLipChTQQC3UFgDp+l5sCiSb3zt7nCoJiVivWw=;" - "path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "OauthHMAC=vU9fV//fsKp9ARyrz/HZx2CqWFCmUygihdl18qR5u78=;" + "domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "OauthExpires=10;domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "BearerToken=accessToken;domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "IdToken=idToken;domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "RefreshToken=refreshToken;domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().Location.get(), - "https://traffic.example.com/test?name=admin&level=trace"}, + "https://traffic.example.com/original_path?var1=1&var2=2"}, }; EXPECT_CALL(decoder_callbacks_, @@ -1450,13 +1456,22 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { filter_->finishGetAccessTokenFlow(); } -TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); - +/** + * Testing oauth state with special characters that must be escaped in JSON. + * + * Expected behavior: the JSON string in the state query parameter should be correctly escaped and + * the final redirect should equal the original request. + */ +TEST_F(OAuth2Test, OAuthTestFullFlowPostWithSpecialCharactersForJson) { + const std::string url_with_special_characters = + R"(/original_path?query="value"&key=val\ue#frag{data}[info]|test\^space)"; + const std::string test_encoded_state_with_special_characters = + "eyJ1cmwiOiJodHRwczovL3RyYWZmaWMuZXhhbXBsZS5jb20vb3JpZ2luYWxfcGF0aD9xdWVyeT1cInZhbHVlXCIma2V5" + "PXZhbFxcdWUjZnJhZzxtZW50PntkYXRhfVtpbmZvXXx0ZXN0XFxec3BhY2UiLCJub25jZSI6IjAwMDAwMDAwMDc1YmNk" + "MTUifQ"; // First construct the initial request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), url_with_special_characters}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -1466,7 +1481,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { Http::TestResponseHeaderMapImpl first_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + @@ -1474,11 +1489,11 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%253Dadmin%" - "2526level%253Dtrace%26nonce%3D1234567890000000" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" - "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%3Dbar%" + TEST_ENCODED_AUTH_SCOPES + "&state=" + test_encoded_state_with_special_characters + + "&resource=oauth2-resource" + + "&resource=http%3A%2F%2Fexample.com" + "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%" + "3Dbar%" "3Fvar1%3D1%26var2%3D2"}, }; @@ -1486,7 +1501,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - // Check that the redirect includes the escaped parameter characters, '?', '&' and '='. + // Check that the redirect includes URL encoded query parameter characters. EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&first_response_headers), true)); // This represents the beginning of the OAuth filter. @@ -1496,15 +1511,13 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { // This represents the callback request from the authorization server. Http::TestRequestHeaderMapImpl second_request_headers{ {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%" - "253Dadmin%2526level%253Dtrace%26nonce%3D1234567890000000"}, + "/_oauth?code=123&state=" + test_encoded_state_with_special_characters}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Scheme.get(), "https"}, }; - // Deliberately fail the HMAC validation check. EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); @@ -1539,6 +1552,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { {Http::Headers::get().SetCookie.get(), "RefreshToken=refreshToken;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().Location.get(), +<<<<<<< HEAD "https://traffic.example.com/test?name=admin&level=trace"}, }; @@ -1642,6 +1656,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { "OauthExpires=;domain=example.com;path=/;Max-Age=;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://traffic.example.com/test?name=admin&level=trace"}, + "https://traffic.example.com" + url_with_special_characters}, }; EXPECT_CALL(decoder_callbacks_, @@ -1899,7 +1914,6 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshToken) { } TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndDefaultRefreshTokenExpiresIn) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, @@ -1947,7 +1961,6 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndDefaultRefr */ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshTokenExpiresInFromJwt) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, @@ -1999,7 +2012,6 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshToke */ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefreshToken) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, @@ -2052,7 +2064,6 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefr */ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndNoExpClaimInRefreshToken) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, @@ -2105,7 +2116,6 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndNoExpClaimI */ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensIdTokenExpiresInFromJwt) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, @@ -2158,7 +2168,6 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensIdTokenExpiresInFromJwt) { */ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensExpiredIdToken) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, @@ -2213,7 +2222,6 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensExpiredIdToken) { */ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensNoExpClaimInIdToken) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, @@ -2340,13 +2348,10 @@ TEST_F(OAuth2Test, CookieValidatorInTransition) { // - The filter gets a new bearer and refresh tokens via the current refresh token // - The filter continues to handler the request without redirection to the user agent TEST_F(OAuth2Test, OAuthTestFullFlowWithUseRefreshToken) { - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */)); // First construct the initial request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -2356,7 +2361,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowWithUseRefreshToken) { Http::TestResponseHeaderMapImpl first_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + @@ -2364,10 +2369,8 @@ TEST_F(OAuth2Test, OAuthTestFullFlowWithUseRefreshToken) { "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%253Dadmin%" - "2526level%253Dtrace%26nonce%3D1234567890000000" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" + TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE + "&resource=oauth2-resource" + + "&resource=http%3A%2F%2Fexample.com" "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%3Dbar%" "3Fvar1%3D1%26var2%3D2"}, }; @@ -2388,10 +2391,8 @@ TEST_F(OAuth2Test, OAuthTestFullFlowWithUseRefreshToken) { // This represents the callback request from the authorization server. Http::TestRequestHeaderMapImpl second_request_headers{ {Http::Headers::get().Cookie.get(), - "OauthNonce=1234567890000000;domain=example.com;path=/;Max-Age=600;secure;HttpOnly"}, - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%" - "253Dadmin%2526level%253Dtrace%26nonce%3D1234567890000000"}, + "OauthNonce=" + TEST_STATE_NONCE + ";domain=example.com;path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Scheme.get(), "https"}, @@ -2421,7 +2422,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowWithUseRefreshToken) { "path=/;Max-Age=;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, {Http::Headers::get().Location.get(), - "https://traffic.example.com/test?name=admin&level=trace"}, + "https://traffic.example.com/original_path?var1=1&var2=2"}, }; EXPECT_CALL(decoder_callbacks_, @@ -2431,7 +2432,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowWithUseRefreshToken) { // the third request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl third_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -2459,11 +2460,10 @@ TEST_F(OAuth2Test, OAuthTestFullFlowWithUseRefreshToken) { } TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenSuccess) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */)); // First construct the initial request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -2497,9 +2497,8 @@ TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenSuccess) { "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=https%3A%2F%2Ftraffic.example.com%2Ftest%3Fname%3Dadmin%26level%3Dtrace" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" + TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE + + "resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" "&resource=https%3A%2F%2Fexample.com"}, }; @@ -2513,13 +2512,10 @@ TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenSuccess) { } TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenFail) { - // Set SystemTime to a fixed point so we get consistent nonce between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(123456789))); - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */)); // First construct the initial request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -2547,7 +2543,7 @@ TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenFail) { Http::TestResponseHeaderMapImpl redirect_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;path=/;Max-Age=600;secure;HttpOnly"}, + "OauthNonce=" + TEST_STATE_NONCE + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + @@ -2555,10 +2551,8 @@ TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenFail) { "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=url%3Dhttps%253A%252F%252Ftraffic.example.com%252Ftest%253Fname%253Dadmin%" - "2526level%253Dtrace%26nonce%3D1234567890000000" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" + TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE + "&resource=oauth2-resource" + + "&resource=http%3A%2F%2Fexample.com" "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%3Dbar%" "3Fvar1%3D1%26var2%3D2"}, }; @@ -2580,11 +2574,10 @@ TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenFail) { * Expected behavior: the filter should should return 401 Unauthorized response. */ TEST_F(OAuth2Test, AjaxRefreshDoesNotRedirect) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */)); // First construct the initial request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -2621,14 +2614,13 @@ TEST_F(OAuth2Test, AjaxRefreshDoesNotRedirect) { } TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */)); const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) - 10; // the third request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, @@ -2687,7 +2679,6 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) { } TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenWithBasicAuth) { - init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_BASIC_AUTH @@ -2696,7 +2687,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenWithBasicAuth) { const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) - 10; Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, diff --git a/test/extensions/filters/http/oauth2/oauth_integration_test.cc b/test/extensions/filters/http/oauth2/oauth_integration_test.cc index 7cb83bc11933..686b17f81682 100644 --- a/test/extensions/filters/http/oauth2/oauth_integration_test.cc +++ b/test/extensions/filters/http/oauth2/oauth_integration_test.cc @@ -317,8 +317,10 @@ name: oauth Http::TestRequestHeaderMapImpl headers{ {":method", "GET"}, - {":path", "/callback?code=foo&state=url%3Dhttp%253A%252F%252Ftraffic.example.com%252Fnot%" - "252F_oauth%26nonce%3D1234567890000000"}, + {":path", + "/callback?code=foo&state=" + "eyJ1cmwiOiJodHRwOi8vdHJhZmZpYy5leGFtcGxlLmNvbS9ub3QvX29hdXRoIiwibm9uY2UiOiIxMjM0NTY3ODkwM" + "DAwMDAwIn0"}, // {"url":"http://traffic.example.com/not/_oauth","nonce":"1234567890000000"} {":scheme", "http"}, {"x-forwarded-proto", "http"}, {":authority", "authority"}, @@ -354,8 +356,10 @@ name: oauth codec_client_ = makeHttpConnection(lookupPort("http")); Http::TestRequestHeaderMapImpl headersWithCookie{ {":method", "GET"}, - {":path", "/callback?code=foo&state=url%3Dhttp%253A%252F%252Ftraffic.example.com%252Fnot%" - "252F_oauth%26nonce%3D1234567890000000"}, + {":path", + "/callback?code=foo&state=" + "eyJ1cmwiOiJodHRwOi8vdHJhZmZpYy5leGFtcGxlLmNvbS9ub3QvX29hdXRoIiwibm9uY2UiOiIxMjM0NTY3ODkwM" + "DAwMDAwIn0"}, // {"url":"http://traffic.example.com/not/_oauth","nonce":"1234567890000000"} {":scheme", "http"}, {"x-forwarded-proto", "http"}, {":authority", "authority"},