Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oauth2 filter: Make OAuth scopes configurable. #14168

Merged
merged 58 commits into from
Jan 23, 2021
Merged
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
80acc89
Add scope to oauth2 filter proto file
andreyprezotto Nov 3, 2020
c276e54
Use configured scopes in authorizarion request
andreyprezotto Nov 3, 2020
d26dd3a
Initialize member in the correct order
andreyprezotto Nov 4, 2020
1577ed6
Update proto files after proto_format.sh
andreyprezotto Nov 4, 2020
47f1fa4
Fix oauth filter tests
andreyprezotto Nov 4, 2020
906a91f
fix semicolon
andreyprezotto Nov 5, 2020
94fa8b1
fix tests
andreyprezotto Nov 5, 2020
2fdf64a
fix tests
andreyprezotto Nov 5, 2020
7c689b9
fix tests after rebase
andreyprezotto Nov 5, 2020
6549060
Add debug log for test
andreyprezotto Nov 6, 2020
5e709f9
Add debug log for test
andreyprezotto Nov 6, 2020
c2567ec
Auth_scope default value if config wasn't set
andreyprezotto Nov 13, 2020
657e21f
add config test for default user
andreyprezotto Nov 13, 2020
a7acd87
add config test for default user
andreyprezotto Nov 13, 2020
5381f74
Revert "Auth_scope default value if config wasn't set"
andreyprezotto Nov 13, 2020
4d9eab9
Revert "Revert "Auth_scope default value if config wasn't set""
andreyprezotto Nov 13, 2020
5e126b9
revert implementation
andreyprezotto Nov 13, 2020
b64e322
change defaul value implementation
andreyprezotto Nov 13, 2020
7359ad3
fix with .empty
andreyprezotto Nov 13, 2020
62f1ff4
fix semicolon
andreyprezotto Nov 13, 2020
b3c2071
fix property name
andreyprezotto Nov 13, 2020
a18a184
semicolon
andreyprezotto Nov 13, 2020
2aa7274
fix tests
andreyprezotto Nov 13, 2020
a097339
Add auth_scopes to the docs
andreyprezotto Nov 16, 2020
47598a2
Update Release Notes
andreyprezotto Nov 16, 2020
6cca203
Fix colon in docs
andreyprezotto Nov 16, 2020
a5d64d0
Fix release notes
andreyprezotto Nov 16, 2020
5aef557
Fix formatting
andreyprezotto Nov 16, 2020
b956a6e
Fix documentation
andreyprezotto Nov 16, 2020
e8393a7
include default value in proto field comment
andreyprezotto Nov 18, 2020
9fa051d
Merge remote-tracking branch 'upstream/master' into oauth_scope
andreyprezotto Nov 24, 2020
ee4cb01
check_format: line order
andreyprezotto Nov 25, 2020
40fe4b8
add missing api_shadow proto files
andreyprezotto Nov 25, 2020
966e04c
Code review changes
andreyprezotto Nov 20, 2020
5109f3e
check_format: fix
andreyprezotto Nov 25, 2020
0e05c16
fix filter_test formatting
andreyprezotto Nov 25, 2020
ecf6616
correct emptyness check
andreyprezotto Nov 26, 2020
21b9d65
Code review changes
andreyprezotto Nov 30, 2020
527a144
Merge branch 'oauth_changes' into oauth_scope
andreyprezotto Dec 1, 2020
f4226a4
Code review changes
andreyprezotto Dec 9, 2020
82fd52b
Refactor OAuth2Test for init config flexibility
andreyprezotto Dec 10, 2020
4625f33
Merge branch 'oauth_review2' into oauth_scope
andreyprezotto Dec 10, 2020
c936cbb
Merge remote-tracking branch 'upstream/master' into oauth_scope
andreyprezotto Dec 10, 2020
aedc096
Fix doc link to field
andreyprezotto Dec 12, 2020
897b290
format fix
andreyprezotto Dec 15, 2020
9705b0d
move encoding logic to config
andreyprezotto Dec 15, 2020
a83fd6f
Merge remote-tracking branch 'upstream/master' into oauth_scope
andreyprezotto Jan 12, 2021
ab0a563
Merge branch 'oauth_scope' of github.com:andreyprezotto/envoy into oa…
andreyprezotto Jan 12, 2021
f9f9f7a
Update current.rst to match master's content
andreyprezotto Jan 12, 2021
f0efcf4
format fix
andreyprezotto Jan 12, 2021
be36649
Change filter stopinteration
andreyprezotto Jan 21, 2021
6c9c0d9
Merge remote-tracking branch 'upstream/master' into oauth_scope
andreyprezotto Jan 21, 2021
8da1892
Change test to use encodedAuthScopes
andreyprezotto Jan 21, 2021
0a33292
remove authScopes
andreyprezotto Jan 21, 2021
e63ec2b
remove authScopes
andreyprezotto Jan 21, 2021
f2cdd88
remove authScopes
andreyprezotto Jan 21, 2021
96ebfcd
Update filter.cc (#1)
ngoyal16 Jan 22, 2021
9fc7acb
Retest CI
andreyprezotto Jan 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion api/envoy/extensions/filters/http/oauth2/v3alpha/oauth.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ message OAuth2Credentials {

// OAuth config
//
// [#next-free-field: 9]
// [#next-free-field: 10]
message OAuth2Config {
// Endpoint on the authorization server to retrieve the access token from.
config.core.v3.HttpUri token_endpoint = 1;
Expand Down Expand Up @@ -74,6 +74,11 @@ message OAuth2Config {

// Any request that matches any of the provided matchers will be passed through without OAuth validation.
repeated config.route.v3.HeaderMatcher pass_through_matcher = 8;

// Optional list of OAuth scopes to be claimed in the authorization request. If not specified,
// defaults to "user" scope.
htuch marked this conversation as resolved.
Show resolved Hide resolved
// OAuth RFC https://tools.ietf.org/html/rfc6749#section-3.3
repeated string auth_scopes = 9;
}

// Filter config.
Expand Down
7 changes: 6 additions & 1 deletion api/envoy/extensions/filters/http/oauth2/v4alpha/oauth.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions docs/root/configuration/http/http_filters/oauth2_filter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ The following is an example configuring the filter.
name: hmac
sds_config:
path: "/etc/envoy/hmac.yaml"
# (Optional): defaults to 'user' scope if not provided
auth_scopes:
- user
- openid
- email

Below is a complete code example of how we employ the filter as one of
:ref:`HttpConnectionManager HTTP filters
Expand Down Expand Up @@ -114,6 +119,11 @@ Below is a complete code example of how we employ the filter as one of
name: hmac
sds_config:
path: "/etc/envoy/hmac.yaml"
# (Optional): defaults to 'user' scope if not provided
auth_scopes:
- user
- openid
- email
- name: envoy.router
tracing: {}
codec_type: "AUTO"
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Minor Behavior Changes
----------------------
*Changes that may cause incompatibilities for some users, but should not for most*

* oauth filter: added the optional parameter :ref:`auth_scopes <envoy_v3_api_field_extensions.filters.http.oauth2.v3alpha.OAuth2Config.auth_scopes>` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider.
* tcp: setting NODELAY in the base connection class. This should have no effect for TCP or HTTP proxying, but may improve throughput in other areas. This behavior can be temporarily reverted by setting `envoy.reloadable_features.always_nodelay` to false.
* upstream: host weight changes now cause a full load balancer rebuild as opposed to happening
atomically inline. This change has been made to support load balancer pre-computation of data
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 27 additions & 4 deletions source/extensions/filters/http/oauth2/filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ constexpr const char* CookieTailHttpOnlyFormatString =
";version=1;path=/;Max-Age={};secure;HttpOnly";

const char* AuthorizationEndpointFormat =
"{}?client_id={}&scope=user&response_type=code&redirect_uri={}&state={}";
"{}?client_id={}&scope={}&response_type=code&redirect_uri={}&state={}";

constexpr absl::string_view UnauthorizedBodyMessage = "OAuth flow failed.";

Expand All @@ -60,6 +60,7 @@ constexpr absl::string_view REDIRECT_RACE = "oauth.race_redirect";
constexpr absl::string_view REDIRECT_LOGGED_IN = "oauth.logged_in";
constexpr absl::string_view REDIRECT_FOR_CREDENTIALS = "oauth.missing_credentials";
constexpr absl::string_view SIGN_OUT = "oauth.sign_out";
constexpr absl::string_view DEFAULT_AUTH_SCOPE = "user";

template <class T>
std::vector<Http::HeaderUtility::HeaderData> headerMatchers(const T& matcher_protos) {
Expand All @@ -73,6 +74,25 @@ std::vector<Http::HeaderUtility::HeaderData> headerMatchers(const T& matcher_pro
return matchers;
}

// Transforms the proto list of 'auth_scopes' into a vector of std::string, also
// handling the default value logic.
std::vector<std::string>
authScopesList(const Protobuf::RepeatedPtrField<std::string>& auth_scopes_protos) {
std::vector<std::string> scopes;

// If 'auth_scopes' is empty it must return a list with the default value.
if (auth_scopes_protos.empty()) {
scopes.emplace_back(DEFAULT_AUTH_SCOPE);
} else {
scopes.reserve(auth_scopes_protos.size());

for (const auto& scope : auth_scopes_protos) {
scopes.emplace_back(scope);
}
}
return scopes;
}

snowp marked this conversation as resolved.
Show resolved Hide resolved
// Sets the auth token as the Bearer token in the authorization header.
void setBearerToken(Http::RequestHeaderMap& headers, const std::string& token) {
headers.setInline(authorization_handle.handle(), absl::StrCat("Bearer ", token));
Expand All @@ -90,6 +110,9 @@ FilterConfig::FilterConfig(
redirect_matcher_(proto_config.redirect_path_matcher()),
signout_path_(proto_config.signout_path()), secret_reader_(secret_reader),
stats_(FilterConfig::generateStats(stats_prefix, scope)),
auth_scopes_(authScopesList(proto_config.auth_scopes())),
encoded_auth_scopes_(
Http::Utility::PercentEncoding::encode(absl::StrJoin(auth_scopes_, " "), ":/=&? ")),
forward_bearer_token_(proto_config.forward_bearer_token()),
pass_through_header_matchers_(headerMatchers(proto_config.pass_through_matcher())) {
if (!cluster_manager.clusters().hasCluster(oauth_token_endpoint_.cluster())) {
Expand Down Expand Up @@ -275,9 +298,9 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he
const std::string escaped_redirect_uri =
Http::Utility::PercentEncoding::encode(redirect_uri, ":/=&?");

const std::string new_url =
fmt::format(AuthorizationEndpointFormat, config_->authorizationEndpoint(),
config_->clientId(), escaped_redirect_uri, escaped_state);
const std::string new_url = fmt::format(
AuthorizationEndpointFormat, config_->authorizationEndpoint(), config_->clientId(),
config_->encodedAuthScopes(), escaped_redirect_uri, escaped_state);
response_headers->setLocation(new_url);
decoder_callbacks_->encodeHeaders(std::move(response_headers), true, REDIRECT_FOR_CREDENTIALS);

Expand Down
4 changes: 4 additions & 0 deletions source/extensions/filters/http/oauth2/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ class FilterConfig {
std::string clientSecret() const { return secret_reader_->clientSecret(); }
std::string tokenSecret() const { return secret_reader_->tokenSecret(); }
FilterStats& stats() { return stats_; }
const std::vector<std::string>& authScopes() const { return auth_scopes_; }
const std::string& encodedAuthScopes() const { return encoded_auth_scopes_; }

private:
static FilterStats generateStats(const std::string& prefix, Stats::Scope& scope);
Expand All @@ -135,6 +137,8 @@ class FilterConfig {
const Matchers::PathMatcher signout_path_;
std::shared_ptr<SecretReader> secret_reader_;
FilterStats stats_;
const std::vector<std::string> auth_scopes_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is no longer used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just removed it

const std::string encoded_auth_scopes_;
const bool forward_bearer_token_ : 1;
const std::vector<Http::HeaderUtility::HeaderData> pass_through_header_matchers_;
};
Expand Down
1 change: 1 addition & 0 deletions source/extensions/filters/http/oauth2/oauth_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ void OAuth2ClientImpl::onSuccess(const Http::AsyncClient::Request&,
const auto response_code = message->headers().Status()->value().getStringView();
if (response_code != "200") {
ENVOY_LOG(debug, "Oauth response code: {}", response_code);
ENVOY_LOG(debug, "Oauth response body: {}", message->bodyAsString());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid of the amount of data that would be logged here.
Wouldn't this even make debug unusable in some cases?
Eg: big pages being proxied

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me clarify this @joaovitor. What is being logged here is the response body of the request to the OAuth provider. This is not the upstream request (to service being proxied in the end).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great @andreyprezotto ... Hope to see this PR merged.
This will improve oauth integration in envoy a lot.
Thanks for this work!

parent_->sendUnauthorizedResponse();
return;
}
Expand Down
8 changes: 8 additions & 0 deletions test/extensions/filters/http/oauth2/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ void expectInvalidSecretConfig(const std::string& failed_secret_name,
signout_path:
path:
exact: /signout
auth_scopes:
- user
- openid
- email
)EOF";

OAuth2Config factory;
Expand Down Expand Up @@ -87,6 +91,10 @@ TEST(ConfigTest, CreateFilter) {
signout_path:
path:
exact: /signout
auth_scopes:
- user
- openid
- email
)EOF";

OAuth2Config factory;
Expand Down
Loading