fix(credential_injector/oauth2): send scopes when specified (envoypro…

…xy#38130) Commit Message: Current code inadvertently shadows the existing body declaration such that when scopes are specified, they are not actually sent to the authorization server. Additional Description: Risk Level: Testing: Added test, verified it errors on current code and is resolved after this fix. Docs Changes: None, this now matches the existing documentation. Release Notes: Added CHANGELOG entry. Platform Specific Features: Signed-off-by: Adam Eijdenberg <[email protected]>
bazmurphy · Jan 29, 2025 · 470261e · 470261e
1 parent 5c9a98c
commit 470261e
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 5 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -27,6 +27,10 @@ bug_fixes:
     matcher would incorrectly handle ``present_match`` configurations by treating them as default present checks. This
     behavior can be temporarily reverted by setting runtime feature
     ``envoy_reloadable_features_enable_new_query_param_present_match_behavior`` to ``false``.
+- area: oauth2
+  change: |
+    Fixed oauth2 credential injector to send scope (if specified) to authorization server
+    when requesting new access token using client_credentials flow.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

diff --git a/source/extensions/http/injected_credentials/oauth2/oauth_client.cc b/source/extensions/http/injected_credentials/oauth2/oauth_client.cc
@@ -39,12 +39,13 @@ OAuth2Client::GetTokenResult OAuth2ClientImpl::asyncGetAccessToken(const std::st
   const auto encoded_secret = Envoy::Http::Utility::PercentEncoding::encode(secret, ":/=&?");
 
   Envoy::Http::RequestMessagePtr request = createPostRequest();
-  const std::string body =
-      fmt::format(GetAccessTokenBodyFormatString, encoded_client_id, encoded_secret);
-  if (!(scopes.empty())) {
+  std::string body;
+  if (scopes.empty()) {
+    body = fmt::format(GetAccessTokenBodyFormatString, encoded_client_id, encoded_secret);
+  } else {
     const auto encoded_scopes = Envoy::Http::Utility::PercentEncoding::encode(scopes, ":/=&?");
-    const std::string body = fmt::format(GetAccessTokenBodyFormatStringWithScopes,
-                                         encoded_client_id, encoded_secret, encoded_scopes);
+    body = fmt::format(GetAccessTokenBodyFormatStringWithScopes, encoded_client_id, encoded_secret,
+                       encoded_scopes);
   }
   request->body().add(body);
   request->headers().setContentLength(body.length());

diff --git a/.../extensions/http/credential_injector/oauth2/credential_injector_oauth_integration_test.cc b/.../extensions/http/credential_injector/oauth2/credential_injector_oauth_integration_test.cc
@@ -65,6 +65,16 @@ class CredentialInjectorIntegrationTest : public HttpIntegrationTest,
     EXPECT_EQ(secret.value(), client_secret);
   }
 
+  virtual void checkScopeInRequest(absl::string_view desired_scope) {
+    std::string request_body = oauth2_request_->body().toString();
+    const auto query_parameters =
+        Http::Utility::QueryParamsMulti::parseParameters(request_body, 0, true);
+    auto actual_scope = query_parameters.getFirstValue("scope");
+
+    ASSERT_TRUE(actual_scope.has_value());
+    EXPECT_EQ(actual_scope.value(), desired_scope);
+  }
+
   void getFakeOuth2Connection() {
     AssertionResult result =
         fake_upstreams_.back()->waitForHttpConnection(*dispatcher_, fake_oauth2_connection_);
@@ -438,6 +448,9 @@ name: envoy.filters.http.credential_injector
   // wait for retried token request and respond with good response
   handleOauth2TokenRequest("test_client_secret");
 
+  // make sure scope is actually sent
+  checkScopeInRequest("scope1");
+
   EXPECT_EQ(
       1UL,
       test_server_