From 0c6a1d2b847bd5608db1b603c9517341758836f9 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 --- changelogs/current.yaml | 4 + 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 | 19 +- .../filters/http/oauth2/filter_test.cc | 424 +++++++----------- .../http/oauth2/oauth_integration_test.cc | 12 +- 7 files changed, 253 insertions(+), 348 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 54e28ca2ad10..a0b145edc1f3 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -69,6 +69,10 @@ minor_behavior_changes: :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 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 90f39890ba7e..69f984d2c583 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 @@ -293,11 +301,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); } @@ -370,7 +379,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; } @@ -456,7 +464,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()) { @@ -482,7 +490,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. @@ -491,12 +498,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) { @@ -506,7 +512,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. @@ -523,11 +529,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); @@ -535,17 +540,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); @@ -583,6 +585,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); @@ -621,7 +627,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; @@ -848,22 +854,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, "", ""}; } @@ -873,15 +878,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 a0e4da2c22d1..01692cadaa18 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -40,28 +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_( THROW_OR_RETURN_VALUE(Secret::ThreadLocalGenericSecretProvider::create( std::move(client_secret_provider), tls, api), std::unique_ptr)), - token_secret_( + hmac_secret_( THROW_OR_RETURN_VALUE(Secret::ThreadLocalGenericSecretProvider::create( - std::move(token_secret_provider), tls, api), + std::move(hmac_secret_provider), tls, api), std::unique_ptr)) {} const std::string& clientSecret() const override { return client_secret_->secret(); } - const std::string& tokenSecret() const override { return token_secret_->secret(); } + const std::string& hmacSecret() const override { return hmac_secret_->secret(); } private: std::unique_ptr client_secret_; - std::unique_ptr token_secret_; + std::unique_ptr hmac_secret_; }; /** @@ -146,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_; } @@ -268,7 +268,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; @@ -293,7 +293,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; @@ -317,6 +316,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.) @@ -335,6 +335,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 63bf7692aad7..a7be6cc29972 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. @@ -425,10 +429,10 @@ 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. @@ -436,24 +440,20 @@ TEST_F(OAuth2Test, PreservesQueryParametersInAuthorizationEndpoint) { EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).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)); @@ -484,10 +484,10 @@ 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. @@ -495,25 +495,20 @@ TEST_F(OAuth2Test, PreservesQueryParametersInAuthorizationEndpointWithUrlEncodin EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).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)); @@ -544,6 +539,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)); @@ -694,10 +691,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}, @@ -732,7 +728,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)); @@ -753,27 +750,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=" + @@ -781,10 +770,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"}, }; @@ -802,11 +789,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"}, @@ -836,7 +821,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_, @@ -897,10 +882,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}, @@ -926,10 +910,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}, @@ -953,11 +940,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}, @@ -984,10 +974,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}, @@ -1211,17 +1206,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{ @@ -1245,12 +1245,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"}, @@ -1258,6 +1263,7 @@ TEST_F(OAuth2Test, OAuthTestCallbackUrlInStateQueryParam) { "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_STATE_NONCE}, }; Http::TestRequestHeaderMapImpl expected_response_headers{ @@ -1283,13 +1289,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"}, }; @@ -1299,22 +1306,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{ @@ -1340,15 +1344,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"}, }; @@ -1356,110 +1359,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(); - // 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().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, - {Http::Headers::get().Scheme.get(), "https"}, - }; - - // 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"}, - {Http::Headers::get().Location.get(), - "https://auth.example.com/oauth/" - "authorize/?client_id=" + - TEST_CLIENT_ID + - "&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%" - "3Fvar1%3D1%26var2%3D2"}, - }; - - // Fail the validation to trigger the OAuth flow. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - - // 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. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(first_request_headers, false)); - - // 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"}, - {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)); - - EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - AuthType::UrlEncodedBody)); - - // Invoke the callback logic. As a side effect, state_ will be populated. - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(second_request_headers, false)); - - EXPECT_EQ(1, config_->stats().oauth_unauthorized_rq_.value()); - EXPECT_EQ(config_->clusterName(), "auth.example.com"); - - // 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().Location.get(), - "https://traffic.example.com/test?name=admin&level=trace"}, - }; - - EXPECT_CALL(decoder_callbacks_, - encodeHeaders_(HeaderMapEqualRef(&second_response_headers), true)); - - 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))); - +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"}, @@ -1469,7 +1380,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 + ";domain=example.com;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + @@ -1477,10 +1388,8 @@ 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" + 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"}, }; @@ -1489,7 +1398,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. @@ -1499,15 +1408,12 @@ 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"}, - {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"}, }; - // Deliberately fail the HMAC validation check. EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); @@ -1532,17 +1438,19 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { // 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=" - "OYnODPsSGabEpZ2LAiPxyjAFgN/7/5Xg24G7jUoUbyI=;" - "path=/;Max-Age=10;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "OauthExpires=10;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=accessToken;path=/;Max-Age=10;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "IdToken=idToken;path=/;Max-Age=10;secure;HttpOnly"}, + "OauthHMAC=vU9fV//fsKp9ARyrz/HZx2CqWFCmUygihdl18qR5u78=;" + "domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refreshToken;path=/;Max-Age=10;secure;HttpOnly"}, + "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_, @@ -1552,25 +1460,21 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { } /** - * Testing oauth state with cookie domain. + * Testing oauth state with special characters that must be escaped in JSON. * - * Expected behavior: Cookie domain should be set to the domain in the config. + * 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, OAuthTestFullFlowPostWithCookieDomain) { - // 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(getConfig(true, false, - ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: - OAuth2Config_AuthType_URL_ENCODED_BODY, - 0, false, false, true /* set_cookie_domain */)); +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"}, @@ -1580,7 +1484,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { Http::TestResponseHeaderMapImpl first_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthNonce=1234567890000000;domain=example.com;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=" + @@ -1588,10 +1492,9 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { "&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_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"}, @@ -1611,10 +1514,9 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { // 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"}, + "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"}, @@ -1634,6 +1536,8 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { 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); @@ -1641,19 +1545,17 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { // 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=" + "OYnODPsSGabEpZ2LAiPxyjAFgN/7/5Xg24G7jUoUbyI=;" + "path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), "OauthExpires=10;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "OauthHMAC=IyFL1rx3SoaHv2rufMo4nRLHPwTCDAY2fnJ1cpIOcpY=;" - "domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), - "OauthExpires=123456799;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"}, + "BearerToken=accessToken;path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), "IdToken=idToken;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refreshToken;domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, + "RefreshToken=refreshToken;path=/;Max-Age=10;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_, @@ -1911,7 +1813,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 */, @@ -1959,7 +1860,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 */, @@ -2011,7 +1911,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 */, @@ -2064,7 +1963,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 */, @@ -2117,7 +2015,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 */, @@ -2170,7 +2067,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 */, @@ -2225,7 +2121,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 */, @@ -2352,13 +2247,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"}, @@ -2368,7 +2260,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=" + @@ -2376,10 +2268,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"}, }; @@ -2400,10 +2290,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"}, @@ -2433,7 +2321,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_, @@ -2443,7 +2331,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"}, @@ -2471,11 +2359,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"}, @@ -2509,9 +2396,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"}, }; @@ -2525,13 +2411,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"}, @@ -2559,7 +2442,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=" + @@ -2567,10 +2450,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"}, }; @@ -2592,11 +2473,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"}, @@ -2633,14 +2513,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"}, @@ -2699,7 +2578,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 @@ -2708,7 +2586,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 b7670f545711..80a2e3453ab6 100644 --- a/test/extensions/filters/http/oauth2/oauth_integration_test.cc +++ b/test/extensions/filters/http/oauth2/oauth_integration_test.cc @@ -326,8 +326,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"}, @@ -363,8 +365,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"},