From ecd968391aadabb013caecc26838d08783663742 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 20 Dec 2021 15:44:54 -0800 Subject: [PATCH] Authorization Matchers: Fix header matcher to check for :method, :path and :authority (#28371) * Authorization Matchers: Fix header matcher to check for :method, :path and :authority * Reviewer comments --- .../security/authorization/evaluate_args.cc | 21 +++++++ .../security/authorization_matchers_test.cc | 60 +++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/src/core/lib/security/authorization/evaluate_args.cc b/src/core/lib/security/authorization/evaluate_args.cc index 99de496eb7c..201929c26ef 100644 --- a/src/core/lib/security/authorization/evaluate_args.cc +++ b/src/core/lib/security/authorization/evaluate_args.cc @@ -115,6 +115,27 @@ absl::optional EvaluateArgs::GetHeaderValue( if (metadata_ == nullptr) { return absl::nullopt; } + // TODO(yashykt): Remove these special cases for known metadata after + // https://github.com/grpc/grpc/pull/28267 is merged + if (key == HttpMethodMetadata::key()) { + auto method = metadata_->get(HttpMethodMetadata()); + return method.has_value() + ? absl::optional( + HttpMethodMetadata::Encode(*method).as_string_view()) + : absl::nullopt; + } + if (key == HttpAuthorityMetadata().key()) { + auto authority = metadata_->get_pointer(HttpAuthorityMetadata()); + return authority != nullptr + ? absl::optional(authority->as_string_view()) + : absl::nullopt; + } + if (key == HttpPathMetadata().key()) { + auto path = metadata_->get_pointer(HttpPathMetadata()); + return path != nullptr + ? absl::optional(path->as_string_view()) + : absl::nullopt; + } return metadata_->GetValue(key, concatenated_value); } diff --git a/test/core/security/authorization_matchers_test.cc b/test/core/security/authorization_matchers_test.cc index 7a19256494b..55d34838f29 100644 --- a/test/core/security/authorization_matchers_test.cc +++ b/test/core/security/authorization_matchers_test.cc @@ -237,6 +237,66 @@ TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherFailedMatch) { EXPECT_FALSE(matcher.Matches(args)); } +TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherMethodSuccess) { + args_.AddPairToMetadata(":method", "GET"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + HeaderAuthorizationMatcher matcher( + HeaderMatcher::Create(/*name=*/":method", HeaderMatcher::Type::kExact, + /*matcher=*/"GET") + .value()); + EXPECT_TRUE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherMethodFail) { + args_.AddPairToMetadata(":method", "GET"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + HeaderAuthorizationMatcher matcher( + HeaderMatcher::Create(/*name=*/":method", HeaderMatcher::Type::kExact, + /*matcher=*/"PUT") + .value()); + EXPECT_FALSE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherAuthoritySuccess) { + args_.AddPairToMetadata(":authority", "localhost"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + HeaderAuthorizationMatcher matcher( + HeaderMatcher::Create(/*name=*/":authority", HeaderMatcher::Type::kExact, + /*matcher=*/"localhost") + .value()); + EXPECT_TRUE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherAuthorityFail) { + args_.AddPairToMetadata(":authority", "localhost"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + HeaderAuthorizationMatcher matcher( + HeaderMatcher::Create(/*name=*/":authority", HeaderMatcher::Type::kExact, + /*matcher=*/"bad_authority") + .value()); + EXPECT_FALSE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherPathSuccess) { + args_.AddPairToMetadata(":path", "/expected/path"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + HeaderAuthorizationMatcher matcher( + HeaderMatcher::Create(/*name=*/":path", HeaderMatcher::Type::kExact, + /*matcher=*/"/expected/path") + .value()); + EXPECT_TRUE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherPathFail) { + args_.AddPairToMetadata(":path", "/expected/path"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + HeaderAuthorizationMatcher matcher( + HeaderMatcher::Create(/*name=*/":path", HeaderMatcher::Type::kExact, + /*matcher=*/"/unexpected/path") + .value()); + EXPECT_FALSE(matcher.Matches(args)); +} + TEST_F(AuthorizationMatchersTest, HeaderAuthorizationMatcherFailedMatchMultivaluedHeader) { args_.AddPairToMetadata("key123", "foo");