-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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. #14034
Changes from 30 commits
80acc89
c276e54
d26dd3a
1577ed6
47f1fa4
906a91f
94fa8b1
2fdf64a
7c689b9
6549060
5e709f9
c2567ec
657e21f
a7acd87
5381f74
4d9eab9
5e126b9
b64e322
7359ad3
62f1ff4
b3c2071
a18a184
2aa7274
a097339
47598a2
6cca203
a5d64d0
5aef557
b956a6e
e8393a7
feb36f7
54af113
8d0ece6
8795f12
990899d
f769059
b0b90ad
6cd68d5
d01b49e
bbfe4fb
96156bc
86fbfae
aa78e42
6bf0a56
81b02b6
854b797
36fabd0
9576515
4cd22a2
5dd7bd8
8771252
b89bcae
7c7fb6c
fb4a4b7
85a911e
7e6b6c7
a52935a
0026696
b1d1fc0
c72c15c
783b773
21a81cf
22a820b
e73172e
e0a80c4
723475a
ab6e2ff
ca66519
51185ab
59945aa
ce172b7
40aaf60
c8d39ea
446df5e
57cfd34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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."; | ||
|
||
|
@@ -61,6 +61,8 @@ 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"; | ||
|
||
const std::string& DEFAULT_AUTH_SCOPE = "user"; | ||
|
||
template <class T> | ||
std::vector<Http::HeaderUtility::HeaderData> headerMatchers(const T& matcher_protos) { | ||
std::vector<Http::HeaderUtility::HeaderData> matchers; | ||
|
@@ -90,6 +92,8 @@ 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_((!proto_config.auth_scopes().empty()) ? proto_config.auth_scopes() | ||
: DEFAULT_AUTH_SCOPE), | ||
forward_bearer_token_(proto_config.forward_bearer_token()), | ||
pass_through_header_matchers_(headerMatchers(proto_config.pass_through_matcher())) { | ||
if (!cluster_manager.get(oauth_token_endpoint_.cluster())) { | ||
|
@@ -275,9 +279,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_->authScopes(), escaped_redirect_uri, escaped_state); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the auth scope need to be URL encoded here or somewhere else? |
||
response_headers->setLocation(new_url); | ||
decoder_callbacks_->encodeHeaders(std::move(response_headers), true, REDIRECT_FOR_CREDENTIALS); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd make this absl::string_view for consistency with the others unless you need this to be a std::string