From 3edaf4499e1ab54178747f9e2f14f827d48f65f6 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 4 Aug 2020 17:31:30 +0800 Subject: [PATCH 1/4] Config Core/Serving authentication to allow unauthenticated access to health probe. --- .../main/java/feast/core/config/CoreSecurityConfig.java | 2 ++ .../java/feast/serving/config/ServingSecurityConfig.java | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/core/src/main/java/feast/core/config/CoreSecurityConfig.java b/core/src/main/java/feast/core/config/CoreSecurityConfig.java index 3e4c2baa9eb..ead6bcb18bc 100644 --- a/core/src/main/java/feast/core/config/CoreSecurityConfig.java +++ b/core/src/main/java/feast/core/config/CoreSecurityConfig.java @@ -17,6 +17,7 @@ package feast.core.config; import feast.proto.core.CoreServiceGrpc; +import io.grpc.health.v1.HealthGrpc; import lombok.extern.slf4j.Slf4j; import net.devh.boot.grpc.server.security.check.AccessPredicate; import net.devh.boot.grpc.server.security.check.GrpcSecurityMetadataSource; @@ -48,6 +49,7 @@ GrpcSecurityMetadataSource grpcSecurityMetadataSource() { // The following endpoints allow unauthenticated access source.set(CoreServiceGrpc.getGetFeastCoreVersionMethod(), AccessPredicate.permitAll()); source.set(CoreServiceGrpc.getUpdateStoreMethod(), AccessPredicate.permitAll()); + source.set(HealthGrpc.getCheckMethod(), AccessPredicate.permitAll()); return source; } } diff --git a/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java index 2d0a46763a7..9456aad1b4d 100644 --- a/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java @@ -18,7 +18,10 @@ import feast.auth.credentials.GoogleAuthCredentials; import feast.auth.credentials.OAuthCredentials; +import feast.proto.serving.ServingServiceGrpc; import io.grpc.CallCredentials; +import io.grpc.health.v1.HealthGrpc; + import java.io.IOException; import net.devh.boot.grpc.server.security.check.AccessPredicate; import net.devh.boot.grpc.server.security.check.GrpcSecurityMetadataSource; @@ -67,6 +70,10 @@ GrpcSecurityMetadataSource grpcSecurityMetadataSource() { // Authentication is enabled for all gRPC endpoints source.setDefault(AccessPredicate.authenticated()); + + // The following endpoints allow unauthenticated access + source.set(ServingServiceGrpc.getGetFeastServingInfoMethod(), AccessPredicate.permitAll()); + source.set(HealthGrpc.getCheckMethod(), AccessPredicate.permitAll()); return source; } From d66768ca6332473acdd970d17288ddcb730b910e Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 4 Aug 2020 17:50:19 +0800 Subject: [PATCH 2/4] Allow unauthenticated requests when only authentication but not authorization is enabled. --- .../feast/auth/config/SecurityConfig.java | 6 ++-- .../auth/CoreServiceAuthenticationIT.java | 29 +++++++++++-------- .../serving/config/ServingSecurityConfig.java | 1 - 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/auth/src/main/java/feast/auth/config/SecurityConfig.java b/auth/src/main/java/feast/auth/config/SecurityConfig.java index 9c831b3076b..378deea1959 100644 --- a/auth/src/main/java/feast/auth/config/SecurityConfig.java +++ b/auth/src/main/java/feast/auth/config/SecurityConfig.java @@ -83,13 +83,13 @@ GrpcAuthenticationReader authenticationReader() { } /** - * Creates an AccessDecisionManager if authentication is enabled. This object determines the - * policy used to make authentication decisions. + * Creates an AccessDecisionManager if authorization is enabled. This object determines the policy + * used to make authorization decisions. * * @return AccessDecisionManager */ @Bean - @ConditionalOnProperty(prefix = "feast.security.authentication", name = "enabled") + @ConditionalOnProperty(prefix = "feast.security.authorization", name = "enabled") AccessDecisionManager accessDecisionManager() { final List> voters = new ArrayList<>(); voters.add(new AccessPredicateVoter()); diff --git a/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java b/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java index 0886bf80b3e..aa02e0c66f4 100644 --- a/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java +++ b/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java @@ -32,7 +32,6 @@ import io.grpc.CallCredentials; import io.grpc.Channel; import io.grpc.ManagedChannelBuilder; -import io.grpc.StatusRuntimeException; import java.util.*; import org.junit.ClassRule; import org.junit.Rule; @@ -121,18 +120,24 @@ public void shouldGetVersionFromFeastCoreAlways() { assertEquals(feastProperties.getVersion(), feastCoreVersionSecure); } + /** + * If authentication is enabled but authorization is disabled, users can still connect to Feast + * Core as anonymous users. They are not forced to authenticate. + */ @Test - public void shouldNotAllowUnauthenticatedFeatureSetListing() { - Exception exception = - assertThrows( - StatusRuntimeException.class, - () -> { - insecureApiClient.simpleListFeatureSets("*"); - }); - - String expectedMessage = "UNAUTHENTICATED: Authentication failed"; - String actualMessage = exception.getMessage(); - assertEquals(actualMessage, expectedMessage); + public void shouldAllowUnauthenticatedFeatureSetListing() { + FeatureSetProto.FeatureSet expectedFeatureSet = DataGenerator.getDefaultFeatureSet(); + insecureApiClient.simpleApplyFeatureSet(expectedFeatureSet); + + List listFeatureSetsResponse = + insecureApiClient.simpleListFeatureSets("*"); + FeatureSetProto.FeatureSet actualFeatureSet = listFeatureSetsResponse.get(0); + + assert listFeatureSetsResponse.size() == 1; + assertEquals( + actualFeatureSet.getSpec().getProject(), expectedFeatureSet.getSpec().getProject()); + assertEquals( + actualFeatureSet.getSpec().getProject(), expectedFeatureSet.getSpec().getProject()); } @Test diff --git a/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java index 9456aad1b4d..839c133387d 100644 --- a/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java @@ -21,7 +21,6 @@ import feast.proto.serving.ServingServiceGrpc; import io.grpc.CallCredentials; import io.grpc.health.v1.HealthGrpc; - import java.io.IOException; import net.devh.boot.grpc.server.security.check.AccessPredicate; import net.devh.boot.grpc.server.security.check.GrpcSecurityMetadataSource; From 04faead99d086fdf23ceaaeea511af62921fa91c Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 4 Aug 2020 18:37:17 +0800 Subject: [PATCH 3/4] Fix ServingServiceOauthAuthenticationIT --- .../ServingServiceOauthAuthenticationIT.java | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java index edd16c24a87..71f022c3425 100644 --- a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java +++ b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java @@ -17,7 +17,6 @@ package feast.serving.it; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.testcontainers.containers.wait.strategy.Wait.forHttp; @@ -26,7 +25,6 @@ import feast.proto.serving.ServingServiceGrpc.ServingServiceBlockingStub; import feast.proto.types.ValueProto.Value; import io.grpc.ManagedChannel; -import io.grpc.StatusRuntimeException; import java.io.File; import java.io.IOException; import java.time.Duration; @@ -87,21 +85,17 @@ static void globalSetup() throws IOException, InitializationError, InterruptedEx } @Test - public void shouldNotAllowUnauthenticatedGetOnlineFeatures() { + public void shouldAllowUnauthenticatedGetOnlineFeatures() { ServingServiceBlockingStub servingStub = AuthTestUtils.getServingServiceStub(false, FEAST_SERVING_PORT, null); GetOnlineFeaturesRequest onlineFeatureRequest = AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); - Exception exception = - assertThrows( - StatusRuntimeException.class, - () -> { - servingStub.getOnlineFeatures(onlineFeatureRequest); - }); - - String expectedMessage = "UNAUTHENTICATED: Authentication failed"; - String actualMessage = exception.getMessage(); - assertEquals(actualMessage, expectedMessage); + GetOnlineFeaturesResponse featureResponse = servingStub.getOnlineFeatures(onlineFeatureRequest); + assertEquals(1, featureResponse.getFieldValuesCount()); + Map fieldsMap = featureResponse.getFieldValues(0).getFieldsMap(); + assertTrue(fieldsMap.containsKey(ENTITY_ID)); + assertTrue(fieldsMap.containsKey(FEATURE_NAME)); + ((ManagedChannel) servingStub.getChannel()).shutdown(); } @Test From d2891d5216c5662892ebc43068aa4f98cd6ea742 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 4 Aug 2020 19:00:45 +0800 Subject: [PATCH 4/4] Add missing applyFeatureSet call to ServingServiceOauthAuthenticationIT --- .../feast/serving/it/ServingServiceOauthAuthenticationIT.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java index 71f022c3425..f1289adc737 100644 --- a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java +++ b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java @@ -86,6 +86,10 @@ static void globalSetup() throws IOException, InitializationError, InterruptedEx @Test public void shouldAllowUnauthenticatedGetOnlineFeatures() { + // apply feature set + CoreSimpleAPIClient coreClient = + AuthTestUtils.getSecureApiClientForCore(FEAST_CORE_PORT, options); + AuthTestUtils.applyFeatureSet(coreClient, PROJECT_NAME, ENTITY_ID, FEATURE_NAME); ServingServiceBlockingStub servingStub = AuthTestUtils.getServingServiceStub(false, FEAST_SERVING_PORT, null); GetOnlineFeaturesRequest onlineFeatureRequest =