Skip to content

Commit

Permalink
fix test
Browse files Browse the repository at this point in the history 
Signed-off-by: Huabing Zhao <[email protected]>
  • Loading branch information
@zhaohuabing
zhaohuabing committed Dec 13, 2024
1 parent d984e69 commit 3c03b55
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 20 deletions.
6 changes: 3 additions & 3 deletions source/extensions/filters/http/oauth2/filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ bool validateCsrfTokenHmac(const std::string& hmac_secret, const std::string& cs
std::string encodeState(const std::string& original_request_url, const std::string& nonce) {
std::string buffer;
absl::string_view sanitized_url = Json::sanitize(buffer, original_request_url);
std::string json = fmt::format(R"({{"url":"{}","nonce":"{}"}})", sanitized_url, nonce);
absl::string_view sanitized_nonce = Json::sanitize(buffer, nonce);
std::string json = fmt::format(R"({{"url":"{}","nonce":"{}"}})", sanitized_url, sanitized_nonce);
return Base64Url::encode(json.data(), json.size());
}

Expand Down Expand Up @@ -557,7 +558,6 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) {
}

const std::string state = encodeState(original_url, csrf_token);

auto query_params = config_->authorizationQueryParams();
query_params.overwrite(queryParamsState, state);

Expand Down Expand Up @@ -907,7 +907,7 @@ CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::Request
// More information can be found at https://datatracker.ietf.org/doc/html/rfc6819#section-5.3.5
std::string nonce = filed_value_pair.at(stateParamsNonce).string_value();
if (!validateCsrfToken(headers, nonce)) {
ENVOY_LOG(error, "nonce cookie does not match nonce in the state: \n{}", nonce);
ENVOY_LOG(error, "csrf token validation failed");
return {false, "", ""};
}
const std::string original_request_url = filed_value_pair.at(stateParamsUrl).string_value();
Expand Down
41 changes: 24 additions & 17 deletions test/extensions/filters/http/oauth2/oauth_integration_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,18 @@ namespace Extensions {
namespace HttpFilters {
namespace Oauth2 {
namespace {

static const std::string TEST_STATE_NONCE =
"8c18b8fcf575b593.qE67JkhE3H/0rpNYWCkQXX65Yzk5gEe7uETE3m8tylY=";
// {"url":"http://traffic.example.com/not/_oauth","nonce":"${extracted}"}
static const std::string TEST_ENCODED_STATE =
"eyJ1cmwiOiJodHRwOi8vdHJhZmZpYy5leGFtcGxlLmNvbS9ub3QvX29hdXRoIiwibm9uY2UiOiI4YzE4YjhmY2Y1NzViNT"
"kzLnFFNjdKa2hFM0gvMHJwTllXQ2tRWFg2NVl6azVnRWU3dUVURTNtOHR5bFk9In0";
static const std::string TEST_STATE_NONCE_1 =
"8c18b8fcf575b593.ZpkXMDNFiinkL87AoSDONKulBruOpaIiSAd7CNkgOEo=";
// {"url":"http://traffic.example.com/not/_oauth","nonce": "${extracted}}"}
static const std::string TEST_ENCODED_STATE_1 =
"eyJ1cmwiOiJodHRwOi8vdHJhZmZpYy5leGFtcGxlLmNvbS9ub3QvX29hdXRoIiwibm9uY2UiOiI4YzE4YjhmY2Y1NzViNT"
"kzLlpwa1hNRE5GaWlua0w4N0FvU0RPTkt1bEJydU9wYUlpU0FkN0NOa2dPRW89In0";
class OauthIntegrationTest : public HttpIntegrationTest,
public Grpc::GrpcClientIntegrationParamTest {
public:
Expand Down Expand Up @@ -321,20 +332,18 @@ name: oauth
oauth2_request_->encodeData(buffer, true);
}

void doAuthenticationFlow(absl::string_view token_secret, absl::string_view hmac_secret) {
void doAuthenticationFlow(absl::string_view token_secret, absl::string_view hmac_secret,
absl::string_view nonce, absl::string_view state) {
codec_client_ = makeHttpConnection(lookupPort("http"));

Http::TestRequestHeaderMapImpl headers{
{":method", "GET"},
{":path",
"/callback?code=foo&state="
"eyJ1cmwiOiJodHRwOi8vdHJhZmZpYy5leGFtcGxlLmNvbS9ub3QvX29hdXRoIiwibm9uY2UiOiIxMjM0NTY3ODkwM"
"DAwMDAwIn0"}, // {"url":"http://traffic.example.com/not/_oauth","nonce":"1234567890000000"}
{":path", absl::StrCat("/callback?code=foo&state=", state)},
{":scheme", "http"},
{"x-forwarded-proto", "http"},
{":authority", "authority"},
{"authority", "Bearer token"},
{"cookie", absl::StrCat(default_cookie_names_.oauth_nonce_, "=1234567890000000")}};
{"cookie", absl::StrCat(default_cookie_names_.oauth_nonce_, "=", nonce)}};

auto encoder_decoder = codec_client_->startRequest(headers);
request_encoder_ = &encoder_decoder.first;
Expand Down Expand Up @@ -365,20 +374,18 @@ name: oauth
codec_client_ = makeHttpConnection(lookupPort("http"));
Http::TestRequestHeaderMapImpl headersWithCookie{
{":method", "GET"},
{":path",
"/callback?code=foo&state="
"eyJ1cmwiOiJodHRwOi8vdHJhZmZpYy5leGFtcGxlLmNvbS9ub3QvX29hdXRoIiwibm9uY2UiOiIxMjM0NTY3ODkwM"
"DAwMDAwIn0"}, // {"url":"http://traffic.example.com/not/_oauth","nonce":"1234567890000000"}
{":path", absl::StrCat("/callback?code=foo&state=", state)},
{":scheme", "http"},
{"x-forwarded-proto", "http"},
{":authority", "authority"},
{"authority", "Bearer token"},
{"cookie", absl::StrCat(default_cookie_names_.oauth_hmac_, "=", hmac)},
{"cookie", absl::StrCat(default_cookie_names_.oauth_expires_, "=", oauth_expires)},
{"cookie", absl::StrCat(default_cookie_names_.oauth_nonce_, "=1234567890000000")},
{"cookie", absl::StrCat(default_cookie_names_.oauth_nonce_, "=", nonce)},
{"cookie", absl::StrCat(default_cookie_names_.bearer_token_, "=", bearer_token)},
{"cookie", absl::StrCat(default_cookie_names_.refresh_token_, "=", refresh_token)},
};

auto encoder_decoder2 = codec_client_->startRequest(headersWithCookie, true);
response = std::move(encoder_decoder2.second);
response->waitForHeaders();
Expand Down Expand Up @@ -478,7 +485,7 @@ TEST_P(OauthIntegrationTest, AuthenticationFlow) {
initialize();

// 1. Do one authentication flow.
doAuthenticationFlow("token_secret", "hmac_secret");
doAuthenticationFlow("token_secret", "hmac_secret", TEST_STATE_NONCE, TEST_ENCODED_STATE);

// 2. Reload secrets.
EXPECT_EQ(test_server_->counter("sds.token.update_success")->value(), 1);
Expand All @@ -490,7 +497,7 @@ TEST_P(OauthIntegrationTest, AuthenticationFlow) {
TestEnvironment::temporaryPath("hmac_secret.yaml"));
test_server_->waitForCounterEq("sds.hmac.update_success", 2, std::chrono::milliseconds(5000));
// 3. Do another one authentication flow.
doAuthenticationFlow("token_secret_1", "hmac_secret_1");
doAuthenticationFlow("token_secret_1", "hmac_secret_1", TEST_STATE_NONCE_1, TEST_ENCODED_STATE_1);
}

TEST_P(OauthIntegrationTest, RefreshTokenFlow) {
Expand Down Expand Up @@ -588,7 +595,7 @@ TEST_P(OauthIntegrationTest, LoadListenerAfterServerIsInitialized) {
test_server_->waitForCounterGe("listener_manager.lds.update_success", 2);
test_server_->waitForGaugeEq("listener_manager.total_listeners_warming", 0);

doAuthenticationFlow("token_secret", "hmac_secret");
doAuthenticationFlow("token_secret", "hmac_secret", TEST_STATE_NONCE, TEST_ENCODED_STATE);
if (lds_connection_ != nullptr) {
AssertionResult result = lds_connection_->close();
RELEASE_ASSERT(result, result.message());
Expand Down Expand Up @@ -664,7 +671,7 @@ TEST_P(OauthIntegrationTestWithBasicAuth, AuthenticationFlow) {
sendLdsResponse({MessageUtil::getYamlStringFromMessage(listener_config_)}, "initial");
};
initialize();
doAuthenticationFlow("token_secret", "hmac_secret");
doAuthenticationFlow("token_secret", "hmac_secret", TEST_STATE_NONCE, TEST_ENCODED_STATE);
}

class OauthUseRefreshTokenDisabled : public OauthIntegrationTest {
Expand Down Expand Up @@ -728,7 +735,7 @@ TEST_P(OauthUseRefreshTokenDisabled, FailRefreshTokenFlow) {
initialize();

// 1. Do one authentication flow.
doAuthenticationFlow("token_secret", "hmac_secret");
doAuthenticationFlow("token_secret", "hmac_secret", TEST_STATE_NONCE, TEST_ENCODED_STATE);

// 2. Reload secrets.
EXPECT_EQ(test_server_->counter("sds.token.update_success")->value(), 1);
Expand Down

0 comments on commit 3c03b55

Please sign in to comment.