Skip to content

Commit

Permalink
oauth2: support disabling redirects for AJAX requests (envoyproxy#32655)
Browse files Browse the repository at this point in the history
Added new parameter `ajax_request_matcher` to optionally not allow OAuth2 authorization redirect when all tokens are expired. Such redirect usually redirects the user to a login page (in authorization code flow) and this behavior is not desired in Ajax requests.

Signed-off-by: Samuel Valis <[email protected]>
  • Loading branch information
samo1 authored Mar 7, 2024
1 parent b7bc420 commit 8318716
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 6 deletions.
7 changes: 6 additions & 1 deletion api/envoy/extensions/filters/http/oauth2/v3/oauth.proto
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ message OAuth2Credentials {

// OAuth config
//
// [#next-free-field: 14]
// [#next-free-field: 15]
message OAuth2Config {
enum AuthType {
// The ``client_id`` and ``client_secret`` will be sent in the URL encoded request body.
Expand Down Expand Up @@ -137,6 +137,11 @@ message OAuth2Config {
// If this value is not set, it will default to ``0s``. In this case, the expiry must be set by
// the authorization server or the OAuth flow will fail.
google.protobuf.Duration default_expires_in = 13;

// Any request that matches any of the provided matchers won't be redirected to OAuth server when tokens are not valid.
// Automatic access token refresh will be performed for these requests, if enabled.
// This behavior can be useful for AJAX requests.
repeated config.route.v3.HeaderMatcher deny_redirect_matcher = 14;
}

// Filter config.
Expand Down
4 changes: 4 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,10 @@ new_features:
- area: tracing
change: |
Added support for variant span attribute type for the OpenTelemetry tracer.
- area: oauth
change: |
:ref:`deny_redirect_matcher <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Config.deny_redirect_matcher>`
to support disabling authorization redirects for specific requests, e.g. AJAX requests.
deprecated:
- area: listener
Expand Down
5 changes: 5 additions & 0 deletions docs/root/configuration/http/http_filters/oauth2_filter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ sending the user to the configured auth endpoint.
an interface for users to provide specific header matching criteria such that, when applicable, the OAuth flow is entirely skipped.
When this occurs, the ``oauth_passthrough`` metric is incremented but ``success`` is not.

:ref:`deny_redirect_matcher <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Config.deny_redirect_matcher>` can be used to specify requests for which
unauthorized response is returned on token expiration and will not automatically redirect to the authorization endpoint. Token refresh can be still performed
during those requests by enabling the :ref:`use_refresh_token <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Config.use_refresh_token>` flag.
This behavior can be useful for AJAX requests which cannot handle redirects correctly.

:ref:`use_refresh_token <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Config.use_refresh_token>` provides
possibility to update access token by using a refresh token. By default after expiration the user is always redirected to the authorization endpoint to log in again.
By enabling this flag a new access token is obtained using by a refresh token without redirecting the user to log in again. This requires the refresh token to be provided by authorization_endpoint when the user logs in.
Expand Down
30 changes: 25 additions & 5 deletions source/extensions/filters/http/oauth2/filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ FilterConfig::FilterConfig(
encoded_resource_query_params_(encodeResourceList(proto_config.resources())),
forward_bearer_token_(proto_config.forward_bearer_token()),
pass_through_header_matchers_(headerMatchers(proto_config.pass_through_matcher())),
deny_redirect_header_matchers_(headerMatchers(proto_config.deny_redirect_matcher())),
cookie_names_(proto_config.credentials().cookie_names()),
auth_type_(getAuthType(proto_config.auth_type())),
use_refresh_token_(proto_config.use_refresh_token().value()),
Expand Down Expand Up @@ -369,10 +370,15 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he
return Http::FilterHeadersStatus::StopAllIterationAndWatermark;
}

ENVOY_LOG(debug, "path {} does not match with redirect matcher. redirecting to OAuth server.",
path_str);
redirectToOAuthServer(headers);
return Http::FilterHeadersStatus::StopIteration;
if (canRedirectToOAuthServer(headers)) {
ENVOY_LOG(debug, "redirecting to OAuth server", path_str);
redirectToOAuthServer(headers);
return Http::FilterHeadersStatus::StopIteration;
} else {
ENVOY_LOG(debug, "unauthorized, redirecting to OAuth server is not allowed", path_str);
sendUnauthorizedResponse();
return Http::FilterHeadersStatus::StopIteration;
}
}

// At this point, we *are* on /_oauth. We believe this request comes from the authorization
Expand Down Expand Up @@ -442,6 +448,16 @@ bool OAuth2Filter::canSkipOAuth(Http::RequestHeaderMap& headers) const {
return false;
}

bool OAuth2Filter::canRedirectToOAuthServer(Http::RequestHeaderMap& headers) const {
for (const auto& matcher : config_->denyRedirectMatchers()) {
if (matcher.matchesHeaders(headers)) {
ENVOY_LOG(debug, "redirect is denied for this request");
return false;
}
}
return true;
}

void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const {
Http::ResponseHeaderMapPtr response_headers{Http::createHeaderMap<Http::ResponseHeaderMapImpl>(
{{Http::Headers::get().Status, std::to_string(enumToInt(Http::Code::Found))}})};
Expand Down Expand Up @@ -608,7 +624,11 @@ void OAuth2Filter::finishRefreshAccessTokenFlow() {
void OAuth2Filter::onRefreshAccessTokenFailure() {
config_->stats().oauth_refreshtoken_failure_.inc();
// We failed to get an access token via the refresh token, so send the user to the oauth endpoint.
redirectToOAuthServer(*request_headers_);
if (canRedirectToOAuthServer(*request_headers_)) {
redirectToOAuthServer(*request_headers_);
} else {
sendUnauthorizedResponse();
}
}

void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers,
Expand Down
5 changes: 5 additions & 0 deletions source/extensions/filters/http/oauth2/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ class FilterConfig {
const std::vector<Http::HeaderUtility::HeaderData>& passThroughMatchers() const {
return pass_through_header_matchers_;
}
const std::vector<Http::HeaderUtility::HeaderData>& denyRedirectMatchers() const {
return deny_redirect_header_matchers_;
}
const envoy::config::core::v3::HttpUri& oauthTokenEndpoint() const {
return oauth_token_endpoint_;
}
Expand Down Expand Up @@ -159,6 +162,7 @@ class FilterConfig {
const std::string encoded_resource_query_params_;
const bool forward_bearer_token_ : 1;
const std::vector<Http::HeaderUtility::HeaderData> pass_through_header_matchers_;
const std::vector<Http::HeaderUtility::HeaderData> deny_redirect_header_matchers_;
const CookieNames cookie_names_;
const AuthType auth_type_;
const bool use_refresh_token_{};
Expand Down Expand Up @@ -274,6 +278,7 @@ class OAuth2Filter : public Http::PassThroughFilter,
// Determines whether or not the current request can skip the entire OAuth flow (HMAC is valid,
// connection is mTLS, etc.)
bool canSkipOAuth(Http::RequestHeaderMap& headers) const;
bool canRedirectToOAuthServer(Http::RequestHeaderMap& headers) const;
void redirectToOAuthServer(Http::RequestHeaderMap& headers) const;

Http::FilterHeadersStatus signOutUser(const Http::RequestHeaderMap& headers);
Expand Down
79 changes: 79 additions & 0 deletions test/extensions/filters/http/oauth2/filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ class OAuth2Test : public testing::TestWithParam<int> {
auto* matcher = p.add_pass_through_matcher();
matcher->set_name(":method");
matcher->mutable_string_match()->set_exact("OPTIONS");
auto* deny_redirect_matcher = p.add_deny_redirect_matcher();
deny_redirect_matcher->set_name("X-Requested-With");
deny_redirect_matcher->mutable_string_match()->set_exact("XMLHttpRequest");
auto credentials = p.mutable_credentials();
credentials->set_client_id(TEST_CLIENT_ID);
credentials->mutable_token_secret()->set_name("secret");
Expand Down Expand Up @@ -780,6 +783,35 @@ TEST_F(OAuth2Test, OAuthOptionsRequestAndContinue) {
EXPECT_EQ(scope_.counterFromString("test.oauth_success").value(), 0);
}

/**
* Scenario: The OAuth filter receives a request without valid OAuth cookies to a non-callback URL
* that matches the deny_redirect_matcher.
*
* Expected behavior: the filter should should return 401 Unauthorized response.
*/
TEST_F(OAuth2Test, AjaxDoesNotRedirect) {
Http::TestRequestHeaderMapImpl request_headers{
{Http::Headers::get().Path.get(), "/anypath"},
{Http::Headers::get().Host.get(), "traffic.example.com"},
{Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post},
{Http::Headers::get().Scheme.get(), "https"},
{"X-Requested-With", "XMLHttpRequest"},
};

// explicitly tell the validator to fail the validation.
EXPECT_CALL(*validator_, setParams(_, _));
EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false));

// Unauthorized response is expected instead of 302 redirect.
EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _));

EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers, false));

EXPECT_EQ(1, config_->stats().oauth_failure_.value());
EXPECT_EQ(0, config_->stats().oauth_unauthorized_rq_.value());
}

// Validates the behavior of the cookie validator.
TEST_F(OAuth2Test, CookieValidator) {
expectValidCookies(
Expand Down Expand Up @@ -2167,6 +2199,53 @@ TEST_F(OAuth2Test, OAuthTestRefreshAccessTokenFail) {
EXPECT_EQ(1, config_->stats().oauth_refreshtoken_failure_.value());
}

/**
* Scenario: The OAuth filter refresh flow fails for a request that matches the
* deny_redirect_matcher.
*
* Expected behavior: the filter should should return 401 Unauthorized response.
*/
TEST_F(OAuth2Test, AjaxRefreshDoesNotRedirect) {

init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */));
// First construct the initial request to the oauth filter with URI parameters.
Http::TestRequestHeaderMapImpl first_request_headers{
{Http::Headers::get().Path.get(), "/test?name=admin&level=trace"},
{Http::Headers::get().Host.get(), "traffic.example.com"},
{Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post},
{Http::Headers::get().Scheme.get(), "https"},
{"X-Requested-With", "XMLHttpRequest"},
};

std::string legit_token{"legit_token"};
EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token));

std::string legit_refresh_token{"legit_refresh_token"};
EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token));

// Fail the validation to trigger the OAuth flow with trying to get the access token using by
// refresh token.
EXPECT_CALL(*validator_, setParams(_, _));
EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false));
EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true));

EXPECT_CALL(*oauth_client_,
asyncRefreshAccessToken(legit_refresh_token, TEST_CLIENT_ID,
"asdf_client_secret_fdsa", AuthType::UrlEncodedBody));

EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark,
filter_->decodeHeaders(first_request_headers, false));

// Unauthorized response is expected instead of 302 redirect.
EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _));

filter_->onRefreshAccessTokenFailure();

EXPECT_EQ(0, config_->stats().oauth_unauthorized_rq_.value());
EXPECT_EQ(1, config_->stats().oauth_refreshtoken_failure_.value());
EXPECT_EQ(1, config_->stats().oauth_failure_.value());
}

TEST_F(OAuth2Test, OAuthTestSetCookiesAfterRefreshAccessToken) {

init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */));
Expand Down
1 change: 1 addition & 0 deletions tools/spelling/spelling_dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ ABI
ACK
ACL
AES
AJAX
AllMuxes
ALPN
ALS
Expand Down

0 comments on commit 8318716

Please sign in to comment.