From a1d786948b55b89d8808c7b60e2f726543f9a02b Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 10 Apr 2025 05:32:35 +0000 Subject: [PATCH 01/20] encrypt oauth2 access and id tokens Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 4 + .../extensions/filters/http/oauth2/filter.cc | 99 +++++-- .../extensions/filters/http/oauth2/filter.h | 8 +- .../filters/http/oauth2/filter_test.cc | 265 +++++++++++------- 4 files changed, 262 insertions(+), 114 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index ea36940c753e2..ba9ddb0d9a2b3 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -63,6 +63,10 @@ minor_behavior_changes: change: | :ref:`AwsCredentialProvider ` now supports all defined credential providers, allowing complete customisation of the credential provider chain when using AWS request signing extension. + FIPS build is updated to use the same version of boringssl as the regular build, per the revised FedRAMP policy. +- area: oauth2 + change: | + The access token, id token and refresh token in the cookies are now encrypted using the HMCA secret. 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/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index f641c51fa41f6..e1680dffbf1c9 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -277,6 +277,7 @@ std::string encodeState(const std::string& original_request_url, const std::stri */ std::string encrypt(const std::string& plaintext, const std::string& secret, Random::RandomGenerator& random) { + std::cout << "XXXXX encrypt hmac secret" << secret << std::endl; // Generate the key from the secret using SHA-256 std::vector key(SHA256_DIGEST_LENGTH); // AES-256 requires 256-bit (32 bytes) key SHA256(reinterpret_cast(secret.c_str()), secret.size(), key.data()); @@ -342,6 +343,7 @@ struct DecryptResult { * Decrypt an AES-256-CBC encrypted string. */ DecryptResult decrypt(const std::string& encrypted, const std::string& secret) { + std::cout << "XXXXX decrypt hmac secret" << secret << std::endl; // Decode the Base64Url-encoded input std::string decoded = Base64Url::decode(encrypted); std::vector combined(decoded.begin(), decoded.end()); @@ -517,7 +519,7 @@ void OAuth2CookieValidator::setParams(const Http::RequestHeaderMap& headers, }); expires_ = findValue(cookies, cookie_names_.oauth_expires_); - token_ = findValue(cookies, cookie_names_.bearer_token_); + access_token_ = findValue(cookies, cookie_names_.bearer_token_); id_token_ = findValue(cookies, cookie_names_.id_token_); refresh_token_ = findValue(cookies, cookie_names_.refresh_token_); hmac_ = findValue(cookies, cookie_names_.oauth_hmac_); @@ -533,9 +535,18 @@ bool OAuth2CookieValidator::hmacIsValid() const { if (!cookie_domain_.empty()) { cookie_domain = cookie_domain_; } - return ((encodeHmacBase64(secret_, cookie_domain, expires_, token_, id_token_, refresh_token_) == - hmac_) || - (encodeHmacHexBase64(secret_, cookie_domain, expires_, token_, id_token_, + std::cout << "xxxxx encodeHmacBase64: " + << encodeHmacBase64(secret_, cookie_domain, expires_, access_token_, id_token_, + refresh_token_) + << std::endl; + std::cout << "xxxxxencodeHmacHexBase64: " + << encodeHmacHexBase64(secret_, cookie_domain, expires_, access_token_, id_token_, + refresh_token_) + << std::endl; + std::cout << "xxxxxxhamc: " << hmac_ << std::endl; + return ((encodeHmacBase64(secret_, cookie_domain, expires_, access_token_, id_token_, + refresh_token_) == hmac_) || + (encodeHmacHexBase64(secret_, cookie_domain, expires_, access_token_, id_token_, refresh_token_) == hmac_)); } @@ -607,6 +618,10 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he return signOutUser(headers); } + // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding + // the request upstream. + decryptAndUpdateOAuthTokens(headers); + if (canSkipOAuth(headers)) { // Update the path header with the query string parameters after a successful OAuth login. // This is necessary if a website requests multiple resources which get redirected to the @@ -706,7 +721,8 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he DecryptResult decrypt_result = decrypt(encrypted_code_verifier, config_->hmacSecret()); if (decrypt_result.error.has_value()) { - ENVOY_LOG(error, "decryption failed: {}", decrypt_result.error.value()); + ENVOY_LOG(error, "failed to decrypt code verifier: {}, error: {}", encrypted_code_verifier, + decrypt_result.error.value()); sendUnauthorizedResponse(); return Http::FilterHeadersStatus::StopIteration; } @@ -747,6 +763,56 @@ bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const { return false; } +// Decrypts the OAuth tokens and updates the OAuth tokens in the request headers before forwarding +// the request upstream. +void OAuth2Filter::decryptAndUpdateOAuthTokens(Http::RequestHeaderMap& headers) { + const CookieNames& cookie_names = config_->cookieNames(); + + absl::flat_hash_map cookies = Http::Utility::parseCookies(headers); + + const std::string encrypted_access_token = findValue(cookies, cookie_names.bearer_token_); + const std::string encrypted_id_token = findValue(cookies, cookie_names.id_token_); + const std::string encrypted_refresh_token = findValue(cookies, cookie_names.refresh_token_); + + if (!encrypted_access_token.empty()) { + cookies.insert_or_assign(cookie_names.bearer_token_, + decryptToken(encrypted_access_token, config_->hmacSecret())); + } + + if (!encrypted_id_token.empty()) { + cookies.insert_or_assign(cookie_names.id_token_, + decryptToken(encrypted_id_token, config_->hmacSecret())); + } + + if (!encrypted_refresh_token.empty()) { + cookies.insert_or_assign(cookie_names.refresh_token_, + decryptToken(encrypted_refresh_token, config_->hmacSecret())); + } + + std::string new_cookies(absl::StrJoin(cookies, "; ", absl::PairFormatter("="))); + headers.setReferenceKey(Http::Headers::get().Cookie, new_cookies); +} + +std::string OAuth2Filter::decryptToken(const std::string& encrypted_token, + const std::string& secret) { + if (encrypted_token.empty()) { + return EMPTY_STRING; + } + + DecryptResult decrypt_result = decrypt(encrypted_token, secret); + if (decrypt_result.error.has_value()) { + ENVOY_LOG(error, "failed to decrypt token: {}, error: {}", encrypted_token, + decrypt_result.error.value()); + // There are two cases: + // 1. The token is a legacy token, so we return the original token. + // 2. The token is encrypted, but the decryption failed due to the HMAC secret is changed. + // In both cases, the client will be redirected to the OAuth server because Cookie validation + // failed. + return encrypted_token; + } + return decrypt_result.plaintext; +} + bool OAuth2Filter::canRedirectToOAuthServer(Http::RequestHeaderMap& headers) const { for (const auto& matcher : config_->denyRedirectMatchers()) { if (matcher->matchesHeaders(headers)) { @@ -819,9 +885,6 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) { // Generate a PKCE code verifier and challenge for the OAuth flow. const std::string code_verifier = generateCodeVerifier(random_); - // Encrypt the code verifier, using HMAC secret as the symmetric key. - const std::string encrypted_code_verifier = - encrypt(code_verifier, config_->hmacSecret(), random_); // Expire the code verifier cookie in 10 minutes. // This should be enough time for the user to complete the OAuth flow. @@ -833,9 +896,10 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) { cookie_tail_http_only = absl::StrCat( fmt::format(CookieDomainFormatString, config_->cookieDomain()), cookie_tail_http_only); } - response_headers->addReferenceKey(Http::Headers::get().SetCookie, - absl::StrCat(config_->cookieNames().code_verifier_, "=", - encrypted_code_verifier, cookie_tail_http_only)); + response_headers->addReferenceKey( + Http::Headers::get().SetCookie, + absl::StrCat(config_->cookieNames().code_verifier_, "=", + encrypt(code_verifier, config_->hmacSecret(), random_), cookie_tail_http_only)); const std::string code_challenge = generateCodeChallenge(code_verifier); query_params.overwrite(queryParamsCodeChallenge, code_challenge); @@ -1154,7 +1218,8 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, if (!access_token_.empty()) { headers.addReferenceKey(Http::Headers::get().SetCookie, - absl::StrCat(cookie_names.bearer_token_, "=", access_token_, + absl::StrCat(cookie_names.bearer_token_, "=", + encrypt(access_token_, config_->hmacSecret(), random_), BuildCookieTail(1))); // BEARER_TOKEN } else if (request_cookies.contains(cookie_names.bearer_token_)) { headers.addReferenceKey( @@ -1164,9 +1229,10 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, } if (!id_token_.empty()) { - headers.addReferenceKey( - Http::Headers::get().SetCookie, - absl::StrCat(cookie_names.id_token_, "=", id_token_, BuildCookieTail(4))); // ID_TOKEN + headers.addReferenceKey(Http::Headers::get().SetCookie, + absl::StrCat(cookie_names.id_token_, "=", + encrypt(id_token_, config_->hmacSecret(), random_), + BuildCookieTail(4))); // ID_TOKEN } else if (request_cookies.contains(cookie_names.id_token_)) { headers.addReferenceKey( Http::Headers::get().SetCookie, @@ -1176,7 +1242,8 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, if (!refresh_token_.empty()) { headers.addReferenceKey(Http::Headers::get().SetCookie, - absl::StrCat(cookie_names.refresh_token_, "=", refresh_token_, + absl::StrCat(cookie_names.refresh_token_, "=", + encrypt(refresh_token_, config_->hmacSecret(), random_), BuildCookieTail(5))); // REFRESH_TOKEN } else if (request_cookies.contains(cookie_names.refresh_token_)) { headers.addReferenceKey( diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index e7ff46d62711d..9c6266061cd74 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -262,13 +262,13 @@ class CookieValidator { virtual bool canUpdateTokenByRefreshToken() const PURE; }; -class OAuth2CookieValidator : public CookieValidator { +class OAuth2CookieValidator : public CookieValidator, Logger::Loggable { public: explicit OAuth2CookieValidator(TimeSource& time_source, const CookieNames& cookie_names, const std::string& cookie_domain) : time_source_(time_source), cookie_names_(cookie_names), cookie_domain_(cookie_domain) {} - const std::string& token() const override { return token_; } + const std::string& token() const override { return access_token_; } const std::string& refreshToken() const override { return refresh_token_; } void setParams(const Http::RequestHeaderMap& headers, const std::string& secret) override; @@ -278,7 +278,7 @@ class OAuth2CookieValidator : public CookieValidator { bool canUpdateTokenByRefreshToken() const override; private: - std::string token_; + std::string access_token_; std::string id_token_; std::string refresh_token_; std::string expires_; @@ -376,6 +376,8 @@ class OAuth2Filter : public Http::PassThroughFilter, const absl::string_view path_str); bool validateCsrfToken(const Http::RequestHeaderMap& headers, const std::string& csrf_token) const; + void decryptAndUpdateOAuthTokens(Http::RequestHeaderMap& headers); + std::string decryptToken(const std::string& encrypted_token, const std::string& secret); }; } // namespace Oauth2 diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 671ad2517480f..f5dab7cef0039 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -48,6 +48,13 @@ static const std::string TEST_CODE_VERIFIER = "Fc1bBwAAAAAVzVsHAAAAABXNWwcAAAAAF static const std::string TEST_ENCRYPTED_CODE_VERIFIER = "Fc1bBwAAAAAVzVsHAAAAABjf6i_Hvf8T2dEuEhPhhDNMlp16az-0dxisL-TzJKaZjOMF8nov_pG377FHmpKcsA"; static const std::string TEST_CODE_CHALLENGE = "YRQaBq_UpkWzfr6JvtNnh7LMfmPVcIKVYdV98ugwmLY"; +static const std::string TEST_ENCRYPTED_ACCESS_TOKEN = + "Fc1bBwAAAAAVzVsHAAAAAHDCo6XWwdgw5IYsxjfymIQ"; //"access_code" +static const std::string TEST_ENCRYPTED_ID_TOKEN = + "Fc1bBwAAAAAVzVsHAAAAAJohQ-XDfnYLdgIQ2yJfRZQ"; //"some-id-token" +static const std::string TEST_ENCRYPTED_REFRESH_TOKEN = + "Fc1bBwAAAAAVzVsHAAAAAERBBlyQ3ASXvDHzyIRDhLwvl1w07AKhjwBz1s4wJGX8"; //"some-refresh-token" +static const std::string TEST_HMAC_SECRET = "asdf_token_secret_fdsa"; namespace { Http::RegisterCustomInlineHeader @@ -60,7 +67,7 @@ class MockSecretReader : public SecretReader { CONSTRUCT_ON_FIRST_USE(std::string, "asdf_client_secret_fdsa"); } const std::string& hmacSecret() const override { - CONSTRUCT_ON_FIRST_USE(std::string, "asdf_token_secret_fdsa"); + CONSTRUCT_ON_FIRST_USE(std::string, TEST_HMAC_SECRET); } }; @@ -240,7 +247,7 @@ class OAuth2Test : public testing::TestWithParam { // Validates the behavior of the cookie validator. void expectValidCookies(const CookieNames& cookie_names, const std::string& cookie_domain) { // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) + 10; @@ -250,16 +257,17 @@ class OAuth2Test : public testing::TestWithParam { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Cookie.get(), fmt::format("{}={}", cookie_names.oauth_expires_, expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=dCu0otMcLoaGF73jrT+R8rGA0pnWyMgNf4+GivGrHEI=")}, + absl::StrCat(cookie_names.bearer_token_, "=" + TEST_ENCRYPTED_ACCESS_TOKEN)}, + {Http::Headers::get().Cookie.get(), + absl::StrCat(cookie_names.oauth_hmac_, "=kBeSwdaRVv9/K+SGQgsRb3mtRt0Q5507OS9+jhozajk=")}, }; auto cookie_validator = std::make_shared(test_time_, cookie_names, cookie_domain); EXPECT_EQ(cookie_validator->token(), ""); EXPECT_EQ(cookie_validator->refreshToken(), ""); - cookie_validator->setParams(request_headers, "mock-secret"); + cookie_validator->setParams(request_headers, TEST_HMAC_SECRET); EXPECT_TRUE(cookie_validator->hmacIsValid()); EXPECT_TRUE(cookie_validator->timestampIsValid()); @@ -861,11 +869,11 @@ TEST_F(OAuth2Test, SetBearerToken) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=604800;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://traffic.example.com/original_path?var1=1&var2=2"}, }; @@ -1363,9 +1371,10 @@ TEST_F(OAuth2Test, CookieValidatorWithCookieDomain) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Cookie.get(), fmt::format("{}={}", cookie_names.oauth_expires_, expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=zgWoFFmB6rbPHQQYQj35H+Fz+GYZgUrh/C48y0WHWRM=")}, + absl::StrCat(cookie_names.bearer_token_, "=", TEST_ENCRYPTED_ACCESS_TOKEN)}, + {Http::Headers::get().Cookie.get(), + absl::StrCat(cookie_names.oauth_hmac_, "=zezN9ca/1VzTUcXlNURDKQqtnhx2XAYwA/i5Ig69Bv4=")}, }; auto cookie_validator = @@ -1373,7 +1382,7 @@ TEST_F(OAuth2Test, CookieValidatorWithCookieDomain) { EXPECT_EQ(cookie_validator->token(), ""); EXPECT_EQ(cookie_validator->refreshToken(), ""); - cookie_validator->setParams(request_headers, "mock-secret"); + cookie_validator->setParams(request_headers, TEST_HMAC_SECRET); EXPECT_TRUE(cookie_validator->hmacIsValid()); EXPECT_TRUE(cookie_validator->timestampIsValid()); @@ -1394,14 +1403,15 @@ TEST_F(OAuth2Test, CookieValidatorSame) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Cookie.get(), fmt::format("{}={}", cookie_names.oauth_expires_, expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=MSq8mkNQGdXx2LKGlLHMwSIj8rLZRnrHE6EWvvTUFx0=")}, + absl::StrCat(cookie_names.bearer_token_, "=", TEST_ENCRYPTED_ACCESS_TOKEN)}, + {Http::Headers::get().Cookie.get(), + absl::StrCat(cookie_names.oauth_hmac_, "=qQ652uASV5JHHwFJT/wmGPmDtUX9d3MG9hnCp47tQao=")}, }; auto cookie_validator = std::make_shared(test_time_, cookie_names, ""); EXPECT_EQ(cookie_validator->token(), ""); - cookie_validator->setParams(request_headers, "mock-secret"); + cookie_validator->setParams(request_headers, TEST_HMAC_SECRET); EXPECT_TRUE(cookie_validator->hmacIsValid()); EXPECT_TRUE(cookie_validator->timestampIsValid()); @@ -1424,12 +1434,13 @@ TEST_F(OAuth2Test, CookieValidatorSame) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Cookie.get(), fmt::format("{}={}", cookie_names.oauth_expires_, new_expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=dbl04CSr6eWF52wdNDCRt/Uw6A4y41wbpmtUWRyD2Fo=")}, + absl::StrCat(cookie_names.bearer_token_, "=", TEST_ENCRYPTED_ACCESS_TOKEN)}, + {Http::Headers::get().Cookie.get(), + absl::StrCat(cookie_names.oauth_hmac_, "=4kx+NUmjgwAlYl2Vs8PpmhPavUukT71UEeQxTZIkW8U=")}, }; - cookie_validator->setParams(request_headers_second, "mock-secret"); + cookie_validator->setParams(request_headers_second, TEST_HMAC_SECRET); EXPECT_TRUE(cookie_validator->hmacIsValid()); EXPECT_TRUE(cookie_validator->timestampIsValid()); @@ -1449,9 +1460,9 @@ TEST_F(OAuth2Test, CookieValidatorInvalidExpiresAt) { {Http::Headers::get().Path.get(), "/anypath"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Cookie.get(), "OauthExpires=notanumber"}, - {Http::Headers::get().Cookie.get(), "BearerToken=xyztoken"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" - "c+1qzyrMmqG8+O4dn7b28OvNNDWcb04yJfNbZCE1zYE="}, + "rtBqtc5bL7fUko5PlJlXRotRKzFqEHdcjEtXwL3nyTM="}, }; auto cookie_validator = std::make_shared( @@ -1459,7 +1470,7 @@ TEST_F(OAuth2Test, CookieValidatorInvalidExpiresAt) { CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken", "OauthNonce", "CodeVerifier"}, ""); - cookie_validator->setParams(request_headers, "mock-secret"); + cookie_validator->setParams(request_headers, TEST_HMAC_SECRET); EXPECT_TRUE(cookie_validator->hmacIsValid()); EXPECT_FALSE(cookie_validator->timestampIsValid()); @@ -1722,7 +1733,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { // 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); + filter_->updateTokens("access_code", "some-id-token", "some-refresh-token", 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. @@ -1734,11 +1745,14 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { {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"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + + ";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"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + + ";domain=example.com;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refreshToken;domain=example.com;path=/;Max-Age=604800;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";domain=example.com;path=/;Max-Age=604800;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://traffic.example.com/original_path?var1=1&var2=2"}, }; @@ -1836,21 +1850,22 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithSpecialCharactersForJson) { // 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); + filter_->updateTokens("access_code", "some-id-token", "some-refresh-token", 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=" - "OYnODPsSGabEpZ2LAiPxyjAFgN/7/5Xg24G7jUoUbyI=;" + "UzbL/bzvWEP8oaoPDfQrD0zu6zC6m0yBOowKx1Mdr6o=;" "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"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refreshToken;path=/;Max-Age=604800;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=604800;secure;HttpOnly"}, {Http::Headers::get().Location.get(), "https://traffic.example.com" + url_with_special_characters}, }; @@ -1881,9 +1896,9 @@ class DisabledIdTokenTests : public OAuth2Test { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=" + access_code_ + ";path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=" + refresh_token_ + ";path=/;Max-Age=600;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, }; init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, @@ -1937,8 +1952,8 @@ TEST_F(DisabledIdTokenTests, SetCookieIgnoresIdTokenWhenDisabledRefreshToken) { EXPECT_EQ(cookies[cookie_names.oauth_hmac_], hmac_without_id_token_); EXPECT_EQ(cookies[cookie_names.oauth_expires_], "1600"); // Uses default_refresh_token_expires_in since not a legitimate JWT. - EXPECT_EQ(cookies[cookie_names.bearer_token_], access_code_); - EXPECT_EQ(cookies[cookie_names.refresh_token_], refresh_token_); + EXPECT_EQ(cookies[cookie_names.bearer_token_], TEST_ENCRYPTED_ACCESS_TOKEN); + EXPECT_EQ(cookies[cookie_names.refresh_token_], TEST_ENCRYPTED_REFRESH_TOKEN); EXPECT_EQ(cookies.contains(cookie_names.id_token_), false); // And ensure when the response comes back, it has the same cookies in the `expected_headers_`. @@ -2059,9 +2074,9 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokens) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2093,11 +2108,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshToken) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=604800;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2133,11 +2148,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndDefaultRefr {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=1200;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=1200;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2185,9 +2200,9 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshToke {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), "RefreshToken=" + refreshToken + ";path=/;Max-Age=2554415000;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, @@ -2237,9 +2252,9 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefr {Http::Headers::get().SetCookie.get(), "OauthExpires=2554515600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), "RefreshToken=" + refreshToken + ";path=/;Max-Age=0;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, @@ -2279,6 +2294,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndNoExpClaimI "eyJhbGciOiJIUzI1NiJ9." "eyJSb2xlIjoiQWRtaW4iLCJJc3N1ZXIiOiJJc3N1ZXIiLCJVc2VybmFtZSI6IkphdmFJblVzZSIsImlhdCI6MTcwODA2" "NDcyOH0.92H-X2Oa4ECNmFLZBWBHP0BJyEHDprLkEIc2JBJYwkI"; + const std::string encrypted_refresh_token = + "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDE7Qej3gaQyBPqvzoNiSVn8-sv2lmZF7nT3OVnBe7X-KK-" + "jOOVaiHesGNEsPt5F0CmkMytmf-t0VMASmnC8FhgnCsRkf2XHL_" + "z18YGJTvbHgc6QDdKUDwGuMTL048BdQYelXZ9nwtNchSkbZIa8yUf5wrZtEvFpOzE-brHaI3LOWmHaQ27h_" + "lm5eH0qKwMy_jXZMXhxzO_-Rrz9XBlVwIMP"; // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2288,11 +2308,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndNoExpClaimI {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=" + refreshToken + ";path=/;Max-Age=1200;secure;HttpOnly"}, + "RefreshToken=" + encrypted_refresh_token + ";path=/;Max-Age=1200;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2332,6 +2352,12 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensIdTokenExpiresInFromJwt) { "eyJ1bmlxdWVfbmFtZSI6ImFsZXhjZWk4OCIsInN1YiI6ImFsZXhjZWk4OCIsImp0aSI6IjQ5ZTFjMzc1IiwiYXVkIjoi" "dGVzdCIsIm5iZiI6MTcwNzQxNDYzNSwiZXhwIjoyNTU0NDE2MDAwLCJpYXQiOjE3MDc0MTQ2MzYsImlzcyI6ImRvdG5l" "dC11c2VyLWp3dHMifQ.LaGOw6x0-m7r-WzxgCIdPnAfp0O1hy6mW4klq9Vs2XM"; + const std::string encrypted_id_token = + "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_" + "amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-" + "x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1V" + "LZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW2" + "3YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2341,11 +2367,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensIdTokenExpiresInFromJwt) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=" + id_token + ";path=/;Max-Age=2554415000;secure;HttpOnly"}, + "IdToken=" + encrypted_id_token + ";path=/;Max-Age=2554415000;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refresh-token;path=/;Max-Age=1200;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=1200;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2384,6 +2410,12 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensExpiredIdToken) { "eyJ1bmlxdWVfbmFtZSI6ImFsZXhjZWk4OCIsInN1YiI6ImFsZXhjZWk4OCIsImp0aSI6IjQ5ZTFjMzc1IiwiYXVkIjoi" "dGVzdCIsIm5iZiI6MTcwNzQxNDYzNSwiZXhwIjoyNTU0NDE2MDAwLCJpYXQiOjE3MDc0MTQ2MzYsImlzcyI6ImRvdG5l" "dC11c2VyLWp3dHMifQ.LaGOw6x0-m7r-WzxgCIdPnAfp0O1hy6mW4klq9Vs2XM"; + const std::string encrypted_id_token = + "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_" + "amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-" + "x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1V" + "LZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW2" + "3YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2393,11 +2425,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensExpiredIdToken) { {Http::Headers::get().SetCookie.get(), "OauthExpires=2554515600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=" + id_token + ";path=/;Max-Age=0;secure;HttpOnly"}, + "IdToken=" + encrypted_id_token + ";path=/;Max-Age=0;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refresh-token;path=/;Max-Age=1200;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=1200;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2437,6 +2469,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensNoExpClaimInIdToken) { "eyJhbGciOiJIUzI1NiJ9." "eyJSb2xlIjoiQWRtaW4iLCJJc3N1ZXIiOiJJc3N1ZXIiLCJVc2VybmFtZSI6IkphdmFJblVzZSIsImlhdCI6MTcwODA2" "NDcyOH0.92H-X2Oa4ECNmFLZBWBHP0BJyEHDprLkEIc2JBJYwkI"; + const std::string encrypted_id_token = + "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDE7Qej3gaQyBPqvzoNiSVn8-sv2lmZF7nT3OVnBe7X-KK-" + "jOOVaiHesGNEsPt5F0CmkMytmf-t0VMASmnC8FhgnCsRkf2XHL_" + "z18YGJTvbHgc6QDdKUDwGuMTL048BdQYelXZ9nwtNchSkbZIa8yUf5wrZtEvFpOzE-brHaI3LOWmHaQ27h_" + "lm5eH0qKwMy_jXZMXhxzO_-Rrz9XBlVwIMP"; // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2446,11 +2483,11 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensNoExpClaimInIdToken) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "IdToken=" + id_token + ";path=/;Max-Age=600;secure;HttpOnly"}, + "IdToken=" + encrypted_id_token + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refresh-token;path=/;Max-Age=1200;secure;HttpOnly"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=1200;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2499,9 +2536,9 @@ TEST_F(OAuth2Test, CookieValidatorInTransition) { {Http::Headers::get().Path.get(), "/_signout"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Cookie.get(), "OauthExpires=1600"}, - {Http::Headers::get().Cookie.get(), "BearerToken=access_code"}, - {Http::Headers::get().Cookie.get(), "IdToken=some-id-token"}, - {Http::Headers::get().Cookie.get(), "RefreshToken=some-refresh-token"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "Y9gCpVnhyaY+ecSxt/ZLZc/OMb8ZNivrVH1RByJxEbs="}, }; @@ -2519,9 +2556,9 @@ TEST_F(OAuth2Test, CookieValidatorInTransition) { {Http::Headers::get().Path.get(), "/_signout"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Cookie.get(), "OauthExpires=1600"}, - {Http::Headers::get().Cookie.get(), "BearerToken=access_code"}, - {Http::Headers::get().Cookie.get(), "IdToken=some-id-token"}, - {Http::Headers::get().Cookie.get(), "RefreshToken=some-refresh-token"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "NjNkODAyYTU1OWUxYzlhNjNlNzljNGIxYjdmNjRiNjVjZmNlMzFiZjE5MzYyYmViNTQ3ZDUxMDcyMjcxMTFiYg=="}, @@ -2888,7 +2925,9 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) - 10; - std::string legit_refresh_token{"legit_refresh_token"}; + std::string legit_refresh_token = "legit_refresh_token"; + std::string encrypted_refresh_token = + "Fc1bBwAAAAAVzVsHAAAAAOh8bHz59OyZPtKMgiX5FWJMyTXqsPjbf1j-Ao8fn1tb"; // the third request to the oauth filter with URI parameters. Http::TestRequestHeaderMapImpl request_headers{ {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, @@ -2896,8 +2935,9 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Cookie.get(), fmt::format("OauthExpires={}", expires_at_s)}, - {Http::Headers::get().Cookie.get(), fmt::format("RefreshToken={}", legit_refresh_token)}, - {Http::Headers::get().Cookie.get(), "BearerToken=xyztoken"}, + {Http::Headers::get().Cookie.get(), fmt::format("RefreshToken={}", encrypted_refresh_token)}, + {Http::Headers::get().Cookie.get(), + "BearerToken=Fc1bBwAAAAAVzVsHAAAAANWU4XU3MGwGHLzWF6KUL0s"}, // xyztoken {Http::Headers::get().Cookie.get(), "OauthHMAC=dCu0otMcLoaGF73jrT+R8rGA0pnWyMgNf4+GivGrHEI="}, }; @@ -2933,10 +2973,12 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) "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"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - fmt::format("RefreshToken={};path=/;Max-Age=604800;secure;HttpOnly", legit_refresh_token)}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + fmt::format("RefreshToken={};path=/;Max-Age=604800;secure;HttpOnly", + encrypted_refresh_token)}, }; EXPECT_THAT(response_headers, HeaderMapEqualRef(&expected_response_headers)); @@ -2946,7 +2988,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) EXPECT_EQ(cookies.at("OauthExpires"), "10"); EXPECT_EQ(cookies.at("BearerToken"), "accessToken"); EXPECT_EQ(cookies.at("IdToken"), "idToken"); - EXPECT_EQ(cookies.at("RefreshToken"), legit_refresh_token); + EXPECT_EQ(cookies.at("RefreshToken"), encrypted_refresh_token); } TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenWithBasicAuth) { @@ -2955,6 +2997,8 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenWithBasicAuth) { OAuth2Config_AuthType_BASIC_AUTH /* authType */)); + // 1. Test sending a request with expired tokens. + // Set the expiration time to 10 seconds in the past to simulate token expiration. const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) - 10; Http::TestRequestHeaderMapImpl request_headers{ @@ -2963,31 +3007,44 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenWithBasicAuth) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Cookie.get(), fmt::format("OauthExpires={}", expires_at_s)}, - {Http::Headers::get().Cookie.get(), "BearerToken=xyztoken"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=dCu0otMcLoaGF73jrT+R8rGA0pnWyMgNf4+GivGrHEI="}, - {Http::Headers::get().Cookie.get(), "RefreshToken=legit_refresh_token"}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, }; - std::string legit_refresh_token{"legit_refresh_token"}; + std::string legit_refresh_token{"some-refresh-token"}; EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); + // Filter should refresh the tokens using the refresh token because the tokens are expired and a + // refresh token is available. EXPECT_CALL(*oauth_client_, asyncRefreshAccessToken(legit_refresh_token, TEST_CLIENT_ID, "asdf_client_secret_fdsa", AuthType::BasicAuth)); + // Filter should stop iteration because the tokens are expired. EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers, false)); + // 2. Test refresh flow succeeds. + // The new tokens received from the refresh flow. + const std::string access_token = "accessToken"; + const std::string id_token = "idToken"; + const std::string refresh_token = "refreshToken"; + const std::string encrypted_id_token = "Fc1bBwAAAAAVzVsHAAAAAPD4z8oLeVyvkfTcl_cw198"; + const std::string encrypted_access_token = "Fc1bBwAAAAAVzVsHAAAAAGUINzc06x19yQYjN4Kb-YA"; + const std::string encrypted_refresh_token = "Fc1bBwAAAAAVzVsHAAAAACWUO4LpH2VJBN_6jSUWDPg"; + + // Filter should continue decoding because the tokens are refreshed. EXPECT_CALL(decoder_callbacks_, continueDecoding()); // 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); + filter_->updateTokens(access_token, id_token, refresh_token, expiredTime); filter_->finishRefreshAccessTokenFlow(); @@ -3001,14 +3058,17 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenWithBasicAuth) { "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"}, + "BearerToken=" + encrypted_access_token + ";path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refreshToken;path=/;Max-Age=604800;secure;HttpOnly"}, + "IdToken=" + encrypted_id_token + ";path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "RefreshToken=" + encrypted_refresh_token + ";path=/;Max-Age=604800;secure;HttpOnly"}, }; + // Test the response headers are set correctly with the new tokens. EXPECT_THAT(response_headers, HeaderMapEqualRef(&expected_response_headers)); + // Test the request headers are updated with the new tokens. auto cookies = Http::Utility::parseCookies(request_headers); EXPECT_EQ(cookies.at("OauthHMAC"), "OYnODPsSGabEpZ2LAiPxyjAFgN/7/5Xg24G7jUoUbyI="); EXPECT_EQ(cookies.at("OauthExpires"), "10"); @@ -3043,11 +3103,14 @@ TEST_F(OAuth2Test, AllCookiesStrictSameSite) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly;SameSite=Strict"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";path=/;Max-Age=604800;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().Location.get(), ""}, }; @@ -3084,11 +3147,13 @@ TEST_F(OAuth2Test, AllCookiesNoneSameSite) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly;SameSite=None"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";path=/;Max-Age=604800;secure;HttpOnly;SameSite=None"}, {Http::Headers::get().Location.get(), ""}, }; @@ -3125,11 +3190,13 @@ TEST_F(OAuth2Test, AllCookiesLaxSameSite) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly;SameSite=Lax"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly;SameSite=Lax"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Lax"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly;SameSite=Lax"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Lax"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly;SameSite=Lax"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";path=/;Max-Age=604800;secure;HttpOnly;SameSite=Lax"}, {Http::Headers::get().Location.get(), ""}, }; @@ -3166,11 +3233,13 @@ TEST_F(OAuth2Test, MixedCookieSameSiteWithDisabled) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly;SameSite=Strict"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";path=/;Max-Age=604800;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().Location.get(), ""}, }; @@ -3207,11 +3276,14 @@ TEST_F(OAuth2Test, MixedCookieSameSiteWithoutDisabled) { {Http::Headers::get().SetCookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, {Http::Headers::get().SetCookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().SetCookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly;SameSite=Lax"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";path=/;Max-Age=604800;secure;HttpOnly;SameSite=Lax"}, {Http::Headers::get().Location.get(), ""}, }; @@ -3289,11 +3361,14 @@ TEST_F(OAuth2Test, CookiesDeletedWhenTokensCleared) { {Http::Headers::get().Cookie.get(), "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly;SameSite=None"}, {Http::Headers::get().Cookie.get(), - "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().Cookie.get(), - "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, {Http::Headers::get().Cookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly;SameSite=Lax"}, + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";path=/;Max-Age=604800;secure;HttpOnly;SameSite=Lax"}, {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_CSRF_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly;SameSite=Strict"}, }; From 40b3bc5ab0ed8b4bb2bd8733f01baa7934db46ac Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 16 Apr 2025 04:45:16 +0000 Subject: [PATCH 02/20] fix test Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/filter.cc | 19 +++--- .../extensions/filters/http/oauth2/filter.h | 2 +- .../filters/http/oauth2/filter_test.cc | 63 ++++++------------- 3 files changed, 33 insertions(+), 51 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index e1680dffbf1c9..3105249a819e5 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -620,7 +620,7 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding // the request upstream. - decryptAndUpdateOAuthTokens(headers); + decryptAndUpdateOAuthTokenCookies(headers); if (canSkipOAuth(headers)) { // Update the path header with the query string parameters after a successful OAuth login. @@ -763,12 +763,15 @@ bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const { return false; } -// Decrypts the OAuth tokens and updates the OAuth tokens in the request headers before forwarding +// Decrypts the OAuth tokens and updates the OAuth tokens in the request cookies before forwarding // the request upstream. -void OAuth2Filter::decryptAndUpdateOAuthTokens(Http::RequestHeaderMap& headers) { - const CookieNames& cookie_names = config_->cookieNames(); - +void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers) { absl::flat_hash_map cookies = Http::Utility::parseCookies(headers); + if(cookies.empty()){ + return; + } + + const CookieNames& cookie_names = config_->cookieNames(); const std::string encrypted_access_token = findValue(cookies, cookie_names.bearer_token_); const std::string encrypted_id_token = findValue(cookies, cookie_names.id_token_); @@ -789,8 +792,10 @@ void OAuth2Filter::decryptAndUpdateOAuthTokens(Http::RequestHeaderMap& headers) decryptToken(encrypted_refresh_token, config_->hmacSecret())); } - std::string new_cookies(absl::StrJoin(cookies, "; ", absl::PairFormatter("="))); - headers.setReferenceKey(Http::Headers::get().Cookie, new_cookies); + if (!encrypted_access_token.empty()||!encrypted_id_token.empty()||!encrypted_refresh_token.empty()){ + std::string new_cookies(absl::StrJoin(cookies, "; ", absl::PairFormatter("="))); + headers.setReferenceKey(Http::Headers::get().Cookie, new_cookies); + } } std::string OAuth2Filter::decryptToken(const std::string& encrypted_token, diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 9c6266061cd74..b6bb4517f08f9 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -376,7 +376,7 @@ class OAuth2Filter : public Http::PassThroughFilter, const absl::string_view path_str); bool validateCsrfToken(const Http::RequestHeaderMap& headers, const std::string& csrf_token) const; - void decryptAndUpdateOAuthTokens(Http::RequestHeaderMap& headers); + void decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers); std::string decryptToken(const std::string& encrypted_token, const std::string& secret); }; diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index f5dab7cef0039..0b706db8171de 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -260,7 +260,7 @@ class OAuth2Test : public testing::TestWithParam { {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=" + TEST_ENCRYPTED_ACCESS_TOKEN)}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=kBeSwdaRVv9/K+SGQgsRb3mtRt0Q5507OS9+jhozajk=")}, + absl::StrCat(cookie_names.oauth_hmac_, "=oMh0+qk68Y4ya4JGQqT+Ja1Y1X58Sc8iATRxPPPG5Yc=")}, }; auto cookie_validator = @@ -1374,7 +1374,7 @@ TEST_F(OAuth2Test, CookieValidatorWithCookieDomain) { {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=", TEST_ENCRYPTED_ACCESS_TOKEN)}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=zezN9ca/1VzTUcXlNURDKQqtnhx2XAYwA/i5Ig69Bv4=")}, + absl::StrCat(cookie_names.oauth_hmac_, "=PHLtlCLTIjfuAocmHmW8QzM3YSTRF6L+E3o6a1+TiS4=")}, }; auto cookie_validator = @@ -1406,7 +1406,7 @@ TEST_F(OAuth2Test, CookieValidatorSame) { {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=", TEST_ENCRYPTED_ACCESS_TOKEN)}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=qQ652uASV5JHHwFJT/wmGPmDtUX9d3MG9hnCp47tQao=")}, + absl::StrCat(cookie_names.oauth_hmac_, "=eYef0itomg0CAjYygAfCLwmS2s1DaiL+N1Ql5V48o4o=")}, }; auto cookie_validator = std::make_shared(test_time_, cookie_names, ""); @@ -1437,7 +1437,7 @@ TEST_F(OAuth2Test, CookieValidatorSame) { {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=", TEST_ENCRYPTED_ACCESS_TOKEN)}, {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=4kx+NUmjgwAlYl2Vs8PpmhPavUukT71UEeQxTZIkW8U=")}, + absl::StrCat(cookie_names.oauth_hmac_, "=VSTrKslW8ZNUqwgP+6Ocm1+7+NcF8GG/e1dqKsq14rc=")}, }; cookie_validator->setParams(request_headers_second, TEST_HMAC_SECRET); @@ -1462,7 +1462,7 @@ TEST_F(OAuth2Test, CookieValidatorInvalidExpiresAt) { {Http::Headers::get().Cookie.get(), "OauthExpires=notanumber"}, {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" - "rtBqtc5bL7fUko5PlJlXRotRKzFqEHdcjEtXwL3nyTM="}, + "042KfjoL8OTsm8r4l6IO5dlxjzkaTDSyCaAibGI00bM="}, }; auto cookie_validator = std::make_shared( @@ -1551,7 +1551,7 @@ TEST_F(OAuth2Test, OAuthTestCallbackUrlInStateQueryParam) { "&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(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" @@ -1569,31 +1569,13 @@ TEST_F(OAuth2Test, OAuthTestCallbackUrlInStateQueryParam) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - std::string legit_token{"legit_token"}; + std::string legit_token{"access_code"}; EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_response_headers), false)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers, false)); - - Http::TestRequestHeaderMapImpl final_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=" + 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_CSRF_TOKEN}, - {Http::CustomHeaders::get().Authorization.get(), "Bearer legit_token"}, - }; - - EXPECT_EQ(request_headers, final_request_headers); } TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { @@ -1606,7 +1588,7 @@ TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=legit_token"}, + {Http::Headers::get().Cookie.get(), "BearerToken=access_code"}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" @@ -1624,7 +1606,7 @@ TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - std::string legit_token{"legit_token"}; + std::string legit_token{"access_code"}; EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); EXPECT_CALL(decoder_callbacks_, @@ -1632,23 +1614,18 @@ TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers, false)); - Http::TestRequestHeaderMapImpl final_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=" + 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=" + TEST_CSRF_TOKEN}, - {Http::CustomHeaders::get().Authorization.get(), "Bearer legit_token"}, - }; + EXPECT_EQ(request_headers.getHostValue(), "traffic.example.com"); + EXPECT_EQ(request_headers.getMethodValue(), Http::Headers::get().MethodValues.Get); + EXPECT_EQ(request_headers.getPathValue(), "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + + "&state=" + TEST_ENCODED_STATE); + auto auth_header = request_headers.get(Http::CustomHeaders::get().Authorization); + EXPECT_EQ(auth_header[0]->value().getStringView(), "Bearer access_code"); - EXPECT_EQ(request_headers, final_request_headers); + auto cookies = Http::Utility::parseCookies(request_headers); + EXPECT_EQ(cookies["OauthExpires"], "123"); + EXPECT_EQ(cookies["BearerToken"], "access_code"); + EXPECT_EQ(cookies["OauthHMAC"], "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMjRlNjMxZTJmNTZkYzRmZTM0ZQ===="); + EXPECT_EQ(cookies["OauthNonce"], TEST_CSRF_TOKEN); } /** From 4d82c5816ac7fd82bc070a92d24763a0a6577964 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 6 Jun 2025 02:45:53 +0000 Subject: [PATCH 03/20] format Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 1 - source/extensions/filters/http/oauth2/filter.cc | 5 +++-- test/extensions/filters/http/oauth2/filter_test.cc | 9 ++++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index ba9ddb0d9a2b3..be00e7bce2a85 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -63,7 +63,6 @@ minor_behavior_changes: change: | :ref:`AwsCredentialProvider ` now supports all defined credential providers, allowing complete customisation of the credential provider chain when using AWS request signing extension. - FIPS build is updated to use the same version of boringssl as the regular build, per the revised FedRAMP policy. - area: oauth2 change: | The access token, id token and refresh token in the cookies are now encrypted using the HMCA secret. diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 3105249a819e5..af5a0409b9deb 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -767,7 +767,7 @@ bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const { // the request upstream. void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers) { absl::flat_hash_map cookies = Http::Utility::parseCookies(headers); - if(cookies.empty()){ + if (cookies.empty()) { return; } @@ -792,7 +792,8 @@ void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& hea decryptToken(encrypted_refresh_token, config_->hmacSecret())); } - if (!encrypted_access_token.empty()||!encrypted_id_token.empty()||!encrypted_refresh_token.empty()){ + if (!encrypted_access_token.empty() || !encrypted_id_token.empty() || + !encrypted_refresh_token.empty()) { std::string new_cookies(absl::StrJoin(cookies, "; ", absl::PairFormatter("="))); headers.setReferenceKey(Http::Headers::get().Cookie, new_cookies); } diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 0b706db8171de..29640aca3663b 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -1616,15 +1616,18 @@ TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { EXPECT_EQ(request_headers.getHostValue(), "traffic.example.com"); EXPECT_EQ(request_headers.getMethodValue(), Http::Headers::get().MethodValues.Get); - EXPECT_EQ(request_headers.getPathValue(), "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + - "&state=" + TEST_ENCODED_STATE); + EXPECT_EQ(request_headers.getPathValue(), + "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + + "&state=" + TEST_ENCODED_STATE); auto auth_header = request_headers.get(Http::CustomHeaders::get().Authorization); EXPECT_EQ(auth_header[0]->value().getStringView(), "Bearer access_code"); auto cookies = Http::Utility::parseCookies(request_headers); EXPECT_EQ(cookies["OauthExpires"], "123"); EXPECT_EQ(cookies["BearerToken"], "access_code"); - EXPECT_EQ(cookies["OauthHMAC"], "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMjRlNjMxZTJmNTZkYzRmZTM0ZQ===="); + EXPECT_EQ( + cookies["OauthHMAC"], + "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMjRlNjMxZTJmNTZkYzRmZTM0ZQ===="); EXPECT_EQ(cookies["OauthNonce"], TEST_CSRF_TOKEN); } From d664edb477b3df2ba88d802d702c79ce1126c071 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 6 Jun 2025 11:33:29 +0000 Subject: [PATCH 04/20] fix test Signed-off-by: Huabing (Robin) Zhao --- .../filters/http/oauth2/filter_test.cc | 72 ++++++++++--------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 29640aca3663b..cfdc469445002 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -1500,6 +1500,7 @@ 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","csrf_token":"${extracted}"} + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); static const std::string state_with_invalid_url = "eyJ1cmwiOiJibGFoIiwiY3NyZl90b2tlbiI6IjAwMDAwMDAwMDc1YmNkMTUifQ"; Http::TestRequestHeaderMapImpl request_headers{ @@ -1509,7 +1510,7 @@ TEST_F(OAuth2Test, OAuthTestInvalidUrlInStateQueryParam) { "/_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(), "BearerToken="+TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" @@ -1528,7 +1529,7 @@ TEST_F(OAuth2Test, OAuthTestInvalidUrlInStateQueryParam) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - std::string legit_token{"legit_token"}; + std::string legit_token{"access_code"}; EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), false)); @@ -1588,7 +1589,7 @@ TEST_F(OAuth2Test, OAuthTestUpdatePathAfterSuccess) { "/_oauth?code=abcdefxyz123&scope=" + TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=access_code"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" @@ -1720,7 +1721,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithCookieDomain) { Http::TestRequestHeaderMapImpl second_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().SetCookie.get(), - "OauthHMAC=vU9fV//fsKp9ARyrz/HZx2CqWFCmUygihdl18qR5u78=;" + "OauthHMAC=seD1HFQMr2pDwXgZKYQ1+D8R/p8tCa2fO8xTmfAgAUg=;" "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"}, @@ -1932,8 +1933,8 @@ TEST_F(DisabledIdTokenTests, SetCookieIgnoresIdTokenWhenDisabledRefreshToken) { EXPECT_EQ(cookies[cookie_names.oauth_hmac_], hmac_without_id_token_); EXPECT_EQ(cookies[cookie_names.oauth_expires_], "1600"); // Uses default_refresh_token_expires_in since not a legitimate JWT. - EXPECT_EQ(cookies[cookie_names.bearer_token_], TEST_ENCRYPTED_ACCESS_TOKEN); - EXPECT_EQ(cookies[cookie_names.refresh_token_], TEST_ENCRYPTED_REFRESH_TOKEN); + EXPECT_EQ(cookies[cookie_names.bearer_token_], "access_code"); + EXPECT_EQ(cookies[cookie_names.refresh_token_], "some-refresh-token"); EXPECT_EQ(cookies.contains(cookie_names.id_token_), false); // And ensure when the response comes back, it has the same cookies in the `expected_headers_`. @@ -2171,6 +2172,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshToke "eyJ1bmlxdWVfbmFtZSI6ImFsZXhjZWk4OCIsInN1YiI6ImFsZXhjZWk4OCIsImp0aSI6IjQ5ZTFjMzc1IiwiYXVkIjoi" "dGVzdCIsIm5iZiI6MTcwNzQxNDYzNSwiZXhwIjoyNTU0NDE2MDAwLCJpYXQiOjE3MDc0MTQ2MzYsImlzcyI6ImRvdG5l" "dC11c2VyLWp3dHMifQ.LaGOw6x0-m7r-WzxgCIdPnAfp0O1hy6mW4klq9Vs2XM"; + const std::string encrypted_refresh_token = "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1VLZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW23YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2184,7 +2186,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshToke {Http::Headers::get().SetCookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=" + refreshToken + ";path=/;Max-Age=2554415000;secure;HttpOnly"}, + "RefreshToken=" + encrypted_refresh_token + ";path=/;Max-Age=2554415000;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2223,6 +2225,8 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefr "eyJ1bmlxdWVfbmFtZSI6ImFsZXhjZWk4OCIsInN1YiI6ImFsZXhjZWk4OCIsImp0aSI6IjQ5ZTFjMzc1IiwiYXVkIjoi" "dGVzdCIsIm5iZiI6MTcwNzQxNDYzNSwiZXhwIjoyNTU0NDE2MDAwLCJpYXQiOjE3MDc0MTQ2MzYsImlzcyI6ImRvdG5l" "dC11c2VyLWp3dHMifQ.LaGOw6x0-m7r-WzxgCIdPnAfp0O1hy6mW4klq9Vs2XM"; +const std::string encrypted_refresh_token = "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1VLZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW23YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; + // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2236,7 +2240,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefr {Http::Headers::get().SetCookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=" + refreshToken + ";path=/;Max-Age=0;secure;HttpOnly"}, + "RefreshToken=" + encrypted_refresh_token + ";path=/;Max-Age=0;secure;HttpOnly"}, {Http::Headers::get().Location.get(), ""}, }; @@ -2315,7 +2319,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensIdTokenExpiresInFromJwt) { OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, 1200 /* default_refresh_token_expires_in */)); TestScopedRuntime scoped_runtime; - oauthHMAC = "UjDfDiq1RHQooE16EhoadVxwOD7sBvrn+S8CZ2k4tvM=;"; + oauthHMAC = "MqrMKGLbdIEogLWZPRffaVTXDGRRveG3gn9bZu5Gd4Q=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); @@ -2357,7 +2361,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensIdTokenExpiresInFromJwt) { EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), true)); - filter_->onGetAccessTokenSuccess("access_code", id_token, "refresh-token", + filter_->onGetAccessTokenSuccess("access_code", id_token, "some-refresh-token", std::chrono::seconds(600)); } @@ -2373,7 +2377,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensExpiredIdToken) { OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, 1200 /* default_refresh_token_expires_in */)); TestScopedRuntime scoped_runtime; - oauthHMAC = "HSyUburg3d4IXM2+5gCiIEn6VvLm584MqFmVEed4Jyc=;"; + oauthHMAC = "eQmiVNw3uAZixmzqtd75kD/0MeSJzS/ROl99NNfWoyU=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(2554515000))); @@ -2415,7 +2419,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensExpiredIdToken) { EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), true)); - filter_->onGetAccessTokenSuccess("access_code", id_token, "refresh-token", + filter_->onGetAccessTokenSuccess("access_code", id_token, "some-refresh-token", std::chrono::seconds(600)); } @@ -2433,7 +2437,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensNoExpClaimInIdToken) { OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, 1200 /* default_refresh_token_expires_in */)); TestScopedRuntime scoped_runtime; - oauthHMAC = "6CyS8TiamKlAVtPpHANqYOwS59gOTCIRXV9j1GtGwqA=;"; + oauthHMAC = "CU0eIzpTJSD/LFOVPaH7ypOQqqBvh4s6Tin3ip9rajk=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); @@ -2473,7 +2477,7 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensNoExpClaimInIdToken) { EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), true)); - filter_->onGetAccessTokenSuccess("access_code", id_token, "refresh-token", + filter_->onGetAccessTokenSuccess("access_code", id_token, "some-refresh-token", std::chrono::seconds(600)); } @@ -2519,8 +2523,7 @@ TEST_F(OAuth2Test, CookieValidatorInTransition) { {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN}, {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, - {Http::Headers::get().Cookie.get(), "OauthHMAC=" - "Y9gCpVnhyaY+ecSxt/ZLZc/OMb8ZNivrVH1RByJxEbs="}, + {Http::Headers::get().Cookie.get(), "OauthHMAC=eK7Kw2VqlnZJiz93KTnZqUar3ajNAe+ubmosGFkyL4I="}, }; auto cookie_validator = std::make_shared( @@ -2540,8 +2543,7 @@ TEST_F(OAuth2Test, CookieValidatorInTransition) { {Http::Headers::get().Cookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN}, {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, {Http::Headers::get().Cookie.get(), - "OauthHMAC=" - "NjNkODAyYTU1OWUxYzlhNjNlNzljNGIxYjdmNjRiNjVjZmNlMzFiZjE5MzYyYmViNTQ3ZDUxMDcyMjcxMTFiYg=="}, + "OauthHMAC=eK7Kw2VqlnZJiz93KTnZqUar3ajNAe+ubmosGFkyL4I="}, }; cookie_validator->setParams(request_headers_hexbase64, "mock-secret"); @@ -2845,7 +2847,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Cookie.get(), fmt::format("OauthExpires={}", expires_at_s)}, - {Http::Headers::get().Cookie.get(), "BearerToken=xyztoken"}, + {Http::Headers::get().Cookie.get(), "BearerToken="+ TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=dCu0otMcLoaGF73jrT+R8rGA0pnWyMgNf4+GivGrHEI="}, }; @@ -2868,7 +2870,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) { // 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); + filter_->updateTokens("access_code", "some-id-token", "some-refresh-token", expiredTime); filter_->finishRefreshAccessTokenFlow(); @@ -2878,24 +2880,24 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) { Http::TestResponseHeaderMapImpl expected_response_headers{ {Http::Headers::get().SetCookie.get(), "OauthHMAC=" - "OYnODPsSGabEpZ2LAiPxyjAFgN/7/5Xg24G7jUoUbyI=;" + "UzbL/bzvWEP8oaoPDfQrD0zu6zC6m0yBOowKx1Mdr6o=;" "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"}, + "BearerToken="+TEST_ENCRYPTED_ACCESS_TOKEN+";path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=refreshToken;path=/;Max-Age=604800;secure;HttpOnly"}, + "RefreshToken=" +TEST_ENCRYPTED_REFRESH_TOKEN+";path=/;Max-Age=604800;secure;HttpOnly"}, }; EXPECT_THAT(response_headers, HeaderMapEqualRef(&expected_response_headers)); auto cookies = Http::Utility::parseCookies(request_headers); - EXPECT_EQ(cookies.at("OauthHMAC"), "OYnODPsSGabEpZ2LAiPxyjAFgN/7/5Xg24G7jUoUbyI="); + EXPECT_EQ(cookies.at("OauthHMAC"), "UzbL/bzvWEP8oaoPDfQrD0zu6zC6m0yBOowKx1Mdr6o="); EXPECT_EQ(cookies.at("OauthExpires"), "10"); - EXPECT_EQ(cookies.at("BearerToken"), "accessToken"); - EXPECT_EQ(cookies.at("IdToken"), "idToken"); - EXPECT_EQ(cookies.at("RefreshToken"), "refreshToken"); + EXPECT_EQ(cookies.at("BearerToken"), "access_code"); + EXPECT_EQ(cookies.at("IdToken"), "some-id-token"); + EXPECT_EQ(cookies.at("RefreshToken"), "some-refresh-token"); } // When a refresh flow succeeds, but a new refresh token isn't received from the OAuth server, the @@ -2917,7 +2919,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) {Http::Headers::get().Cookie.get(), fmt::format("OauthExpires={}", expires_at_s)}, {Http::Headers::get().Cookie.get(), fmt::format("RefreshToken={}", encrypted_refresh_token)}, {Http::Headers::get().Cookie.get(), - "BearerToken=Fc1bBwAAAAAVzVsHAAAAANWU4XU3MGwGHLzWF6KUL0s"}, // xyztoken + "BearerToken="+TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=dCu0otMcLoaGF73jrT+R8rGA0pnWyMgNf4+GivGrHEI="}, }; @@ -2939,7 +2941,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) // 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", "", expiredTime); + filter_->updateTokens("access_code", "some-id-token", "", expiredTime); filter_->finishRefreshAccessTokenFlow(); @@ -2949,7 +2951,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) Http::TestResponseHeaderMapImpl expected_response_headers{ {Http::Headers::get().SetCookie.get(), "OauthHMAC=" - "AWc2PEcPGGXlOtGGLOsT6rFnW9qxOVk0NvfBpZHRY3I=;" + "xQCNvPMLwq3rF1dB/mSwyVz7kcIZai8pD8rS5SNLgRU=;" "path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), "OauthExpires=10;path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), @@ -2964,11 +2966,11 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) EXPECT_THAT(response_headers, HeaderMapEqualRef(&expected_response_headers)); auto cookies = Http::Utility::parseCookies(request_headers); - EXPECT_EQ(cookies.at("OauthHMAC"), "AWc2PEcPGGXlOtGGLOsT6rFnW9qxOVk0NvfBpZHRY3I="); + EXPECT_EQ(cookies.at("OauthHMAC"), "xQCNvPMLwq3rF1dB/mSwyVz7kcIZai8pD8rS5SNLgRU="); EXPECT_EQ(cookies.at("OauthExpires"), "10"); - EXPECT_EQ(cookies.at("BearerToken"), "accessToken"); - EXPECT_EQ(cookies.at("IdToken"), "idToken"); - EXPECT_EQ(cookies.at("RefreshToken"), encrypted_refresh_token); + EXPECT_EQ(cookies.at("BearerToken"), "access_code"); + EXPECT_EQ(cookies.at("IdToken"), "some-id-token"); + EXPECT_EQ(cookies.at("RefreshToken"), "legit_refresh_token"); } TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenWithBasicAuth) { From 81f2d6800716187c288ef41220731d24d526c748 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Mon, 9 Jun 2025 01:01:37 +0000 Subject: [PATCH 05/20] remove debug info Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/filter.cc | 13 +------- .../filters/http/oauth2/filter_test.cc | 32 ++++++++++++------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index af5a0409b9deb..a549fad04909c 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -277,7 +277,6 @@ std::string encodeState(const std::string& original_request_url, const std::stri */ std::string encrypt(const std::string& plaintext, const std::string& secret, Random::RandomGenerator& random) { - std::cout << "XXXXX encrypt hmac secret" << secret << std::endl; // Generate the key from the secret using SHA-256 std::vector key(SHA256_DIGEST_LENGTH); // AES-256 requires 256-bit (32 bytes) key SHA256(reinterpret_cast(secret.c_str()), secret.size(), key.data()); @@ -343,7 +342,6 @@ struct DecryptResult { * Decrypt an AES-256-CBC encrypted string. */ DecryptResult decrypt(const std::string& encrypted, const std::string& secret) { - std::cout << "XXXXX decrypt hmac secret" << secret << std::endl; // Decode the Base64Url-encoded input std::string decoded = Base64Url::decode(encrypted); std::vector combined(decoded.begin(), decoded.end()); @@ -535,15 +533,6 @@ bool OAuth2CookieValidator::hmacIsValid() const { if (!cookie_domain_.empty()) { cookie_domain = cookie_domain_; } - std::cout << "xxxxx encodeHmacBase64: " - << encodeHmacBase64(secret_, cookie_domain, expires_, access_token_, id_token_, - refresh_token_) - << std::endl; - std::cout << "xxxxxencodeHmacHexBase64: " - << encodeHmacHexBase64(secret_, cookie_domain, expires_, access_token_, id_token_, - refresh_token_) - << std::endl; - std::cout << "xxxxxxhamc: " << hmac_ << std::endl; return ((encodeHmacBase64(secret_, cookie_domain, expires_, access_token_, id_token_, refresh_token_) == hmac_) || (encodeHmacHexBase64(secret_, cookie_domain, expires_, access_token_, id_token_, @@ -763,7 +752,7 @@ bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const { return false; } -// Decrypts the OAuth tokens and updates the OAuth tokens in the request cookies before forwarding +// Decrpyts the OAuth tokens and updates the OAuth tokens in the request cookies before forwarding // the request upstream. void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers) { absl::flat_hash_map cookies = Http::Utility::parseCookies(headers); diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index cfdc469445002..44f07a7d007d4 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -1510,7 +1510,7 @@ TEST_F(OAuth2Test, OAuthTestInvalidUrlInStateQueryParam) { "/_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="+TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=" "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" @@ -2172,7 +2172,12 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshToke "eyJ1bmlxdWVfbmFtZSI6ImFsZXhjZWk4OCIsInN1YiI6ImFsZXhjZWk4OCIsImp0aSI6IjQ5ZTFjMzc1IiwiYXVkIjoi" "dGVzdCIsIm5iZiI6MTcwNzQxNDYzNSwiZXhwIjoyNTU0NDE2MDAwLCJpYXQiOjE3MDc0MTQ2MzYsImlzcyI6ImRvdG5l" "dC11c2VyLWp3dHMifQ.LaGOw6x0-m7r-WzxgCIdPnAfp0O1hy6mW4klq9Vs2XM"; - const std::string encrypted_refresh_token = "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1VLZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW23YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; + const std::string encrypted_refresh_token = + "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_" + "amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-" + "x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1V" + "LZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW2" + "3YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2225,8 +2230,12 @@ TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefr "eyJ1bmlxdWVfbmFtZSI6ImFsZXhjZWk4OCIsInN1YiI6ImFsZXhjZWk4OCIsImp0aSI6IjQ5ZTFjMzc1IiwiYXVkIjoi" "dGVzdCIsIm5iZiI6MTcwNzQxNDYzNSwiZXhwIjoyNTU0NDE2MDAwLCJpYXQiOjE3MDc0MTQ2MzYsImlzcyI6ImRvdG5l" "dC11c2VyLWp3dHMifQ.LaGOw6x0-m7r-WzxgCIdPnAfp0O1hy6mW4klq9Vs2XM"; -const std::string encrypted_refresh_token = "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1VLZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW23YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; - + const std::string encrypted_refresh_token = + "Fc1bBwAAAAAVzVsHAAAAANmnPnluIb9exn3WlbkgaDHNTVoZUE-1O8H_" + "amXtsHZWG04QXuzJxsFxxe58HpCeWYx7QYi886mP3fCWDBrOJZ4DkwJjQXtvp9VdmKhCr1qCYQ9mSdv6GY50g-aOOr-" + "x1wXNGCfnURYA48u2BulYuHqG2FzNAfbPo8uNO0IS3CUNE3C9gLcs4gHq9AjMwXVe3PLxV0ihrcXCUVp0ao9R2k2Ki1V" + "LZpaH6ntay0IUJft2hjvq3lVvtCakEH0LYmzx9G0MGwaqiaeeFBNQyCY9iji5BOAfFezKnLKAvsYn2egVDHEFXCCSUW2" + "3YEA57eGNDrs1PIZXRvLrjyJCiBE-0Iiq74MgHSG6usBK21wks8VOGyIy3qRkz-LcmgLX9ZB1lA"; // Expected response after the callback is complete. Http::TestRequestHeaderMapImpl expected_headers{ @@ -2542,8 +2551,7 @@ TEST_F(OAuth2Test, CookieValidatorInTransition) { {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN}, {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, - {Http::Headers::get().Cookie.get(), - "OauthHMAC=eK7Kw2VqlnZJiz93KTnZqUar3ajNAe+ubmosGFkyL4I="}, + {Http::Headers::get().Cookie.get(), "OauthHMAC=eK7Kw2VqlnZJiz93KTnZqUar3ajNAe+ubmosGFkyL4I="}, }; cookie_validator->setParams(request_headers_hexbase64, "mock-secret"); @@ -2847,7 +2855,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Cookie.get(), fmt::format("OauthExpires={}", expires_at_s)}, - {Http::Headers::get().Cookie.get(), "BearerToken="+ TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=dCu0otMcLoaGF73jrT+R8rGA0pnWyMgNf4+GivGrHEI="}, }; @@ -2884,10 +2892,11 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) { "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="+TEST_ENCRYPTED_ACCESS_TOKEN+";path=/;Max-Age=10;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, + "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, {Http::Headers::get().SetCookie.get(), - "RefreshToken=" +TEST_ENCRYPTED_REFRESH_TOKEN+";path=/;Max-Age=604800;secure;HttpOnly"}, + "IdToken=" + TEST_ENCRYPTED_ID_TOKEN + ";path=/;Max-Age=10;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + ";path=/;Max-Age=604800;secure;HttpOnly"}, }; EXPECT_THAT(response_headers, HeaderMapEqualRef(&expected_response_headers)); @@ -2918,8 +2927,7 @@ TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessTokenNoNewRefreshToken) {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Cookie.get(), fmt::format("OauthExpires={}", expires_at_s)}, {Http::Headers::get().Cookie.get(), fmt::format("RefreshToken={}", encrypted_refresh_token)}, - {Http::Headers::get().Cookie.get(), - "BearerToken="+TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthHMAC=dCu0otMcLoaGF73jrT+R8rGA0pnWyMgNf4+GivGrHEI="}, }; From 4c5e9eae3c855426711cba00ad891ad474244911 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Mon, 9 Jun 2025 01:56:02 +0000 Subject: [PATCH 06/20] fix test Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 7 ++++--- test/extensions/filters/http/oauth2/filter_test.cc | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index a549fad04909c..74b4ffbfda32b 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -799,10 +799,11 @@ std::string OAuth2Filter::decryptToken(const std::string& encrypted_token, ENVOY_LOG(error, "failed to decrypt token: {}, error: {}", encrypted_token, decrypt_result.error.value()); // There are two cases: - // 1. The token is a legacy token, so we return the original token. + // 1. The token is a legacy unencrypted token. + // In this case, we return the token as-is to allow the request to proceed. // 2. The token is encrypted, but the decryption failed due to the HMAC secret is changed. - // In both cases, the client will be redirected to the OAuth server because Cookie validation - // failed. + // In this case, we return the original encrypted token, the HMAC validation will fail + // and the user wil return encrypted_token; } return decrypt_result.plaintext; diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 44f07a7d007d4..8d864a37342c1 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -1499,10 +1499,11 @@ 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","csrf_token":"${extracted}"} test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + static const std::string state_with_invalid_url = - "eyJ1cmwiOiJibGFoIiwiY3NyZl90b2tlbiI6IjAwMDAwMDAwMDc1YmNkMTUifQ"; + "eyJ1cmwiOiJibGFoIiwiY3NyZl90b2tlbiI6IjAwMDAwMDAwMDc1YmNkMTUubmE2a3J1NHgxcEhnb2NTSWVVL21kdEhZ" + "bjU4R2gxYnF3ZVM0WFhvaXFWZz0ifQ"; Http::TestRequestHeaderMapImpl request_headers{ {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, From 178f3c1b76fbeb07fa9401e24dde52717f0ea1e4 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Mon, 9 Jun 2025 09:00:58 +0000 Subject: [PATCH 07/20] fix integration test Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/filter.cc | 4 +- .../http/oauth2/oauth_integration_test.cc | 87 +++++++++++++++++-- 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 74b4ffbfda32b..152853fb6e2d4 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -752,7 +752,7 @@ bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const { return false; } -// Decrpyts the OAuth tokens and updates the OAuth tokens in the request cookies before forwarding +// Decrypt the OAuth tokens and updates the OAuth tokens in the request cookies before forwarding // the request upstream. void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers) { absl::flat_hash_map cookies = Http::Utility::parseCookies(headers); @@ -803,7 +803,7 @@ std::string OAuth2Filter::decryptToken(const std::string& encrypted_token, // In this case, we return the token as-is to allow the request to proceed. // 2. The token is encrypted, but the decryption failed due to the HMAC secret is changed. // In this case, we return the original encrypted token, the HMAC validation will fail - // and the user wil + // and the user will be redirected to the OAuth server for re-authentication. return encrypted_token; } return decrypt_result.plaintext; diff --git a/test/extensions/filters/http/oauth2/oauth_integration_test.cc b/test/extensions/filters/http/oauth2/oauth_integration_test.cc index a95107ce415d2..5b8dd1005ba71 100644 --- a/test/extensions/filters/http/oauth2/oauth_integration_test.cc +++ b/test/extensions/filters/http/oauth2/oauth_integration_test.cc @@ -15,6 +15,7 @@ namespace Extensions { namespace HttpFilters { namespace Oauth2 { namespace { + static const std::string TEST_STATE_CSRF_TOKEN = "8c18b8fcf575b593.qE67JkhE3H/0rpNYWCkQXX65Yzk5gEe7uETE3m8tylY="; // {"url":"http://traffic.example.com/not/_oauth","csrf_token":"${extracted}"} @@ -31,6 +32,69 @@ static const std::string TEST_ENCRYPTED_CODE_VERIFIER = "Fc1bBwAAAAAVzVsHAAAAACcWO_WnprqLTdaCdFE7rj83_Jej1OihEIfOcQJFRCQZirutZ-XL7LK2G2KgRnVCCA"; static const std::string TEST_ENCRYPTED_CODE_VERIFIER_1 = "Fc1bBwAAAAAVzVsHAAAAANRgXgBre6UErcWdPGZOl-o0px-SribGBqMNhaB6Smp-pjDSB20RXanapU6gVN4E1A"; +static const std::string TEST_ENCRYPTED_ACCESS_TOKEN = + "Fc1bBwAAAAAVzVsHAAAAALw-JhWF2XQOvdUKxWoMN1w"; // "bar" +static const std::string TEST_ENCRYPTED_REFRESH_TOKEN = + "Fc1bBwAAAAAVzVsHAAAAAM9NnfacsjScJzcyWlSKX6E"; // "foo" + +/** + * Decrypt an AES-256-CBC encrypted string. + */ +std::string decrypt(absl::string_view encrypted, absl::string_view secret) { + // Decode the Base64Url-encoded input + std::string decoded = Base64Url::decode(encrypted); + std::vector combined(decoded.begin(), decoded.end()); + + if (combined.size() <= 16) { + return ""; + } + + // Extract the IV (first 16 bytes) + std::vector iv(combined.begin(), combined.begin() + 16); + + // Extract the ciphertext (remaining bytes) + std::vector ciphertext(combined.begin() + 16, combined.end()); + + // Generate the key from the secret using SHA-256 + std::vector key(SHA256_DIGEST_LENGTH); + SHA256(reinterpret_cast((std::string(secret)).c_str()), secret.size(), + key.data()); + + EVP_CIPHER_CTX* ctx = EVP_CIPHER_CTX_new(); + RELEASE_ASSERT(ctx, "Failed to create context"); + + std::vector plaintext(ciphertext.size() + EVP_MAX_BLOCK_LENGTH); + int len = 0, plaintext_len = 0; + + // Initialize decryption operation + if (EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), nullptr, key.data(), iv.data()) != 1) { + EVP_CIPHER_CTX_free(ctx); + return ""; + } + + // Decrypt the ciphertext + if (EVP_DecryptUpdate(ctx, plaintext.data(), &len, ciphertext.data(), ciphertext.size()) != 1) { + EVP_CIPHER_CTX_free(ctx); + return ""; + } + plaintext_len += len; + + // Finalize decryption + if (EVP_DecryptFinal_ex(ctx, plaintext.data() + len, &len) != 1) { + EVP_CIPHER_CTX_free(ctx); + return ""; + } + + plaintext_len += len; + + EVP_CIPHER_CTX_free(ctx); + + // Resize to actual plaintext length + plaintext.resize(plaintext_len); + + return std::string(plaintext.begin(), plaintext.end()); +} + class OauthIntegrationTest : public HttpIntegrationTest, public Grpc::GrpcClientIntegrationParamTest { public: @@ -39,6 +103,7 @@ class OauthIntegrationTest : public HttpIntegrationTest, skip_tag_extraction_rule_check_ = true; enableHalfClose(true); } + envoy::service::discovery::v3::DiscoveryResponse genericSecretResponse(absl::string_view name, absl::string_view value) { envoy::extensions::transport_sockets::tls::v3::Secret secret; @@ -278,12 +343,13 @@ name: oauth validate_headers.addReferenceKey( Http::Headers::get().Cookie, absl::StrCat(default_cookie_names_.oauth_expires_, "=", expires)); - validate_headers.addReferenceKey(Http::Headers::get().Cookie, - absl::StrCat(default_cookie_names_.bearer_token_, "=", token)); - validate_headers.addReferenceKey( Http::Headers::get().Cookie, - absl::StrCat(default_cookie_names_.refresh_token_, "=", refreshToken)); + absl::StrCat(default_cookie_names_.bearer_token_, "=", decrypt(token, hmac_secret))); + + validate_headers.addReferenceKey(Http::Headers::get().Cookie, + absl::StrCat(default_cookie_names_.refresh_token_, "=", + decrypt(refreshToken, hmac_secret))); OAuth2CookieValidator validator{api_->timeSource(), default_cookie_names_, ""}; validator.setParams(validate_headers, std::string(hmac_secret)); @@ -407,10 +473,15 @@ name: oauth codec_client_ = makeHttpConnection(lookupPort("http")); Http::TestRequestHeaderMapImpl headers{ - {":method", "GET"}, {":path", "/request1"}, - {":scheme", "http"}, {"x-forwarded-proto", "http"}, - {":authority", "authority"}, {"Cookie", "RefreshToken=efddf321;BearerToken=ff1234fc"}, - {":authority", "authority"}, {"authority", "Bearer token"}}; + {":method", "GET"}, + {":path", "/request1"}, + {":scheme", "http"}, + {"x-forwarded-proto", "http"}, + {":authority", "authority"}, + {"Cookie", "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN + + ";BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {":authority", "authority"}, + {"authority", "Bearer token"}}; auto encoder_decoder = codec_client_->startRequest(headers); request_encoder_ = &encoder_decoder.first; From 07c3367061bd78e944d15a58e65d8a5279961937 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 13 Jun 2025 13:45:25 +0000 Subject: [PATCH 08/20] fix example verification Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/filter.cc | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 152853fb6e2d4..236d086d93bc4 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -571,6 +571,10 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, * 5) user is unauthorized */ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { + // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding + // the request upstream. + decryptAndUpdateOAuthTokenCookies(headers); + // Skip Filter and continue chain if a Passthrough header is matching // Must be done before the sanitation of the authorization header, // otherwise the authorization header might be altered or removed @@ -607,10 +611,6 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he return signOutUser(headers); } - // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding - // the request upstream. - decryptAndUpdateOAuthTokenCookies(headers); - if (canSkipOAuth(headers)) { // Update the path header with the query string parameters after a successful OAuth login. // This is necessary if a website requests multiple resources which get redirected to the @@ -766,24 +766,34 @@ void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& hea const std::string encrypted_id_token = findValue(cookies, cookie_names.id_token_); const std::string encrypted_refresh_token = findValue(cookies, cookie_names.refresh_token_); - if (!encrypted_access_token.empty()) { - cookies.insert_or_assign(cookie_names.bearer_token_, - decryptToken(encrypted_access_token, config_->hmacSecret())); - } - - if (!encrypted_id_token.empty()) { - cookies.insert_or_assign(cookie_names.id_token_, - decryptToken(encrypted_id_token, config_->hmacSecret())); - } - - if (!encrypted_refresh_token.empty()) { - cookies.insert_or_assign(cookie_names.refresh_token_, - decryptToken(encrypted_refresh_token, config_->hmacSecret())); - } - if (!encrypted_access_token.empty() || !encrypted_id_token.empty() || !encrypted_refresh_token.empty()) { - std::string new_cookies(absl::StrJoin(cookies, "; ", absl::PairFormatter("="))); + std::string new_cookies = + std::string(headers.get(Http::Headers::get().Cookie)[0]->value().getStringView()); + if (!encrypted_access_token.empty()) { + size_t pos = new_cookies.find(encrypted_access_token); + if (pos != std::string::npos) { + new_cookies.replace(pos, encrypted_access_token.length(), + decryptToken(encrypted_access_token, config_->hmacSecret())); + } + } + + if (!encrypted_id_token.empty()) { + size_t pos = new_cookies.find(encrypted_id_token); + if (pos != std::string::npos) { + new_cookies.replace(pos, encrypted_id_token.length(), + decryptToken(encrypted_id_token, config_->hmacSecret())); + } + } + + // todo: we don't need to pass the refresh token to upstream services + if (!encrypted_refresh_token.empty()) { + size_t pos = new_cookies.find(encrypted_refresh_token); + if (pos != std::string::npos) { + new_cookies.replace(pos, encrypted_refresh_token.length(), + decryptToken(encrypted_refresh_token, config_->hmacSecret())); + } + } headers.setReferenceKey(Http::Headers::get().Cookie, new_cookies); } } From cf8da0f3ccbe513962aeea1f58ed06f108d19628 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sat, 14 Jun 2025 05:35:54 +0000 Subject: [PATCH 09/20] Revert "fix example verification" This reverts commit 5261a1ff8623e538373c55af9e905c159837c8ea. Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/filter.cc | 50 ++++++++----------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 236d086d93bc4..152853fb6e2d4 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -571,10 +571,6 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, * 5) user is unauthorized */ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { - // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding - // the request upstream. - decryptAndUpdateOAuthTokenCookies(headers); - // Skip Filter and continue chain if a Passthrough header is matching // Must be done before the sanitation of the authorization header, // otherwise the authorization header might be altered or removed @@ -611,6 +607,10 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he return signOutUser(headers); } + // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding + // the request upstream. + decryptAndUpdateOAuthTokenCookies(headers); + if (canSkipOAuth(headers)) { // Update the path header with the query string parameters after a successful OAuth login. // This is necessary if a website requests multiple resources which get redirected to the @@ -766,34 +766,24 @@ void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& hea const std::string encrypted_id_token = findValue(cookies, cookie_names.id_token_); const std::string encrypted_refresh_token = findValue(cookies, cookie_names.refresh_token_); - if (!encrypted_access_token.empty() || !encrypted_id_token.empty() || - !encrypted_refresh_token.empty()) { - std::string new_cookies = - std::string(headers.get(Http::Headers::get().Cookie)[0]->value().getStringView()); - if (!encrypted_access_token.empty()) { - size_t pos = new_cookies.find(encrypted_access_token); - if (pos != std::string::npos) { - new_cookies.replace(pos, encrypted_access_token.length(), - decryptToken(encrypted_access_token, config_->hmacSecret())); - } - } + if (!encrypted_access_token.empty()) { + cookies.insert_or_assign(cookie_names.bearer_token_, + decryptToken(encrypted_access_token, config_->hmacSecret())); + } - if (!encrypted_id_token.empty()) { - size_t pos = new_cookies.find(encrypted_id_token); - if (pos != std::string::npos) { - new_cookies.replace(pos, encrypted_id_token.length(), - decryptToken(encrypted_id_token, config_->hmacSecret())); - } - } + if (!encrypted_id_token.empty()) { + cookies.insert_or_assign(cookie_names.id_token_, + decryptToken(encrypted_id_token, config_->hmacSecret())); + } - // todo: we don't need to pass the refresh token to upstream services - if (!encrypted_refresh_token.empty()) { - size_t pos = new_cookies.find(encrypted_refresh_token); - if (pos != std::string::npos) { - new_cookies.replace(pos, encrypted_refresh_token.length(), - decryptToken(encrypted_refresh_token, config_->hmacSecret())); - } - } + if (!encrypted_refresh_token.empty()) { + cookies.insert_or_assign(cookie_names.refresh_token_, + decryptToken(encrypted_refresh_token, config_->hmacSecret())); + } + + if (!encrypted_access_token.empty() || !encrypted_id_token.empty() || + !encrypted_refresh_token.empty()) { + std::string new_cookies(absl::StrJoin(cookies, "; ", absl::PairFormatter("="))); headers.setReferenceKey(Http::Headers::get().Cookie, new_cookies); } } From eb08d5eadc412ee4e0ebc1a0eea49afe9b983fe9 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sat, 14 Jun 2025 05:40:05 +0000 Subject: [PATCH 10/20] fixt test Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 152853fb6e2d4..bd136560c766e 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -571,6 +571,10 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, * 5) user is unauthorized */ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { + // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding + // the request upstream. + decryptAndUpdateOAuthTokenCookies(headers); + // Skip Filter and continue chain if a Passthrough header is matching // Must be done before the sanitation of the authorization header, // otherwise the authorization header might be altered or removed @@ -607,10 +611,6 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he return signOutUser(headers); } - // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding - // the request upstream. - decryptAndUpdateOAuthTokenCookies(headers); - if (canSkipOAuth(headers)) { // Update the path header with the query string parameters after a successful OAuth login. // This is necessary if a website requests multiple resources which get redirected to the From 3eff601938c9b0cd350b4545937a4771da2ba455 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sat, 14 Jun 2025 06:44:58 +0000 Subject: [PATCH 11/20] more test Signed-off-by: Huabing (Robin) Zhao --- .../filters/http/oauth2/filter_test.cc | 37 +++++++++++++++++++ tools/spelling/spelling_dictionary.txt | 1 + 2 files changed, 38 insertions(+) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 8d864a37342c1..0320f68414118 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -3390,6 +3390,43 @@ TEST_F(OAuth2Test, CookiesDeletedWhenTokensCleared) { filter_->onGetAccessTokenSuccess("", "", "", expiredTime); } +// Ensure that the token cookies are decrypted before forwarding the request +TEST_F(OAuth2Test, CookiesDecryptedBeforeForwarding) { + // Initialize with use_refresh_token set to false + init(getConfig(true /* forward_bearer_token */)); + + // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthHMAC=4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ="}, + {Http::Headers::get().Cookie.get(), "OauthExpires=1600"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), "IdToken=" + TEST_ENCRYPTED_ID_TOKEN}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_CSRF_TOKEN}, + }; + + // cookie-validation mocking + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + + // return reference mocking + std::string access_token{"access_code"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(access_token)); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + + // Expect the request headers to be updated with the decrypted tokens + auto cookies = Http::Utility::parseCookies(request_headers); + EXPECT_EQ(cookies.at("BearerToken"), "access_code"); + EXPECT_EQ(cookies.at("IdToken"), "some-id-token"); + EXPECT_EQ(cookies.at("RefreshToken"), "some-refresh-token"); +} + } // namespace Oauth2 } // namespace HttpFilters } // namespace Extensions diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index cb602768c7dab..b7382838f51d8 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -90,6 +90,7 @@ ETB ETX FS FIXME +decrypted gperf HEXDIG HEXDIGIT From b184c0db921e1a2333e609a82d7256be84d3a55e Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 24 Jun 2025 07:17:00 +0000 Subject: [PATCH 12/20] minor change Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index be00e7bce2a85..da018e59c9606 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -65,7 +65,7 @@ minor_behavior_changes: providers, allowing complete customisation of the credential provider chain when using AWS request signing extension. - area: oauth2 change: | - The access token, id token and refresh token in the cookies are now encrypted using the HMCA secret. + The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* From c2e059dd4fe47f5aaf5e8a96e052f658135a03b5 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 24 Jun 2025 08:12:28 +0000 Subject: [PATCH 13/20] add runtime guard Signed-off-by: Huabing (Robin) Zhao --- source/common/runtime/runtime_features.cc | 1 + .../extensions/filters/http/oauth2/filter.cc | 32 +++++++++++-------- .../extensions/filters/http/oauth2/filter.h | 3 +- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 4a80557bbd740..67316166fb7b1 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -66,6 +66,7 @@ RUNTIME_GUARD(envoy_reloadable_features_mmdb_files_reload_enabled); RUNTIME_GUARD(envoy_reloadable_features_no_extension_lookup_by_name); RUNTIME_GUARD(envoy_reloadable_features_normalize_rds_provider_config); RUNTIME_GUARD(envoy_reloadable_features_oauth2_use_refresh_token); +RUNTIME_GUARD(envoy_reloadable_features_oauth2_encrypt_token); RUNTIME_GUARD(envoy_reloadable_features_original_dst_rely_on_idle_timeout); RUNTIME_GUARD(envoy_reloadable_features_original_src_fix_port_exhaustion); RUNTIME_GUARD(envoy_reloadable_features_prefer_ipv6_dns_on_macos); diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 31ab23a603040..3b4f988b98776 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -774,18 +774,15 @@ void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& hea const std::string encrypted_refresh_token = findValue(cookies, cookie_names.refresh_token_); if (!encrypted_access_token.empty()) { - cookies.insert_or_assign(cookie_names.bearer_token_, - decryptToken(encrypted_access_token, config_->hmacSecret())); + cookies.insert_or_assign(cookie_names.bearer_token_, decryptToken(encrypted_access_token)); } if (!encrypted_id_token.empty()) { - cookies.insert_or_assign(cookie_names.id_token_, - decryptToken(encrypted_id_token, config_->hmacSecret())); + cookies.insert_or_assign(cookie_names.id_token_, decryptToken(encrypted_id_token)); } if (!encrypted_refresh_token.empty()) { - cookies.insert_or_assign(cookie_names.refresh_token_, - decryptToken(encrypted_refresh_token, config_->hmacSecret())); + cookies.insert_or_assign(cookie_names.refresh_token_, decryptToken(encrypted_refresh_token)); } if (!encrypted_access_token.empty() || !encrypted_id_token.empty() || @@ -795,13 +792,23 @@ void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& hea } } -std::string OAuth2Filter::decryptToken(const std::string& encrypted_token, - const std::string& secret) { +std::string OAuth2Filter::encryptToken(const std::string& token) const { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_token")) { + return token; + } + return encrypt(token, config_->hmacSecret(), random_); +} + +std::string OAuth2Filter::decryptToken(const std::string& encrypted_token) const { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_token")) { + return encrypted_token; + } + if (encrypted_token.empty()) { return EMPTY_STRING; } - DecryptResult decrypt_result = decrypt(encrypted_token, secret); + DecryptResult decrypt_result = decrypt(encrypted_token, config_->hmacSecret()); if (decrypt_result.error.has_value()) { ENVOY_LOG(error, "failed to decrypt token: {}, error: {}", encrypted_token, decrypt_result.error.value()); @@ -1224,7 +1231,7 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, if (!access_token_.empty()) { headers.addReferenceKey(Http::Headers::get().SetCookie, absl::StrCat(cookie_names.bearer_token_, "=", - encrypt(access_token_, config_->hmacSecret(), random_), + encryptToken(access_token_), BuildCookieTail(1))); // BEARER_TOKEN } else if (request_cookies.contains(cookie_names.bearer_token_)) { headers.addReferenceKey( @@ -1235,8 +1242,7 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, if (!id_token_.empty()) { headers.addReferenceKey(Http::Headers::get().SetCookie, - absl::StrCat(cookie_names.id_token_, "=", - encrypt(id_token_, config_->hmacSecret(), random_), + absl::StrCat(cookie_names.id_token_, "=", encryptToken(id_token_), BuildCookieTail(4))); // ID_TOKEN } else if (request_cookies.contains(cookie_names.id_token_)) { headers.addReferenceKey( @@ -1248,7 +1254,7 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, if (!refresh_token_.empty()) { headers.addReferenceKey(Http::Headers::get().SetCookie, absl::StrCat(cookie_names.refresh_token_, "=", - encrypt(refresh_token_, config_->hmacSecret(), random_), + encryptToken(refresh_token_), BuildCookieTail(5))); // REFRESH_TOKEN } else if (request_cookies.contains(cookie_names.refresh_token_)) { headers.addReferenceKey( diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index b85a08fbf98c1..8e3a18424059c 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -383,7 +383,8 @@ class OAuth2Filter : public Http::PassThroughFilter, bool validateCsrfToken(const Http::RequestHeaderMap& headers, const std::string& csrf_token) const; void decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers); - std::string decryptToken(const std::string& encrypted_token, const std::string& secret); + std::string encryptToken(const std::string& token) const; + std::string decryptToken(const std::string& encrypted_token) const; }; } // namespace Oauth2 From 2322541ab708ac3e5a07569c6e8b274e3003252d Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 24 Jun 2025 08:15:23 +0000 Subject: [PATCH 14/20] update change log Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 3 ++- source/common/runtime/runtime_features.cc | 2 +- source/extensions/filters/http/oauth2/filter.cc | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index b69d4a54ad45b..e36b905dada90 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -69,7 +69,8 @@ minor_behavior_changes: a work in progress field. - area: oauth2 change: | - The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. + The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. This behavior can + be reverted by setting the runtime guard ``"envoy.reloadable_features.oauth2_encrypt_tokens"`` to ``false``. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 67316166fb7b1..20c05be8e9ad6 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -66,7 +66,7 @@ RUNTIME_GUARD(envoy_reloadable_features_mmdb_files_reload_enabled); RUNTIME_GUARD(envoy_reloadable_features_no_extension_lookup_by_name); RUNTIME_GUARD(envoy_reloadable_features_normalize_rds_provider_config); RUNTIME_GUARD(envoy_reloadable_features_oauth2_use_refresh_token); -RUNTIME_GUARD(envoy_reloadable_features_oauth2_encrypt_token); +RUNTIME_GUARD(envoy_reloadable_features_oauth2_encrypt_tokens); RUNTIME_GUARD(envoy_reloadable_features_original_dst_rely_on_idle_timeout); RUNTIME_GUARD(envoy_reloadable_features_original_src_fix_port_exhaustion); RUNTIME_GUARD(envoy_reloadable_features_prefer_ipv6_dns_on_macos); diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 3b4f988b98776..bf69f188d511d 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -793,14 +793,14 @@ void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& hea } std::string OAuth2Filter::encryptToken(const std::string& token) const { - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_token")) { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { return token; } return encrypt(token, config_->hmacSecret(), random_); } std::string OAuth2Filter::decryptToken(const std::string& encrypted_token) const { - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_token")) { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { return encrypted_token; } From 7d015d697e1a11ccc7615beae07c19d056153d87 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 24 Jun 2025 08:39:40 +0000 Subject: [PATCH 15/20] make methods const Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 9 +++++---- source/extensions/filters/http/oauth2/filter.h | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index bf69f188d511d..8fc7c4d1ebc3a 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -761,7 +761,7 @@ bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const { // Decrypt the OAuth tokens and updates the OAuth tokens in the request cookies before forwarding // the request upstream. -void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers) { +void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers) const { absl::flat_hash_map cookies = Http::Utility::parseCookies(headers); if (cookies.empty()) { return; @@ -933,7 +933,7 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) { /** * Modifies the state of the filter by adding response headers to the decoder_callbacks */ -Http::FilterHeadersStatus OAuth2Filter::signOutUser(const Http::RequestHeaderMap& headers) { +Http::FilterHeadersStatus OAuth2Filter::signOutUser(const Http::RequestHeaderMap& headers) const { Http::ResponseHeaderMapPtr response_headers{Http::createHeaderMap( {{Http::Headers::get().Status, std::to_string(enumToInt(Http::Code::Found))}})}; std::string cookie_domain; @@ -1275,8 +1275,9 @@ void OAuth2Filter::sendUnauthorizedResponse() { // * Does the query parameters contain the code and state? // * Does the state contain the original request URL and the CSRF token? // * Does the CSRF token in the state match the one in the cookie? -CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::RequestHeaderMap& headers, - const absl::string_view path_str) { +CallbackValidationResult +OAuth2Filter::validateOAuthCallback(const Http::RequestHeaderMap& headers, + const absl::string_view path_str) const { // Return 401 unauthorized if the query parameters contain an error response. const auto query_parameters = Http::Utility::QueryParamsMulti::parseQueryString(path_str); if (query_parameters.getFirstValue(queryParamsError).has_value()) { diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 8e3a18424059c..b4b43a0574607 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -368,7 +368,7 @@ class OAuth2Filter : public Http::PassThroughFilter, bool canRedirectToOAuthServer(Http::RequestHeaderMap& headers) const; void redirectToOAuthServer(Http::RequestHeaderMap& headers); - Http::FilterHeadersStatus signOutUser(const Http::RequestHeaderMap& headers); + Http::FilterHeadersStatus signOutUser(const Http::RequestHeaderMap& headers) const; std::string getEncodedToken() const; std::string getExpiresTimeForRefreshToken(const std::string& refresh_token, @@ -379,10 +379,10 @@ class OAuth2Filter : public Http::PassThroughFilter, void addResponseCookies(Http::ResponseHeaderMap& headers, const std::string& encoded_token) const; const std::string& bearerPrefix() const; CallbackValidationResult validateOAuthCallback(const Http::RequestHeaderMap& headers, - const absl::string_view path_str); + const absl::string_view path_str) const; bool validateCsrfToken(const Http::RequestHeaderMap& headers, const std::string& csrf_token) const; - void decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers); + void decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& headers) const; std::string encryptToken(const std::string& token) const; std::string decryptToken(const std::string& encrypted_token) const; }; From 6a39095e3dd1ef9619ef1560a3b8a18a4979b93f Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 24 Jun 2025 08:53:10 +0000 Subject: [PATCH 16/20] minor change Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 8fc7c4d1ebc3a..ea0f377424510 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -580,7 +580,9 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding // the request upstream. - decryptAndUpdateOAuthTokenCookies(headers); + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { + decryptAndUpdateOAuthTokenCookies(headers); + } // Skip Filter and continue chain if a Passthrough header is matching // Must be done before the sanitation of the authorization header, @@ -800,10 +802,6 @@ std::string OAuth2Filter::encryptToken(const std::string& token) const { } std::string OAuth2Filter::decryptToken(const std::string& encrypted_token) const { - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { - return encrypted_token; - } - if (encrypted_token.empty()) { return EMPTY_STRING; } From b3e0f6635e3752e7dd82ca28574aa0217ba45098 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 24 Jun 2025 09:29:12 +0000 Subject: [PATCH 17/20] fix test Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 1 - source/common/runtime/runtime_features.cc | 2 +- source/extensions/filters/http/oauth2/filter.cc | 8 ++++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index e36b905dada90..fb5b5c962ab89 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -71,7 +71,6 @@ minor_behavior_changes: change: | The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. This behavior can be reverted by setting the runtime guard ``"envoy.reloadable_features.oauth2_encrypt_tokens"`` to ``false``. - bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* - area: conn_pool diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 20c05be8e9ad6..ab40f3550837e 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -65,8 +65,8 @@ RUNTIME_GUARD(envoy_reloadable_features_local_reply_traverses_filter_chain_after RUNTIME_GUARD(envoy_reloadable_features_mmdb_files_reload_enabled); RUNTIME_GUARD(envoy_reloadable_features_no_extension_lookup_by_name); RUNTIME_GUARD(envoy_reloadable_features_normalize_rds_provider_config); -RUNTIME_GUARD(envoy_reloadable_features_oauth2_use_refresh_token); RUNTIME_GUARD(envoy_reloadable_features_oauth2_encrypt_tokens); +RUNTIME_GUARD(envoy_reloadable_features_oauth2_use_refresh_token); RUNTIME_GUARD(envoy_reloadable_features_original_dst_rely_on_idle_timeout); RUNTIME_GUARD(envoy_reloadable_features_original_src_fix_port_exhaustion); RUNTIME_GUARD(envoy_reloadable_features_prefer_ipv6_dns_on_macos); diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index ea0f377424510..2b78958cf6c3e 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -580,7 +580,7 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { // Decrypt the OAuth tokens and update the OAuth tokens in the request headers before forwarding // the request upstream. - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { decryptAndUpdateOAuthTokenCookies(headers); } @@ -795,10 +795,10 @@ void OAuth2Filter::decryptAndUpdateOAuthTokenCookies(Http::RequestHeaderMap& hea } std::string OAuth2Filter::encryptToken(const std::string& token) const { - if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { - return token; + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth2_encrypt_tokens")) { + return encrypt(token, config_->hmacSecret(), random_); } - return encrypt(token, config_->hmacSecret(), random_); + return token; } std::string OAuth2Filter::decryptToken(const std::string& encrypted_token) const { From ac63f13244c4fce0961df59e03e6ac29808286f8 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 24 Jun 2025 20:54:20 +0800 Subject: [PATCH 18/20] Update changelogs/current.yaml Co-authored-by: phlax Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index fb5b5c962ab89..4daa48cdc54c9 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -70,7 +70,7 @@ minor_behavior_changes: - area: oauth2 change: | The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. This behavior can - be reverted by setting the runtime guard ``"envoy.reloadable_features.oauth2_encrypt_tokens"`` to ``false``. + be reverted by setting the runtime guard ``envoy.reloadable_features.oauth2_encrypt_tokens`` to ``false``. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* - area: conn_pool From 488d2dc369dcfec392853caa38918ebb2377a92f Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 25 Jun 2025 10:15:09 +0000 Subject: [PATCH 19/20] address comment Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 4daa48cdc54c9..338f4b90d386c 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -71,6 +71,7 @@ minor_behavior_changes: change: | The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. This behavior can be reverted by setting the runtime guard ``envoy.reloadable_features.oauth2_encrypt_tokens`` to ``false``. + bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* - area: conn_pool From 05163db26c423abfff720333c202776521012dd9 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 4 Jul 2025 08:30:48 +0000 Subject: [PATCH 20/20] tests for runtime flag disabled Signed-off-by: Huabing (Robin) Zhao --- .../filters/http/oauth2/filter_test.cc | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 83b6243ee7a40..8a05660cfecdc 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -1030,6 +1030,64 @@ TEST_F(OAuth2Test, SetBearerToken) { EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_success").value(), 1); } +TEST_F(OAuth2Test, SetBearerTokenWithEncryptionDisabled) { + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues({{"envoy.reloadable_features.oauth2_encrypt_tokens", "false"}}); + + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + + // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Cookie.get(), + "OauthNonce=" + TEST_CSRF_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier=" + TEST_ENCRYPTED_CODE_VERIFIER + ";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}, + }; + + 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, + TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); + + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(request_headers, false)); + + // 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 response_headers{ + {Http::Headers::get().Status.get(), "302"}, + {Http::Headers::get().SetCookie.get(), "OauthHMAC=" + "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;" + "path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "BearerToken=access_code;path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "IdToken=some-id-token;path=/;Max-Age=600;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), + "RefreshToken=some-refresh-token;path=/;Max-Age=604800;secure;HttpOnly"}, + {Http::Headers::get().Location.get(), + "https://traffic.example.com/original_path?var1=1&var2=2"}, + }; + + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + + filter_->onGetAccessTokenSuccess("access_code", "some-id-token", "some-refresh-token", + std::chrono::seconds(600)); + + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_success").value(), 1); +} + /** * Scenario: The OAuth filter receives a request without valid OAuth cookies to a non-callback URL * (indicating that the user needs to re-validate cookies or get 401'd). @@ -3570,6 +3628,46 @@ TEST_F(OAuth2Test, CookiesDecryptedBeforeForwarding) { EXPECT_EQ(cookies.at("RefreshToken"), "some-refresh-token"); } +// Ensure that the token cookies are decrypted before forwarding the request +TEST_F(OAuth2Test, CookiesDecryptedBeforeForwardingWithEncryptionDisabled) { + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues({{"envoy.reloadable_features.oauth2_encrypt_tokens", "false"}}); + + // Initialize with use_refresh_token set to false + init(getConfig(true /* forward_bearer_token */)); + + // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthHMAC=4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ="}, + {Http::Headers::get().Cookie.get(), "OauthExpires=1600"}, + {Http::Headers::get().Cookie.get(), "BearerToken=access_code"}, + {Http::Headers::get().Cookie.get(), "IdToken=some-id-token"}, + {Http::Headers::get().Cookie.get(), "RefreshToken=some-refresh-token"}, + {Http::Headers::get().Cookie.get(), "OauthNonce=" + TEST_CSRF_TOKEN}, + }; + + // cookie-validation mocking + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + + // return reference mocking + std::string access_token{"access_code"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(access_token)); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + + // Expect the request headers to be updated with the decrypted tokens + auto cookies = Http::Utility::parseCookies(request_headers); + EXPECT_EQ(cookies.at("BearerToken"), "access_code"); + EXPECT_EQ(cookies.at("IdToken"), "some-id-token"); + EXPECT_EQ(cookies.at("RefreshToken"), "some-refresh-token"); +} + } // namespace Oauth2 } // namespace HttpFilters } // namespace Extensions