fix: allow oauth in internal proxy

Signed-off-by: Mohammed Shetaya <mohammed.shetaya@procore.com>

Author: MohammedShetaya

api/envoy/extensions/filters/http/oauth2/v3/oauth.proto

@@ -151,7 +151,7 @@ message OAuth2Credentials {
 
 // OAuth config
 //
-// [#next-free-field: 27]
+// [#next-free-field: 29]
 message OAuth2Config {
   enum AuthType {
     // The ``client_id`` and ``client_secret`` will be sent in the URL encoded request body.
@@ -197,6 +197,21 @@ message OAuth2Config {
   // This URI should not contain any query parameters.
   string redirect_uri = 4 [(validate.rules).string = {min_len: 1}];
 
+  // Optional list of allowed redirect domains for the URL encoded in the OAuth2 state parameter.
+  // When set, the filter validates the host of the original request URL decoded from the state
+  // parameter against this list before redirecting. Supports exact matches (e.g., "example.com")
+  // and wildcard prefix matches (e.g., "*.example.com" matches "foo.example.com").
+  // If not set, no domain restriction is applied.
+  repeated string allowed_redirect_domains = 27;
+
+  // When set to true, the host and scheme encoded in the OAuth2 state parameter
+  // will be derived from the configured redirect_uri instead of from the
+  // request's :authority and :scheme headers. This is useful when Envoy is
+  // deployed behind an external load balancer whose hostname differs from
+  // Envoy's internal hostname.
+  // Default is false.
+  bool match_redirect_url_to_redirect_uri = 28;
+
   // Matching criteria used to determine whether a path appears to be the result of a redirect from the authorization server.
   type.matcher.v3.PathMatcher redirect_path_matcher = 5
       [(validate.rules).message = {required: true}];

source/extensions/filters/http/oauth2/filter.cc

@@ -302,6 +302,33 @@ std::string generateCodeChallenge(const std::string& code_verifier) {
   return Base64Url::encode(sha256_string.data(), sha256_string.size());
 }
 
+bool isHostAllowedDomain(absl::string_view host, const std::vector<std::string>& allowed_domains) {
+  if (allowed_domains.empty()) {
+    return true;
+  }
+
+  // Strip port from host if present (e.g., "example.com:8080" -> "example.com")
+  auto colon_pos = host.rfind(':');
+  absl::string_view hostname =
+      (colon_pos != absl::string_view::npos) ? host.substr(0, colon_pos) : host;
+
+  for (const auto& domain : allowed_domains) {
+    if (absl::StartsWith(domain, "*.")) {
+      // Wildcard match: "*.example.com" matches "foo.example.com"
+      absl::string_view suffix = absl::string_view(domain).substr(1);
+      if (absl::EndsWith(hostname, suffix) && hostname.size() > suffix.size()) {
+        return true;
+      }
+    } else {
+      // Exact match
+      if (absl::EqualsIgnoreCase(hostname, domain)) {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 /**
  * Encodes the state parameter for the OAuth2 flow.
  * The state parameter is a base64Url encoded JSON object containing the original request URL, a
@@ -456,6 +483,9 @@ FilterConfig::FilterConfig(
       authorization_query_params_(buildAutorizationQueryParams(proto_config)),
       client_id_(proto_config.credentials().client_id()),
       redirect_uri_(proto_config.redirect_uri()),
+      allowed_redirect_domains_(proto_config.allowed_redirect_domains().begin(),
+                                proto_config.allowed_redirect_domains().end()),
+      match_redirect_url_to_redirect_uri_(proto_config.match_redirect_url_to_redirect_uri()),
       redirect_matcher_(proto_config.redirect_path_matcher(), context),
       signout_path_(proto_config.signout_path(), context), secret_reader_(secret_reader),
       stats_(FilterConfig::generateStats(stats_prefix, proto_config.stat_prefix(), scope)),
@@ -915,7 +945,22 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) {
   if (Http::Utility::schemeIsHttp(headers.getSchemeValue())) {
     scheme = Http::Headers::get().SchemeValues.Http;
   }
-  const std::string base_path = absl::StrCat(scheme, "://", host_);
+
+  // Format redirect_uri — needed for the query param, and when
+  // match_redirect_url_to_redirect_uri is set, also for deriving the state URL host.
+  auto formatter = THROW_OR_RETURN_VALUE(Formatter::FormatterImpl::create(config_->redirectUri()),
+                                         Formatter::FormatterPtr);
+  const auto redirect_uri = formatter->format({&headers}, decoder_callbacks_->streamInfo());
+
+  std::string base_path = absl::StrCat(scheme, "://", host_);
+
+  Http::Utility::Url parsed_redirect_uri;
+  if (config_->matchRedirectUrlToRedirectUri() &&
+      parsed_redirect_uri.initialize(redirect_uri, false)) {
+    base_path =
+        absl::StrCat(parsed_redirect_uri.scheme(), "://", parsed_redirect_uri.hostAndPort());
+  }
+
   const std::string original_url = absl::StrCat(base_path, headers.Path()->value().getStringView());
 
   const CookieNames& cookie_names = config_->cookieNames();
@@ -949,9 +994,6 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) {
   auto query_params = config_->authorizationQueryParams();
   query_params.overwrite(queryParamsState, state);
 
-  Formatter::FormatterPtr formatter = THROW_OR_RETURN_VALUE(
-      Formatter::FormatterImpl::create(config_->redirectUri()), Formatter::FormatterPtr);
-  const auto redirect_uri = formatter->format({&headers}, decoder_callbacks_->streamInfo());
   const std::string escaped_redirect_uri = Http::Utility::PercentEncoding::urlEncode(redirect_uri);
   query_params.overwrite(queryParamsRedirectUri, escaped_redirect_uri);
 
@@ -1496,6 +1538,13 @@ CallbackValidationResult OAuth2Filter::validateState(const Http::RequestHeaderMa
             fmt::format("State url can not be initialized: {}", original_request_url)};
   }
 
+  // Validate the host of the original request URL against allowed redirect domains.
+  if (!isHostAllowedDomain(url.hostAndPort(), config_->allowedRedirectDomains())) {
+    return {false, "", "", flow_id,
+            fmt::format("State url host is not in the allowed redirect domains: {}",
+                        url.hostAndPort())};
+  }
+
   return {true, "", original_request_url, flow_id, ""};
 }
 

source/extensions/filters/http/oauth2/filter.h

@@ -155,6 +155,10 @@ class FilterConfig : public Logger::Loggable<Logger::Id::oauth2> {
     return authorization_query_params_;
   }
   const std::string& redirectUri() const { return redirect_uri_; }
+  const std::vector<std::string>& allowedRedirectDomains() const {
+    return allowed_redirect_domains_;
+  }
+  bool matchRedirectUrlToRedirectUri() const { return match_redirect_url_to_redirect_uri_; }
   const Matchers::PathMatcher& redirectPathMatcher() const { return redirect_matcher_; }
   const Matchers::PathMatcher& signoutPath() const { return signout_path_; }
   std::string clientSecret() const { return secret_reader_->clientSecret(); }
@@ -220,6 +224,8 @@ class FilterConfig : public Logger::Loggable<Logger::Id::oauth2> {
   const Http::Utility::QueryParamsMulti authorization_query_params_;
   const std::string client_id_;
   const std::string redirect_uri_;
+  const std::vector<std::string> allowed_redirect_domains_;
+  const bool match_redirect_url_to_redirect_uri_;
   const Matchers::PathMatcher redirect_matcher_;
   const Matchers::PathMatcher signout_path_;
   std::shared_ptr<SecretReader> secret_reader_;

test/extensions/filters/http/oauth2/config_test.cc

@@ -870,6 +870,50 @@ TEST(ConfigTest, ValidPartitionedConfigs) {
   EXPECT_TRUE(result.ok());
 }
 
+TEST(ConfigTest, AllowedRedirectDomainsAndMatchRedirectUrl) {
+  const std::string yaml = R"EOF(
+config:
+  token_endpoint:
+    cluster: foo
+    uri: oauth.com/token
+    timeout: 3s
+  credentials:
+    client_id: "secret"
+    token_secret:
+      name: token
+    hmac_secret:
+      name: hmac
+  authorization_endpoint: https://oauth.com/oauth/authorize/
+  redirect_uri: "https://external.example.com/callback"
+  redirect_path_matcher:
+    path:
+      exact: /callback
+  signout_path:
+    path:
+      exact: /signout
+  allowed_redirect_domains:
+  - "example.com"
+  - "*.example.com"
+  match_redirect_url_to_redirect_uri: true
+  )EOF";
+
+  OAuth2Config factory;
+  ProtobufTypes::MessagePtr proto_config = factory.createEmptyConfigProto();
+  TestUtility::loadFromYaml(yaml, *proto_config);
+  NiceMock<Server::Configuration::MockFactoryContext> context;
+  context.server_factory_context_.cluster_manager_.initializeClusters({"foo"}, {});
+
+  NiceMock<Secret::MockSecretManager> secret_manager;
+  ON_CALL(context.server_factory_context_, secretManager())
+      .WillByDefault(ReturnRef(secret_manager));
+  ON_CALL(secret_manager, findStaticGenericSecretProvider(_))
+      .WillByDefault(Return(std::make_shared<Secret::GenericSecretConfigProviderImpl>(
+          envoy::extensions::transport_sockets::tls::v3::GenericSecret())));
+
+  const auto result = factory.createFilterFactoryFromProto(*proto_config, "stats", context);
+  EXPECT_TRUE(result.ok());
+}
+
 } // namespace Oauth2
 } // namespace HttpFilters
 } // namespace Extensions

test/extensions/filters/http/oauth2/filter_test.cc

@@ -4838,6 +4838,202 @@ TEST_F(OAuth2Test, OAuthTestCustomCookiePaths) {
   }
 }
 
+// Helper to build a FilterConfig with allowed_redirect_domains and
+// match_redirect_url_to_redirect_uri.
+class OAuth2RedirectDomainTest : public OAuth2Test {
+public:
+  OAuth2RedirectDomainTest() : OAuth2Test(false) {}
+
+  FilterConfigSharedPtr
+  getConfigWithRedirectDomains(const std::vector<std::string>& allowed_domains,
+                               bool match_redirect_url = false,
+                               const std::string& redirect_uri = "") {
+    envoy::extensions::filters::http::oauth2::v3::OAuth2Config p;
+    auto* endpoint = p.mutable_token_endpoint();
+    endpoint->set_cluster("auth.example.com");
+    endpoint->set_uri("auth.example.com/_oauth");
+    endpoint->mutable_timeout()->set_seconds(1);
+    p.set_redirect_uri(redirect_uri.empty() ? "%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK
+                                          : redirect_uri);
+    p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK);
+    p.set_authorization_endpoint("https://auth.example.com/oauth/authorize/");
+    p.mutable_signout_path()->mutable_path()->set_exact("/_signout");
+    p.set_forward_bearer_token(true);
+    p.add_auth_scopes("user");
+    p.add_auth_scopes("openid");
+    p.add_auth_scopes("email");
+    p.add_resources("oauth2-resource");
+    p.add_resources("http://example.com");
+    p.add_resources("https://example.com/some/path%2F..%2F/utf8\xc3\x83;foo=bar?var1=1&var2=2");
+    auto credentials = p.mutable_credentials();
+    credentials->set_client_id(TEST_CLIENT_ID);
+    credentials->mutable_token_secret()->set_name("secret");
+    credentials->mutable_hmac_secret()->set_name("hmac");
+    p.mutable_use_refresh_token()->set_value(false);
+
+    for (const auto& domain : allowed_domains) {
+      p.add_allowed_redirect_domains(domain);
+    }
+    p.set_match_redirect_url_to_redirect_uri(match_redirect_url);
+
+    MessageUtil::validate(p, ProtobufMessage::getStrictValidationVisitor());
+
+    auto secret_reader = std::make_shared<MockSecretReader>();
+    return std::make_shared<FilterConfig>(p, factory_context_.server_factory_context_,
+                                          secret_reader, scope_, "test.");
+  }
+};
+
+/**
+ * Scenario: allowed_redirect_domains is set and the state URL host is not in the list.
+ *
+ * Expected behavior: the filter should reject the callback with 401.
+ */
+TEST_F(OAuth2RedirectDomainTest, AllowedRedirectDomainsRejectsInvalidHost) {
+  // TEST_ENCODED_STATE contains url with host "traffic.example.com".
+  // Only allow "other.example.com".
+  init(getConfigWithRedirectDomains({"other.example.com"}));
+
+  Http::TestRequestHeaderMapImpl request_headers{
+      {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE},
+      {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN},
+      {Http::Headers::get().Cookie.get(),
+       "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER},
+      {Http::Headers::get().Host.get(), "traffic.example.com"},
+      {Http::Headers::get().Scheme.get(), "https"},
+      {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get},
+  };
+
+  EXPECT_CALL(*validator_, setParams(_, _));
+  EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false));
+
+  EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers, false));
+}
+
+/**
+ * Scenario: allowed_redirect_domains is set and the state URL host matches exactly.
+ *
+ * Expected behavior: the filter should allow the callback and proceed with token exchange.
+ */
+TEST_F(OAuth2RedirectDomainTest, AllowedRedirectDomainsAllowsExactMatch) {
+  // TEST_ENCODED_STATE contains url with host "traffic.example.com".
+  init(getConfigWithRedirectDomains({"traffic.example.com"}));
+
+  Http::TestRequestHeaderMapImpl request_headers{
+      {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE},
+      {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN},
+      {Http::Headers::get().Cookie.get(),
+       "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER},
+      {Http::Headers::get().Host.get(), "traffic.example.com"},
+      {Http::Headers::get().Scheme.get(), "https"},
+      {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get},
+  };
+
+  EXPECT_CALL(*validator_, setParams(_, _));
+  EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false));
+
+  EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa",
+                                                  "https://traffic.example.com" + TEST_CALLBACK,
+                                                  TEST_CODE_VERIFIER, AuthType::UrlEncodedBody));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer,
+            filter_->decodeHeaders(request_headers, false));
+}
+
+/**
+ * Scenario: allowed_redirect_domains is set with a wildcard and the state URL host matches.
+ *
+ * Expected behavior: "*.example.com" should match "traffic.example.com".
+ */
+TEST_F(OAuth2RedirectDomainTest, AllowedRedirectDomainsAllowsWildcardMatch) {
+  // TEST_ENCODED_STATE contains url with host "traffic.example.com".
+  init(getConfigWithRedirectDomains({"*.example.com"}));
+
+  Http::TestRequestHeaderMapImpl request_headers{
+      {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE},
+      {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN},
+      {Http::Headers::get().Cookie.get(),
+       "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER},
+      {Http::Headers::get().Host.get(), "traffic.example.com"},
+      {Http::Headers::get().Scheme.get(), "https"},
+      {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get},
+  };
+
+  EXPECT_CALL(*validator_, setParams(_, _));
+  EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false));
+
+  EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa",
+                                                  "https://traffic.example.com" + TEST_CALLBACK,
+                                                  TEST_CODE_VERIFIER, AuthType::UrlEncodedBody));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer,
+            filter_->decodeHeaders(request_headers, false));
+}
+
+// {"url":"https://external.example.com/original_path?var1=1&var2=2",
+//  "csrf_token":"00000000075bcd15.na6kru4x1pHgocSIeU/mdtHYn58Gh1bqweS4XXoiqVg=",
+//  "flow_id":"00000000075bcd15"}
+static const std::string TEST_ENCODED_STATE_EXTERNAL_HOST =
+    "eyJ1cmwiOiJodHRwczovL2V4dGVybmFsLmV4YW1wbGUuY29tL29yaWdpbmFsX3BhdGg_dmFyMT0xJnZhcjI9MiIsIm"
+    "NzcmZfdG9rZW4iOiIwMDAwMDAwMDA3NWJjZDE1Lm5hNmtydTR4MXBIZ29jU0llVS9tZHRIWW41OEdoMWJxd2VTNFhY"
+    "b2lxVmc9IiwiZmxvd19pZCI6IjAwMDAwMDAwMDc1YmNkMTUifQ";
+
+/**
+ * Scenario: match_redirect_url_to_redirect_uri is enabled with a static redirect_uri pointing
+ * to an external host. The internal :authority is "traffic.example.com" but the redirect_uri
+ * uses "external.example.com".
+ *
+ * Expected behavior: the state URL encoded in the redirect to the IdP should use
+ * "external.example.com" as the host, not "traffic.example.com".
+ */
+TEST_F(OAuth2RedirectDomainTest, MatchRedirectUrlToRedirectUri) {
+  init(getConfigWithRedirectDomains({}, true, "https://external.example.com/_oauth"));
+
+  Http::TestRequestHeaderMapImpl first_request_headers{
+      {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"},
+      {Http::Headers::get().Host.get(), "traffic.example.com"},
+      {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get},
+      {Http::Headers::get().Scheme.get(), "https"},
+  };
+
+  Http::TestResponseHeaderMapImpl first_response_headers{
+      {Http::Headers::get().Status.get(), "302"},
+      {Http::Headers::get().SetCookie.get(),
+       "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"},
+      {Http::Headers::get().SetCookie.get(),
+       "OauthNonce=" + TEST_CSRF_TOKEN + ";path=/;Max-Age=600;secure;HttpOnly"},
+      {Http::Headers::get().SetCookie.get(),
+       "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER +
+           ";path=/;Max-Age=600;secure;HttpOnly"},
+      {Http::Headers::get().SetCookie.get(),
+       "CodeVerifier=" + TEST_ENCRYPTED_CODE_VERIFIER + ";path=/;Max-Age=600;secure;HttpOnly"},
+      {Http::Headers::get().Location.get(),
+       "https://auth.example.com/oauth/"
+       "authorize/?client_id=" +
+           TEST_CLIENT_ID + "&code_challenge=" + TEST_CODE_CHALLENGE +
+           "&code_challenge_method=S256"
+           "&redirect_uri=https%3A%2F%2Fexternal.example.com%2F_oauth"
+           "&response_type=code"
+           "&scope=" +
+           TEST_ENCODED_AUTH_SCOPES + "&state=" + TEST_ENCODED_STATE_EXTERNAL_HOST +
+           "&resource=oauth2-resource"
+           "&resource=http%3A%2F%2Fexample.com"
+           "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%3Dbar%"
+           "3Fvar1%3D1%26var2%3D2"},
+  };
+
+  EXPECT_CALL(*validator_, setParams(_, _));
+  EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false));
+
+  EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&first_response_headers), true));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(first_request_headers, false));
+}
+
 } // namespace Oauth2
 } // namespace HttpFilters
 } // namespace Extensions