From d827c878de0311ac00959a167f923bb4df680ad9 Mon Sep 17 00:00:00 2001 From: "Andres March (vistaprint)" Date: Tue, 9 Jun 2020 16:54:08 +0000 Subject: [PATCH 01/18] Auth --- core/pom.xml | 88 +++++++- .../GoogleOpenIDAuthenticationProvider.java | 68 +++++++ .../authorization/AuthorizationProvider.java | 38 ++++ .../Keto/KetoAuthorizationProvider.java | 108 ++++++++++ .../config/CoreGRpcServerBuilderConfig.java | 30 --- .../feast/core/config/FeastProperties.java | 37 ++++ .../feast/core/config/SecurityConfig.java | 144 +++++++++++++ .../java/feast/core/grpc/CoreServiceImpl.java | 24 ++- ...gementService.java => ProjectService.java} | 44 +++- core/src/main/resources/application.yml | 27 ++- .../feast/core/grpc/CoreServiceAuthTest.java | 147 ++++++++++++++ .../core/service/ProjectServiceTest.java | 104 ++++++++++ infra/scripts/test-end-to-end-batch.sh | 27 ++- sdk/python/feast/client.py | 189 +++++++++--------- sdk/python/feast/config.py | 3 +- sdk/python/feast/constants.py | 45 ++++- sdk/python/feast/grpc/auth.py | 117 +++++++++++ sdk/python/feast/grpc/grpc.py | 53 +++++ sdk/python/tests/feast_core_server.py | 31 +++ sdk/python/tests/test_client.py | 125 ++++++++++-- 20 files changed, 1262 insertions(+), 187 deletions(-) create mode 100644 core/src/main/java/feast/core/auth/authentication/GoogleOID/GoogleOpenIDAuthenticationProvider.java create mode 100644 core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java create mode 100644 core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java delete mode 100644 core/src/main/java/feast/core/config/CoreGRpcServerBuilderConfig.java create mode 100644 core/src/main/java/feast/core/config/SecurityConfig.java rename core/src/main/java/feast/core/service/{AccessManagementService.java => ProjectService.java} (58%) create mode 100644 core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java create mode 100644 core/src/test/java/feast/core/service/ProjectServiceTest.java create mode 100644 sdk/python/feast/grpc/auth.py create mode 100644 sdk/python/feast/grpc/grpc.py diff --git a/core/pom.xml b/core/pom.xml index 2245be63aeb..573e86a643e 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -109,11 +109,56 @@ org.apache.logging.log4j log4j-web - - - io.github.lognet - grpc-spring-boot-starter - + + org.springframework.security + spring-security-core + 5.3.0.RELEASE + + + org.springframework.security + spring-security-config + 5.3.0.RELEASE + + + org.springframework.security.oauth + spring-security-oauth2 + 2.4.0.RELEASE + + + org.springframework.security + spring-security-oauth2-client + 5.3.0.RELEASE + + + org.springframework.security + spring-security-web + 5.3.0.RELEASE + + + org.springframework.security + spring-security-oauth2-resource-server + 5.3.0.RELEASE + + + org.springframework.security + spring-security-oauth2-jose + 5.3.0.RELEASE + + + net.devh + grpc-server-spring-boot-starter + 2.4.0.RELEASE + + + com.nimbusds + nimbus-jose-jwt + 8.2.1 + + + org.springframework.security + spring-security-oauth2-core + 5.3.0.RELEASE + org.springframework.boot @@ -207,7 +252,28 @@ io.prometheus simpleclient_servlet + + com.google.api.client + google-api-client-googleapis-auth-oauth + 1.2.3-alpha + + + com.auth0 + jwks-rsa + 0.11.0 + + + com.auth0 + java-jwt + 3.10.0 + + + + sh.ory.keto + keto-client + 0.4.4-alpha.1 + com.jayway.jsonpath @@ -258,5 +324,17 @@ flyway-core ${flyway.version} + + org.springframework + spring-test + 5.1.3.RELEASE + test + + + org.junit.jupiter + junit-jupiter + RELEASE + test + diff --git a/core/src/main/java/feast/core/auth/authentication/GoogleOID/GoogleOpenIDAuthenticationProvider.java b/core/src/main/java/feast/core/auth/authentication/GoogleOID/GoogleOpenIDAuthenticationProvider.java new file mode 100644 index 00000000000..103ed5e2635 --- /dev/null +++ b/core/src/main/java/feast/core/auth/authentication/GoogleOID/GoogleOpenIDAuthenticationProvider.java @@ -0,0 +1,68 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.auth.authentication.GoogleOID; + +import java.util.Map; +import org.springframework.security.authentication.AuthenticationProvider; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.oauth2.jwt.NimbusJwtDecoder; +import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter; +import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationProvider; + +/** + * Google Open ID Authentication Provider. This provider is used to validate incoming requests to + * Feast Core. + */ +public class GoogleOpenIDAuthenticationProvider implements AuthenticationProvider { + + private JwtAuthenticationProvider authProvider; + + /** + * @param options String K/V pair of options to initialize the AuthenticationProvider with. Only + * one option is currently configurable, the jwkEndpointURI. + */ + public GoogleOpenIDAuthenticationProvider(Map options) { + + // Endpoint used to retrieve certificates to validate JWT token + String jwkEndpointURI = "https://www.googleapis.com/oauth2/v3/certs"; + + // Provide a custom endpoint to retrieve certificates + if (options != null) { + jwkEndpointURI = options.get("jwkEndpointURI"); + } + authProvider = + new JwtAuthenticationProvider(NimbusJwtDecoder.withJwkSetUri(jwkEndpointURI).build()); + authProvider.setJwtAuthenticationConverter(new JwtAuthenticationConverter()); + } + + /** + * Authenticate a request based on its Spring Security Authentication object + * + * @param authentication Authentication object which contains a JWT to validate + * @return Returns the same authentication object after authentication + */ + @Override + public Authentication authenticate(Authentication authentication) throws AuthenticationException { + return authProvider.authenticate(authentication); + } + + @Override + public boolean supports(Class aClass) { + return authProvider.supports(aClass); + } +} diff --git a/core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java b/core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java new file mode 100644 index 00000000000..0209b90b9ff --- /dev/null +++ b/core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java @@ -0,0 +1,38 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.auth.authorization; + +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.core.Authentication; + +/** + * AuthorizationProvider is the base interface that each AuthorizationProvider needs to implement in + * order to authorize requests to Feast Core + */ +public interface AuthorizationProvider { + + /** + * Validates whether a user is within a project. Throws an AccessDeniedException if user is not + * within the project. + * + * @param project Name of the Feast project + * @param authentication Spring Security Authentication object + * @throws AccessDeniedException + */ + void checkIfProjectMember(String project, Authentication authentication) + throws AccessDeniedException; +} diff --git a/core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java b/core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java new file mode 100644 index 00000000000..7f744e5c79e --- /dev/null +++ b/core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java @@ -0,0 +1,108 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.auth.authorization.Keto; + +import feast.core.auth.authorization.AuthorizationProvider; +import java.util.List; +import java.util.Map; +import org.hibernate.validator.internal.constraintvalidators.bv.EmailValidator; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.core.Authentication; +import org.springframework.security.oauth2.jwt.Jwt; +import sh.ory.keto.ApiClient; +import sh.ory.keto.ApiException; +import sh.ory.keto.Configuration; +import sh.ory.keto.api.EnginesApi; +import sh.ory.keto.model.OryAccessControlPolicyRole; + +/** Authorization Provider implementation for Ory Keto */ +public class KetoAuthorizationProvider implements AuthorizationProvider { + + private final EnginesApi apiInstance; + + /** + * Initializes the KetoAuthorizationProvider + * + * @param options String K/V pair of options to initialize the provider with. Expects at least a + * "basePath" for the provider URL + */ + public KetoAuthorizationProvider(Map options) { + if (options == null) { + throw new IllegalArgumentException("Cannot pass empty or null options to KetoAuth"); + } + ApiClient defaultClient = Configuration.getDefaultApiClient(); + defaultClient.setBasePath(options.get("basePath")); + this.apiInstance = new EnginesApi(defaultClient); + } + + /** + * Validates whether a user is within a project. Throws an AccessDeniedException if user is not + * within the project. + * + * @param project Name of the Feast project + * @param authentication Spring Security Authentication object + * @throws AccessDeniedException + */ + public void checkIfProjectMember(String project, Authentication authentication) + throws AccessDeniedException { + String email = getEmailFromAuth(authentication); + try { + // Get all roles from Keto + List roles = + this.apiInstance.listOryAccessControlPolicyRoles("glob", 500L, 500L, email); + + // Loop through all roles the user has + for (OryAccessControlPolicyRole role : roles) { + // If the user has an admin or project specific role, return. + if (("roles:admin").equals(role.getId()) + || (String.format("roles:feast:%s-member", project)).equals(role.getId())) { + return; + } + } + } catch (ApiException e) { + System.err.println("Exception when calling EnginesApi#doOryAccessControlPoliciesAllow"); + System.err.println("Status code: " + e.getCode()); + System.err.println("Reason: " + e.getResponseBody()); + System.err.println("Response headers: " + e.getResponseHeaders()); + e.printStackTrace(); + } + // Could not determine project membership, deny access. + throw new AccessDeniedException( + String.format("Access denied to project %s for user %s", project, email)); + } + + /** + * Get user email from their authentication object. + * + * @param authentication Spring Security Authentication object, used to extract user details + * @return String user email + */ + private String getEmailFromAuth(Authentication authentication) { + Jwt principle = ((Jwt) authentication.getPrincipal()); + Map claims = principle.getClaims(); + String email = (String) claims.get("email"); + + if (email.isEmpty()) { + throw new IllegalStateException("JWT does not have a valid email set."); + } + boolean validEmail = (new EmailValidator()).isValid(email, null); + if (!validEmail) { + throw new IllegalStateException("JWT contains an invalid email address"); + } + return email; + } +} diff --git a/core/src/main/java/feast/core/config/CoreGRpcServerBuilderConfig.java b/core/src/main/java/feast/core/config/CoreGRpcServerBuilderConfig.java deleted file mode 100644 index e025c7b2978..00000000000 --- a/core/src/main/java/feast/core/config/CoreGRpcServerBuilderConfig.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 The Feast Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feast.core.config; - -import io.grpc.ServerBuilder; -import io.grpc.protobuf.services.ProtoReflectionService; -import org.lognet.springboot.grpc.GRpcServerBuilderConfigurer; -import org.springframework.stereotype.Component; - -@Component -public class CoreGRpcServerBuilderConfig extends GRpcServerBuilderConfigurer { - @Override - public void configure(ServerBuilder serverBuilder) { - serverBuilder.addService(ProtoReflectionService.newInstance()); - } -} diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index 07303107a43..5b9f74f605f 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -59,6 +59,7 @@ public FeastProperties() {} @NotNull /* Feast Kafka stream properties */ private StreamProperties stream; + private SecurityProperties security; /** Feast job properties. These properties are used for ingestion jobs. */ @Getter @@ -255,4 +256,40 @@ public void validate() { } } } + + @Getter + @Setter + public static class SecurityProperties { + + private AuthenticationProperties authentication; + private AuthorizationProperties authorization; + + @Getter + @Setter + public static class AuthenticationProperties { + + // Enable authentication + private boolean enabled; + + // Named authentication provider to use + private String provider; + + // K/V options to initialize the provider with + private Map options; + } + + @Getter + @Setter + public static class AuthorizationProperties { + + // Enable authorization. Authentication must be enabled if authorization is enabled. + private boolean enabled; + + // Named authorization provider to use. + private String provider; + + // K/V options to initialize the provider with + private Map options; + } + } } diff --git a/core/src/main/java/feast/core/config/SecurityConfig.java b/core/src/main/java/feast/core/config/SecurityConfig.java new file mode 100644 index 00000000000..855d1b1354c --- /dev/null +++ b/core/src/main/java/feast/core/config/SecurityConfig.java @@ -0,0 +1,144 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.config; + +import feast.core.CoreServiceGrpc; +import feast.core.auth.authentication.GoogleOID.GoogleOpenIDAuthenticationProvider; +import feast.core.auth.authorization.AuthorizationProvider; +import feast.core.auth.authorization.Keto.KetoAuthorizationProvider; +import feast.core.config.FeastProperties.SecurityProperties; +import java.util.ArrayList; +import java.util.List; +import net.devh.boot.grpc.server.security.authentication.BearerAuthenticationReader; +import net.devh.boot.grpc.server.security.authentication.GrpcAuthenticationReader; +import net.devh.boot.grpc.server.security.check.AccessPredicate; +import net.devh.boot.grpc.server.security.check.AccessPredicateVoter; +import net.devh.boot.grpc.server.security.check.GrpcSecurityMetadataSource; +import net.devh.boot.grpc.server.security.check.ManualGrpcSecurityMetadataSource; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.AccessDecisionManager; +import org.springframework.security.access.AccessDecisionVoter; +import org.springframework.security.access.vote.UnanimousBased; +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.AuthenticationProvider; +import org.springframework.security.authentication.ProviderManager; +import org.springframework.security.oauth2.server.resource.BearerTokenAuthenticationToken; + +@Configuration +public class SecurityConfig { + + private final SecurityProperties securityProperties; + + public SecurityConfig(FeastProperties feastProperties) { + this.securityProperties = feastProperties.getSecurity(); + } + + /** + * Initializes an AuthenticationManager if authentication has been enabled. + * + * @return AuthenticationManager + */ + @Bean + @ConditionalOnProperty(prefix = "feast.security.authentication", name = "enabled") + AuthenticationManager authenticationManager() { + final List providers = new ArrayList<>(); + + if (securityProperties.getAuthentication().isEnabled()) { + switch (securityProperties.getAuthentication().getProvider()) { + case "GoogleOpenID": + providers.add( + new GoogleOpenIDAuthenticationProvider( + securityProperties.getAuthentication().getOptions())); + break; + default: + throw new IllegalArgumentException( + "Please configure an Authentication Provider if you have enabled authentication."); + } + } + return new ProviderManager(providers); + } + + /** + * Creates an AuthenticationReader that the AuthenticationManager will use to authenticate + * requests + * + * @return GrpcAuthenticationReader + */ + @Bean + @ConditionalOnProperty(prefix = "feast.security.authentication", name = "enabled") + GrpcAuthenticationReader authenticationReader() { + return new BearerAuthenticationReader(BearerTokenAuthenticationToken::new); + } + + /** + * Creates a SecurityMetadataSource when authentication is enabled. This allows for the + * configuration of endpoint level security rules. + * + * @return GrpcSecurityMetadataSource + */ + @Bean + @ConditionalOnProperty(prefix = "feast.security.authentication", name = "enabled") + GrpcSecurityMetadataSource grpcSecurityMetadataSource() { + final ManualGrpcSecurityMetadataSource source = new ManualGrpcSecurityMetadataSource(); + + // Authentication is enabled for all gRPC endpoints + source.setDefault(AccessPredicate.authenticated()); + + // The following endpoints allow unauthenticated access + source.set(CoreServiceGrpc.getGetFeastCoreVersionMethod(), AccessPredicate.permitAll()); + + return source; + } + + /** + * Creates an AccessDecisionManager if authorization is enabled. This object determines the policy + * used to make authorization decisions. + * + * @return AccessDecisionManager + */ + @Bean + @ConditionalOnProperty(prefix = "feast.security.authorization", name = "enabled") + AccessDecisionManager accessDecisionManager() { + final List> voters = new ArrayList<>(); + voters.add(new AccessPredicateVoter()); + return new UnanimousBased(voters); + } + + /** + * Creates an AuthorizationProvider based on Feast configuration. This provider is available + * through the security service. + * + * @return AuthorizationProvider used to validate access to Feast resources. + */ + @Bean + @ConditionalOnProperty(prefix = "feast.security.authorization", name = "enabled") + AuthorizationProvider authorizationProvider() { + if (securityProperties.getAuthentication().isEnabled() + && securityProperties.getAuthorization().isEnabled()) { + switch (securityProperties.getAuthorization().getProvider()) { + case "KetoAuthorization": + return new KetoAuthorizationProvider(securityProperties.getAuthorization().getOptions()); + default: + throw new IllegalArgumentException( + "Please configure an Authorization Provider if you have enabled authorization."); + } + } + return null; + } +} diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java index 8f17e78b001..0941f4f4048 100644 --- a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java @@ -22,8 +22,8 @@ import feast.core.exception.RetrievalException; import feast.core.grpc.interceptors.MonitoringInterceptor; import feast.core.model.Project; -import feast.core.service.AccessManagementService; import feast.core.service.JobService; +import feast.core.service.ProjectService; import feast.core.service.SpecService; import feast.core.service.StatsService; import feast.proto.core.CoreServiceGrpc.CoreServiceImplBase; @@ -35,29 +35,28 @@ import java.util.NoSuchElementException; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; -import org.lognet.springboot.grpc.GRpcService; +import net.devh.boot.grpc.server.service.GrpcService; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.context.SecurityContextHolder; /** Implementation of the feast core GRPC service. */ @Slf4j -@GRpcService(interceptors = {MonitoringInterceptor.class}) +@GrpcService(interceptors = {MonitoringInterceptor.class}) public class CoreServiceImpl extends CoreServiceImplBase { private final FeastProperties feastProperties; private SpecService specService; - private AccessManagementService accessManagementService; private JobService jobService; private StatsService statsService; + private ProjectService projectService; @Autowired public CoreServiceImpl( SpecService specService, - AccessManagementService accessManagementService, StatsService statsService, JobService jobService, FeastProperties feastProperties) { this.specService = specService; - this.accessManagementService = accessManagementService; this.jobService = jobService; this.feastProperties = feastProperties; this.statsService = statsService; @@ -176,6 +175,10 @@ public void listStores( @Override public void applyFeatureSet( ApplyFeatureSetRequest request, StreamObserver responseObserver) { + + projectService.checkIfProjectMember( + SecurityContextHolder.getContext(), request.getFeatureSet().getSpec().getProject()); + try { ApplyFeatureSetResponse response = specService.applyFeatureSet(request.getFeatureSet()); responseObserver.onNext(response); @@ -212,7 +215,7 @@ public void updateStore( public void createProject( CreateProjectRequest request, StreamObserver responseObserver) { try { - accessManagementService.createProject(request.getName()); + projectService.createProject(request.getName()); responseObserver.onNext(CreateProjectResponse.getDefaultInstance()); responseObserver.onCompleted(); } catch (Exception e) { @@ -225,8 +228,11 @@ public void createProject( @Override public void archiveProject( ArchiveProjectRequest request, StreamObserver responseObserver) { + + projectService.checkIfProjectMember(SecurityContextHolder.getContext(), request.getName()); + try { - accessManagementService.archiveProject(request.getName()); + projectService.archiveProject(request.getName()); responseObserver.onNext(ArchiveProjectResponse.getDefaultInstance()); responseObserver.onCompleted(); } catch (IllegalArgumentException e) { @@ -251,7 +257,7 @@ public void archiveProject( public void listProjects( ListProjectsRequest request, StreamObserver responseObserver) { try { - List projects = accessManagementService.listProjects(); + List projects = projectService.listProjects(); responseObserver.onNext( ListProjectsResponse.newBuilder() .addAllProjects(projects.stream().map(Project::getName).collect(Collectors.toList())) diff --git a/core/src/main/java/feast/core/service/AccessManagementService.java b/core/src/main/java/feast/core/service/ProjectService.java similarity index 58% rename from core/src/main/java/feast/core/service/AccessManagementService.java rename to core/src/main/java/feast/core/service/ProjectService.java index 5b02d6f3c4a..d70391f060d 100644 --- a/core/src/main/java/feast/core/service/AccessManagementService.java +++ b/core/src/main/java/feast/core/service/ProjectService.java @@ -16,27 +16,49 @@ */ package feast.core.service; +import feast.core.auth.authorization.AuthorizationProvider; +import feast.core.config.FeastProperties; +import feast.core.config.FeastProperties.SecurityProperties; import feast.core.dao.ProjectRepository; import feast.core.model.Project; import java.util.List; import java.util.Optional; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; @Slf4j @Service -public class AccessManagementService { +public class ProjectService { + + private SecurityProperties securityProperties; + private AuthorizationProvider authorizationProvider; private ProjectRepository projectRepository; + public ProjectService( + FeastProperties feastProperties, + ProjectRepository projectRepository, + AuthorizationProvider authorizationProvider) { + this.projectRepository = projectRepository; + this.authorizationProvider = authorizationProvider; + this.securityProperties = feastProperties.getSecurity(); + } + @Autowired - public AccessManagementService(ProjectRepository projectRepository) { + public ProjectService( + FeastProperties feastProperties, + ProjectRepository projectRepository, + ObjectProvider authorizationProvider) { this.projectRepository = projectRepository; // create default project if it does not yet exist. if (!projectRepository.existsById(Project.DEFAULT_NAME)) { this.createProject(Project.DEFAULT_NAME); } + this.authorizationProvider = authorizationProvider.getIfUnique(); + this.securityProperties = feastProperties.getSecurity(); } /** @@ -44,7 +66,6 @@ public AccessManagementService(ProjectRepository projectRepository) { * * @param name Name of project to be created */ - @Transactional public void createProject(String name) { if (projectRepository.existsById(name)) { throw new IllegalArgumentException(String.format("Project already exists: %s", name)); @@ -58,7 +79,6 @@ public void createProject(String name) { * * @param name Name of the project to be archived */ - @Transactional public void archiveProject(String name) { Optional project = projectRepository.findById(name); if (!project.isPresent()) { @@ -80,4 +100,18 @@ public void archiveProject(String name) { public List listProjects() { return projectRepository.findAllByArchivedIsFalse(); } + + /** + * Determine whether a user belongs to a Project + * + * @param securityContext User's Spring Security Context. Used to identify user. + * @param project Name of the project for which membership should be tested. + */ + public void checkIfProjectMember(SecurityContext securityContext, String project) { + Authentication authentication = securityContext.getAuthentication(); + if (!this.securityProperties.getAuthorization().isEnabled()) { + return; + } + this.authorizationProvider.checkIfProjectMember(project, authentication); + } } diff --git a/core/src/main/resources/application.yml b/core/src/main/resources/application.yml index 35390e7163e..8dec0d6380b 100644 --- a/core/src/main/resources/application.yml +++ b/core/src/main/resources/application.yml @@ -15,13 +15,6 @@ # # -grpc: - # The port number Feast Serving GRPC service should listen on - port: 6565 - # This allows client to discover GRPC endpoints easily - # https://github.com/grpc/grpc-java/blob/master/documentation/server-reflection-tutorial.md - enable-reflection: true - feast: jobs: # Job update polling interval in milliseconds: how often Feast checks if new jobs should be sent to the runner. @@ -77,12 +70,30 @@ feast: bootstrapServers: localhost:9092 replicationFactor: 1 partitions: 1 - specsOptions: specsTopic: feast-specs specsAckTopic: feast-specs-ack notifyIntervalMilliseconds: 1000 + + security: + authentication: + enabled: false + provider: GoogleOpenID + authorization: + enabled: false + provider: KetoAuthorization + options: + basePath: http://localhost:3000 +grpc: + server: + # The port that Feast Core gRPC service listens on + port: 6565 + security: + enabled: false + certificateChainPath: server.crt + privateKeyPath: server.key + spring: jpa: properties.hibernate: diff --git a/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java new file mode 100644 index 00000000000..6ca91c85733 --- /dev/null +++ b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java @@ -0,0 +1,147 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.grpc; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.CoreServiceProto.ApplyFeatureSetRequest; +import feast.core.CoreServiceProto.ApplyFeatureSetResponse; +import feast.core.FeatureSetProto; +import feast.core.FeatureSetProto.FeatureSetStatus; +import feast.core.SourceProto.KafkaSourceConfig; +import feast.core.SourceProto.SourceType; +import feast.core.auth.authorization.AuthorizationProvider; +import feast.core.config.FeastProperties; +import feast.core.config.FeastProperties.SecurityProperties; +import feast.core.dao.ProjectRepository; +import feast.core.model.FeatureSet; +import feast.core.model.Field; +import feast.core.model.Source; +import feast.core.service.ProjectService; +import feast.core.service.SpecService; +import feast.types.ValueProto.ValueType.Enum; +import io.grpc.internal.testing.StreamRecorder; +import java.sql.Date; +import java.time.Instant; +import java.util.Arrays; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; + +@SpringBootTest +class CoreServiceAuthTest { + + private CoreServiceImpl coreService; + private SpecService specService; + private ProjectService projectService; + private ProjectRepository projectRepository; + private AuthorizationProvider authProvider; + + CoreServiceAuthTest() { + specService = mock(SpecService.class); + projectRepository = mock(ProjectRepository.class); + authProvider = mock(AuthorizationProvider.class); + FeastProperties.SecurityProperties.AuthorizationProperties authProp = + new FeastProperties.SecurityProperties.AuthorizationProperties(); + authProp.setEnabled(true); + FeastProperties.SecurityProperties sp = new SecurityProperties(); + sp.setAuthorization(authProp); + FeastProperties feastProperties = new FeastProperties(); + feastProperties.setSecurity(sp); + projectService = new ProjectService(feastProperties, projectRepository, authProvider); + coreService = new CoreServiceImpl(specService, projectService); + } + + @Test + void cantApplyFeatureSetIfNotProjectMember() throws InvalidProtocolBufferException { + + String project = "project1"; + Authentication auth = mock(Authentication.class); + SecurityContext context = mock(SecurityContext.class); + SecurityContextHolder.setContext(context); + when(context.getAuthentication()).thenReturn(auth); + + doThrow(AccessDeniedException.class).when(authProvider).checkIfProjectMember(project, auth); + + StreamRecorder responseObserver = StreamRecorder.create(); + FeatureSetProto.FeatureSet incomingFeatureSet = newDummyFeatureSet("f2", 1, project).toProto(); + FeatureSetProto.FeatureSetSpec incomingFeatureSetSpec = + incomingFeatureSet.getSpec().toBuilder().clearVersion().build(); + FeatureSetProto.FeatureSet spec = + FeatureSetProto.FeatureSet.newBuilder().setSpec(incomingFeatureSetSpec).build(); + ApplyFeatureSetRequest request = + ApplyFeatureSetRequest.newBuilder().setFeatureSet(spec).build(); + + assertThrows( + AccessDeniedException.class, () -> coreService.applyFeatureSet(request, responseObserver)); + } + + @Test + void canApplyFeatureSetIfProjectMember() throws InvalidProtocolBufferException { + + String project = "project1"; + Authentication auth = mock(Authentication.class); + SecurityContext context = mock(SecurityContext.class); + SecurityContextHolder.setContext(context); + when(context.getAuthentication()).thenReturn(auth); + + StreamRecorder responseObserver = StreamRecorder.create(); + FeatureSetProto.FeatureSet incomingFeatureSet = newDummyFeatureSet("f2", 1, project).toProto(); + FeatureSetProto.FeatureSetSpec incomingFeatureSetSpec = + incomingFeatureSet.getSpec().toBuilder().clearVersion().build(); + FeatureSetProto.FeatureSet spec = + FeatureSetProto.FeatureSet.newBuilder().setSpec(incomingFeatureSetSpec).build(); + ApplyFeatureSetRequest request = + ApplyFeatureSetRequest.newBuilder().setFeatureSet(spec).build(); + + coreService.applyFeatureSet(request, responseObserver); + } + + private FeatureSet newDummyFeatureSet(String name, int version, String project) { + Field feature = new Field("feature", Enum.INT64); + Field entity = new Field("entity", Enum.STRING); + + Source defaultSource = + new Source( + SourceType.KAFKA, + KafkaSourceConfig.newBuilder() + .setBootstrapServers("kafka:9092") + .setTopic("my-topic") + .build(), + true); + + FeatureSet fs = + new FeatureSet( + name, + project, + version, + 100L, + Arrays.asList(entity), + Arrays.asList(feature), + defaultSource, + FeatureSetStatus.STATUS_READY); + fs.setCreated(Date.from(Instant.ofEpochSecond(10L))); + return fs; + } +} diff --git a/core/src/test/java/feast/core/service/ProjectServiceTest.java b/core/src/test/java/feast/core/service/ProjectServiceTest.java new file mode 100644 index 00000000000..a36bd322955 --- /dev/null +++ b/core/src/test/java/feast/core/service/ProjectServiceTest.java @@ -0,0 +1,104 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.service; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +import feast.core.auth.authorization.AuthorizationProvider; +import feast.core.config.FeastProperties; +import feast.core.config.FeastProperties.SecurityProperties; +import feast.core.dao.ProjectRepository; +import feast.core.model.Project; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.Mock; + +public class ProjectServiceTest { + + @Mock private ProjectRepository projectRepository; + + @Rule public final ExpectedException expectedException = ExpectedException.none(); + + private ProjectService projectService; + + @Before + public void setUp() { + initMocks(this); + projectRepository = mock(ProjectRepository.class); + FeastProperties.SecurityProperties.AuthorizationProperties authProp = + new FeastProperties.SecurityProperties.AuthorizationProperties(); + authProp.setEnabled(false); + FeastProperties.SecurityProperties sp = new SecurityProperties(); + sp.setAuthorization(authProp); + FeastProperties feastProperties = new FeastProperties(); + feastProperties.setSecurity(sp); + projectService = + new ProjectService(feastProperties, projectRepository, mock(AuthorizationProvider.class)); + } + + @Test + public void shouldCreateProjectIfItDoesntExist() { + String projectName = "project1"; + Project project = new Project(projectName); + when(projectRepository.saveAndFlush(any(Project.class))).thenReturn(project); + projectService.createProject(projectName); + verify(projectRepository, times(1)).saveAndFlush(any()); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldNotCreateProjectIfItExist() { + String projectName = "project1"; + when(projectRepository.existsById(projectName)).thenReturn(true); + projectService.createProject(projectName); + } + + @Test + public void shouldArchiveProjectIfItExists() { + String projectName = "project1"; + when(projectRepository.findById(projectName)).thenReturn(Optional.of(new Project(projectName))); + projectService.archiveProject(projectName); + verify(projectRepository, times(1)).saveAndFlush(any(Project.class)); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldNotArchiveProjectIfItIsAlreadyArchived() { + String projectName = "project1"; + when(projectRepository.findById(projectName)).thenReturn(Optional.empty()); + projectService.archiveProject(projectName); + } + + @Test + public void shouldListProjects() { + String projectName = "project1"; + Project project = new Project(projectName); + List expected = Arrays.asList(project); + when(projectRepository.findAllByArchivedIsFalse()).thenReturn(expected); + List actual = projectService.listProjects(); + Assert.assertEquals(expected, actual); + } +} diff --git a/infra/scripts/test-end-to-end-batch.sh b/infra/scripts/test-end-to-end-batch.sh index 6763e417095..6e6bf01964b 100755 --- a/infra/scripts/test-end-to-end-batch.sh +++ b/infra/scripts/test-end-to-end-batch.sh @@ -56,7 +56,27 @@ else echo "[DEBUG] Skipping building jars" fi -cat < /tmp/core.warehouse.application.yml +echo " +============================================================ +Starting Feast Core +============================================================ +" +# Start Feast Core in background +cat < /tmp/core.application.yml +grpc: + server: + port: 6565 + security: + enabled: false + +security: + authentication: + enabled: false + provider: None + authorization: + enabled: false + provider: None + feast: jobs: polling_interval_milliseconds: 10000 @@ -66,6 +86,11 @@ feast: type: DirectRunner options: tempLocation: gs://${TEMP_BUCKET}/tempLocation +grpc: + server: + port: 6565 + security: + enabled: false EOF diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 9225759a6ec..f31ade35037 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -29,14 +29,17 @@ import pyarrow as pa import pyarrow.parquet as pq from google.protobuf.timestamp_pb2 import Timestamp - +import feast.grpc.auth as feast_auth from feast.config import Config from feast.constants import ( - CONFIG_CORE_SECURE_KEY, + CONFIG_CORE_ENABLE_AUTH_KEY, + CONFIG_CORE_ENABLE_SSL_KEY, + CONFIG_CORE_SERVER_SSL_CERT_KEY, CONFIG_CORE_URL_KEY, CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY, CONFIG_PROJECT_KEY, - CONFIG_SERVING_SECURE_KEY, + CONFIG_SERVING_ENABLE_SSL_KEY, + CONFIG_SERVING_SERVER_SSL_CERT_KEY, CONFIG_SERVING_URL_KEY, FEAST_DEFAULT_OPTIONS, ) @@ -66,6 +69,7 @@ from feast.feature import Feature, FeatureRef from feast.feature_set import Entity, FeatureSet, FeatureSetRef from feast.job import IngestJob, RetrievalJob +from feast.grpc.grpc import create_grpc_channel from feast.loaders.abstract_producer import get_producer from feast.loaders.file import export_source_to_staging_location from feast.loaders.ingest import KAFKA_CHUNK_PRODUCTION_TIMEOUT, get_feature_row_chunks @@ -96,13 +100,15 @@ class Client: def __init__(self, options: Optional[Dict[str, str]] = None, **kwargs): """ The Feast Client should be initialized with at least one service url - - Args: + Please see constants.py for configuration options. Commonly used options + or arguments include: core_url: Feast Core URL. Used to manage features serving_url: Feast Serving URL. Used to retrieve features project: Sets the active project. This field is optional. core_secure: Use client-side SSL/TLS for Core gRPC API serving_secure: Use client-side SSL/TLS for Serving gRPC API + + Args: options: Configuration options to initialize client with **kwargs: Additional keyword arguments that will be used as configuration options along with "options" @@ -112,10 +118,53 @@ def __init__(self, options: Optional[Dict[str, str]] = None, **kwargs): options = dict() self._config = Config(options={**options, **kwargs}) - self.__core_channel: grpc.Channel = None - self.__serving_channel: grpc.Channel = None self._core_service_stub: CoreServiceStub = None self._serving_service_stub: ServingServiceStub = None + self._auth_metadata = None + + # Configure Auth Metadata Plugin if auth is enabled + if self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY): + self._auth_metadata = feast_auth.get_auth_metadata_plugin(self._config) + + @property + def _core_service(self): + """ + Creates or returns the gRPC Feast Core Service Stub + + Returns: CoreServiceStub + """ + if not self._core_service_stub: + channel = create_grpc_channel( + url=self._config.get(CONFIG_CORE_URL_KEY), + enable_ssl=self._config.getboolean(CONFIG_CORE_ENABLE_SSL_KEY), + enable_auth=self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY), + ssl_server_cert_path=self._config.get(CONFIG_CORE_SERVER_SSL_CERT_KEY), + auth_metadata_plugin=self._auth_metadata, + timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + ) + self._core_service_stub = CoreServiceStub(channel) + return self._core_service_stub + + @property + def _serving_service(self): + """ + Creates or returns the gRPC Feast Serving Service Stub + + Returns: ServingServiceStub + """ + if not self._serving_service_stub: + channel = create_grpc_channel( + url=self._config.get(CONFIG_SERVING_URL_KEY), + enable_ssl=self._config.getboolean(CONFIG_SERVING_ENABLE_SSL_KEY), + enable_auth=False, + ssl_server_cert_path=self._config.get( + CONFIG_SERVING_SERVER_SSL_CERT_KEY + ), + auth_metadata_plugin=None, + timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + ) + self._serving_service_stub = ServingServiceStub(channel) + return self._serving_service_stub @property def core_url(self) -> str: @@ -165,7 +214,7 @@ def core_secure(self) -> bool: Returns: Whether client-side SSL/TLS is enabled """ - return self._config.getboolean(CONFIG_CORE_SECURE_KEY) + return self._config.getboolean(CONFIG_CORE_ENABLE_SSL_KEY) @core_secure.setter def core_secure(self, value: bool): @@ -175,7 +224,7 @@ def core_secure(self, value: bool): Args: value: True to enable client-side SSL/TLS """ - self._config.set(CONFIG_CORE_SECURE_KEY, value) + self._config.set(CONFIG_CORE_ENABLE_SSL_KEY, value) @property def serving_secure(self) -> bool: @@ -185,7 +234,7 @@ def serving_secure(self) -> bool: Returns: Whether client-side SSL/TLS is enabled """ - return self._config.getboolean(CONFIG_SERVING_SECURE_KEY) + return self._config.getboolean(CONFIG_SERVING_ENABLE_SSL_KEY) @serving_secure.setter def serving_secure(self, value: bool): @@ -195,7 +244,7 @@ def serving_secure(self, value: bool): Args: value: True to enable client-side SSL/TLS """ - self._config.set(CONFIG_SERVING_SECURE_KEY, value) + self._config.set(CONFIG_SERVING_ENABLE_SSL_KEY, value) def version(self): """ @@ -210,90 +259,22 @@ def version(self): } if self.serving_url: - self._connect_serving() - serving_version = self._serving_service_stub.GetFeastServingInfo( + serving_version = self._serving_service.GetFeastServingInfo( GetFeastServingInfoRequest(), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), ).version result["serving"] = {"url": self.serving_url, "version": serving_version} if self.core_url: - self._connect_core() - core_version = self._core_service_stub.GetFeastCoreVersion( + core_version = self._core_service.GetFeastCoreVersion( GetFeastCoreVersionRequest(), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + metadata=self._get_grpc_metadata(), ).version result["core"] = {"url": self.core_url, "version": core_version} return result - def _connect_core(self, skip_if_connected: bool = True): - """ - Connect to Core API - - Args: - skip_if_connected: Do not attempt to connect if already connected - """ - if skip_if_connected and self._core_service_stub: - return - - if not self.core_url: - raise ValueError("Please set Feast Core URL.") - - if self.__core_channel is None: - if self.core_secure or self.core_url.endswith(":443"): - self.__core_channel = grpc.secure_channel( - self.core_url, grpc.ssl_channel_credentials() - ) - else: - self.__core_channel = grpc.insecure_channel(self.core_url) - - try: - grpc.channel_ready_future(self.__core_channel).result( - timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY) - ) - except grpc.FutureTimeoutError: - raise ConnectionError( - f"Connection timed out while attempting to connect to Feast " - f"Core gRPC server {self.core_url} " - ) - else: - self._core_service_stub = CoreServiceStub(self.__core_channel) - - def _connect_serving(self, skip_if_connected=True): - """ - Connect to Serving API - - Args: - skip_if_connected: Do not attempt to connect if already connected - """ - - if skip_if_connected and self._serving_service_stub: - return - - if not self.serving_url: - raise ValueError("Please set Feast Serving URL.") - - if self.__serving_channel is None: - if self.serving_secure or self.serving_url.endswith(":443"): - self.__serving_channel = grpc.secure_channel( - self.serving_url, grpc.ssl_channel_credentials() - ) - else: - self.__serving_channel = grpc.insecure_channel(self.serving_url) - - try: - grpc.channel_ready_future(self.__serving_channel).result( - timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY) - ) - except grpc.FutureTimeoutError: - raise ConnectionError( - f"Connection timed out while attempting to connect to Feast " - f"Serving gRPC server {self.serving_url} " - ) - else: - self._serving_service_stub = ServingServiceStub(self.__serving_channel) - @property def project(self) -> Union[str, None]: """ @@ -323,10 +304,11 @@ def list_projects(self) -> List[str]: List of project names """ - self._connect_core() - response = self._core_service_stub.ListProjects( + + response = self._core_service.ListProjects( ListProjectsRequest(), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + metadata=self._get_grpc_metadata(), ) # type: ListProjectsResponse return list(response.projects) @@ -338,10 +320,10 @@ def create_project(self, project: str): project: Name of project """ - self._connect_core() - self._core_service_stub.CreateProject( + self._core_service.CreateProject( CreateProjectRequest(name=project), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + metadata=self._get_grpc_metadata(), ) # type: CreateProjectResponse def archive_project(self, project): @@ -359,6 +341,7 @@ def archive_project(self, project): self._core_service_stub.ArchiveProject( ArchiveProjectRequest(name=project), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + metadata=self._get_grpc_metadata(), ) # type: ArchiveProjectResponse except grpc.RpcError as e: raise grpc.RpcError(e.details()) @@ -392,7 +375,6 @@ def _apply_feature_set(self, feature_set: FeatureSet): Args: feature_set: Feature set that will be registered """ - self._connect_core() feature_set.is_valid() feature_set_proto = feature_set.to_proto() @@ -402,9 +384,10 @@ def _apply_feature_set(self, feature_set: FeatureSet): # Convert the feature set to a request and send to Feast Core try: - apply_fs_response = self._core_service_stub.ApplyFeatureSet( + apply_fs_response = self._core_service.ApplyFeatureSet( ApplyFeatureSetRequest(feature_set=feature_set_proto), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + metadata=self._get_grpc_metadata(), ) # type: ApplyFeatureSetResponse except grpc.RpcError as e: raise grpc.RpcError(e.details()) @@ -439,7 +422,6 @@ def list_feature_sets( Returns: List of feature sets """ - self._connect_core() if project is None: if self.project is not None: @@ -455,10 +437,10 @@ def list_feature_sets( ) # Get latest feature sets from Feast Core - feature_set_protos = self._core_service_stub.ListFeatureSets( - ListFeatureSetsRequest(filter=filter) + feature_set_protos = self._core_service.ListFeatureSets( + ListFeatureSetsRequest(filter=filter), metadata=self._get_grpc_metadata(), ) # type: ListFeatureSetsResponse - + # Extract feature sets and return feature_sets = [] for feature_set_proto in feature_set_protos.feature_sets: @@ -481,7 +463,6 @@ def get_feature_set( Returns either the specified feature set, or raises an exception if none is found """ - self._connect_core() if project is None: if self.project is not None: @@ -491,7 +472,8 @@ def get_feature_set( try: get_feature_set_response = self._core_service_stub.GetFeatureSet( - GetFeatureSetRequest(project=project, name=name.strip()) + GetFeatureSetRequest(project=project, name=name.strip()), + metadata=self._get_grpc_metadata(), ) # type: GetFeatureSetResponse except grpc.RpcError as e: raise grpc.RpcError(e.details()) @@ -608,10 +590,13 @@ def get_batch_features( """ self._connect_serving() + feature_references = _build_feature_references( + feature_refs=feature_refs, default_project=default_project + ) # Retrieve serving information to determine store type and # staging location - serving_info = self._serving_service_stub.GetFeastServingInfo( + serving_info = self._serving_service.GetFeastServingInfo( GetFeastServingInfoRequest(), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), ) # type: GetFeastServingInfoResponse @@ -619,7 +604,7 @@ def get_batch_features( if serving_info.type != FeastServingType.FEAST_SERVING_TYPE_BATCH: raise Exception( f'You are connected to a store "{self.serving_url}" which ' - f"does not support batch retrieval" + f"does not support batch retrieval " ) if isinstance(entity_rows, pd.DataFrame): @@ -698,7 +683,6 @@ def get_online_features( Each EntityRow provided will yield one record, which contains data fields with data value and field status metadata (if included). """ - self._connect_serving() try: response = self._serving_service_stub.GetOnlineFeatures( @@ -1004,6 +988,17 @@ def get_statistics( request ).dataset_feature_statistics_list + def _get_grpc_metadata(self): + """ + Returns a metadata tuple to attach to gRPC requests. This is primarily + used when authentication is enabled but SSL/TLS is disabled. + + Returns: Tuple of metadata to attach to each gRPC call + """ + if self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY) and self._auth_metadata: + return self._auth_metadata.get_signed_meta() + return () + def _build_feature_references( feature_ref_strs: List[str], project: Optional[str] = None diff --git a/sdk/python/feast/config.py b/sdk/python/feast/config.py index fd35b6e5d87..71800e24a78 100644 --- a/sdk/python/feast/config.py +++ b/sdk/python/feast/config.py @@ -67,7 +67,8 @@ def _init_config(path: str): def _get_feast_env_vars(): """ - Get environmental variables that start with FEAST_ + Get environmental variables that start with "FEAST_" + Returns: Dict of Feast environmental variables (stripped of prefix) """ feast_env_vars = {} diff --git a/sdk/python/feast/constants.py b/sdk/python/feast/constants.py index 4b22a8e975d..48e9c8feeca 100644 --- a/sdk/python/feast/constants.py +++ b/sdk/python/feast/constants.py @@ -14,23 +14,35 @@ # limitations under the License. # -# General constants DATETIME_COLUMN = "datetime" + +# Environmental variable to specify Feast configuration file location FEAST_CONFIG_FILE_ENV_KEY = "FEAST_CONFIG" + +# Default prefix to Feast environmental variables CONFIG_FEAST_ENV_VAR_PREFIX = "FEAST_" + +# Default directory to Feast configuration file CONFIG_FILE_DEFAULT_DIRECTORY = ".feast" + +# Default Feast configuration file name CONFIG_FILE_NAME = "config" -CONFIG_FILE_SECTION = "general" +# Default section in Feast configuration file to specify options +CONFIG_FILE_SECTION = "general" -# Feast configuration options +# Feast Configuration Options +CONFIG_PROJECT_KEY = "project" CONFIG_CORE_URL_KEY = "core_url" +CONFIG_CORE_ENABLE_SSL_KEY = "core_enable_ssl" +CONFIG_CORE_ENABLE_AUTH_KEY = "core_enable_auth" +CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY = "core_auth_token" +CONFIG_CORE_SERVER_SSL_CERT_KEY = "core_server_ssl_cert" CONFIG_SERVING_URL_KEY = "serving_url" -CONFIG_PROJECT_KEY = "project" -CONFIG_CORE_SECURE_KEY = "core_secure" -CONFIG_SERVING_SECURE_KEY = "serving_secure" +CONFIG_SERVING_ENABLE_SSL_KEY = "serving_enable_ssl" +CONFIG_SERVING_SERVER_SSL_CERT_KEY = "serving_server_ssl_cert" CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY = "grpc_connection_timeout_default" -CONFIG_GRPC_CONNECTION_TIMEOUT_APPLY_KEY = "grpc_connection_timeout_apply_key" +CONFIG_GRPC_CONNECTION_TIMEOUT_APPLY_KEY = "grpc_connection_timeout_apply" CONFIG_BATCH_FEATURE_REQUEST_WAIT_TIME_SECONDS_KEY = ( "batch_feature_request_wait_time_seconds" ) @@ -40,13 +52,28 @@ # Configuration option default values FEAST_DEFAULT_OPTIONS = { + # Default Feast project to use CONFIG_PROJECT_KEY: "default", + # Default Feast Core URL CONFIG_CORE_URL_KEY: "localhost:6565", - CONFIG_CORE_SECURE_KEY: "False", + # Enable or disable TLS/SSL to Feast Core + CONFIG_CORE_ENABLE_SSL_KEY: "False", + # Enable user authentication to Feast Core + CONFIG_CORE_ENABLE_AUTH_KEY: "False", + # Path to certificate(s) to secure connection to Feast Core + CONFIG_CORE_SERVER_SSL_CERT_KEY: "", + # Default Feast Serving URL CONFIG_SERVING_URL_KEY: "localhost:6565", - CONFIG_SERVING_SECURE_KEY: "False", + # Enable or disable TLS/SSL to Feast Serving + CONFIG_SERVING_ENABLE_SSL_KEY: "False", + # Path to certificate(s) to secure connection to Feast Serving + CONFIG_SERVING_SERVER_SSL_CERT_KEY: "", + # Default connection timeout to Feast Serving and Feast Core (in seconds) CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY: "3", + # Default gRPC connection timeout when sending an ApplyFeatureSet command to + # Feast Core (in seconds) CONFIG_GRPC_CONNECTION_TIMEOUT_APPLY_KEY: "600", + # Time to wait for batch feature requests before timing out. CONFIG_BATCH_FEATURE_REQUEST_WAIT_TIME_SECONDS_KEY: "600", CONFIG_TIMEOUT_KEY: "21600", CONFIG_MAX_WAIT_INTERVAL_KEY: "60", diff --git a/sdk/python/feast/grpc/auth.py b/sdk/python/feast/grpc/auth.py new file mode 100644 index 00000000000..cd4bb2de3f7 --- /dev/null +++ b/sdk/python/feast/grpc/auth.py @@ -0,0 +1,117 @@ +import grpc +from google.auth.exceptions import DefaultCredentialsError + +from feast.config import Config +from feast.constants import CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY + + +def get_auth_metadata_plugin(config: Config): + """ + Get an Authentication Metadata Plugin. This plugin is used in gRPC to + sign requests. Please see the following URL for more details + https://grpc.github.io/grpc/python/_modules/grpc.html#AuthMetadataPlugin + + New plugins can be added to this function. For the time being we only + support Google Open ID authentication. + + Returns: Returns an implementation of grpc.AuthMetadataPlugin + + Args: + config: Feast Configuration object + """ + return GoogleOpenIDAuthMetadataPlugin(config) + + +class GoogleOpenIDAuthMetadataPlugin(grpc.AuthMetadataPlugin): + """A `gRPC AuthMetadataPlugin`_ that inserts the credentials into each + request. + + .. _gRPC AuthMetadataPlugin: + http://www.grpc.io/grpc/python/grpc.html#grpc.AuthMetadataPlugin + """ + + def __init__(self, config: Config): + """ + Initializes a GoogleOpenIDAuthMetadataPlugin, used to sign gRPC requests + Args: + config: Feast Configuration object + """ + super(GoogleOpenIDAuthMetadataPlugin, self).__init__() + from google.auth.transport import requests + + self._static_token = None + self._token = None + + # If provided, set a static token + if config.exists(CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY): + self._static_token = config.get(CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY) + + self._request = requests.Request() + self._refresh_token() + + def get_signed_meta(self): + """ Creates a signed authorization metadata token.""" + return (("authorization", "Bearer {}".format(self._token)),) + + def _refresh_token(self): + """ Refreshes Google ID token and persists it in memory """ + + # Use static token if available + if self._static_token: + self._token = self._static_token + return + + # Try to find ID Token from Gcloud SDK + from google.auth import jwt + import subprocess + + cli_output = subprocess.run( + ["gcloud", "auth", "print-identity-token"], stdout=subprocess.PIPE + ) + token = cli_output.stdout.decode("utf-8").strip() + try: + jwt.decode(token, verify=False) # Ensure the token is valid + self._token = token + return + except ValueError: + pass # GCloud command not successful + + # Try to use Google Auth library to find ID Token + from google import auth as google_auth + + try: + credentials, _ = google_auth.default(["openid", "email"]) + credentials.refresh(self._request) + if hasattr(credentials, "id_token"): + self._token = credentials.id_token + return + except DefaultCredentialsError: + pass # Could not determine credentials, skip + + # Raise exception otherwise + raise RuntimeError( + "Could not determine Google ID token. Please ensure that the " + "Google Cloud SDK is installed or that a service account can be " + "found using the GOOGLE_APPLICATION_CREDENTIALS environmental " + "variable." + ) + + def set_static_token(self, token): + """ + Define a static token to return + + Args: + token: String token + """ + self._static_token = token + + +def __call__(self, context, callback): + """Passes authorization metadata into the given callback. + + Args: + context (grpc.AuthMetadataContext): The RPC context. + callback (grpc.AuthMetadataPluginCallback): The callback that will + be invoked to pass in the authorization metadata. + """ + callback(self.get_signed_meta(), None) diff --git a/sdk/python/feast/grpc/grpc.py b/sdk/python/feast/grpc/grpc.py new file mode 100644 index 00000000000..28b21437a37 --- /dev/null +++ b/sdk/python/feast/grpc/grpc.py @@ -0,0 +1,53 @@ +import grpc + + +def create_grpc_channel( + url: str, + enable_ssl: bool = False, + enable_auth: bool = False, + ssl_server_cert_path: str = None, + auth_metadata_plugin: grpc.AuthMetadataPlugin = None, + timeout: int = 3, +) -> grpc.Channel: + """ + Create a gRPC channel + Args: + url: gRPC URL to connect to + enable_ssl: Enable TLS/SSL, optionally provide a server side certificate + enable_auth: Enable user auth + ssl_server_cert_path: (optional) Path to certificate (used with + "enable SSL") + auth_metadata_plugin: Metadata plugin to use to sign requests, only used + with "enable auth" when SSL/TLS is enabled + timeout: Connection timeout to server + + Returns: Returns a grpc.Channel + """ + if not url: + raise ValueError("Unable to create gRPC channel. URL has not been defined.") + + if enable_ssl or url.endswith(":443"): + # User has provided a public key certificate + if ssl_server_cert_path: + with open(ssl_server_cert_path, "rb",) as f: + credentials = grpc.ssl_channel_credentials(f.read()) + # Guess the certificate location + else: + credentials = grpc.ssl_channel_credentials() + + # Authentication is enabled, add the metadata plugin in order to sign + # requests + if enable_auth: + credentials = grpc.composite_channel_credentials( + credentials, grpc.metadata_call_credentials(auth_metadata_plugin), + ) + channel = grpc.secure_channel(url, credentials=credentials) + else: + channel = grpc.insecure_channel(url) + try: + grpc.channel_ready_future(channel).result(timeout=timeout) + return channel + except grpc.FutureTimeoutError: + raise ConnectionError( + f"Connection timed out while attempting to connect to {url}" + ) diff --git a/sdk/python/tests/feast_core_server.py b/sdk/python/tests/feast_core_server.py index 3ac1b17d003..da3cc4e69b7 100644 --- a/sdk/python/tests/feast_core_server.py +++ b/sdk/python/tests/feast_core_server.py @@ -21,6 +21,37 @@ _logger = logging.getLogger(__name__) _ONE_DAY_IN_SECONDS = 60 * 60 * 24 +_SIGNATURE_HEADER_KEY = "authorization" + + +class DisallowAuthInterceptor(grpc.ServerInterceptor): + def __init__(self): + def abort(ignored_request, context): + context.abort(grpc.StatusCode.UNAUTHENTICATED, "Invalid signature") + + self._abortion = grpc.unary_unary_rpc_method_handler(abort) + + def intercept_service(self, continuation, handler_call_details): + print(handler_call_details.invocation_metadata) + if "Bearer" in handler_call_details.invocation_metadata[0][1]: + return self._abortion + else: + return continuation(handler_call_details) + + +class AllowAuthInterceptor(grpc.ServerInterceptor): + def __init__(self): + def abort(ignored_request, context): + context.abort(grpc.StatusCode.UNAUTHENTICATED, "Invalid signature") + + self._abortion = grpc.unary_unary_rpc_method_handler(abort) + + def intercept_service(self, continuation, handler_call_details): + print(handler_call_details.invocation_metadata) + if "Bearer" in handler_call_details.invocation_metadata[0][1]: + return continuation(handler_call_details) + else: + return self._abortion class CoreServicer(Core.CoreServiceServicer): diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 9a65a984e35..1d0a8647525 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -66,7 +66,11 @@ from feast.source import KafkaSource from feast.types import Value_pb2 as ValueProto from feast.value_type import ValueType -from feast_core_server import CoreServicer +from feast_core_server import ( + AllowAuthInterceptor, + CoreServicer, + DisallowAuthInterceptor, +) from feast_serving_server import ServingServicer CORE_URL = "core.feast.example.com" @@ -74,28 +78,29 @@ _PRIVATE_KEY_RESOURCE_PATH = "data/localhost.key" _CERTIFICATE_CHAIN_RESOURCE_PATH = "data/localhost.pem" _ROOT_CERTIFICATE_RESOURCE_PATH = "data/localhost.crt" +_FAKE_JWT_TOKEN = ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0N" + "TY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDI" + "yfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c" +) class TestClient: @pytest.fixture - def secure_mock_client(self, mocker): + def secure_mock_client(self): client = Client( core_url=CORE_URL, serving_url=SERVING_URL, - core_secure=True, - serving_secure=True, + core_enable_ssl=True, + serving_enable_ssl=True, ) - mocker.patch.object(client, "_connect_core") - mocker.patch.object(client, "_connect_serving") client._core_url = CORE_URL client._serving_url = SERVING_URL return client @pytest.fixture - def mock_client(self, mocker): + def mock_client(self): client = Client(core_url=CORE_URL, serving_url=SERVING_URL) - mocker.patch.object(client, "_connect_core") - mocker.patch.object(client, "_connect_serving") client._core_url = CORE_URL client._serving_url = SERVING_URL return client @@ -137,18 +142,53 @@ def secure_core_server(self, server_credentials): def secure_serving_server(self, server_credentials): server = grpc.server(futures.ThreadPoolExecutor(max_workers=10)) Serving.add_ServingServiceServicer_to_server(ServingServicer(), server) - server.add_secure_port("[::]:50054", server_credentials) server.start() yield server server.stop(0) + @pytest.fixture + def secure_core_server_with_auth(self, server_credentials): + server = grpc.server( + futures.ThreadPoolExecutor(max_workers=10), + interceptors=(AllowAuthInterceptor(),), + ) + Core.add_CoreServiceServicer_to_server(CoreServicer(), server) + server.add_secure_port("[::]:50055", server_credentials) + server.start() + yield server + server.stop(0) + + @pytest.fixture + def insecure_core_server_with_auth(self, server_credentials): + server = grpc.server( + futures.ThreadPoolExecutor(max_workers=10), + interceptors=(AllowAuthInterceptor(),), + ) + Core.add_CoreServiceServicer_to_server(CoreServicer(), server) + server.add_insecure_port("[::]:50056") + server.start() + yield server + server.stop(0) + + @pytest.fixture + def insecure_core_server_that_blocks_auth(self, server_credentials): + server = grpc.server( + futures.ThreadPoolExecutor(max_workers=10), + interceptors=(DisallowAuthInterceptor(),), + ) + Core.add_CoreServiceServicer_to_server(CoreServicer(), server) + server.add_insecure_port("[::]:50057") + server.start() + yield server + server.stop(0) + @pytest.fixture def secure_client(self, secure_core_server, secure_serving_server): root_certificate_credentials = pkgutil.get_data( __name__, _ROOT_CERTIFICATE_RESOURCE_PATH ) - # this is needed to establish a secure connection using self-signed certificates, for the purpose of the test + ssl_channel_credentials = grpc.ssl_channel_credentials( root_certificates=root_certificate_credentials ) @@ -159,8 +199,27 @@ def secure_client(self, secure_core_server, secure_serving_server): yield Client( core_url="localhost:50053", serving_url="localhost:50054", - core_secure=True, - serving_secure=True, + core_enable_ssl=True, + serving_enable_ssl=True, + ) + + @pytest.fixture + def secure_core_client_with_auth(self, secure_core_server_with_auth): + root_certificate_credentials = pkgutil.get_data( + __name__, _ROOT_CERTIFICATE_RESOURCE_PATH + ) + ssl_channel_credentials = grpc.ssl_channel_credentials( + root_certificates=root_certificate_credentials + ) + with mock.patch( + "grpc.ssl_channel_credentials", + MagicMock(return_value=ssl_channel_credentials), + ): + yield Client( + core_url="localhost:50055", + core_enable_ssl=True, + core_enable_auth=True, + core_auth_token=_FAKE_JWT_TOKEN, ) @pytest.fixture @@ -892,7 +951,7 @@ def test_feature_set_types_success(self, test_client, dataframe, mocker): test_client.apply(all_types_fs) mocker.patch.object( - test_client._core_service_stub, + test_client._core_service, "GetFeatureSet", return_value=GetFeatureSetResponse(feature_set=all_types_fs.to_proto()), ) @@ -907,15 +966,15 @@ def test_secure_channel_creation_with_secure_client(self, _mocked_obj): client = Client( core_url="localhost:50051", serving_url="localhost:50052", - serving_secure=True, - core_secure=True, + serving_enable_ssl=True, + core_enable_ssl=True, ) with mock.patch("grpc.secure_channel") as _grpc_mock, mock.patch( "grpc.ssl_channel_credentials", MagicMock(return_value="test") ) as _mocked_credentials: - client._connect_serving() + _ = client._serving_service _grpc_mock.assert_called_with( - client.serving_url, _mocked_credentials.return_value + client.serving_url, credentials=_mocked_credentials.return_value ) @mock.patch("grpc.channel_ready_future") @@ -926,9 +985,9 @@ def test_secure_channel_creation_with_secure_serving_url( with mock.patch("grpc.secure_channel") as _grpc_mock, mock.patch( "grpc.ssl_channel_credentials", MagicMock(return_value="test") ) as _mocked_credentials: - client._connect_serving() + _ = client._serving_service _grpc_mock.assert_called_with( - client.serving_url, _mocked_credentials.return_value + client.serving_url, credentials=_mocked_credentials.return_value ) @patch("grpc.channel_ready_future") @@ -937,7 +996,29 @@ def test_secure_channel_creation_with_secure_core_url(self, _mocked_obj): with mock.patch("grpc.secure_channel") as _grpc_mock, mock.patch( "grpc.ssl_channel_credentials", MagicMock(return_value="test") ) as _mocked_credentials: - client._connect_core() + _ = client._core_service _grpc_mock.assert_called_with( - client.core_url, _mocked_credentials.return_value + client.core_url, credentials=_mocked_credentials.return_value ) + + @mock.patch("grpc.channel_ready_future") + def test_auth_success_with_secure_channel_on_core_url( + self, secure_core_client_with_auth + ): + secure_core_client_with_auth.list_feature_sets() + + def test_auth_success_with_insecure_channel_on_core_url( + self, insecure_core_server_with_auth + ): + client = Client( + core_url="localhost:50056", + core_enable_auth=True, + core_auth_token=_FAKE_JWT_TOKEN, + ) + client.list_feature_sets() + + def test_no_auth_sent_when_auth_disabled( + self, insecure_core_server_that_blocks_auth + ): + client = Client(core_url="localhost:50057") + client.list_feature_sets() From 78f7266976327e8b44a39f13b1f9d79e343dd436 Mon Sep 17 00:00:00 2001 From: Jayanth Kumar M J Date: Mon, 15 Jun 2020 17:27:57 -0400 Subject: [PATCH 02/18] Authentication and Authorization Client sdk authentication, Server Authentication and authorization. --- core/pom.xml | 42 ++-- ... => DefaultJwtAuthenticationProvider.java} | 6 +- .../authorization/AuthorizationProvider.java | 9 +- .../authorization/AuthorizationResult.java | 41 ++++ .../Keto/KetoAuthorizationProvider.java | 14 +- .../feast/core/config/FeastProperties.java | 15 +- .../feast/core/config/SecurityConfig.java | 10 +- .../java/feast/core/grpc/CoreServiceImpl.java | 2 + .../feast/core/grpc/HealthServiceImpl.java | 14 +- .../main/java/feast/core/model/Entity.java | 2 +- .../main/java/feast/core/model/Feature.java | 2 +- .../feast/core/service/ProjectService.java | 7 +- core/src/main/resources/application.yml | 4 +- .../java/feast/core/auth/AuthConfigTest.java | 42 ++++ .../java/feast/core/auth/AuthTestConfig.java | 40 ++++ .../feast/core/grpc/CoreServiceAuthTest.java | 60 ++++-- .../service/AccessManagementServiceTest.java | 74 ------- .../core/service/ProjectServiceTest.java | 11 + .../src/test/resources/application.properties | 3 + sdk/python/feast/client.py | 16 +- sdk/python/feast/constants.py | 15 ++ sdk/python/feast/grpc/auth.py | 149 ++++++++++++- sdk/python/setup.py | 4 +- sdk/python/tests/grpc/test_auth.py | 200 ++++++++++++++++++ sdk/python/tests/test_client.py | 2 +- tests/e2e/bq/bq-batch-retrieval.py | 8 +- tests/e2e/bq/feature-stats.py | 13 +- tests/e2e/redis/basic-ingest-redis-serving.py | 26 ++- tests/e2e/requirements.txt | 2 +- 29 files changed, 650 insertions(+), 183 deletions(-) rename core/src/main/java/feast/core/auth/authentication/{GoogleOID/GoogleOpenIDAuthenticationProvider.java => DefaultJwtAuthenticationProvider.java} (92%) create mode 100644 core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java create mode 100644 core/src/test/java/feast/core/auth/AuthConfigTest.java create mode 100644 core/src/test/java/feast/core/auth/AuthTestConfig.java delete mode 100644 core/src/test/java/feast/core/service/AccessManagementServiceTest.java create mode 100644 sdk/python/tests/grpc/test_auth.py diff --git a/core/pom.xml b/core/pom.xml index 573e86a643e..8f920fbe0c0 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -191,7 +191,6 @@ com.google.protobuf protobuf-java-util - com.google.guava @@ -213,7 +212,6 @@ google-api-services-dataflow v1b3-rev266-1.25.0 - org.hibernate hibernate-core @@ -281,12 +279,6 @@ 2.2.0 test - - org.mockito - mockito-core - test - - org.springframework.boot spring-boot-test @@ -326,15 +318,39 @@ org.springframework + + javax.validation + validation-api + 2.0.0.Final + + + org.hibernate.validator + hibernate-validator + 6.1.2.Final + + + org.hibernate.validator + hibernate-validator-annotation-processor + 6.1.2.Final + + + + org.mockito + mockito-core + 2.23.0 + test + + + org.springframework spring-test 5.1.3.RELEASE - test - - + test + + org.junit.jupiter junit-jupiter RELEASE test - - + + diff --git a/core/src/main/java/feast/core/auth/authentication/GoogleOID/GoogleOpenIDAuthenticationProvider.java b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java similarity index 92% rename from core/src/main/java/feast/core/auth/authentication/GoogleOID/GoogleOpenIDAuthenticationProvider.java rename to core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java index 103ed5e2635..2036adb1671 100644 --- a/core/src/main/java/feast/core/auth/authentication/GoogleOID/GoogleOpenIDAuthenticationProvider.java +++ b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.core.auth.authentication.GoogleOID; +package feast.core.auth.authentication; import java.util.Map; import org.springframework.security.authentication.AuthenticationProvider; @@ -28,7 +28,7 @@ * Google Open ID Authentication Provider. This provider is used to validate incoming requests to * Feast Core. */ -public class GoogleOpenIDAuthenticationProvider implements AuthenticationProvider { +public class DefaultJwtAuthenticationProvider implements AuthenticationProvider { private JwtAuthenticationProvider authProvider; @@ -36,7 +36,7 @@ public class GoogleOpenIDAuthenticationProvider implements AuthenticationProvide * @param options String K/V pair of options to initialize the AuthenticationProvider with. Only * one option is currently configurable, the jwkEndpointURI. */ - public GoogleOpenIDAuthenticationProvider(Map options) { + public DefaultJwtAuthenticationProvider(Map options) { // Endpoint used to retrieve certificates to validate JWT token String jwkEndpointURI = "https://www.googleapis.com/oauth2/v3/certs"; diff --git a/core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java b/core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java index 0209b90b9ff..3d504be3ffd 100644 --- a/core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java +++ b/core/src/main/java/feast/core/auth/authorization/AuthorizationProvider.java @@ -16,7 +16,6 @@ */ package feast.core.auth.authorization; -import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.Authentication; /** @@ -26,13 +25,11 @@ public interface AuthorizationProvider { /** - * Validates whether a user is within a project. Throws an AccessDeniedException if user is not - * within the project. + * Validates whether a user is allowed access to the project * * @param project Name of the Feast project * @param authentication Spring Security Authentication object - * @throws AccessDeniedException + * @return AuthorizationResult result of authorization query */ - void checkIfProjectMember(String project, Authentication authentication) - throws AccessDeniedException; + AuthorizationResult checkAccess(String project, Authentication authentication); } diff --git a/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java b/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java new file mode 100644 index 00000000000..6b0d41f43e7 --- /dev/null +++ b/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java @@ -0,0 +1,41 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.auth.authorization; + +import com.google.auto.value.AutoValue; +import java.util.Optional; +import javax.annotation.Nullable; + +@AutoValue +public abstract class AuthorizationResult { + public static AuthorizationResult create( + @Nullable boolean allowed, @Nullable String failureReason) { + return new AutoValue_AuthorizationResult(allowed, Optional.ofNullable(failureReason)); + } + + public static AuthorizationResult failed(@Nullable String failureReason) { + return new AutoValue_AuthorizationResult(false, Optional.ofNullable(failureReason)); + } + + public static AuthorizationResult success() { + return new AutoValue_AuthorizationResult(true, Optional.empty()); + } + + public abstract boolean allowed(); + + public abstract Optional failureReason(); +} diff --git a/core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java b/core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java index 7f744e5c79e..0c0197f0f4a 100644 --- a/core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java +++ b/core/src/main/java/feast/core/auth/authorization/Keto/KetoAuthorizationProvider.java @@ -17,10 +17,10 @@ package feast.core.auth.authorization.Keto; import feast.core.auth.authorization.AuthorizationProvider; +import feast.core.auth.authorization.AuthorizationResult; import java.util.List; import java.util.Map; import org.hibernate.validator.internal.constraintvalidators.bv.EmailValidator; -import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.jwt.Jwt; import sh.ory.keto.ApiClient; @@ -50,15 +50,13 @@ public KetoAuthorizationProvider(Map options) { } /** - * Validates whether a user is within a project. Throws an AccessDeniedException if user is not - * within the project. + * Validates whether a user has access to the project * * @param project Name of the Feast project * @param authentication Spring Security Authentication object - * @throws AccessDeniedException + * @return AuthorizationResult result of authorization query */ - public void checkIfProjectMember(String project, Authentication authentication) - throws AccessDeniedException { + public AuthorizationResult checkAccess(String project, Authentication authentication) { String email = getEmailFromAuth(authentication); try { // Get all roles from Keto @@ -70,7 +68,7 @@ public void checkIfProjectMember(String project, Authentication authentication) // If the user has an admin or project specific role, return. if (("roles:admin").equals(role.getId()) || (String.format("roles:feast:%s-member", project)).equals(role.getId())) { - return; + return AuthorizationResult.success(); } } } catch (ApiException e) { @@ -81,7 +79,7 @@ public void checkIfProjectMember(String project, Authentication authentication) e.printStackTrace(); } // Could not determine project membership, deny access. - throw new AccessDeniedException( + return AuthorizationResult.failed( String.format("Access denied to project %s for user %s", project, email)); } diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index 5b9f74f605f..4bca210cd15 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -20,9 +20,17 @@ import feast.core.validators.OneOfStrings; import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; import javax.annotation.PostConstruct; -import javax.validation.*; +import javax.validation.ConstraintViolation; +import javax.validation.ConstraintViolationException; +import javax.validation.Validation; +import javax.validation.Validator; +import javax.validation.ValidatorFactory; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; import javax.validation.constraints.Positive; @@ -31,9 +39,11 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.info.BuildProperties; +import org.springframework.stereotype.Component; @Getter @Setter +@Component @ConfigurationProperties(prefix = "feast", ignoreInvalidFields = true) public class FeastProperties { @@ -59,6 +69,7 @@ public FeastProperties() {} @NotNull /* Feast Kafka stream properties */ private StreamProperties stream; + private SecurityProperties security; /** Feast job properties. These properties are used for ingestion jobs. */ diff --git a/core/src/main/java/feast/core/config/SecurityConfig.java b/core/src/main/java/feast/core/config/SecurityConfig.java index 855d1b1354c..f77e20ba107 100644 --- a/core/src/main/java/feast/core/config/SecurityConfig.java +++ b/core/src/main/java/feast/core/config/SecurityConfig.java @@ -16,11 +16,11 @@ */ package feast.core.config; -import feast.core.CoreServiceGrpc; -import feast.core.auth.authentication.GoogleOID.GoogleOpenIDAuthenticationProvider; +import feast.core.auth.authentication.DefaultJwtAuthenticationProvider; import feast.core.auth.authorization.AuthorizationProvider; import feast.core.auth.authorization.Keto.KetoAuthorizationProvider; import feast.core.config.FeastProperties.SecurityProperties; +import feast.proto.core.CoreServiceGrpc; import java.util.ArrayList; import java.util.List; import net.devh.boot.grpc.server.security.authentication.BearerAuthenticationReader; @@ -61,9 +61,9 @@ AuthenticationManager authenticationManager() { if (securityProperties.getAuthentication().isEnabled()) { switch (securityProperties.getAuthentication().getProvider()) { - case "GoogleOpenID": + case "jwt": providers.add( - new GoogleOpenIDAuthenticationProvider( + new DefaultJwtAuthenticationProvider( securityProperties.getAuthentication().getOptions())); break; default: @@ -132,7 +132,7 @@ AuthorizationProvider authorizationProvider() { if (securityProperties.getAuthentication().isEnabled() && securityProperties.getAuthorization().isEnabled()) { switch (securityProperties.getAuthorization().getProvider()) { - case "KetoAuthorization": + case "keto": return new KetoAuthorizationProvider(securityProperties.getAuthorization().getOptions()); default: throw new IllegalArgumentException( diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java index 0941f4f4048..f4f79288198 100644 --- a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java @@ -53,10 +53,12 @@ public class CoreServiceImpl extends CoreServiceImplBase { @Autowired public CoreServiceImpl( SpecService specService, + ProjectService projectService, StatsService statsService, JobService jobService, FeastProperties feastProperties) { this.specService = specService; + this.projectService = projectService; this.jobService = jobService; this.feastProperties = feastProperties; this.statsService = statsService; diff --git a/core/src/main/java/feast/core/grpc/HealthServiceImpl.java b/core/src/main/java/feast/core/grpc/HealthServiceImpl.java index 3bd2f8748fe..0a1f10109ca 100644 --- a/core/src/main/java/feast/core/grpc/HealthServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/HealthServiceImpl.java @@ -16,7 +16,7 @@ */ package feast.core.grpc; -import feast.core.service.AccessManagementService; +import feast.core.service.ProjectService; import io.grpc.Status; import io.grpc.health.v1.HealthGrpc.HealthImplBase; import io.grpc.health.v1.HealthProto.HealthCheckRequest; @@ -24,24 +24,24 @@ import io.grpc.health.v1.HealthProto.HealthCheckResponse.ServingStatus; import io.grpc.stub.StreamObserver; import lombok.extern.slf4j.Slf4j; -import org.lognet.springboot.grpc.GRpcService; +import net.devh.boot.grpc.server.service.GrpcService; import org.springframework.beans.factory.annotation.Autowired; @Slf4j -@GRpcService +@GrpcService public class HealthServiceImpl extends HealthImplBase { - private final AccessManagementService accessManagementService; + private final ProjectService projectService; @Autowired - public HealthServiceImpl(AccessManagementService accessManagementService) { - this.accessManagementService = accessManagementService; + public HealthServiceImpl(ProjectService projectService) { + this.projectService = projectService; } @Override public void check( HealthCheckRequest request, StreamObserver responseObserver) { try { - accessManagementService.listProjects(); + projectService.listProjects(); responseObserver.onNext( HealthCheckResponse.newBuilder().setStatus(ServingStatus.SERVING).build()); responseObserver.onCompleted(); diff --git a/core/src/main/java/feast/core/model/Entity.java b/core/src/main/java/feast/core/model/Entity.java index 6133d492fcc..a5fd8c1b05d 100644 --- a/core/src/main/java/feast/core/model/Entity.java +++ b/core/src/main/java/feast/core/model/Entity.java @@ -44,7 +44,7 @@ public class Entity { public Entity() {} - private Entity(String name, ValueType.Enum type) { + public Entity(String name, ValueType.Enum type) { this.setName(name); this.setType(type.toString()); } diff --git a/core/src/main/java/feast/core/model/Feature.java b/core/src/main/java/feast/core/model/Feature.java index 87978e65ce3..7229f13af87 100644 --- a/core/src/main/java/feast/core/model/Feature.java +++ b/core/src/main/java/feast/core/model/Feature.java @@ -86,7 +86,7 @@ public Feature() {} // retrieved from or written to. private boolean archived = false; - private Feature(String name, ValueType.Enum type) { + public Feature(String name, ValueType.Enum type) { this.setName(name); this.setType(type.toString()); } diff --git a/core/src/main/java/feast/core/service/ProjectService.java b/core/src/main/java/feast/core/service/ProjectService.java index d70391f060d..e8fe49f8f91 100644 --- a/core/src/main/java/feast/core/service/ProjectService.java +++ b/core/src/main/java/feast/core/service/ProjectService.java @@ -17,6 +17,7 @@ package feast.core.service; import feast.core.auth.authorization.AuthorizationProvider; +import feast.core.auth.authorization.AuthorizationResult; import feast.core.config.FeastProperties; import feast.core.config.FeastProperties.SecurityProperties; import feast.core.dao.ProjectRepository; @@ -26,6 +27,7 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContext; import org.springframework.stereotype.Service; @@ -112,6 +114,9 @@ public void checkIfProjectMember(SecurityContext securityContext, String project if (!this.securityProperties.getAuthorization().isEnabled()) { return; } - this.authorizationProvider.checkIfProjectMember(project, authentication); + AuthorizationResult result = this.authorizationProvider.checkAccess(project, authentication); + if (!result.allowed()) { + throw new AccessDeniedException(result.failureReason().orElse("AccessDenied")); + } } } diff --git a/core/src/main/resources/application.yml b/core/src/main/resources/application.yml index 8dec0d6380b..cda671ce72d 100644 --- a/core/src/main/resources/application.yml +++ b/core/src/main/resources/application.yml @@ -78,10 +78,10 @@ feast: security: authentication: enabled: false - provider: GoogleOpenID + provider: jwt authorization: enabled: false - provider: KetoAuthorization + provider: none options: basePath: http://localhost:3000 diff --git a/core/src/test/java/feast/core/auth/AuthConfigTest.java b/core/src/test/java/feast/core/auth/AuthConfigTest.java new file mode 100644 index 00000000000..c54493489a4 --- /dev/null +++ b/core/src/test/java/feast/core/auth/AuthConfigTest.java @@ -0,0 +1,42 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.auth; + +import static org.junit.Assert.assertNotNull; + +import feast.core.config.SecurityConfig; +import javax.inject.Inject; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit.jupiter.SpringExtension; + +@ExtendWith(SpringExtension.class) +@SpringBootTest(classes = {AuthTestConfig.class, SecurityConfig.class}) +@TestPropertySource(locations = {"classpath:application.properties", "classpath:application.yml"}) +class AuthConfigTest { + + @Inject AuthenticationManager authManager; + @Inject AuthTestConfig config; + + @Test + void canConfigureAuth() { + assertNotNull(authManager); + } +} diff --git a/core/src/test/java/feast/core/auth/AuthTestConfig.java b/core/src/test/java/feast/core/auth/AuthTestConfig.java new file mode 100644 index 00000000000..50f29499fcf --- /dev/null +++ b/core/src/test/java/feast/core/auth/AuthTestConfig.java @@ -0,0 +1,40 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2019 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.auth; + +import feast.core.config.FeastProperties; +import java.util.Properties; +import javax.inject.Inject; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.info.BuildProperties; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.context.annotation.Bean; + +/** Configuration of JPA related services and beans for the core application. */ +@TestConfiguration +@EnableConfigurationProperties(FeastProperties.class) +public class AuthTestConfig { + + public @Inject FeastProperties feast; + + @Bean + public BuildProperties buildProperties() { + Properties props = new Properties(); + props.put("version", "test"); + return new BuildProperties(props); + } +} diff --git a/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java index 6ca91c85733..87e29902e57 100644 --- a/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java +++ b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java @@ -17,32 +17,41 @@ package feast.core.grpc; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.doThrow; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.protobuf.InvalidProtocolBufferException; -import feast.core.CoreServiceProto.ApplyFeatureSetRequest; -import feast.core.CoreServiceProto.ApplyFeatureSetResponse; -import feast.core.FeatureSetProto; -import feast.core.FeatureSetProto.FeatureSetStatus; -import feast.core.SourceProto.KafkaSourceConfig; -import feast.core.SourceProto.SourceType; import feast.core.auth.authorization.AuthorizationProvider; +import feast.core.auth.authorization.AuthorizationResult; import feast.core.config.FeastProperties; import feast.core.config.FeastProperties.SecurityProperties; import feast.core.dao.ProjectRepository; +import feast.core.model.Entity; +import feast.core.model.Feature; import feast.core.model.FeatureSet; -import feast.core.model.Field; import feast.core.model.Source; +import feast.core.service.JobService; import feast.core.service.ProjectService; import feast.core.service.SpecService; -import feast.types.ValueProto.ValueType.Enum; +import feast.core.service.StatsService; +import feast.proto.core.CoreServiceProto.ApplyFeatureSetRequest; +import feast.proto.core.CoreServiceProto.ApplyFeatureSetResponse; +import feast.proto.core.FeatureSetProto; +import feast.proto.core.FeatureSetProto.FeatureSetStatus; +import feast.proto.core.SourceProto.KafkaSourceConfig; +import feast.proto.core.SourceProto.SourceType; +import feast.proto.types.ValueProto.ValueType.Enum; import io.grpc.internal.testing.StreamRecorder; import java.sql.Date; import java.time.Instant; import java.util.Arrays; +import java.util.HashMap; import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.Authentication; @@ -53,15 +62,16 @@ class CoreServiceAuthTest { private CoreServiceImpl coreService; - private SpecService specService; private ProjectService projectService; - private ProjectRepository projectRepository; - private AuthorizationProvider authProvider; + + @Mock private SpecService specService; + @Mock private ProjectRepository projectRepository; + @Mock private AuthorizationProvider authProvider; + @Mock private StatsService statsService; + @Mock private JobService jobService; CoreServiceAuthTest() { - specService = mock(SpecService.class); - projectRepository = mock(ProjectRepository.class); - authProvider = mock(AuthorizationProvider.class); + MockitoAnnotations.initMocks(this); FeastProperties.SecurityProperties.AuthorizationProperties authProp = new FeastProperties.SecurityProperties.AuthorizationProperties(); authProp.setEnabled(true); @@ -70,7 +80,8 @@ class CoreServiceAuthTest { FeastProperties feastProperties = new FeastProperties(); feastProperties.setSecurity(sp); projectService = new ProjectService(feastProperties, projectRepository, authProvider); - coreService = new CoreServiceImpl(specService, projectService); + coreService = + new CoreServiceImpl(specService, projectService, statsService, jobService, feastProperties); } @Test @@ -82,12 +93,14 @@ void cantApplyFeatureSetIfNotProjectMember() throws InvalidProtocolBufferExcepti SecurityContextHolder.setContext(context); when(context.getAuthentication()).thenReturn(auth); - doThrow(AccessDeniedException.class).when(authProvider).checkIfProjectMember(project, auth); + doReturn(AuthorizationResult.failed(null)) + .when(authProvider) + .checkAccess(anyString(), any(Authentication.class)); StreamRecorder responseObserver = StreamRecorder.create(); FeatureSetProto.FeatureSet incomingFeatureSet = newDummyFeatureSet("f2", 1, project).toProto(); FeatureSetProto.FeatureSetSpec incomingFeatureSetSpec = - incomingFeatureSet.getSpec().toBuilder().clearVersion().build(); + incomingFeatureSet.getSpec().toBuilder().build(); FeatureSetProto.FeatureSet spec = FeatureSetProto.FeatureSet.newBuilder().setSpec(incomingFeatureSetSpec).build(); ApplyFeatureSetRequest request = @@ -105,11 +118,14 @@ void canApplyFeatureSetIfProjectMember() throws InvalidProtocolBufferException { SecurityContext context = mock(SecurityContext.class); SecurityContextHolder.setContext(context); when(context.getAuthentication()).thenReturn(auth); + doReturn(AuthorizationResult.success()) + .when(authProvider) + .checkAccess(anyString(), any(Authentication.class)); StreamRecorder responseObserver = StreamRecorder.create(); FeatureSetProto.FeatureSet incomingFeatureSet = newDummyFeatureSet("f2", 1, project).toProto(); FeatureSetProto.FeatureSetSpec incomingFeatureSetSpec = - incomingFeatureSet.getSpec().toBuilder().clearVersion().build(); + incomingFeatureSet.getSpec().toBuilder().build(); FeatureSetProto.FeatureSet spec = FeatureSetProto.FeatureSet.newBuilder().setSpec(incomingFeatureSetSpec).build(); ApplyFeatureSetRequest request = @@ -119,8 +135,8 @@ void canApplyFeatureSetIfProjectMember() throws InvalidProtocolBufferException { } private FeatureSet newDummyFeatureSet(String name, int version, String project) { - Field feature = new Field("feature", Enum.INT64); - Field entity = new Field("entity", Enum.STRING); + Feature feature = new Feature("feature", Enum.INT64); + Entity entity = new Entity("entity", Enum.STRING); Source defaultSource = new Source( @@ -135,11 +151,11 @@ private FeatureSet newDummyFeatureSet(String name, int version, String project) new FeatureSet( name, project, - version, 100L, Arrays.asList(entity), Arrays.asList(feature), defaultSource, + new HashMap(), FeatureSetStatus.STATUS_READY); fs.setCreated(Date.from(Instant.ofEpochSecond(10L))); return fs; diff --git a/core/src/test/java/feast/core/service/AccessManagementServiceTest.java b/core/src/test/java/feast/core/service/AccessManagementServiceTest.java deleted file mode 100644 index 15be203709f..00000000000 --- a/core/src/test/java/feast/core/service/AccessManagementServiceTest.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 The Feast Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feast.core.service; - -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; -import static org.mockito.MockitoAnnotations.initMocks; - -import feast.core.dao.ProjectRepository; -import feast.core.model.Project; -import java.util.Optional; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.mockito.Mock; - -public class AccessManagementServiceTest { - @Rule public ExpectedException expectedException = ExpectedException.none(); - // mocks - @Mock private ProjectRepository projectRepository; - // dummy models - private Project defaultProject; - private Project testProject; - - // test target - private AccessManagementService accessService; - - @Before - public void setup() { - initMocks(this); - // setup dummy models for testing - this.defaultProject = new Project(Project.DEFAULT_NAME); - this.testProject = new Project("project"); - // setup test target - when(this.projectRepository.existsById(Project.DEFAULT_NAME)).thenReturn(false); - this.accessService = new AccessManagementService(this.projectRepository); - } - - @Test - public void testDefaultProjectCreateInConstructor() { - verify(this.projectRepository).saveAndFlush(this.defaultProject); - } - - @Test - public void testArchiveProject() { - when(this.projectRepository.findById("project")).thenReturn(Optional.of(this.testProject)); - this.accessService.archiveProject("project"); - this.testProject.setArchived(true); - verify(this.projectRepository).saveAndFlush(this.testProject); - // reset archived flag - this.testProject.setArchived(false); - } - - @Test - public void shouldNotArchiveDefaultProject() { - expectedException.expect(IllegalArgumentException.class); - this.accessService.archiveProject(Project.DEFAULT_NAME); - } -} diff --git a/core/src/test/java/feast/core/service/ProjectServiceTest.java b/core/src/test/java/feast/core/service/ProjectServiceTest.java index a36bd322955..ae9fc661b3e 100644 --- a/core/src/test/java/feast/core/service/ProjectServiceTest.java +++ b/core/src/test/java/feast/core/service/ProjectServiceTest.java @@ -61,6 +61,11 @@ public void setUp() { new ProjectService(feastProperties, projectRepository, mock(AuthorizationProvider.class)); } + @Test + public void testDefaultProjectCreateInConstructor() { + verify(this.projectRepository).saveAndFlush(new Project(Project.DEFAULT_NAME)); + } + @Test public void shouldCreateProjectIfItDoesntExist() { String projectName = "project1"; @@ -85,6 +90,12 @@ public void shouldArchiveProjectIfItExists() { verify(projectRepository, times(1)).saveAndFlush(any(Project.class)); } + @Test + public void shouldNotArchiveDefaultProject() { + expectedException.expect(IllegalArgumentException.class); + this.projectService.archiveProject(Project.DEFAULT_NAME); + } + @Test(expected = IllegalArgumentException.class) public void shouldNotArchiveProjectIfItIsAlreadyArchived() { String projectName = "project1"; diff --git a/core/src/test/resources/application.properties b/core/src/test/resources/application.properties index 33c45dd0c5f..3020fb841c8 100644 --- a/core/src/test/resources/application.properties +++ b/core/src/test/resources/application.properties @@ -16,6 +16,9 @@ # grpc.port=${GRPC_PORT:6565} +feast.security.authentication.enabled = true +feast.security.authorization.enabled = false + feast.core.projectId = ${PROJECT_ID:} feast.core.datasetPrefix = ${DATASET_PREFIX:fs} diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index f31ade35037..808c739788d 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -68,8 +68,8 @@ from feast.core.FeatureSet_pb2 import FeatureSetStatus from feast.feature import Feature, FeatureRef from feast.feature_set import Entity, FeatureSet, FeatureSetRef -from feast.job import IngestJob, RetrievalJob from feast.grpc.grpc import create_grpc_channel +from feast.job import IngestJob, RetrievalJob from feast.loaders.abstract_producer import get_producer from feast.loaders.file import export_source_to_staging_location from feast.loaders.ingest import KAFKA_CHUNK_PRODUCTION_TIMEOUT, get_feature_row_chunks @@ -336,7 +336,6 @@ def archive_project(self, project): project: Name of project to archive """ - self._connect_core() try: self._core_service_stub.ArchiveProject( ArchiveProjectRequest(name=project), @@ -440,7 +439,7 @@ def list_feature_sets( feature_set_protos = self._core_service.ListFeatureSets( ListFeatureSetsRequest(filter=filter), metadata=self._get_grpc_metadata(), ) # type: ListFeatureSetsResponse - + # Extract feature sets and return feature_sets = [] for feature_set_proto in feature_set_protos.feature_sets: @@ -589,7 +588,6 @@ def get_batch_features( >>> print(df) """ - self._connect_serving() feature_references = _build_feature_references( feature_refs=feature_refs, default_project=default_project ) @@ -649,11 +647,11 @@ def get_batch_features( # Retrieve Feast Job object to manage life cycle of retrieval try: - response = self._serving_service_stub.GetBatchFeatures(request) + response = self._serving_service.GetBatchFeatures(request) except grpc.RpcError as e: raise grpc.RpcError(e.details()) - return RetrievalJob(response.job, self._serving_service_stub) + return RetrievalJob(response.job, self._serving_service) def get_online_features( self, @@ -685,7 +683,7 @@ def get_online_features( """ try: - response = self._serving_service_stub.GetOnlineFeatures( + response = self._serving_service.GetOnlineFeatures( GetOnlineFeaturesRequest( omit_entities_in_response=omit_entities, features=_build_feature_references( @@ -748,7 +746,6 @@ def list_ingest_jobs( Returns: List of IngestJobs matching the given filters """ - self._connect_core() # construct list request feature_set_ref = None list_filter = ListIngestionJobsRequest.Filter( @@ -772,7 +769,6 @@ def restart_ingest_job(self, job: IngestJob): Args: job: IngestJob to restart """ - self._connect_core() request = RestartIngestionJobRequest(id=job.id) try: self._core_service_stub.RestartIngestionJob(request) @@ -789,7 +785,6 @@ def stop_ingest_job(self, job: IngestJob): Args: job: IngestJob to restart """ - self._connect_core() request = StopIngestionJobRequest(id=job.id) try: self._core_service_stub.StopIngestionJob(request) @@ -957,7 +952,6 @@ def get_statistics( Returns a tensorflow DatasetFeatureStatisticsList containing TFDV featureStatistics. """ - self._connect_core() if ingestion_ids is not None and ( start_date is not None or end_date is not None ): diff --git a/sdk/python/feast/constants.py b/sdk/python/feast/constants.py index 48e9c8feeca..911432326a9 100644 --- a/sdk/python/feast/constants.py +++ b/sdk/python/feast/constants.py @@ -13,6 +13,13 @@ # See the License for the specific language governing permissions and # limitations under the License. # +from enum import Enum + + +class AuthProvider(Enum): + GOOGLE = "google" + OAUTH = "oauth" + DATETIME_COLUMN = "datetime" @@ -46,6 +53,12 @@ CONFIG_BATCH_FEATURE_REQUEST_WAIT_TIME_SECONDS_KEY = ( "batch_feature_request_wait_time_seconds" ) +CONFIG_OAUTH_GRANT_TYPE_KEY = "oauth_grant_type" +CONFIG_OAUTH_CLIENT_ID_KEY = "oauth_client_id" +CONFIG_OAUTH_CLIENT_SECRET_KEY = "oauth_client_secret" +CONFIG_OAUTH_AUDIENCE_KEY = "oauth_audience" +CONFIG_OAUTH_TOKEN_REQUEST_URL_KEY = "oauth_token_request_url" +CONFIG_CORE_AUTH_PROVIDER = "core_auth_provider" CONFIG_TIMEOUT_KEY = "timeout" CONFIG_MAX_WAIT_INTERVAL_KEY = "max_wait_interval" @@ -77,4 +90,6 @@ CONFIG_BATCH_FEATURE_REQUEST_WAIT_TIME_SECONDS_KEY: "600", CONFIG_TIMEOUT_KEY: "21600", CONFIG_MAX_WAIT_INTERVAL_KEY: "60", + # Authentication Provider - Google OpenID/OAuth + CONFIG_CORE_AUTH_PROVIDER: "google", } diff --git a/sdk/python/feast/grpc/auth.py b/sdk/python/feast/grpc/auth.py index cd4bb2de3f7..581d45f559d 100644 --- a/sdk/python/feast/grpc/auth.py +++ b/sdk/python/feast/grpc/auth.py @@ -1,8 +1,34 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2018-2020 The Feast Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from http import HTTPStatus + import grpc from google.auth.exceptions import DefaultCredentialsError from feast.config import Config -from feast.constants import CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY +from feast.constants import ( + CONFIG_CORE_AUTH_PROVIDER, + CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY, + CONFIG_OAUTH_AUDIENCE_KEY, + CONFIG_OAUTH_CLIENT_ID_KEY, + CONFIG_OAUTH_CLIENT_SECRET_KEY, + CONFIG_OAUTH_GRANT_TYPE_KEY, + CONFIG_OAUTH_TOKEN_REQUEST_URL_KEY, + AuthProvider, +) def get_auth_metadata_plugin(config: Config): @@ -19,7 +45,109 @@ def get_auth_metadata_plugin(config: Config): Args: config: Feast Configuration object """ - return GoogleOpenIDAuthMetadataPlugin(config) + if AuthProvider(config.get(CONFIG_CORE_AUTH_PROVIDER)) == AuthProvider.GOOGLE: + return GoogleOpenIDAuthMetadataPlugin(config) + elif AuthProvider(config.get(CONFIG_CORE_AUTH_PROVIDER)) == AuthProvider.OAUTH: + return OAuthMetadataPlugin(config) + else: + raise RuntimeError( + "Could not determine OAuth provider." + 'Must be set to either "google" or "oauth"' + ) + + +class OAuthMetadataPlugin(grpc.AuthMetadataPlugin): + """A `gRPC AuthMetadataPlugin`_ that inserts the credentials into each + request. + + .. _gRPC AuthMetadataPlugin: + http://www.grpc.io/grpc/python/grpc.html#grpc.AuthMetadataPlugin + """ + + def __init__(self, config: Config): + """ + Initializes an OAuthMetadataPlugin, used to sign gRPC requests + Args: + config: Feast Configuration object + """ + super(OAuthMetadataPlugin, self).__init__() + + self._static_token = None + self._token = None + + # If provided, set a static token + if config.exists(CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY): + self._static_token = config.get(CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY) + + if ( + config.exists(CONFIG_OAUTH_GRANT_TYPE_KEY) + and config.exists(CONFIG_OAUTH_CLIENT_ID_KEY) + and config.exists(CONFIG_OAUTH_CLIENT_SECRET_KEY) + and config.exists(CONFIG_OAUTH_AUDIENCE_KEY) + and config.exists(CONFIG_OAUTH_TOKEN_REQUEST_URL_KEY) + ): + self._refresh_token(config) + else: + raise RuntimeError( + " Please ensure that the " + "necessary parameters are passed to the client - " + "oauth_grant_type, oauth_client_id, oauth_client_secret, " + "oauth_audience, oauth_token_request_url." + ) + + def get_signed_meta(self): + """ Creates a signed authorization metadata token.""" + return (("authorization", "Bearer {}".format(self._token)),) + + def _refresh_token(self, config: Config): + """ Refreshes OAuth token and persists it in memory """ + + # Use static token if available + if self._static_token: + self._token = self._static_token + return + + import json + import requests + + headers_token = {"content-type": "application/json"} + data_token = { + "grant_type": config.get(CONFIG_OAUTH_GRANT_TYPE_KEY), + "client_id": config.get(CONFIG_OAUTH_CLIENT_ID_KEY), + "client_secret": config.get(CONFIG_OAUTH_CLIENT_SECRET_KEY), + "audience": config.get(CONFIG_OAUTH_AUDIENCE_KEY), + } + data_token = json.dumps(data_token) + response_token = requests.post( + config.get(CONFIG_OAUTH_TOKEN_REQUEST_URL_KEY), + headers=headers_token, + data=data_token, + ) + if response_token.status_code == HTTPStatus.OK: + return response_token.json().get("access_token") + else: + raise RuntimeError( + f"Could not fetch OAuth token, got response : {response_token.status_code}" + ) + + def set_static_token(self, token): + """ + Define a static token to return + + Args: + token: String token + """ + self._static_token = token + + def __call__(self, context, callback): + """Passes authorization metadata into the given callback. + + Args: + context (grpc.AuthMetadataContext): The RPC context. + callback (grpc.AuthMetadataPluginCallback): The callback that will + be invoked to pass in the authorization metadata. + """ + callback(self.get_signed_meta(), None) class GoogleOpenIDAuthMetadataPlugin(grpc.AuthMetadataPlugin): @@ -105,13 +233,12 @@ def set_static_token(self, token): """ self._static_token = token + def __call__(self, context, callback): + """Passes authorization metadata into the given callback. -def __call__(self, context, callback): - """Passes authorization metadata into the given callback. - - Args: - context (grpc.AuthMetadataContext): The RPC context. - callback (grpc.AuthMetadataPluginCallback): The callback that will - be invoked to pass in the authorization metadata. - """ - callback(self.get_signed_meta(), None) + Args: + context (grpc.AuthMetadataContext): The RPC context. + callback (grpc.AuthMetadataPluginCallback): The callback that will + be invoked to pass in the authorization metadata. + """ + callback(self.get_signed_meta(), None) diff --git a/sdk/python/setup.py b/sdk/python/setup.py index b0d49ccabcf..f1126c16c80 100644 --- a/sdk/python/setup.py +++ b/sdk/python/setup.py @@ -25,8 +25,8 @@ REQUIRED = [ "Click==7.*", - "google-api-core==1.14.*", - "google-auth==1.6.*", + "google-api-core==1.20.*", + "google-auth<2.0dev,>=1.14.0", "google-cloud-bigquery==1.18.*", "google-cloud-storage==1.20.*", "google-cloud-core==1.0.*", diff --git a/sdk/python/tests/grpc/test_auth.py b/sdk/python/tests/grpc/test_auth.py new file mode 100644 index 00000000000..1ac77c132d3 --- /dev/null +++ b/sdk/python/tests/grpc/test_auth.py @@ -0,0 +1,200 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2018-2020 The Feast Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json +from http import HTTPStatus +from unittest.mock import call, patch + +from pytest import fixture, raises + +from feast.config import Config +from feast.grpc.auth import ( + GoogleOpenIDAuthMetadataPlugin, + OAuthMetadataPlugin, + get_auth_metadata_plugin, +) + +AUDIENCE = "https://testaudience.io/" + +AUTH_URL = "https://test.auth.com/v2/token" + +HEADERS = {"content-type": "application/json"} + +DATA = json.dumps( + { + "grant_type": "client_credentials", + "client_id": "fakeID", + "client_secret": "fakeSecret", + "audience": AUDIENCE, + } +) + + +class MockResponse: + def __init__(self, json_data, status_code): + self.json_data = json_data + self.status_code = status_code + + def json(self): + return self.json_data + + +class GoogleSubprocessResponse: + def __init__(self, output): + self.stdout = output["stdout"] + + +class GoogleDecodeResponse: + def __init__(self, stdout): + self.stdout = stdout + + +class GoogleDefaultResponse: + def __init__(self, id_token): + self.id_token = id_token + + def refresh(self, request): + pass + + +class GoogleDefaultErrorResponse: + def __init__(self, id_token): + self.wrong_attribute = id_token + + def refresh(self, request): + pass + + +@fixture +def config_oauth(): + config_dict = { + "core_url": "localhost:50051", + "core_enable_auth": True, + "core_auth_provider": "oauth", + "oauth_grant_type": "client_credentials", + "oauth_client_id": "fakeID", + "oauth_client_secret": "fakeSecret", + "oauth_audience": AUDIENCE, + "oauth_token_request_url": AUTH_URL, + } + return Config(config_dict) + + +@fixture +def config_google(): + config_dict = { + "core_url": "localhost:50051", + "core_enable_auth": True, + "core_auth_provider": "google", + "oauth_grant_type": "client_credentials", + "oauth_client_id": "fakeID", + "oauth_client_secret": "fakeSecret", + "oauth_audience": AUDIENCE, + "oauth_token_request_url": AUTH_URL, + } + return Config(config_dict) + + +@fixture +def config_with_missing_variable(): + config_dict = { + "core_url": "localhost:50051", + "core_enable_auth": True, + "core_auth_provider": "oauth", + "oauth_grant_type": "client_credentials", + "oauth_client_id": "fakeID", + "oauth_client_secret": "fakeSecret", + "oauth_token_request_url": AUTH_URL, + } + return Config(config_dict) + + +@patch( + "requests.post", + return_value=MockResponse({"access_token": "mock_token"}, HTTPStatus.OK), +) +def test_get_auth_metadata_plugin_oauth_should_pass(post, config_oauth): + auth_metadata_plugin = get_auth_metadata_plugin(config_oauth) + assert isinstance(auth_metadata_plugin, OAuthMetadataPlugin) + assert post.call_count == 1 + assert post.call_args == call(AUTH_URL, headers=HEADERS, data=DATA) + + +@patch( + "requests.post", + return_value=MockResponse({"access_token": "mock_token"}, HTTPStatus.UNAUTHORIZED), +) +def test_get_auth_metadata_plugin_oauth_should_raise_when_response_is_not_200( + post, config_oauth +): + with raises(RuntimeError): + get_auth_metadata_plugin(config_oauth) + assert post.call_count == 1 + assert post.call_args == call(AUTH_URL, headers=HEADERS, data=DATA) + + +def test_get_auth_metadata_plugin_oauth_should_raise_when_config_is_incorrect( + config_with_missing_variable, +): + with raises(RuntimeError): + get_auth_metadata_plugin(config_with_missing_variable) + + +@patch("google.auth.jwt.decode", return_value=GoogleDecodeResponse("jwt_token")) +@patch( + "subprocess.run", + return_value=GoogleSubprocessResponse({"stdout": "std_output".encode("utf-8")}), +) +def test_get_auth_metadata_plugin_google_should_pass_with_token_from_gcloud_sdk( + subprocess, jwt, config_google +): + auth_metadata_plugin = get_auth_metadata_plugin(config_google) + assert isinstance(auth_metadata_plugin, GoogleOpenIDAuthMetadataPlugin) + + +@patch( + "google.auth.default", + return_value=[ + GoogleDefaultResponse("fake_token"), + GoogleDefaultResponse("project_id"), + ], +) +@patch( + "subprocess.run", + return_value=GoogleSubprocessResponse({"stdout": "std_output".encode("utf-8")}), +) +def test_get_auth_metadata_plugin_google_should_pass_with_token_from_google_auth_lib( + subprocess, default, config_google +): + auth_metadata_plugin = get_auth_metadata_plugin(config_google) + assert isinstance(auth_metadata_plugin, GoogleOpenIDAuthMetadataPlugin) + + +@patch( + "google.auth.default", + return_value=[ + GoogleDefaultErrorResponse("fake_token"), + GoogleDefaultErrorResponse("project_id"), + ], +) +@patch( + "subprocess.run", + return_value=GoogleSubprocessResponse({"stdout": "std_output".encode("utf-8")}), +) +def test_get_auth_metadata_plugin_google_should_raise_when_token_validation_fails( + subprocess, default, config_google +): + with raises(RuntimeError): + get_auth_metadata_plugin(config_google) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 1d0a8647525..0c4eddef21c 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -719,7 +719,7 @@ def test_get_batch_features(self, mocked_client, mocker): } ), feature_refs=["driver:driver_id", "driver_id"], - project="driver_project", + default_project="driver_project", ) # Type: GetBatchFeaturesResponse assert response.id == "123" and response.status == JobStatus.JOB_STATUS_DONE diff --git a/tests/e2e/bq/bq-batch-retrieval.py b/tests/e2e/bq/bq-batch-retrieval.py index e5e75eff1a5..3918f69fb3a 100644 --- a/tests/e2e/bq/bq-batch-retrieval.py +++ b/tests/e2e/bq/bq-batch-retrieval.py @@ -3,14 +3,18 @@ import random import time import uuid -from datetime import datetime -from datetime import timedelta +from datetime import datetime, timedelta from urllib.parse import urlparse import numpy as np import pandas as pd import pytest import pytz +from google.cloud import bigquery, storage +from google.cloud.storage import Blob +from google.protobuf.duration_pb2 import Duration +from pandavro import to_avro + from feast.client import Client from feast.core.CoreService_pb2 import ListStoresRequest from feast.core.IngestionJob_pb2 import IngestionJobStatus diff --git a/tests/e2e/bq/feature-stats.py b/tests/e2e/bq/feature-stats.py index 6650d67fb19..72778e8f28d 100644 --- a/tests/e2e/bq/feature-stats.py +++ b/tests/e2e/bq/feature-stats.py @@ -1,11 +1,16 @@ +import os +import time +import uuid +from datetime import datetime, timedelta + import pandas as pd import pytest import pytz -import uuid -import time -import os -from datetime import datetime, timedelta +from google.protobuf.duration_pb2 import Duration +from google.protobuf.json_format import MessageToDict +import tensorflow_data_validation as tfdv +from deepdiff import DeepDiff from feast.client import Client from feast.entity import Entity from feast.feature import Feature diff --git a/tests/e2e/redis/basic-ingest-redis-serving.py b/tests/e2e/redis/basic-ingest-redis-serving.py index 5facfe7ee39..9607767f6f8 100644 --- a/tests/e2e/redis/basic-ingest-redis-serving.py +++ b/tests/e2e/redis/basic-ingest-redis-serving.py @@ -1,6 +1,7 @@ -import pytest import math +import os import random +import tempfile import time import grpc from collections import OrderedDict @@ -20,15 +21,28 @@ from feast.wait import wait_retry_backoff from feast.constants import FEAST_DEFAULT_OPTIONS, CONFIG_PROJECT_KEY from google.protobuf.duration_pb2 import Duration +import uuid from datetime import datetime -import pytz -import pandas as pd +import grpc import numpy as np -import tempfile -import os +import pandas as pd +import pytest +import pytz +from google.protobuf.duration_pb2 import Duration + +from feast.client import Client +from feast.constants import CONFIG_PROJECT_KEY, FEAST_DEFAULT_OPTIONS +from feast.core import CoreService_pb2 +from feast.core.CoreService_pb2_grpc import CoreServiceStub +from feast.core.IngestionJob_pb2 import IngestionJobStatus +from feast.entity import Entity from feast.feature import Feature -import uuid +from feast.feature_set import FeatureSet, FeatureSetRef +from feast.serving.ServingService_pb2 import (GetOnlineFeaturesRequest, + GetOnlineFeaturesResponse) +from feast.type_map import ValueType +from feast.types.Value_pb2 import Value as Value FLOAT_TOLERANCE = 0.00001 PROJECT_NAME = 'basic_' + uuid.uuid4().hex.upper()[0:6] diff --git a/tests/e2e/requirements.txt b/tests/e2e/requirements.txt index 040febc4983..d219c3280b6 100644 --- a/tests/e2e/requirements.txt +++ b/tests/e2e/requirements.txt @@ -9,5 +9,5 @@ pytest-timeout==1.3.3 pytest-ordering==0.6.* tensorflow-data-validation==0.21.2 deepdiff==4.3.2 -tensorflow==2.1.0 +tensorflow==2.2.0 tfx-bsl==0.21.* # lock to 0.21 \ No newline at end of file From ae57b74ad875f096b7a9a9e7860bebc5085eacbc Mon Sep 17 00:00:00 2001 From: dr3s Date: Tue, 16 Jun 2020 17:50:16 -0400 Subject: [PATCH 03/18] PR Feedback fixes * Updating docs * Pr comments and end to end test fixes * Adding get token check and minor refactoring * changing autovalue to lombok * removed lognet --- core/pom.xml | 1 - .../authorization/AuthorizationResult.java | 20 +++++++------- .../feast/core/service/ProjectService.java | 4 +-- sdk/python/feast/client.py | 8 ++++-- sdk/python/feast/grpc/auth.py | 6 ++--- sdk/python/tests/grpc/test_auth.py | 27 ++++++++++--------- sdk/python/tests/test_client.py | 2 +- 7 files changed, 37 insertions(+), 31 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 8f920fbe0c0..83bfc84a1f0 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -175,7 +175,6 @@ org.springframework.boot spring-boot-configuration-processor - io.grpc diff --git a/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java b/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java index 6b0d41f43e7..5b5f492f09e 100644 --- a/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java +++ b/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java @@ -16,26 +16,28 @@ */ package feast.core.auth.authorization; -import com.google.auto.value.AutoValue; import java.util.Optional; import javax.annotation.Nullable; +import lombok.AllArgsConstructor; +import lombok.Getter; + +@Getter +@AllArgsConstructor +public class AuthorizationResult { -@AutoValue -public abstract class AuthorizationResult { public static AuthorizationResult create( @Nullable boolean allowed, @Nullable String failureReason) { - return new AutoValue_AuthorizationResult(allowed, Optional.ofNullable(failureReason)); + return new AuthorizationResult(allowed, Optional.ofNullable(failureReason)); } public static AuthorizationResult failed(@Nullable String failureReason) { - return new AutoValue_AuthorizationResult(false, Optional.ofNullable(failureReason)); + return new AuthorizationResult(false, Optional.ofNullable(failureReason)); } public static AuthorizationResult success() { - return new AutoValue_AuthorizationResult(true, Optional.empty()); + return new AuthorizationResult(true, Optional.empty()); } - public abstract boolean allowed(); - - public abstract Optional failureReason(); + private boolean allowed; + private Optional failureReason; } diff --git a/core/src/main/java/feast/core/service/ProjectService.java b/core/src/main/java/feast/core/service/ProjectService.java index e8fe49f8f91..59e02f67771 100644 --- a/core/src/main/java/feast/core/service/ProjectService.java +++ b/core/src/main/java/feast/core/service/ProjectService.java @@ -115,8 +115,8 @@ public void checkIfProjectMember(SecurityContext securityContext, String project return; } AuthorizationResult result = this.authorizationProvider.checkAccess(project, authentication); - if (!result.allowed()) { - throw new AccessDeniedException(result.failureReason().orElse("AccessDenied")); + if (!result.isAllowed()) { + throw new AccessDeniedException(result.getFailureReason().orElse("AccessDenied")); } } } diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 808c739788d..5804a3ec6b6 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -107,6 +107,10 @@ def __init__(self, options: Optional[Dict[str, str]] = None, **kwargs): project: Sets the active project. This field is optional. core_secure: Use client-side SSL/TLS for Core gRPC API serving_secure: Use client-side SSL/TLS for Serving gRPC API + core_enable_auth: Enable authentication and authorization + core_auth_provider: Authentication provider – "google" or "oauth" + if core_auth_provider is "oauth", the following fields are mandatory – + oauth_grant_type, oauth_client_id, oauth_client_secret, oauth_audience, oauth_token_request_url Args: options: Configuration options to initialize client with @@ -583,13 +587,13 @@ def get_batch_features( >>> } >>> ) >>> feature_retrieval_job = feast_client.get_batch_features( - >>> feature_refs, entity_rows, default_project="my_project") + >>> feature_refs, entity_rows, project="my_project") >>> df = feature_retrieval_job.to_dataframe() >>> print(df) """ feature_references = _build_feature_references( - feature_refs=feature_refs, default_project=default_project + feature_ref_strs=feature_refs, project=project ) # Retrieve serving information to determine store type and diff --git a/sdk/python/feast/grpc/auth.py b/sdk/python/feast/grpc/auth.py index 581d45f559d..3fc4e261415 100644 --- a/sdk/python/feast/grpc/auth.py +++ b/sdk/python/feast/grpc/auth.py @@ -78,8 +78,8 @@ def __init__(self, config: Config): # If provided, set a static token if config.exists(CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY): self._static_token = config.get(CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY) - - if ( + self._refresh_token(config) + elif ( config.exists(CONFIG_OAUTH_GRANT_TYPE_KEY) and config.exists(CONFIG_OAUTH_CLIENT_ID_KEY) and config.exists(CONFIG_OAUTH_CLIENT_SECRET_KEY) @@ -124,7 +124,7 @@ def _refresh_token(self, config: Config): data=data_token, ) if response_token.status_code == HTTPStatus.OK: - return response_token.json().get("access_token") + self._token = response_token.json().get("access_token") else: raise RuntimeError( f"Could not fetch OAuth token, got response : {response_token.status_code}" diff --git a/sdk/python/tests/grpc/test_auth.py b/sdk/python/tests/grpc/test_auth.py index 1ac77c132d3..90896ee925f 100644 --- a/sdk/python/tests/grpc/test_auth.py +++ b/sdk/python/tests/grpc/test_auth.py @@ -51,12 +51,7 @@ def json(self): return self.json_data -class GoogleSubprocessResponse: - def __init__(self, output): - self.stdout = output["stdout"] - - -class GoogleDecodeResponse: +class GoogleMockResponse: def __init__(self, stdout): self.stdout = stdout @@ -130,6 +125,9 @@ def test_get_auth_metadata_plugin_oauth_should_pass(post, config_oauth): assert isinstance(auth_metadata_plugin, OAuthMetadataPlugin) assert post.call_count == 1 assert post.call_args == call(AUTH_URL, headers=HEADERS, data=DATA) + assert auth_metadata_plugin.get_signed_meta() == ( + ("authorization", "Bearer mock_token"), + ) @patch( @@ -152,16 +150,18 @@ def test_get_auth_metadata_plugin_oauth_should_raise_when_config_is_incorrect( get_auth_metadata_plugin(config_with_missing_variable) -@patch("google.auth.jwt.decode", return_value=GoogleDecodeResponse("jwt_token")) +@patch("google.auth.jwt.decode", return_value=GoogleMockResponse("jwt_token")) @patch( - "subprocess.run", - return_value=GoogleSubprocessResponse({"stdout": "std_output".encode("utf-8")}), + "subprocess.run", return_value=GoogleMockResponse("std_output".encode("utf-8")), ) def test_get_auth_metadata_plugin_google_should_pass_with_token_from_gcloud_sdk( subprocess, jwt, config_google ): auth_metadata_plugin = get_auth_metadata_plugin(config_google) assert isinstance(auth_metadata_plugin, GoogleOpenIDAuthMetadataPlugin) + assert auth_metadata_plugin.get_signed_meta() == ( + ("authorization", "Bearer std_output"), + ) @patch( @@ -172,14 +172,16 @@ def test_get_auth_metadata_plugin_google_should_pass_with_token_from_gcloud_sdk( ], ) @patch( - "subprocess.run", - return_value=GoogleSubprocessResponse({"stdout": "std_output".encode("utf-8")}), + "subprocess.run", return_value=GoogleMockResponse("std_output".encode("utf-8")), ) def test_get_auth_metadata_plugin_google_should_pass_with_token_from_google_auth_lib( subprocess, default, config_google ): auth_metadata_plugin = get_auth_metadata_plugin(config_google) assert isinstance(auth_metadata_plugin, GoogleOpenIDAuthMetadataPlugin) + assert auth_metadata_plugin.get_signed_meta() == ( + ("authorization", "Bearer fake_token"), + ) @patch( @@ -190,8 +192,7 @@ def test_get_auth_metadata_plugin_google_should_pass_with_token_from_google_auth ], ) @patch( - "subprocess.run", - return_value=GoogleSubprocessResponse({"stdout": "std_output".encode("utf-8")}), + "subprocess.run", return_value=GoogleMockResponse("std_output".encode("utf-8")), ) def test_get_auth_metadata_plugin_google_should_raise_when_token_validation_fails( subprocess, default, config_google diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 0c4eddef21c..1d0a8647525 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -719,7 +719,7 @@ def test_get_batch_features(self, mocked_client, mocker): } ), feature_refs=["driver:driver_id", "driver_id"], - default_project="driver_project", + project="driver_project", ) # Type: GetBatchFeaturesResponse assert response.id == "123" and response.status == JobStatus.JOB_STATUS_DONE From b810d1d1529e2d99f7752eb3b9020e0e1ea20e29 Mon Sep 17 00:00:00 2001 From: Jayanth Kumar M J Date: Thu, 18 Jun 2020 15:16:06 -0400 Subject: [PATCH 04/18] Authentication and Authorization End to end test PR comments and end to end test with google auth. --- .prow/config.yaml | 2 +- core/pom.xml | 1 - .../authorization/AuthorizationResult.java | 23 ++++++++++++++++ infra/scripts/test-end-to-end-batch.sh | 26 +------------------ sdk/python/feast/client.py | 4 --- tests/e2e/requirements.txt | 2 +- 6 files changed, 26 insertions(+), 32 deletions(-) diff --git a/.prow/config.yaml b/.prow/config.yaml index b32b38f6555..1d79fdb3d2d 100644 --- a/.prow/config.yaml +++ b/.prow/config.yaml @@ -157,7 +157,7 @@ presubmits: - name: test-end-to-end-auth decorate: true - always_run: false + always_run: true spec: containers: - image: maven:3.6-jdk-11 diff --git a/core/pom.xml b/core/pom.xml index 83bfc84a1f0..a77f48112a7 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -288,7 +288,6 @@ spring-boot-test-autoconfigure test - javax.xml.bind jaxb-api diff --git a/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java b/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java index 5b5f492f09e..bbf4220b31e 100644 --- a/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java +++ b/core/src/main/java/feast/core/auth/authorization/AuthorizationResult.java @@ -21,19 +21,42 @@ import lombok.AllArgsConstructor; import lombok.Getter; +/** + * Implementation of AuthorizationProvider will return AuthorizationResult after validating incoming + * requests to Feast Core. AuthorizationResult provides methods to check if user is authorized to + * perform an action or not. + */ @Getter @AllArgsConstructor public class AuthorizationResult { + /** + * Method to create AuthorizationResult Object. + * + * @param allowed True If user is authorized, False otherwise. + * @param failureReason Reason for authorization failure, if any + * @return AuthorizationResult Object. + */ public static AuthorizationResult create( @Nullable boolean allowed, @Nullable String failureReason) { return new AuthorizationResult(allowed, Optional.ofNullable(failureReason)); } + /** + * Method to create failed AuthorizationResult Object. + * + * @param failureReason Reason for authorization failure, if any or Null + * @return AuthorizationResult Object. + */ public static AuthorizationResult failed(@Nullable String failureReason) { return new AuthorizationResult(false, Optional.ofNullable(failureReason)); } + /** + * Method to create Success AuthorizationResult Object. + * + * @return AuthorizationResult Object. + */ public static AuthorizationResult success() { return new AuthorizationResult(true, Optional.empty()); } diff --git a/infra/scripts/test-end-to-end-batch.sh b/infra/scripts/test-end-to-end-batch.sh index 6e6bf01964b..60f09cb4163 100755 --- a/infra/scripts/test-end-to-end-batch.sh +++ b/infra/scripts/test-end-to-end-batch.sh @@ -56,27 +56,8 @@ else echo "[DEBUG] Skipping building jars" fi -echo " -============================================================ -Starting Feast Core -============================================================ -" # Start Feast Core in background -cat < /tmp/core.application.yml -grpc: - server: - port: 6565 - security: - enabled: false - -security: - authentication: - enabled: false - provider: None - authorization: - enabled: false - provider: None - +cat < /tmp/core.warehouse.application.yml feast: jobs: polling_interval_milliseconds: 10000 @@ -86,11 +67,6 @@ feast: type: DirectRunner options: tempLocation: gs://${TEMP_BUCKET}/tempLocation -grpc: - server: - port: 6565 - security: - enabled: false EOF diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 5804a3ec6b6..370d325dc42 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -592,10 +592,6 @@ def get_batch_features( >>> print(df) """ - feature_references = _build_feature_references( - feature_ref_strs=feature_refs, project=project - ) - # Retrieve serving information to determine store type and # staging location serving_info = self._serving_service.GetFeastServingInfo( diff --git a/tests/e2e/requirements.txt b/tests/e2e/requirements.txt index d219c3280b6..040febc4983 100644 --- a/tests/e2e/requirements.txt +++ b/tests/e2e/requirements.txt @@ -9,5 +9,5 @@ pytest-timeout==1.3.3 pytest-ordering==0.6.* tensorflow-data-validation==0.21.2 deepdiff==4.3.2 -tensorflow==2.2.0 +tensorflow==2.1.0 tfx-bsl==0.21.* # lock to 0.21 \ No newline at end of file From 2ec7e27ed8b6d637568a651af89070dd8553df9c Mon Sep 17 00:00:00 2001 From: e10112844 Date: Thu, 18 Jun 2020 17:57:42 -0400 Subject: [PATCH 05/18] list_features_by_ref to use updated core service method. --- sdk/python/feast/client.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 370d325dc42..723884f9fbb 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -506,8 +506,6 @@ def list_features_by_ref( >>> features = list_features_by_ref(project="test_project", entities=["driver_id"], labels={"key1":"val1","key2":"val2"}) >>> print(features) """ - self._connect_core() - if project is None: if self.project is not None: project = self.project @@ -518,7 +516,7 @@ def list_features_by_ref( project=project, entities=entities, labels=labels ) - feature_protos = self._core_service_stub.ListFeatures( + feature_protos = self._core_service.ListFeatures( ListFeaturesRequest(filter=filter) ) # type: ListFeaturesResponse From 983dffd162a1ed9b9b723961fec6f077888ddc62 Mon Sep 17 00:00:00 2001 From: e10112844 Date: Fri, 19 Jun 2020 15:42:28 -0400 Subject: [PATCH 06/18] Fix for failing test after rebase. --- core/pom.xml | 23 +++++++++++++++++++ .../feast/core/grpc/CoreServiceAuthTest.java | 21 +++++++++-------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index a77f48112a7..4ced0bf9d65 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -278,6 +278,13 @@ 2.2.0 test + + org.mockito + mockito-core + 2.23.0 + test + + org.springframework.boot spring-boot-test @@ -309,6 +316,22 @@ 6.1.2.Final + + javax.validation + validation-api + 2.0.0.Final + + + org.hibernate.validator + hibernate-validator + 6.1.2.Final + + + org.hibernate.validator + hibernate-validator-annotation-processor + 6.1.2.Final + + org.flywaydb flyway-core diff --git a/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java index 87e29902e57..35f47a22725 100644 --- a/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java +++ b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java @@ -41,6 +41,7 @@ import feast.proto.core.CoreServiceProto.ApplyFeatureSetResponse; import feast.proto.core.FeatureSetProto; import feast.proto.core.FeatureSetProto.FeatureSetStatus; +import feast.proto.core.SourceProto; import feast.proto.core.SourceProto.KafkaSourceConfig; import feast.proto.core.SourceProto.SourceType; import feast.proto.types.ValueProto.ValueType.Enum; @@ -137,16 +138,16 @@ void canApplyFeatureSetIfProjectMember() throws InvalidProtocolBufferException { private FeatureSet newDummyFeatureSet(String name, int version, String project) { Feature feature = new Feature("feature", Enum.INT64); Entity entity = new Entity("entity", Enum.STRING); - - Source defaultSource = - new Source( - SourceType.KAFKA, - KafkaSourceConfig.newBuilder() - .setBootstrapServers("kafka:9092") - .setTopic("my-topic") - .build(), - true); - + SourceProto.Source sourceSpec = + SourceProto.Source.newBuilder() + .setType(SourceType.KAFKA) + .setKafkaSourceConfig( + KafkaSourceConfig.newBuilder() + .setBootstrapServers("kafka:9092") + .setTopic("my-topic") + .build()) + .build(); + Source defaultSource = Source.fromProto(sourceSpec); FeatureSet fs = new FeatureSet( name, From 080916c4f2e1cf450f41eb699c1090345582a687 Mon Sep 17 00:00:00 2001 From: e10112844 Date: Fri, 19 Jun 2020 15:59:18 -0400 Subject: [PATCH 07/18] Removed boolean conversion. --- tests/e2e/redis/basic-ingest-redis-serving.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/e2e/redis/basic-ingest-redis-serving.py b/tests/e2e/redis/basic-ingest-redis-serving.py index 9607767f6f8..d93ed499871 100644 --- a/tests/e2e/redis/basic-ingest-redis-serving.py +++ b/tests/e2e/redis/basic-ingest-redis-serving.py @@ -117,8 +117,7 @@ def allow_dirty(pytestconfig): @pytest.fixture(scope='module') def enable_auth(pytestconfig): - return True if pytestconfig.getoption( - "enable_auth").lower() == "true" else False + return pytestconfig.getoption("enable_auth") @pytest.fixture(scope='module') From 23329050e72c9d6b2bf9338c14f4591426b11ece Mon Sep 17 00:00:00 2001 From: e10112844 Date: Fri, 19 Jun 2020 16:26:37 -0400 Subject: [PATCH 08/18] Added missing fixture dependency. --- tests/e2e/redis/basic-ingest-redis-serving.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/redis/basic-ingest-redis-serving.py b/tests/e2e/redis/basic-ingest-redis-serving.py index d93ed499871..618cc45d9a1 100644 --- a/tests/e2e/redis/basic-ingest-redis-serving.py +++ b/tests/e2e/redis/basic-ingest-redis-serving.py @@ -121,7 +121,7 @@ def enable_auth(pytestconfig): @pytest.fixture(scope='module') -def client(core_url, serving_url, allow_dirty): +def client(core_url, serving_url, allow_dirty, enable_auth): # Get client for core and serving # if enable_auth is True, Google Id token will be # passed in the metadata for authentication. From 741153260c3a30cf3e5a83935dc9f0ac281db2d6 Mon Sep 17 00:00:00 2001 From: e10112844 Date: Fri, 19 Jun 2020 18:15:32 -0400 Subject: [PATCH 09/18] Corrected overriding yaml for auth, removed redundant echo. --- infra/scripts/test-end-to-end.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/infra/scripts/test-end-to-end.sh b/infra/scripts/test-end-to-end.sh index 34bcba54103..29d146de4d3 100755 --- a/infra/scripts/test-end-to-end.sh +++ b/infra/scripts/test-end-to-end.sh @@ -45,11 +45,6 @@ else echo "[DEBUG] Skipping building jars" fi -echo " -============================================================ -Starting Feast Core -============================================================ -" # Start Feast Core with auth if enabled cat < /tmp/core.warehouse.application.yml feast: @@ -59,7 +54,12 @@ feast: runners: - name: direct type: DirectRunner - options:{} + options: {} + stream: + type: kafka + options: + topic: feast-features + bootstrapServers: "kafka:9092,localhost:9094" security: authentication: enabled: true From 3e2fbc73e5d09408940ad1ca893c4cfab5913ce1 Mon Sep 17 00:00:00 2001 From: Willem Pienaar <6728866+woop@users.noreply.github.com> Date: Mon, 22 Jun 2020 01:34:02 +0800 Subject: [PATCH 10/18] Allow gcloud command to run without exiting tests --- infra/scripts/test-end-to-end.sh | 1 + sdk/python/feast/grpc/auth.py | 17 ++++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/infra/scripts/test-end-to-end.sh b/infra/scripts/test-end-to-end.sh index 29d146de4d3..d0885978477 100755 --- a/infra/scripts/test-end-to-end.sh +++ b/infra/scripts/test-end-to-end.sh @@ -90,6 +90,7 @@ ORIGINAL_DIR=$(pwd) cd tests/e2e set +e +export GOOGLE_APPLICATION_CREDENTIALS=/etc/gcloud/service-account.json pytest redis/* --enable_auth=${ENABLE_AUTH} --junitxml=${LOGS_ARTIFACT_PATH}/python-sdk-test-report.xml TEST_EXIT_CODE=$? diff --git a/sdk/python/feast/grpc/auth.py b/sdk/python/feast/grpc/auth.py index 3fc4e261415..d146d851a9d 100644 --- a/sdk/python/feast/grpc/auth.py +++ b/sdk/python/feast/grpc/auth.py @@ -193,15 +193,15 @@ def _refresh_token(self): from google.auth import jwt import subprocess - cli_output = subprocess.run( - ["gcloud", "auth", "print-identity-token"], stdout=subprocess.PIPE - ) - token = cli_output.stdout.decode("utf-8").strip() try: + cli_output = subprocess.run( + ["gcloud", "auth", "print-identity-token"], stdout=subprocess.PIPE + ) + token = cli_output.stdout.decode("utf-8").strip() jwt.decode(token, verify=False) # Ensure the token is valid self._token = token return - except ValueError: + except (ValueError, FileNotFoundError): pass # GCloud command not successful # Try to use Google Auth library to find ID Token @@ -218,10 +218,9 @@ def _refresh_token(self): # Raise exception otherwise raise RuntimeError( - "Could not determine Google ID token. Please ensure that the " - "Google Cloud SDK is installed or that a service account can be " - "found using the GOOGLE_APPLICATION_CREDENTIALS environmental " - "variable." + "Could not determine Google ID token. Please ensure that the Google Cloud SDK is installed and run: " + "\"gcloud auth application-default login\" or ensure that a service account can be found by setting" + " the GOOGLE_APPLICATION_CREDENTIALS environmental variable to its path." ) def set_static_token(self, token): From b52dfde1b45ee0dc2fcfb14240130ed6ad91f68d Mon Sep 17 00:00:00 2001 From: e10112844 Date: Sun, 21 Jun 2020 13:37:48 -0400 Subject: [PATCH 11/18] Lint error. --- sdk/python/feast/client.py | 1 + sdk/python/feast/grpc/auth.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 723884f9fbb..f4c822e018a 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -29,6 +29,7 @@ import pyarrow as pa import pyarrow.parquet as pq from google.protobuf.timestamp_pb2 import Timestamp + import feast.grpc.auth as feast_auth from feast.config import Config from feast.constants import ( diff --git a/sdk/python/feast/grpc/auth.py b/sdk/python/feast/grpc/auth.py index d146d851a9d..0ec08d33655 100644 --- a/sdk/python/feast/grpc/auth.py +++ b/sdk/python/feast/grpc/auth.py @@ -219,7 +219,7 @@ def _refresh_token(self): # Raise exception otherwise raise RuntimeError( "Could not determine Google ID token. Please ensure that the Google Cloud SDK is installed and run: " - "\"gcloud auth application-default login\" or ensure that a service account can be found by setting" + '"gcloud auth application-default login" or ensure that a service account can be found by setting' " the GOOGLE_APPLICATION_CREDENTIALS environmental variable to its path." ) From 2beee64f76a6501c14a7ad85126d82f5e8ca1668 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Mon, 22 Jun 2020 10:42:11 +0800 Subject: [PATCH 12/18] Set GOOGLE_APPLICATION_CREDENTIALS for auth testW --- infra/scripts/test-end-to-end.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infra/scripts/test-end-to-end.sh b/infra/scripts/test-end-to-end.sh index d0885978477..dece13f4e01 100755 --- a/infra/scripts/test-end-to-end.sh +++ b/infra/scripts/test-end-to-end.sh @@ -8,7 +8,7 @@ if [[ -n $1 ]]; then fi echo "Authenication enabled : ${ENABLE_AUTH}" -test -z ${GOOGLE_APPLICATION_CREDENTIALS} && GOOGLE_APPLICATION_CREDENTIALS="/etc/service-account/service-account.json" +test -z ${GOOGLE_APPLICATION_CREDENTIALS} && GOOGLE_APPLICATION_CREDENTIALS="/etc/gcloud/service-account.json" test -z ${SKIP_BUILD_JARS} && SKIP_BUILD_JARS="false" test -z ${GOOGLE_CLOUD_PROJECT} && GOOGLE_CLOUD_PROJECT="kf-feast" test -z ${TEMP_BUCKET} && TEMP_BUCKET="feast-templocation-kf-feast" From 0e0b7a9a068310507f8b00a4a0580dc2efefeb16 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Mon, 22 Jun 2020 11:04:15 +0800 Subject: [PATCH 13/18] Add gcloud sdk installation to auth tests --- infra/scripts/test-end-to-end.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/infra/scripts/test-end-to-end.sh b/infra/scripts/test-end-to-end.sh index dece13f4e01..d32f68a6464 100755 --- a/infra/scripts/test-end-to-end.sh +++ b/infra/scripts/test-end-to-end.sh @@ -35,6 +35,7 @@ This script will run end-to-end tests for Feast Core and Online Serving. source ${SCRIPTS_DIR}/setup-common-functions.sh install_test_tools +install_gcloud_sdk install_and_start_local_redis install_and_start_local_postgres install_and_start_local_zookeeper_and_kafka From 812f199a3ae685fd52d3abd7be6b233004d25576 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Tue, 23 Jun 2020 22:02:13 +0800 Subject: [PATCH 14/18] Add transactions back to projects, revert to AccessManagementService, and make Auth Config generic --- core/pom.xml | 39 ------------------- .../DefaultJwtAuthenticationProvider.java | 6 +-- .../feast/core/config/FeastProperties.java | 2 + .../java/feast/core/grpc/CoreServiceImpl.java | 19 ++++----- .../feast/core/grpc/HealthServiceImpl.java | 10 ++--- ...vice.java => AccessManagementService.java} | 10 +++-- core/src/main/resources/application.yml | 3 ++ .../feast/core/grpc/CoreServiceAuthTest.java | 10 +++-- ....java => AccessManagementServiceTest.java} | 21 +++++----- sdk/python/tests/grpc/test_auth.py | 3 +- sdk/python/tests/test_client.py | 4 +- sdk/python/tests/test_config.py | 1 - sdk/python/tests/test_feature_set.py | 2 +- 13 files changed, 49 insertions(+), 81 deletions(-) rename core/src/main/java/feast/core/service/{ProjectService.java => AccessManagementService.java} (95%) rename core/src/test/java/feast/core/service/{ProjectServiceTest.java => AccessManagementServiceTest.java} (85%) diff --git a/core/pom.xml b/core/pom.xml index 4ced0bf9d65..737a5c1d685 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -278,12 +278,6 @@ 2.2.0 test - - org.mockito - mockito-core - 2.23.0 - test - org.springframework.boot @@ -300,45 +294,12 @@ jaxb-api - - javax.validation - validation-api - 2.0.0.Final - - - org.hibernate.validator - hibernate-validator - 6.1.2.Final - - - org.hibernate.validator - hibernate-validator-annotation-processor - 6.1.2.Final - - - - javax.validation - validation-api - 2.0.0.Final - - - org.hibernate.validator - hibernate-validator - 6.1.2.Final - - - org.hibernate.validator - hibernate-validator-annotation-processor - 6.1.2.Final - org.flywaydb flyway-core ${flyway.version} - - org.springframework javax.validation validation-api diff --git a/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java index 2036adb1671..bff96ca3ece 100644 --- a/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java +++ b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java @@ -37,14 +37,10 @@ public class DefaultJwtAuthenticationProvider implements AuthenticationProvider * one option is currently configurable, the jwkEndpointURI. */ public DefaultJwtAuthenticationProvider(Map options) { - // Endpoint used to retrieve certificates to validate JWT token - String jwkEndpointURI = "https://www.googleapis.com/oauth2/v3/certs"; + String jwkEndpointURI = options.get("jwkEndpointURI"); // Provide a custom endpoint to retrieve certificates - if (options != null) { - jwkEndpointURI = options.get("jwkEndpointURI"); - } authProvider = new JwtAuthenticationProvider(NimbusJwtDecoder.withJwkSetUri(jwkEndpointURI).build()); authProvider.setJwtAuthenticationConverter(new JwtAuthenticationConverter()); diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index 4bca210cd15..62d0d3bea56 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -283,6 +283,7 @@ public static class AuthenticationProperties { private boolean enabled; // Named authentication provider to use + @OneOfStrings({"jwt"}) private String provider; // K/V options to initialize the provider with @@ -297,6 +298,7 @@ public static class AuthorizationProperties { private boolean enabled; // Named authorization provider to use. + @OneOfStrings({"none", "keto"}) private String provider; // K/V options to initialize the provider with diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java index f4f79288198..d014f582a3f 100644 --- a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java @@ -22,8 +22,8 @@ import feast.core.exception.RetrievalException; import feast.core.grpc.interceptors.MonitoringInterceptor; import feast.core.model.Project; +import feast.core.service.AccessManagementService; import feast.core.service.JobService; -import feast.core.service.ProjectService; import feast.core.service.SpecService; import feast.core.service.StatsService; import feast.proto.core.CoreServiceGrpc.CoreServiceImplBase; @@ -48,17 +48,17 @@ public class CoreServiceImpl extends CoreServiceImplBase { private SpecService specService; private JobService jobService; private StatsService statsService; - private ProjectService projectService; + private AccessManagementService accessManagementService; @Autowired public CoreServiceImpl( SpecService specService, - ProjectService projectService, + AccessManagementService accessManagementService, StatsService statsService, JobService jobService, FeastProperties feastProperties) { this.specService = specService; - this.projectService = projectService; + this.accessManagementService = accessManagementService; this.jobService = jobService; this.feastProperties = feastProperties; this.statsService = statsService; @@ -178,7 +178,7 @@ public void listStores( public void applyFeatureSet( ApplyFeatureSetRequest request, StreamObserver responseObserver) { - projectService.checkIfProjectMember( + accessManagementService.checkIfProjectMember( SecurityContextHolder.getContext(), request.getFeatureSet().getSpec().getProject()); try { @@ -217,7 +217,7 @@ public void updateStore( public void createProject( CreateProjectRequest request, StreamObserver responseObserver) { try { - projectService.createProject(request.getName()); + accessManagementService.createProject(request.getName()); responseObserver.onNext(CreateProjectResponse.getDefaultInstance()); responseObserver.onCompleted(); } catch (Exception e) { @@ -231,10 +231,11 @@ public void createProject( public void archiveProject( ArchiveProjectRequest request, StreamObserver responseObserver) { - projectService.checkIfProjectMember(SecurityContextHolder.getContext(), request.getName()); + accessManagementService.checkIfProjectMember( + SecurityContextHolder.getContext(), request.getName()); try { - projectService.archiveProject(request.getName()); + accessManagementService.archiveProject(request.getName()); responseObserver.onNext(ArchiveProjectResponse.getDefaultInstance()); responseObserver.onCompleted(); } catch (IllegalArgumentException e) { @@ -259,7 +260,7 @@ public void archiveProject( public void listProjects( ListProjectsRequest request, StreamObserver responseObserver) { try { - List projects = projectService.listProjects(); + List projects = accessManagementService.listProjects(); responseObserver.onNext( ListProjectsResponse.newBuilder() .addAllProjects(projects.stream().map(Project::getName).collect(Collectors.toList())) diff --git a/core/src/main/java/feast/core/grpc/HealthServiceImpl.java b/core/src/main/java/feast/core/grpc/HealthServiceImpl.java index 0a1f10109ca..b83a05b7f03 100644 --- a/core/src/main/java/feast/core/grpc/HealthServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/HealthServiceImpl.java @@ -16,7 +16,7 @@ */ package feast.core.grpc; -import feast.core.service.ProjectService; +import feast.core.service.AccessManagementService; import io.grpc.Status; import io.grpc.health.v1.HealthGrpc.HealthImplBase; import io.grpc.health.v1.HealthProto.HealthCheckRequest; @@ -30,18 +30,18 @@ @Slf4j @GrpcService public class HealthServiceImpl extends HealthImplBase { - private final ProjectService projectService; + private final AccessManagementService accessManagementService; @Autowired - public HealthServiceImpl(ProjectService projectService) { - this.projectService = projectService; + public HealthServiceImpl(AccessManagementService accessManagementService) { + this.accessManagementService = accessManagementService; } @Override public void check( HealthCheckRequest request, StreamObserver responseObserver) { try { - projectService.listProjects(); + accessManagementService.listProjects(); responseObserver.onNext( HealthCheckResponse.newBuilder().setStatus(ServingStatus.SERVING).build()); responseObserver.onCompleted(); diff --git a/core/src/main/java/feast/core/service/ProjectService.java b/core/src/main/java/feast/core/service/AccessManagementService.java similarity index 95% rename from core/src/main/java/feast/core/service/ProjectService.java rename to core/src/main/java/feast/core/service/AccessManagementService.java index 59e02f67771..f0ab5714644 100644 --- a/core/src/main/java/feast/core/service/ProjectService.java +++ b/core/src/main/java/feast/core/service/AccessManagementService.java @@ -31,16 +31,17 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContext; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; @Slf4j @Service -public class ProjectService { +public class AccessManagementService { private SecurityProperties securityProperties; private AuthorizationProvider authorizationProvider; private ProjectRepository projectRepository; - public ProjectService( + public AccessManagementService( FeastProperties feastProperties, ProjectRepository projectRepository, AuthorizationProvider authorizationProvider) { @@ -50,7 +51,7 @@ public ProjectService( } @Autowired - public ProjectService( + public AccessManagementService( FeastProperties feastProperties, ProjectRepository projectRepository, ObjectProvider authorizationProvider) { @@ -68,6 +69,7 @@ public ProjectService( * * @param name Name of project to be created */ + @Transactional public void createProject(String name) { if (projectRepository.existsById(name)) { throw new IllegalArgumentException(String.format("Project already exists: %s", name)); @@ -81,6 +83,7 @@ public void createProject(String name) { * * @param name Name of the project to be archived */ + @Transactional public void archiveProject(String name) { Optional project = projectRepository.findById(name); if (!project.isPresent()) { @@ -99,6 +102,7 @@ public void archiveProject(String name) { * * @return List of active projects */ + @Transactional public List listProjects() { return projectRepository.findAllByArchivedIsFalse(); } diff --git a/core/src/main/resources/application.yml b/core/src/main/resources/application.yml index cda671ce72d..3fb513c3355 100644 --- a/core/src/main/resources/application.yml +++ b/core/src/main/resources/application.yml @@ -79,6 +79,9 @@ feast: authentication: enabled: false provider: jwt + options: + jwkEndpointURI: "https://www.googleapis.com/oauth2/v3/certs"; + authorization: enabled: false provider: none diff --git a/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java index 35f47a22725..471b2c78d48 100644 --- a/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java +++ b/core/src/test/java/feast/core/grpc/CoreServiceAuthTest.java @@ -33,8 +33,8 @@ import feast.core.model.Feature; import feast.core.model.FeatureSet; import feast.core.model.Source; +import feast.core.service.AccessManagementService; import feast.core.service.JobService; -import feast.core.service.ProjectService; import feast.core.service.SpecService; import feast.core.service.StatsService; import feast.proto.core.CoreServiceProto.ApplyFeatureSetRequest; @@ -63,7 +63,7 @@ class CoreServiceAuthTest { private CoreServiceImpl coreService; - private ProjectService projectService; + private AccessManagementService accessManagementService; @Mock private SpecService specService; @Mock private ProjectRepository projectRepository; @@ -80,9 +80,11 @@ class CoreServiceAuthTest { sp.setAuthorization(authProp); FeastProperties feastProperties = new FeastProperties(); feastProperties.setSecurity(sp); - projectService = new ProjectService(feastProperties, projectRepository, authProvider); + accessManagementService = + new AccessManagementService(feastProperties, projectRepository, authProvider); coreService = - new CoreServiceImpl(specService, projectService, statsService, jobService, feastProperties); + new CoreServiceImpl( + specService, accessManagementService, statsService, jobService, feastProperties); } @Test diff --git a/core/src/test/java/feast/core/service/ProjectServiceTest.java b/core/src/test/java/feast/core/service/AccessManagementServiceTest.java similarity index 85% rename from core/src/test/java/feast/core/service/ProjectServiceTest.java rename to core/src/test/java/feast/core/service/AccessManagementServiceTest.java index ae9fc661b3e..6e7affa4128 100644 --- a/core/src/test/java/feast/core/service/ProjectServiceTest.java +++ b/core/src/test/java/feast/core/service/AccessManagementServiceTest.java @@ -38,13 +38,13 @@ import org.junit.rules.ExpectedException; import org.mockito.Mock; -public class ProjectServiceTest { +public class AccessManagementServiceTest { @Mock private ProjectRepository projectRepository; @Rule public final ExpectedException expectedException = ExpectedException.none(); - private ProjectService projectService; + private AccessManagementService accessManagementService; @Before public void setUp() { @@ -57,8 +57,9 @@ public void setUp() { sp.setAuthorization(authProp); FeastProperties feastProperties = new FeastProperties(); feastProperties.setSecurity(sp); - projectService = - new ProjectService(feastProperties, projectRepository, mock(AuthorizationProvider.class)); + accessManagementService = + new AccessManagementService( + feastProperties, projectRepository, mock(AuthorizationProvider.class)); } @Test @@ -71,7 +72,7 @@ public void shouldCreateProjectIfItDoesntExist() { String projectName = "project1"; Project project = new Project(projectName); when(projectRepository.saveAndFlush(any(Project.class))).thenReturn(project); - projectService.createProject(projectName); + accessManagementService.createProject(projectName); verify(projectRepository, times(1)).saveAndFlush(any()); } @@ -79,28 +80,28 @@ public void shouldCreateProjectIfItDoesntExist() { public void shouldNotCreateProjectIfItExist() { String projectName = "project1"; when(projectRepository.existsById(projectName)).thenReturn(true); - projectService.createProject(projectName); + accessManagementService.createProject(projectName); } @Test public void shouldArchiveProjectIfItExists() { String projectName = "project1"; when(projectRepository.findById(projectName)).thenReturn(Optional.of(new Project(projectName))); - projectService.archiveProject(projectName); + accessManagementService.archiveProject(projectName); verify(projectRepository, times(1)).saveAndFlush(any(Project.class)); } @Test public void shouldNotArchiveDefaultProject() { expectedException.expect(IllegalArgumentException.class); - this.projectService.archiveProject(Project.DEFAULT_NAME); + this.accessManagementService.archiveProject(Project.DEFAULT_NAME); } @Test(expected = IllegalArgumentException.class) public void shouldNotArchiveProjectIfItIsAlreadyArchived() { String projectName = "project1"; when(projectRepository.findById(projectName)).thenReturn(Optional.empty()); - projectService.archiveProject(projectName); + accessManagementService.archiveProject(projectName); } @Test @@ -109,7 +110,7 @@ public void shouldListProjects() { Project project = new Project(projectName); List expected = Arrays.asList(project); when(projectRepository.findAllByArchivedIsFalse()).thenReturn(expected); - List actual = projectService.listProjects(); + List actual = accessManagementService.listProjects(); Assert.assertEquals(expected, actual); } } diff --git a/sdk/python/tests/grpc/test_auth.py b/sdk/python/tests/grpc/test_auth.py index 90896ee925f..5045f357a0c 100644 --- a/sdk/python/tests/grpc/test_auth.py +++ b/sdk/python/tests/grpc/test_auth.py @@ -17,14 +17,13 @@ from http import HTTPStatus from unittest.mock import call, patch -from pytest import fixture, raises - from feast.config import Config from feast.grpc.auth import ( GoogleOpenIDAuthMetadataPlugin, OAuthMetadataPlugin, get_auth_metadata_plugin, ) +from pytest import fixture, raises AUDIENCE = "https://testaudience.io/" diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 1d0a8647525..c529d97ffdd 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -22,14 +22,13 @@ import grpc import pandas as pd import pandavro -import pytest from google.protobuf.duration_pb2 import Duration -from mock import MagicMock, patch from pytz import timezone import dataframes import feast.core.CoreService_pb2_grpc as Core import feast.serving.ServingService_pb2_grpc as Serving +import pytest from feast.client import Client from feast.core.CoreService_pb2 import ( GetFeastCoreVersionResponse, @@ -72,6 +71,7 @@ DisallowAuthInterceptor, ) from feast_serving_server import ServingServicer +from mock import MagicMock, patch CORE_URL = "core.feast.example.com" SERVING_URL = "serving.example.com" diff --git a/sdk/python/tests/test_config.py b/sdk/python/tests/test_config.py index 9ed34a736a2..8c30f2562cc 100644 --- a/sdk/python/tests/test_config.py +++ b/sdk/python/tests/test_config.py @@ -16,7 +16,6 @@ from tempfile import mkstemp import pytest - from feast.config import Config diff --git a/sdk/python/tests/test_feature_set.py b/sdk/python/tests/test_feature_set.py index 5f36d8fe691..31689403a64 100644 --- a/sdk/python/tests/test_feature_set.py +++ b/sdk/python/tests/test_feature_set.py @@ -19,12 +19,12 @@ import grpc import pandas as pd -import pytest import pytz from google.protobuf import json_format import dataframes import feast.core.CoreService_pb2_grpc as Core +import pytest from feast.client import Client from feast.entity import Entity from feast.feature_set import ( From c164525117da4e02c8d5fdf99fdb60598332f8ff Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Tue, 23 Jun 2020 23:04:01 +0800 Subject: [PATCH 15/18] Fix typo in core configuration --- core/src/main/resources/application.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/application.yml b/core/src/main/resources/application.yml index 3fb513c3355..57728e0dc8a 100644 --- a/core/src/main/resources/application.yml +++ b/core/src/main/resources/application.yml @@ -80,7 +80,7 @@ feast: enabled: false provider: jwt options: - jwkEndpointURI: "https://www.googleapis.com/oauth2/v3/certs"; + jwkEndpointURI: "https://www.googleapis.com/oauth2/v3/certs" authorization: enabled: false From 4c967292395582cbbdfa2c6ea49a5b196b708cba Mon Sep 17 00:00:00 2001 From: Willem Pienaar <6728866+woop@users.noreply.github.com> Date: Tue, 23 Jun 2020 23:09:07 +0800 Subject: [PATCH 16/18] Update documentation to remove Google terminology --- .../auth/authentication/DefaultJwtAuthenticationProvider.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java index bff96ca3ece..418107998aa 100644 --- a/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java +++ b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java @@ -25,8 +25,7 @@ import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationProvider; /** - * Google Open ID Authentication Provider. This provider is used to validate incoming requests to - * Feast Core. + * Json Web Token Authentication Provider used to validate incoming requests to Feast Core. */ public class DefaultJwtAuthenticationProvider implements AuthenticationProvider { From ffc94e18fdcb5126fd7cad23e76564a58add73c4 Mon Sep 17 00:00:00 2001 From: Willem Pienaar <6728866+woop@users.noreply.github.com> Date: Tue, 23 Jun 2020 23:12:48 +0800 Subject: [PATCH 17/18] Remove stream specific configuration in e2e tests --- infra/scripts/test-end-to-end.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/infra/scripts/test-end-to-end.sh b/infra/scripts/test-end-to-end.sh index d32f68a6464..6e969bc7fc2 100755 --- a/infra/scripts/test-end-to-end.sh +++ b/infra/scripts/test-end-to-end.sh @@ -56,11 +56,6 @@ feast: - name: direct type: DirectRunner options: {} - stream: - type: kafka - options: - topic: feast-features - bootstrapServers: "kafka:9092,localhost:9094" security: authentication: enabled: true From dee0e4bae65c3e6621a1869e5e9ef5744ea60910 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Tue, 23 Jun 2020 23:22:32 +0800 Subject: [PATCH 18/18] Fix linting --- .../auth/authentication/DefaultJwtAuthenticationProvider.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java index 418107998aa..a39e673adaa 100644 --- a/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java +++ b/core/src/main/java/feast/core/auth/authentication/DefaultJwtAuthenticationProvider.java @@ -24,9 +24,7 @@ import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationProvider; -/** - * Json Web Token Authentication Provider used to validate incoming requests to Feast Core. - */ +/** Json Web Token Authentication Provider used to validate incoming requests to Feast Core. */ public class DefaultJwtAuthenticationProvider implements AuthenticationProvider { private JwtAuthenticationProvider authProvider;