Skip to content

Commit

Permalink
oauth2: removes exceptions (#37541)
Browse files Browse the repository at this point in the history
Commit Message: oauth2: removes exceptions
Additional Description:
Risk Level: low
Testing: existing ones
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

---------

Signed-off-by: Takeshi Yoneda <[email protected]>
  • Loading branch information
mathetake authored Dec 11, 2024
1 parent 0c6a1d2 commit 6e845a5
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 24 deletions.
10 changes: 5 additions & 5 deletions source/extensions/filters/http/oauth2/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ secretsProvider(const envoy::extensions::transport_sockets::tls::v3::SdsSecretCo
}
} // namespace

Http::FilterFactoryCb OAuth2Config::createFilterFactoryFromProtoTyped(
absl::StatusOr<Http::FilterFactoryCb> OAuth2Config::createFilterFactoryFromProtoTyped(
const envoy::extensions::filters::http::oauth2::v3::OAuth2& proto,
const std::string& stats_prefix, Server::Configuration::FactoryContext& context) {
if (!proto.has_config()) {
throw EnvoyException("config must be present for global config");
return absl::InvalidArgumentError("config must be present for global config");
}

const auto& proto_config = proto.config();
Expand All @@ -56,16 +56,16 @@ Http::FilterFactoryCb OAuth2Config::createFilterFactoryFromProtoTyped(
auto secret_provider_client_secret = secretsProvider(
client_secret, secret_manager, transport_socket_factory, context.initManager());
if (secret_provider_client_secret == nullptr) {
throw EnvoyException("invalid token secret configuration");
return absl::InvalidArgumentError("invalid token secret configuration");
}
auto secret_provider_hmac_secret =
secretsProvider(hmac_secret, secret_manager, transport_socket_factory, context.initManager());
if (secret_provider_hmac_secret == nullptr) {
throw EnvoyException("invalid HMAC secret configuration");
return absl::InvalidArgumentError("invalid HMAC secret configuration");
}

if (proto_config.preserve_authorization_header() && proto_config.forward_bearer_token()) {
throw EnvoyException(
return absl::InvalidArgumentError(
"invalid combination of forward_bearer_token and preserve_authorization_header "
"configuration. If forward_bearer_token is set to true, then "
"preserve_authorization_header must be false");
Expand Down
6 changes: 3 additions & 3 deletions source/extensions/filters/http/oauth2/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ namespace Extensions {
namespace HttpFilters {
namespace Oauth2 {

class OAuth2Config : public Extensions::HttpFilters::Common::FactoryBase<
class OAuth2Config : public Extensions::HttpFilters::Common::ExceptionFreeFactoryBase<
envoy::extensions::filters::http::oauth2::v3::OAuth2> {
public:
OAuth2Config() : FactoryBase("envoy.filters.http.oauth2") {}
OAuth2Config() : ExceptionFreeFactoryBase("envoy.filters.http.oauth2") {}

Http::FilterFactoryCb
absl::StatusOr<Http::FilterFactoryCb>
createFilterFactoryFromProtoTyped(const envoy::extensions::filters::http::oauth2::v3::OAuth2&,
const std::string&,
Server::Configuration::FactoryContext&) override;
Expand Down
29 changes: 14 additions & 15 deletions test/extensions/filters/http/oauth2/config_test.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
#include <memory>
#include <string>

#include "envoy/extensions/filters/http/oauth2/v3/oauth.pb.h"

#include "source/common/protobuf/message_validator_impl.h"
Expand All @@ -10,7 +7,6 @@

#include "test/mocks/server/factory_context.h"

#include "gmock/gmock.h"
#include "gtest/gtest.h"

namespace Envoy {
Expand All @@ -25,7 +21,7 @@ namespace {

// This loads one of the secrets in credentials, and fails the other one.
void expectInvalidSecretConfig(const std::string& failed_secret_name,
const std::string& exception_message) {
const std::string& status_message) {
const std::string yaml = R"EOF(
config:
token_endpoint:
Expand Down Expand Up @@ -74,9 +70,9 @@ void expectInvalidSecretConfig(const std::string& failed_secret_name,
.WillByDefault(Return(std::make_shared<Secret::GenericSecretConfigProviderImpl>(
envoy::extensions::transport_sockets::tls::v3::GenericSecret())));

EXPECT_THROW_WITH_MESSAGE(
factory.createFilterFactoryFromProto(*proto_config, "stats", context).status().IgnoreError(),
EnvoyException, exception_message);
const auto result = factory.createFilterFactoryFromProto(*proto_config, "stats", context);
EXPECT_FALSE(result.ok());
EXPECT_EQ(result.status().message(), status_message);
}

} // namespace
Expand Down Expand Up @@ -164,9 +160,10 @@ TEST(ConfigTest, CreateFilterMissingConfig) {
envoy::extensions::filters::http::oauth2::v3::OAuth2 proto_config;

NiceMock<Server::Configuration::MockFactoryContext> factory_context;
EXPECT_THROW_WITH_MESSAGE(
config.createFilterFactoryFromProtoTyped(proto_config, "whatever", factory_context),
EnvoyException, "config must be present for global config");
const auto result =
config.createFilterFactoryFromProtoTyped(proto_config, "whatever", factory_context);
EXPECT_FALSE(result.ok());
EXPECT_EQ(result.status().message(), "config must be present for global config");
}

TEST(ConfigTest, WrongCookieName) {
Expand Down Expand Up @@ -275,10 +272,12 @@ TEST(ConfigTest, WrongCombinationOfPreserveAuthorizationAndForwardBearer) {
.WillByDefault(Return(std::make_shared<Secret::GenericSecretConfigProviderImpl>(
envoy::extensions::transport_sockets::tls::v3::GenericSecret())));

EXPECT_THROW_WITH_REGEX(
factory.createFilterFactoryFromProto(*proto_config, "stats", context).status().IgnoreError(),
EnvoyException,
"invalid combination of forward_bearer_token and preserve_authorization_header");
const auto result = factory.createFilterFactoryFromProto(*proto_config, "stats", context);
EXPECT_FALSE(result.ok());
EXPECT_EQ(result.status().message(),
"invalid combination of forward_bearer_token and preserve_authorization_header "
"configuration. If forward_bearer_token is set to true, then "
"preserve_authorization_header must be false");
}

} // namespace Oauth2
Expand Down
1 change: 0 additions & 1 deletion tools/code_format/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ paths:
- source/extensions/filters/http/json_to_metadata
- source/extensions/filters/http/jwt_authn
- source/extensions/filters/http/local_ratelimit
- source/extensions/filters/http/oauth2
- source/extensions/filters/http/file_system_buffer
- source/extensions/filters/http/header_to_metadata
- source/extensions/filters/http/on_demand
Expand Down

0 comments on commit 6e845a5

Please sign in to comment.