Skip to content

Commit

Permalink
Merge branch 'main' into mobile-pull-request-target
Browse files Browse the repository at this point in the history
  • Loading branch information
Yannic committed Mar 16, 2023
2 parents 3b56d83 + 4e6956f commit a208467
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 35 deletions.
14 changes: 14 additions & 0 deletions api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2384,4 +2384,18 @@ message FilterConfig {
// not support the specified filter, it may ignore the map entry rather
// than rejecting the config.
bool is_optional = 2;

// If true, the filter is disabled in the route or virtual host and the ``config`` field is ignored.
//
// .. note::
//
// This field will take effect when the request arrive and filter chain is created for the request.
// If initial route is selected for the request and a filter is disabled in the initial route, then
// the filter will not be added to the filter chain.
// And if the request is mutated later and re-match to another route, the disabled filter by the
// initial route will not be added back to the filter chain because the filter chain is already
// created and it is too late to change the chain.
//
// This field only make sense for the downstream HTTP filters for now.
bool disabled = 3;
}
9 changes: 9 additions & 0 deletions api/envoy/extensions/filters/http/oauth2/v3/oauth.proto
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
//

message OAuth2Credentials {
// [#next-free-field: 6]
message CookieNames {
// Cookie name to hold OAuth bearer token value. When the authentication server validates the
// client and returns an authorization token back to the OAuth filter, no matter what format
Expand All @@ -38,6 +39,14 @@ message OAuth2Credentials {
// Cookie name to hold OAuth expiry value. Defaults to ``OauthExpires``.
string oauth_expires = 3
[(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME ignore_empty: true}];

// Cookie name to hold the id token. Defaults to ``IdToken``.
string id_token = 4
[(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME ignore_empty: true}];

// Cookie name to hold the refresh token. Defaults to ``RefreshToken``.
string refresh_token = 5
[(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME ignore_empty: true}];
}

// The client_id to be used in the authorize calls. This value will be URL encoded when sent to the OAuth server.
Expand Down
3 changes: 3 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ new_features:
- area: access_log
change: |
enhanced observability into local close for :ref:`%RESPONSE_CODE_DETAILS% <config_http_conn_man_details>`.
- area: oauth filter
change: |
extended :ref:`cookie_names <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Credentials.cookie_names>` to allow overriding (default) cookie names (``IdToken``, ``RefreshToken``) set by the filter.
- area: tracing
change: |
allow :ref:`grpc_service <envoy_v3_api_field_config.trace.v3.OpenTelemetryConfig.grpc_service>` to be optional. This enables a means to disable collection of traces.
Expand Down
3 changes: 2 additions & 1 deletion docs/root/configuration/http/http_filters/oauth2_filter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ The OAuth filter's flow involves:
:ref:`hmac_secret <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Credentials.hmac_secret>`
to assist in encoding.
* The filter calls ``continueDecoding()`` to unblock the filter chain.
* The filter sets ``IdToken`` and ``RefreshToken`` cookies if they are provided by Identity provider along with ``AccessToken``.
* The filter sets ``IdToken`` and ``RefreshToken`` cookies if they are provided by Identity provider along with ``AccessToken``. These cookie names
can be customized by setting :ref:`cookie_names <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Credentials.cookie_names>`.

When the authn server validates the client and returns an authorization token back to the OAuth filter,
no matter what format that token is, if
Expand Down
18 changes: 14 additions & 4 deletions mobile/library/objective-c/EnvoyEngineImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,7 @@ - (int)runWithConfig:(EnvoyConfiguration *)config logLevel:(NSString *)logLevel
options->setConcurrency(1);
return reinterpret_cast<Envoy::Engine *>(_engineHandle)->run(std::move(options));
} @catch (NSException *exception) {
NSLog(@"[Envoy] exception caught: %@", exception);
[NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyError" object:self];
[self logException:exception];
return kEnvoyFailure;
}
}
Expand All @@ -545,8 +544,7 @@ - (int)runWithYAML:(NSString *)yaml
@try {
return (int)run_engine(_engineHandle, yaml.UTF8String, logLevel.UTF8String, "");
} @catch (NSException *exception) {
NSLog(@"[Envoy] exception caught: %@", exception);
[NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyError" object:self];
[self logException:exception];
return kEnvoyFailure;
}
}
Expand Down Expand Up @@ -609,4 +607,16 @@ - (void)terminateNotification:(NSNotification *)notification {
terminate_engine(_engineHandle, /* release */ false);
}

- (void)logException:(NSException *)exception {
NSLog(@"[Envoy] exception caught: %@", exception);

NSString *message = [NSString stringWithFormat:@"%@;%@;%@", exception.name, exception.reason,
exception.callStackSymbols.description];
ENVOY_LOG_EVENT_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::misc), error,
"handled_cxx_exception", [message UTF8String]);

[NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyHandledCXXException"
object:exception];
}

@end
29 changes: 12 additions & 17 deletions source/extensions/filters/http/oauth2/filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,9 @@ namespace {
Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
authorization_handle(Http::CustomHeaders::get().Authorization);

// Deleted OauthHMAC cookie.
constexpr const char* SignoutCookieValue =
constexpr const char* CookieDeleteFormatString =
"{}=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT";

// Deleted BearerToken cookie.
constexpr const char* SignoutBearerTokenValue =
"{}=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT";

constexpr absl::string_view SignoutIdTokenValue =
"IdToken=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT";

constexpr absl::string_view SignoutRefreshTokenValue =
"RefreshToken=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT";

constexpr const char* CookieTailFormatString = ";version=1;path=/;Max-Age={};secure";

constexpr const char* CookieTailHttpOnlyFormatString =
Expand Down Expand Up @@ -189,7 +178,8 @@ void OAuth2CookieValidator::setParams(const Http::RequestHeaderMap& headers,
const std::string& secret) {
const auto& cookies = Http::Utility::parseCookies(headers, [this](absl::string_view key) -> bool {
return key == cookie_names_.oauth_expires_ || key == cookie_names_.bearer_token_ ||
key == cookie_names_.oauth_hmac_ || key == cookie_names_.id_token_;
key == cookie_names_.oauth_hmac_ || key == cookie_names_.id_token_ ||
key == cookie_names_.refresh_token_;
});

expires_ = findValue(cookies, cookie_names_.oauth_expires_);
Expand Down Expand Up @@ -444,12 +434,16 @@ Http::FilterHeadersStatus OAuth2Filter::signOutUser(const Http::RequestHeaderMap
const std::string new_path = absl::StrCat(headers.getSchemeValue(), "://", host_, "/");
response_headers->addReferenceKey(
Http::Headers::get().SetCookie,
fmt::format(SignoutCookieValue, config_->cookieNames().oauth_hmac_));
fmt::format(CookieDeleteFormatString, config_->cookieNames().oauth_hmac_));
response_headers->addReferenceKey(
Http::Headers::get().SetCookie,
fmt::format(SignoutBearerTokenValue, config_->cookieNames().bearer_token_));
response_headers->addReferenceKey(Http::Headers::get().SetCookie, SignoutIdTokenValue);
response_headers->addReferenceKey(Http::Headers::get().SetCookie, SignoutRefreshTokenValue);
fmt::format(CookieDeleteFormatString, config_->cookieNames().bearer_token_));
response_headers->addReferenceKey(
Http::Headers::get().SetCookie,
fmt::format(CookieDeleteFormatString, config_->cookieNames().id_token_));
response_headers->addReferenceKey(
Http::Headers::get().SetCookie,
fmt::format(CookieDeleteFormatString, config_->cookieNames().refresh_token_));
response_headers->setLocation(new_path);
decoder_callbacks_->encodeHeaders(std::move(response_headers), true, SIGN_OUT);

Expand Down Expand Up @@ -543,6 +537,7 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers,
}
}
}

void OAuth2Filter::sendUnauthorizedResponse() {
config_->stats().oauth_failure_.inc();
decoder_callbacks_->sendLocalReply(Http::Code::Unauthorized, UnauthorizedBodyMessage, nullptr,
Expand Down
12 changes: 8 additions & 4 deletions source/extensions/filters/http/oauth2/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,25 @@ struct CookieNames {
CookieNames(const envoy::extensions::filters::http::oauth2::v3::OAuth2Credentials::CookieNames&
cookie_names)
: CookieNames(cookie_names.bearer_token(), cookie_names.oauth_hmac(),
cookie_names.oauth_expires()) {}
cookie_names.oauth_expires(), cookie_names.id_token(),
cookie_names.refresh_token()) {}

CookieNames(const std::string& bearer_token, const std::string& oauth_hmac,
const std::string& oauth_expires)
const std::string& oauth_expires, const std::string& id_token,
const std::string& refresh_token)
: bearer_token_(bearer_token.empty() ? "BearerToken" : bearer_token),
oauth_hmac_(oauth_hmac.empty() ? "OauthHMAC" : oauth_hmac),
oauth_expires_(oauth_expires.empty() ? "OauthExpires" : oauth_expires), id_token_(IdToken),
refresh_token_(RefreshToken) {}
oauth_expires_(oauth_expires.empty() ? OauthExpires : oauth_expires),
id_token_(id_token.empty() ? IdToken : id_token),
refresh_token_(refresh_token.empty() ? RefreshToken : refresh_token) {}

const std::string bearer_token_;
const std::string oauth_hmac_;
const std::string oauth_expires_;
const std::string id_token_;
const std::string refresh_token_;

static constexpr absl::string_view OauthExpires = "OauthExpires";
static constexpr absl::string_view IdToken = "IdToken";
static constexpr absl::string_view RefreshToken = "RefreshToken";
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Network::FilterStatus Filter::onData(Network::ListenerFilterBuffer& buffer) {
}

Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) {
ENVOY_LOG(debug, "http inspector: new connection accepted");
ENVOY_LOG(trace, "http inspector: new connection accepted");

const Network::ConnectionSocket& socket = cb.socket();

Expand Down Expand Up @@ -136,7 +136,7 @@ void Filter::done(bool success) {
// TODO(yxue): use detected protocol from http inspector and support h2c token in HCM
protocol = Http::Utility::AlpnNames::get().Http2c;
}
ENVOY_LOG(trace, "http inspector: set application protocol to {}", protocol);
ENVOY_LOG(debug, "http inspector: set application protocol to {}", protocol);

cb_->socket().setRequestedApplicationProtocols({protocol});
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Network::Address::InstanceConstSharedPtr OriginalDstFilter::getOriginalDst(Netwo
}

Network::FilterStatus OriginalDstFilter::onAccept(Network::ListenerFilterCallbacks& cb) {
ENVOY_LOG(debug, "original_dst: new connection accepted");
ENVOY_LOG(trace, "original_dst: new connection accepted");
Network::ConnectionSocket& socket = cb.socket();

if (socket.addressType() == Network::Address::Type::Ip) {
Expand Down Expand Up @@ -63,7 +63,7 @@ Network::FilterStatus OriginalDstFilter::onAccept(Network::ListenerFilterCallbac
}
}
#endif
ENVOY_LOG(trace, "original_dst: set destination to {}", original_local_address->asString());
ENVOY_LOG(debug, "original_dst: set destination to {}", original_local_address->asString());

// Restore the local address to the original one.
socket.connectionInfoProvider().restoreLocalAddress(original_local_address);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ Filter::Filter(const ConfigSharedPtr& config) : config_(config), ssl_(config_->n
}

Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) {
ENVOY_LOG(debug, "tls inspector: new connection accepted");
ENVOY_LOG(trace, "tls inspector: new connection accepted");
cb_ = &cb;

return Network::FilterStatus::StopIteration;
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/http/oauth2/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ TEST(ConfigTest, CreateFilter) {
bearer_token: BearerToken
oauth_hmac: OauthHMAC
oauth_expires: OauthExpires
id_token: IdToken
refresh_token: RefreshToken
authorization_endpoint: https://oauth.com/oauth/authorize/
redirect_uri: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/callback"
redirect_path_matcher:
Expand Down
43 changes: 40 additions & 3 deletions test/extensions/filters/http/oauth2/filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -787,12 +787,14 @@ TEST_F(OAuth2Test, OAuthOptionsRequestAndContinue_oauth_header_passthrough_fix)

// Validates the behavior of the cookie validator.
TEST_F(OAuth2Test, CookieValidator) {
expectValidCookies(CookieNames{"BearerToken", "OauthHMAC", "OauthExpires"});
expectValidCookies(
CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"});
}

// Validates the behavior of the cookie validator with custom cookie names.
TEST_F(OAuth2Test, CookieValidatorWithCustomNames) {
expectValidCookies(CookieNames{"CustomBearerToken", "CustomOauthHMAC", "CustomOauthExpires"});
expectValidCookies(CookieNames{"CustomBearerToken", "CustomOauthHMAC", "CustomOauthExpires",
"CustomIdToken", "CustomRefreshToken"});
}

// Validates the behavior of the cookie validator when the expires_at value is not a valid integer.
Expand All @@ -810,7 +812,8 @@ TEST_F(OAuth2Test, CookieValidatorInvalidExpiresAt) {
};

auto cookie_validator = std::make_shared<OAuth2CookieValidator>(
test_time_, CookieNames{"BearerToken", "OauthHMAC", "OauthExpires"});
test_time_,
CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"});
cookie_validator->setParams(request_headers, "mock-secret");

EXPECT_TRUE(cookie_validator->hmacIsValid());
Expand Down Expand Up @@ -1204,6 +1207,40 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) {
filter_->finishGetAccessTokenFlow();
}

/**
* Testing oauth response after tokens are set.
*
* Expected behavior: cookies are set.
*/
TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokens) {

// Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs.
test_time_.setSystemTime(SystemTime(std::chrono::seconds(0)));

// Expected response after the callback is complete.
Http::TestRequestHeaderMapImpl expected_headers{
{Http::Headers::get().Status.get(), "302"},
{Http::Headers::get().SetCookie.get(),
"OauthHMAC="
"OTBhMzEwNjk4YzJiNjIxMTcwMTE0ZDE2NjUyNjIyNmI1YmE0Y2NhNTQ3ZWYwZGYzZTNhYjEwNzhmZmQyZGY4Zg==;"
"version=1;path=/;Max-Age=600;secure;HttpOnly"},
{Http::Headers::get().SetCookie.get(),
"OauthExpires=600;version=1;path=/;Max-Age=600;secure;HttpOnly"},
{Http::Headers::get().SetCookie.get(),
"BearerToken=access_code;version=1;path=/;Max-Age=600;secure"},
{Http::Headers::get().SetCookie.get(),
"IdToken=some-id-token;version=1;path=/;Max-Age=600;secure"},
{Http::Headers::get().SetCookie.get(),
"RefreshToken=some-refresh-token;version=1;path=/;Max-Age=600;secure"},
{Http::Headers::get().Location.get(), ""},
};

EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), true));

filter_->onGetAccessTokenSuccess("access_code", "some-id-token", "some-refresh-token",
std::chrono::seconds(600));
}

TEST_F(OAuth2Test, OAuthBearerTokenFlowFromHeader) {
Http::TestRequestHeaderMapImpl request_headers{
{Http::Headers::get().Path.get(), "/test?role=bearer"},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ name: oauth
codec_client_->close();
}

const CookieNames default_cookie_names_{"BearerToken", "OauthHMAC", "OauthExpires"};
const CookieNames default_cookie_names_{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken",
"RefreshToken"};
envoy::config::listener::v3::Listener listener_config_;
std::string listener_name_{"http"};
FakeHttpConnectionPtr lds_connection_;
Expand Down

0 comments on commit a208467

Please sign in to comment.