Skip to content

Commit

Permalink
[release-1.4] istio_authn: bypass CORS preflight request (#2564)
Browse files Browse the repository at this point in the history
* istio_authn: bypass CORS preflight request

* fix comment

* fix lint
  • Loading branch information
istio-testing authored Nov 14, 2019
1 parent ea144fc commit 44089f7
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/envoy/http/authn/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ envoy_cc_library(
"//src/envoy/utils:filter_names_lib",
"//src/envoy/utils:utils_lib",
"//src/istio/authn:context_proto_cc_proto",
"@envoy//source/common/http:headers_lib",
],
)

Expand Down
30 changes: 30 additions & 0 deletions src/envoy/http/authn/http_filter_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,5 +165,35 @@ TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) {
EXPECT_EQ("200", response->headers().Status()->value().getStringView());
}

TEST_P(AuthenticationFilterIntegrationTest, CORSPreflight) {
config_helper_.addFilter(kAuthnFilterWithJwt);
initialize();

// The AuthN filter requires JWT but should bypass CORS preflight request even
// it doesn't have JWT token.
codec_client_ =
makeHttpConnection(makeClientConnection((lookupPort("http"))));
auto headers = Http::TestHeaderMapImpl{
{":method", "OPTIONS"},
{":path", "/"},
{":scheme", "http"},
{":authority", "host"},
{"x-forwarded-for", "10.0.0.1"},
{"access-control-request-method", "GET"},
{"origin", "example.com"},
};
auto response = codec_client_->makeHeaderOnlyRequest(headers);

// Wait for request to upstream (backend)
waitForNextUpstreamRequest();
// Send backend response.
upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}},
true);

response->waitForEndStream();
EXPECT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().Status()->value().getStringView());
}

} // namespace
} // namespace Envoy
18 changes: 18 additions & 0 deletions src/envoy/http/authn/origin_authenticator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "absl/strings/match.h"
#include "authentication/v1alpha1/policy.pb.h"
#include "common/http/headers.h"
#include "src/envoy/http/authn/authn_utils.h"

using istio::authn::Payload;
Expand All @@ -28,6 +29,15 @@ namespace Http {
namespace Istio {
namespace AuthN {

bool isCORSPreflightRequest(const Http::HeaderMap& headers) {
return headers.Method() &&
headers.Method()->value().getStringView() ==
Http::Headers::get().MethodValues.Options &&
headers.Origin() && !headers.Origin()->value().empty() &&
headers.AccessControlRequestMethod() &&
!headers.AccessControlRequestMethod()->value().empty();
}

OriginAuthenticator::OriginAuthenticator(FilterContext* filter_context,
const iaapi::Policy& policy)
: AuthenticatorBase(filter_context), policy_(policy) {}
Expand All @@ -46,6 +56,14 @@ bool OriginAuthenticator::run(Payload* payload) {
return false;
}

if (isCORSPreflightRequest(filter_context()->headerMap())) {
// The CORS preflight doesn't include user credentials, allow regardless of
// JWT policy. See
// http://www.w3.org/TR/cors/#cross-origin-request-with-preflight.
ENVOY_LOG(debug, "CORS preflight request allowed regardless of JWT policy");
return true;
}

absl::string_view request_path;
if (filter_context()->headerMap().Path() != nullptr) {
request_path =
Expand Down
18 changes: 18 additions & 0 deletions src/envoy/http/authn/origin_authenticator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ class OriginAuthenticatorTest : public testing::TestWithParam<bool> {
header_.removePath();
header_.addCopy(":path", path);
}

void addHeader(const std::string& key, const std::string& value) {
header_.addCopy(key, value);
}
};

TEST_P(OriginAuthenticatorTest, Empty) {
Expand Down Expand Up @@ -278,6 +282,20 @@ TEST_P(OriginAuthenticatorTest, SingleMethodFail) {
filter_context_.authenticationResult()));
}

TEST_P(OriginAuthenticatorTest, CORSPreflight) {
ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(kSingleOriginMethodPolicy,
&policy_));

createAuthenticator();

EXPECT_CALL(*authenticator_, validateJwt(_, _)).Times(0);

addHeader(":method", "OPTIONS");
addHeader("origin", "example.com");
addHeader("access-control-request-method", "GET");
EXPECT_TRUE(authenticator_->run(payload_));
}

TEST_P(OriginAuthenticatorTest, TriggeredWithNullPath) {
ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(
kSingleOriginMethodWithTriggerRulePolicy, &policy_));
Expand Down

0 comments on commit 44089f7

Please sign in to comment.