From 07aed5a8c16b7e3d2fdd9d0f9dd9ea3796ca3bbf Mon Sep 17 00:00:00 2001 From: e10112844 Date: Wed, 8 Jul 2020 09:24:52 -0400 Subject: [PATCH 1/8] Authentication and authorization for feast serving, squashed on 07/21 --- auth/pom.xml | 6 + .../feast/auth/config/SecurityConfig.java | 6 +- .../CoreAuthenticationProperties.java | 56 ++++++++ .../credentials/GoogleAuthCredentials.java | 86 +++++++++++++ .../auth/credentials/OAuthCredentials.java | 120 ++++++++++++++++++ .../auth/service/AuthorizationService.java | 63 +++++++++ core/pom.xml | 2 +- .../feast/core/config/FeastProperties.java | 16 +++ .../java/feast/core/grpc/CoreServiceImpl.java | 40 ++++-- .../feast/core/grpc/HealthServiceImpl.java | 10 +- ...gementService.java => ProjectService.java} | 48 +------ .../feast/core/auth/CoreServiceAuthTest.java | 12 +- ...rviceTest.java => ProjectServiceTest.java} | 31 ++--- .../docker-compose/docker-compose.online.yml | 5 + infra/scripts/test-end-to-end-batch.sh | 4 - .../scripts/test-end-to-end-redis-cluster.sh | 4 - infra/scripts/test-end-to-end.sh | 38 ++++-- pom.xml | 3 +- sdk/python/feast/client.py | 13 +- sdk/python/tests/test_client.py | 53 +++++++- serving/pom.xml | 60 +++++++-- .../feast/serving/config/FeastProperties.java | 85 +++++++++++++ .../serving/config/ServingSecurityConfig.java | 94 ++++++++++++++ .../serving/config/SpecServiceConfig.java | 8 +- .../controller/HealthServiceController.java | 4 +- .../ServingServiceGRpcController.java | 32 ++++- .../feast/serving/specs/CoreSpecService.java | 12 +- serving/src/main/resources/application.yml | 42 +++++- .../ServingServiceGRpcControllerTest.java | 62 ++++++++- tests/e2e/redis/basic-ingest-redis-serving.py | 37 ++++-- 30 files changed, 889 insertions(+), 163 deletions(-) create mode 100644 auth/src/main/java/feast/auth/credentials/CoreAuthenticationProperties.java create mode 100644 auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java create mode 100644 auth/src/main/java/feast/auth/credentials/OAuthCredentials.java create mode 100644 auth/src/main/java/feast/auth/service/AuthorizationService.java rename core/src/main/java/feast/core/service/{AccessManagementService.java => ProjectService.java} (53%) rename core/src/test/java/feast/core/service/{AccessManagementServiceTest.java => ProjectServiceTest.java} (72%) create mode 100644 serving/src/main/java/feast/serving/config/ServingSecurityConfig.java diff --git a/auth/pom.xml b/auth/pom.xml index d9b5f78f8d..43abadb508 100644 --- a/auth/pom.xml +++ b/auth/pom.xml @@ -23,6 +23,7 @@ 1.3.2 4.13 2.8.0 + 0.20.0 @@ -153,6 +154,11 @@ junit 4.12 + + com.google.auth + google-auth-library-oauth2-http + ${google-auth-library-oauth2-http-version} + diff --git a/auth/src/main/java/feast/auth/config/SecurityConfig.java b/auth/src/main/java/feast/auth/config/SecurityConfig.java index 378deea195..9c831b3076 100644 --- a/auth/src/main/java/feast/auth/config/SecurityConfig.java +++ b/auth/src/main/java/feast/auth/config/SecurityConfig.java @@ -83,13 +83,13 @@ GrpcAuthenticationReader authenticationReader() { } /** - * Creates an AccessDecisionManager if authorization is enabled. This object determines the policy - * used to make authorization decisions. + * Creates an AccessDecisionManager if authentication is enabled. This object determines the + * policy used to make authentication decisions. * * @return AccessDecisionManager */ @Bean - @ConditionalOnProperty(prefix = "feast.security.authorization", name = "enabled") + @ConditionalOnProperty(prefix = "feast.security.authentication", name = "enabled") AccessDecisionManager accessDecisionManager() { final List> voters = new ArrayList<>(); voters.add(new AccessPredicateVoter()); diff --git a/auth/src/main/java/feast/auth/credentials/CoreAuthenticationProperties.java b/auth/src/main/java/feast/auth/credentials/CoreAuthenticationProperties.java new file mode 100644 index 0000000000..e307dfb1c8 --- /dev/null +++ b/auth/src/main/java/feast/auth/credentials/CoreAuthenticationProperties.java @@ -0,0 +1,56 @@ +/* + * 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.auth.credentials; + +import feast.common.validators.OneOfStrings; +import java.util.Map; + +public class CoreAuthenticationProperties { + // needs to be set to true if authentication is enabled on core + private boolean enabled; + + // authentication provider to use + @OneOfStrings({"google", "oauth"}) + private String provider; + + // K/V options to initialize the provider. + Map options; + + public boolean isEnabled() { + return enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + public String getProvider() { + return provider; + } + + public void setProvider(String provider) { + this.provider = provider; + } + + public Map getOptions() { + return options; + } + + public void setOptions(Map options) { + this.options = options; + } +} diff --git a/auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java b/auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java new file mode 100644 index 0000000000..b5f6a1d786 --- /dev/null +++ b/auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java @@ -0,0 +1,86 @@ +/* + * 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.auth.credentials; + +import static io.grpc.Metadata.ASCII_STRING_MARSHALLER; + +import com.google.auth.oauth2.IdTokenCredentials; +import com.google.auth.oauth2.ServiceAccountCredentials; +import io.grpc.CallCredentials; +import io.grpc.Metadata; +import io.grpc.Status; +import java.io.IOException; +import java.time.Instant; +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.Executor; + +/* + * Google auth provider's callCredentials Implementation for serving. + * Used by CoreSpecService to connect to core. + */ +public class GoogleAuthCredentials extends CallCredentials { + private String accessToken; + private Instant tokenExpiryTime; + private final IdTokenCredentials credentials; + private static final String BEARER_TYPE = "Bearer"; + private static final Metadata.Key AUTHORIZATION_METADATA_KEY = + Metadata.Key.of("Authorization", ASCII_STRING_MARSHALLER); + + public GoogleAuthCredentials(Map options) throws IOException { + + String targetAudience = options.getOrDefault("audience", "https://localhost"); + ServiceAccountCredentials serviceCreds = + (ServiceAccountCredentials) + ServiceAccountCredentials.getApplicationDefault() + .createScoped(Arrays.asList("openid", "email")); + + credentials = + IdTokenCredentials.newBuilder() + .setIdTokenProvider(serviceCreds) + .setTargetAudience(targetAudience) + .build(); + } + + @Override + public void applyRequestMetadata( + RequestInfo requestInfo, Executor appExecutor, MetadataApplier applier) { + appExecutor.execute( + () -> { + try { + // Fetches new token if it is not available or if token has expired. + if (this.accessToken == null || Instant.now().isAfter(this.tokenExpiryTime)) { + credentials.refreshIfExpired(); + this.accessToken = credentials.getIdToken().getTokenValue(); + this.tokenExpiryTime = credentials.getIdToken().getExpirationTime().toInstant(); + } + Metadata headers = new Metadata(); + headers.put( + AUTHORIZATION_METADATA_KEY, String.format("%s %s", BEARER_TYPE, this.accessToken)); + applier.apply(headers); + } catch (Throwable e) { + applier.fail(Status.UNAUTHENTICATED.withCause(e)); + } + }); + } + + @Override + public void thisUsesUnstableApi() { + // TODO Auto-generated method stub + + } +} diff --git a/auth/src/main/java/feast/auth/credentials/OAuthCredentials.java b/auth/src/main/java/feast/auth/credentials/OAuthCredentials.java new file mode 100644 index 0000000000..e7ad47f377 --- /dev/null +++ b/auth/src/main/java/feast/auth/credentials/OAuthCredentials.java @@ -0,0 +1,120 @@ +/* + * 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.auth.credentials; + +import static io.grpc.Metadata.ASCII_STRING_MARSHALLER; + +import com.nimbusds.jose.util.JSONObjectUtils; +import io.grpc.CallCredentials; +import io.grpc.Metadata; +import io.grpc.Status; +import java.time.Instant; +import java.util.Map; +import java.util.concurrent.Executor; +import javax.security.sasl.AuthenticationException; +import net.minidev.json.JSONObject; +import okhttp3.FormBody; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; +import org.springframework.security.oauth2.jwt.NimbusJwtDecoder; + +/* + * Oauth Credentials Implementation for serving. + * Used by CoreSpecService to connect to core. + */ +public class OAuthCredentials extends CallCredentials { + + private static final String JWK_ENDPOINT_URI = "jwkEndpointURI"; + static final String APPLICATION_JSON = "application/json"; + static final String CONTENT_TYPE = "content-type"; + static final String BEARER_TYPE = "Bearer"; + static final String GRANT_TYPE = "grant_type"; + static final String CLIENT_ID = "client_id"; + static final String CLIENT_SECRET = "client_secret"; + static final String AUDIENCE = "audience"; + static final String OAUTH_URL = "oauth_url"; + static final Metadata.Key AUTHORIZATION_METADATA_KEY = + Metadata.Key.of("Authorization", ASCII_STRING_MARSHALLER); + + private OkHttpClient httpClient; + private Request request; + private String accessToken; + private Instant tokenExpiryTime; + private NimbusJwtDecoder jwtDecoder; + + public OAuthCredentials(Map options) { + this.httpClient = new OkHttpClient(); + if (!(options.containsKey(GRANT_TYPE) + && options.containsKey(CLIENT_ID) + && options.containsKey(AUDIENCE) + && options.containsKey(CLIENT_SECRET) + && options.containsKey(OAUTH_URL) + && options.containsKey(JWK_ENDPOINT_URI))) { + throw new AssertionError( + "please configure the properties:" + + " grant_type, client_id, client_secret, audience, oauth_url, jwkEndpointURI"); + } + RequestBody requestBody = + new FormBody.Builder() + .add(GRANT_TYPE, options.get(GRANT_TYPE)) + .add(CLIENT_ID, options.get(CLIENT_ID)) + .add(CLIENT_SECRET, options.get(CLIENT_SECRET)) + .add(AUDIENCE, options.get(AUDIENCE)) + .build(); + this.request = + new Request.Builder() + .url(options.get(OAUTH_URL)) + .addHeader(CONTENT_TYPE, APPLICATION_JSON) + .post(requestBody) + .build(); + this.jwtDecoder = NimbusJwtDecoder.withJwkSetUri(options.get(JWK_ENDPOINT_URI)).build(); + } + + @Override + public void thisUsesUnstableApi() { + // TODO Auto-generated method stub + + } + + @Override + public void applyRequestMetadata( + RequestInfo requestInfo, Executor appExecutor, MetadataApplier applier) { + appExecutor.execute( + () -> { + try { + // Fetches new token if it is not available or if token has expired. + if (this.accessToken == null || Instant.now().isAfter(this.tokenExpiryTime)) { + Response response = httpClient.newCall(request).execute(); + if (!response.isSuccessful()) { + throw new AuthenticationException(response.message()); + } + JSONObject json = JSONObjectUtils.parse(response.body().string()); + this.accessToken = json.getAsString("access_token"); + this.tokenExpiryTime = jwtDecoder.decode(this.accessToken).getExpiresAt(); + } + Metadata headers = new Metadata(); + headers.put( + AUTHORIZATION_METADATA_KEY, String.format("%s %s", BEARER_TYPE, this.accessToken)); + applier.apply(headers); + } catch (Throwable e) { + applier.fail(Status.UNAUTHENTICATED.withCause(e)); + } + }); + } +} diff --git a/auth/src/main/java/feast/auth/service/AuthorizationService.java b/auth/src/main/java/feast/auth/service/AuthorizationService.java new file mode 100644 index 0000000000..0d0d4c4934 --- /dev/null +++ b/auth/src/main/java/feast/auth/service/AuthorizationService.java @@ -0,0 +1,63 @@ +/* + * 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.auth.service; + +import feast.auth.authorization.AuthorizationProvider; +import feast.auth.authorization.AuthorizationResult; +import feast.auth.config.SecurityProperties; +import lombok.AllArgsConstructor; +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; + +@AllArgsConstructor +@Service +public class AuthorizationService { + + private final SecurityProperties securityProperties; + private final AuthorizationProvider authorizationProvider; + + @Autowired + public AuthorizationService( + SecurityProperties securityProperties, + ObjectProvider authorizationProvider) { + this.securityProperties = securityProperties; + this.authorizationProvider = authorizationProvider.getIfAvailable(); + } + + /** + * Determine whether a user has access to a project. + * + * @param securityContext Spring Security Context used to identify a user or service. + * @param project Name of the project for which membership should be tested. + */ + public void authorizeRequest(SecurityContext securityContext, String project) { + Authentication authentication = securityContext.getAuthentication(); + if (!this.securityProperties.getAuthorization().isEnabled()) { + return; + } + + AuthorizationResult result = + this.authorizationProvider.checkAccessToProject(project, authentication); + if (!result.isAllowed()) { + throw new AccessDeniedException(result.getFailureReason().orElse("AccessDenied")); + } + } +} diff --git a/core/pom.xml b/core/pom.xml index 1ac4136ec2..bacc113352 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -126,7 +126,7 @@ org.springframework.security.oauth spring-security-oauth2 - 2.5.0.RELEASE + ${spring.security.oauth2.version} org.springframework.security diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index 799000631d..c50b32b174 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -17,6 +17,8 @@ package feast.core.config; import feast.auth.config.SecurityProperties; +import feast.auth.config.SecurityProperties.AuthenticationProperties; +import feast.auth.config.SecurityProperties.AuthorizationProperties; import feast.common.validators.OneOfStrings; import feast.core.config.FeastProperties.StreamProperties.FeatureStreamOptions; import java.net.InetAddress; @@ -278,5 +280,19 @@ public void validate() { + e.getMessage()); } } + + // Validate AuthenticationProperties + Set> authenticationPropsViolations = + validator.validate(getSecurity().getAuthentication()); + if (!authenticationPropsViolations.isEmpty()) { + throw new ConstraintViolationException(authenticationPropsViolations); + } + + // Validate AuthorizationProperties + Set> authorizationPropsViolations = + validator.validate(getSecurity().getAuthorization()); + if (!authorizationPropsViolations.isEmpty()) { + throw new ConstraintViolationException(authorizationPropsViolations); + } } } diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java index e0698c3e61..558933931e 100644 --- a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java @@ -18,12 +18,13 @@ import com.google.api.gax.rpc.InvalidArgumentException; import com.google.protobuf.InvalidProtocolBufferException; +import feast.auth.service.AuthorizationService; import feast.core.config.FeastProperties; 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; @@ -49,20 +50,23 @@ public class CoreServiceImpl extends CoreServiceImplBase { private SpecService specService; private JobService jobService; private StatsService statsService; - private AccessManagementService accessManagementService; + private ProjectService projectService; + private final AuthorizationService authorizationService; @Autowired public CoreServiceImpl( SpecService specService, - AccessManagementService accessManagementService, + ProjectService projectService, StatsService statsService, JobService jobService, - FeastProperties feastProperties) { + FeastProperties feastProperties, + AuthorizationService authorizationService) { this.specService = specService; - this.accessManagementService = accessManagementService; + this.projectService = projectService; this.jobService = jobService; this.feastProperties = feastProperties; this.statsService = statsService; + this.authorizationService = authorizationService; } @Override @@ -180,9 +184,11 @@ public void applyFeatureSet( ApplyFeatureSetRequest request, StreamObserver responseObserver) { String projectId = null; + try { projectId = request.getFeatureSet().getSpec().getProject(); - accessManagementService.checkIfProjectMember(SecurityContextHolder.getContext(), projectId); + authorizationService.authorizeRequest( + SecurityContextHolder.getContext(), projectId); ApplyFeatureSetResponse response = specService.applyFeatureSet(request.getFeatureSet()); responseObserver.onNext(response); responseObserver.onCompleted(); @@ -225,7 +231,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) { @@ -238,12 +244,11 @@ public void createProject( @Override public void archiveProject( ArchiveProjectRequest request, StreamObserver responseObserver) { - - accessManagementService.checkIfProjectMember( - SecurityContextHolder.getContext(), request.getName()); - + String projectId = null; try { - accessManagementService.archiveProject(request.getName()); + projectId = request.getName(); + authorizationService.authorizeRequest(SecurityContextHolder.getContext(), projectId); + projectService.archiveProject(projectId); responseObserver.onNext(ArchiveProjectResponse.getDefaultInstance()); responseObserver.onCompleted(); } catch (IllegalArgumentException e) { @@ -257,7 +262,14 @@ public void archiveProject( log.error("Attempted to archive an unsupported project:", e); responseObserver.onError( Status.UNIMPLEMENTED.withDescription(e.getMessage()).withCause(e).asRuntimeException()); - } catch (Exception e) { + } catch (AccessDeniedException e) { + log.info(String.format("User prevented from accessing project: %s", projectId)); + responseObserver.onError( + Status.PERMISSION_DENIED + .withDescription(e.getMessage()) + .withCause(e) + .asRuntimeException()); + } catch (Exception e) { log.error("Exception has occurred in the createProject method: ", e); responseObserver.onError( Status.INTERNAL.withDescription(e.getMessage()).withCause(e).asRuntimeException()); @@ -268,7 +280,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/grpc/HealthServiceImpl.java b/core/src/main/java/feast/core/grpc/HealthServiceImpl.java index b83a05b7f0..0a1f10109c 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; @@ -30,18 +30,18 @@ @Slf4j @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/service/AccessManagementService.java b/core/src/main/java/feast/core/service/ProjectService.java similarity index 53% rename from core/src/main/java/feast/core/service/AccessManagementService.java rename to core/src/main/java/feast/core/service/ProjectService.java index 342455e551..308c79bccf 100644 --- a/core/src/main/java/feast/core/service/AccessManagementService.java +++ b/core/src/main/java/feast/core/service/ProjectService.java @@ -16,50 +16,24 @@ */ package feast.core.service; -import feast.auth.authorization.AuthorizationProvider; -import feast.auth.authorization.AuthorizationResult; -import feast.auth.config.SecurityProperties; -import feast.core.config.FeastProperties; 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.access.AccessDeniedException; -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; - // TODO: Remove duplication of constructor - public AccessManagementService( - FeastProperties feastProperties, - ProjectRepository projectRepository, - AuthorizationProvider authorizationProvider) { - this.projectRepository = projectRepository; - this.authorizationProvider = authorizationProvider; - this.securityProperties = feastProperties.getSecurity(); - } - @Autowired - public AccessManagementService( - FeastProperties feastProperties, - ProjectRepository projectRepository, - ObjectProvider authorizationProvider) { + public ProjectService(ProjectRepository projectRepository) { this.projectRepository = projectRepository; - this.authorizationProvider = authorizationProvider.getIfUnique(); - this.securityProperties = feastProperties.getSecurity(); } /** @@ -104,22 +78,4 @@ 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 projectId Id (name) of the project for which membership should be tested. - */ - public void checkIfProjectMember(SecurityContext securityContext, String projectId) { - Authentication authentication = securityContext.getAuthentication(); - if (!this.securityProperties.getAuthorization().isEnabled()) { - return; - } - AuthorizationResult result = - this.authorizationProvider.checkAccessToProject(projectId, authentication); - if (!result.isAllowed()) { - throw new AccessDeniedException(result.getFailureReason().orElse("Access Denied")); - } - } } diff --git a/core/src/test/java/feast/core/auth/CoreServiceAuthTest.java b/core/src/test/java/feast/core/auth/CoreServiceAuthTest.java index 06f63f96d8..f9e8becd0e 100644 --- a/core/src/test/java/feast/core/auth/CoreServiceAuthTest.java +++ b/core/src/test/java/feast/core/auth/CoreServiceAuthTest.java @@ -27,6 +27,7 @@ import feast.auth.authorization.AuthorizationProvider; import feast.auth.authorization.AuthorizationResult; import feast.auth.config.SecurityProperties; +import feast.auth.service.AuthorizationService; import feast.core.config.FeastProperties; import feast.core.dao.ProjectRepository; import feast.core.grpc.CoreServiceImpl; @@ -34,8 +35,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; @@ -61,7 +62,7 @@ public class CoreServiceAuthTest { private CoreServiceImpl coreService; - private AccessManagementService accessManagementService; + private ProjectService projectService; @Mock private SpecService specService; @Mock private ProjectRepository projectRepository; @@ -78,11 +79,12 @@ public CoreServiceAuthTest() { sp.setAuthorization(authProp); FeastProperties feastProperties = new FeastProperties(); feastProperties.setSecurity(sp); - accessManagementService = - new AccessManagementService(feastProperties, projectRepository, authProvider); + projectService = new ProjectService(projectRepository); + AuthorizationService authService = + new AuthorizationService(feastProperties.getSecurity(), authProvider); coreService = new CoreServiceImpl( - specService, accessManagementService, statsService, jobService, feastProperties); + specService, projectService, statsService, jobService, feastProperties, authService); } @Test diff --git a/core/src/test/java/feast/core/service/AccessManagementServiceTest.java b/core/src/test/java/feast/core/service/ProjectServiceTest.java similarity index 72% rename from core/src/test/java/feast/core/service/AccessManagementServiceTest.java rename to core/src/test/java/feast/core/service/ProjectServiceTest.java index 1def250037..e09580f24e 100644 --- a/core/src/test/java/feast/core/service/AccessManagementServiceTest.java +++ b/core/src/test/java/feast/core/service/ProjectServiceTest.java @@ -22,9 +22,6 @@ import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; -import feast.auth.authorization.AuthorizationProvider; -import feast.auth.config.SecurityProperties; -import feast.core.config.FeastProperties; import feast.core.dao.ProjectRepository; import feast.core.model.Project; import java.util.Arrays; @@ -37,28 +34,18 @@ import org.junit.rules.ExpectedException; import org.mockito.Mock; -public class AccessManagementServiceTest { +public class ProjectServiceTest { @Mock private ProjectRepository projectRepository; @Rule public final ExpectedException expectedException = ExpectedException.none(); - private AccessManagementService accessManagementService; + private ProjectService projectService; @Before public void setUp() { initMocks(this); projectRepository = mock(ProjectRepository.class); - SecurityProperties.AuthorizationProperties authProp = - new SecurityProperties.AuthorizationProperties(); - authProp.setEnabled(false); - SecurityProperties sp = new SecurityProperties(); - sp.setAuthorization(authProp); - FeastProperties feastProperties = new FeastProperties(); - feastProperties.setSecurity(sp); - - accessManagementService = - new AccessManagementService( - feastProperties, projectRepository, mock(AuthorizationProvider.class)); + projectService = new ProjectService(projectRepository); } @Test @@ -66,7 +53,7 @@ public void shouldCreateProjectIfItDoesntExist() { String projectName = "project1"; Project project = new Project(projectName); when(projectRepository.saveAndFlush(project)).thenReturn(project); - accessManagementService.createProject(projectName); + projectService.createProject(projectName); verify(projectRepository, times(1)).saveAndFlush(project); } @@ -74,7 +61,7 @@ public void shouldCreateProjectIfItDoesntExist() { public void shouldNotCreateProjectIfItExist() { String projectName = "project1"; when(projectRepository.existsById(projectName)).thenReturn(true); - accessManagementService.createProject(projectName); + projectService.createProject(projectName); } @Test @@ -82,21 +69,21 @@ public void shouldArchiveProjectIfItExists() { String projectName = "project1"; Project project = new Project(projectName); when(projectRepository.findById(projectName)).thenReturn(Optional.of(project)); - accessManagementService.archiveProject(projectName); + projectService.archiveProject(projectName); verify(projectRepository, times(1)).saveAndFlush(project); } @Test public void shouldNotArchiveDefaultProject() { expectedException.expect(IllegalArgumentException.class); - this.accessManagementService.archiveProject(Project.DEFAULT_NAME); + this.projectService.archiveProject(Project.DEFAULT_NAME); } @Test(expected = IllegalArgumentException.class) public void shouldNotArchiveProjectIfItIsAlreadyArchived() { String projectName = "project1"; when(projectRepository.findById(projectName)).thenReturn(Optional.empty()); - accessManagementService.archiveProject(projectName); + projectService.archiveProject(projectName); } @Test @@ -105,7 +92,7 @@ public void shouldListProjects() { Project project = new Project(projectName); List expected = Arrays.asList(project); when(projectRepository.findAllByArchivedIsFalse()).thenReturn(expected); - List actual = accessManagementService.listProjects(); + List actual = projectService.listProjects(); Assert.assertEquals(expected, actual); } } diff --git a/infra/docker-compose/docker-compose.online.yml b/infra/docker-compose/docker-compose.online.yml index b01d0882fb..0e5a3cfaec 100644 --- a/infra/docker-compose/docker-compose.online.yml +++ b/infra/docker-compose/docker-compose.online.yml @@ -5,11 +5,16 @@ services: image: ${FEAST_SERVING_IMAGE}:${FEAST_VERSION} volumes: - ./serving/${FEAST_ONLINE_SERVING_CONFIG}:/etc/feast/application.yml + # Required if authentication is enabled on core and + # provider is 'google'. GOOGLE_APPLICATION_CREDENTIALS is used for connecting to core. + - ./gcp-service-accounts/${FEAST_BATCH_SERVING_GCP_SERVICE_ACCOUNT_KEY}:/etc/gcloud/service-accounts/key.json depends_on: - redis ports: - 6566:6566 restart: on-failure + environment: + GOOGLE_APPLICATION_CREDENTIALS: /etc/gcloud/service-accounts/key.json command: - java - -jar diff --git a/infra/scripts/test-end-to-end-batch.sh b/infra/scripts/test-end-to-end-batch.sh index e81a0ddd6c..fa8ad9c032 100755 --- a/infra/scripts/test-end-to-end-batch.sh +++ b/infra/scripts/test-end-to-end-batch.sh @@ -114,10 +114,6 @@ feast: tracing: enabled: false -grpc: - port: 6566 - enable-reflection: true - server: port: 8081 diff --git a/infra/scripts/test-end-to-end-redis-cluster.sh b/infra/scripts/test-end-to-end-redis-cluster.sh index 9094fc3a2e..be38fe765b 100755 --- a/infra/scripts/test-end-to-end-redis-cluster.sh +++ b/infra/scripts/test-end-to-end-redis-cluster.sh @@ -67,10 +67,6 @@ feast: tracing: enabled: false -grpc: - port: 6566 - enable-reflection: true - spring: main: web-environment: false diff --git a/infra/scripts/test-end-to-end.sh b/infra/scripts/test-end-to-end.sh index 75bacd3560..84d65aebe2 100755 --- a/infra/scripts/test-end-to-end.sh +++ b/infra/scripts/test-end-to-end.sh @@ -2,10 +2,7 @@ set -e set -o pipefail -ENABLE_AUTH="False" -if [[ -n $1 ]]; then - ENABLE_AUTH=$1 -fi +[[ $1 == "True" ]] && ENABLE_AUTH="true" || ENABLE_AUTH="false" echo "Authenication enabled : ${ENABLE_AUTH}" test -z ${GOOGLE_APPLICATION_CREDENTIALS} && GOOGLE_APPLICATION_CREDENTIALS="/etc/gcloud/service-account.json" @@ -60,20 +57,13 @@ feast: authentication: enabled: true provider: jwt + options: + jwkEndpointURI: "https://www.googleapis.com/oauth2/v3/certs" authorization: enabled: false provider: none EOF -if [[ ${ENABLE_AUTH} = "True" ]]; - then - print_banner "Starting 'Feast core with auth'." - start_feast_core /tmp/core.warehouse.application.yml - else - print_banner "Starting 'Feast core without auth'." - start_feast_core -fi - cat < /tmp/serving.warehouse.application.yml feast: stores: @@ -86,8 +76,30 @@ feast: subscriptions: - name: "*" project: "*" + core-authentication: + enabled: $ENABLE_AUTH + provider: google + security: + authentication: + enabled: $ENABLE_AUTH + provider: jwt + authorization: + enabled: false + provider: none EOF +if [[ ${ENABLE_AUTH} = "true" ]]; + then + print_banner "Starting Feast core with auth" + start_feast_core /tmp/core.warehouse.application.yml + print_banner "Starting Feast Serving with auth" + else + print_banner "Starting Feast core without auth" + start_feast_core + print_banner "Starting Feast Serving without auth" +fi + + start_feast_serving /tmp/serving.warehouse.application.yml install_python_with_miniconda_and_feast_sdk diff --git a/pom.xml b/pom.xml index a1a63b6f49..d618d22bde 100644 --- a/pom.xml +++ b/pom.xml @@ -68,7 +68,8 @@ 6.0.8 2.9.9 2.0.2 - + 2.5.0.RELEASE + false diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index c39c696d99..c11c01f64f 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -165,11 +165,11 @@ def _serving_service(self): 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, + enable_auth=self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY), ssl_server_cert_path=self._config.get( CONFIG_SERVING_SERVER_SSL_CERT_KEY ), - auth_metadata_plugin=None, + auth_metadata_plugin=self._auth_metadata, timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), ) self._serving_service_stub = ServingServiceStub(channel) @@ -271,6 +271,7 @@ def version(self): serving_version = self._serving_service.GetFeastServingInfo( GetFeastServingInfoRequest(), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + metadata=self._get_grpc_metadata(), ).version result["serving"] = {"url": self.serving_url, "version": serving_version} @@ -619,6 +620,7 @@ def get_historical_features( serving_info = self._serving_service.GetFeastServingInfo( GetFeastServingInfoRequest(), timeout=self._config.getint(CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY), + metadata=self._get_grpc_metadata(), ) # type: GetFeastServingInfoResponse if serving_info.type != FeastServingType.FEAST_SERVING_TYPE_BATCH: @@ -669,7 +671,9 @@ def get_historical_features( # Retrieve Feast Job object to manage life cycle of retrieval try: - response = self._serving_service.GetBatchFeatures(request) + response = self._serving_service.GetBatchFeatures( + request, metadata=self._get_grpc_metadata() + ) except grpc.RpcError as e: raise grpc.RpcError(e.details()) @@ -722,7 +726,8 @@ def get_online_features( features=_build_feature_references(feature_ref_strs=feature_refs), entity_rows=_infer_online_entity_rows(entity_rows), project=project if project is not None else self.project, - ) + ), + metadata=self._get_grpc_metadata(), ) except grpc.RpcError as e: raise grpc.RpcError(e.details()) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 8712847b4b..11b04f8c5d 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -81,6 +81,7 @@ "TY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDI" "yfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c" ) +AUTH_METADATA = (("authorization", f"Bearer {_FAKE_JWT_TOKEN}"),) class TestClient: @@ -103,6 +104,32 @@ def mock_client(self): client._serving_url = SERVING_URL return client + @pytest.fixture + def mock_client_with_auth(self): + client = Client( + core_url=CORE_URL, + serving_url=SERVING_URL, + core_enable_auth=True, + core_auth_token=_FAKE_JWT_TOKEN, + ) + client._core_url = CORE_URL + client._serving_url = SERVING_URL + return client + + @pytest.fixture + def secure_mock_client_with_auth(self): + client = Client( + core_url=CORE_URL, + serving_url=SERVING_URL, + core_enable_ssl=True, + serving_enable_ssl=True, + core_enable_auth=True, + core_auth_token=_FAKE_JWT_TOKEN, + ) + client._core_url = CORE_URL + client._serving_url = SERVING_URL + return client + @pytest.fixture def server_credentials(self): private_key = pkgutil.get_data(__name__, _PRIVATE_KEY_RESOURCE_PATH) @@ -257,10 +284,21 @@ def test_version(self, mocked_client, mocker): ) @pytest.mark.parametrize( - "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + "mocked_client,auth_metadata", + [ + (pytest.lazy_fixture("mock_client"), ()), + (pytest.lazy_fixture("mock_client_with_auth"), (AUTH_METADATA)), + (pytest.lazy_fixture("secure_mock_client"), ()), + (pytest.lazy_fixture("secure_mock_client_with_auth"), (AUTH_METADATA)), + ], + ids=[ + "mock_client_without_auth", + "mock_client_with_auth", + "secure_mock_client_without_auth", + "secure_mock_client_with_auth", + ], ) - def test_get_online_features(self, mocked_client, mocker): + def test_get_online_features(self, mocked_client, auth_metadata, mocker): ROW_COUNT = 300 mocked_client._serving_service_stub = Serving.ServingServiceStub( @@ -312,7 +350,7 @@ def int_val(x): project="driver_project", ) # type: GetOnlineFeaturesResponse mocked_client._serving_service_stub.GetOnlineFeatures.assert_called_with( - request + request, metadata=auth_metadata ) got_fields = got_response.field_values[0].fields @@ -606,7 +644,12 @@ def test_stop_ingest_job(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [ + pytest.lazy_fixture("mock_client"), + pytest.lazy_fixture("mock_client_with_auth"), + pytest.lazy_fixture("secure_mock_client"), + pytest.lazy_fixture("secure_mock_client_with_auth"), + ], ) def test_get_historical_features(self, mocked_client, mocker): diff --git a/serving/pom.xml b/serving/pom.xml index d00e5e5366..651050d759 100644 --- a/serving/pom.xml +++ b/serving/pom.xml @@ -103,6 +103,12 @@ feast-common ${project.version} + + + dev.feast + feast-auth + ${project.version} + @@ -137,13 +143,7 @@ true - - - io.github.lognet - grpc-spring-boot-starter - - - + org.springframework.boot spring-boot-starter-actuator @@ -266,12 +266,56 @@ embedded-redis test - jakarta.validation jakarta.validation-api ${jakarta.validation.api.version} + + org.springframework.security + spring-security-core + ${spring.security.version} + + + org.springframework.security + spring-security-config + ${spring.security.version} + + + org.springframework.security.oauth + spring-security-oauth2 + ${spring.security.oauth2.version} + + + org.springframework.security + spring-security-oauth2-client + ${spring.security.version} + + + org.springframework.security + spring-security-web + ${spring.security.version} + + + org.springframework.security + spring-security-oauth2-jose + ${spring.security.version} + + + net.devh + grpc-server-spring-boot-starter + ${grpc.spring.boot.starter.version} + + + com.nimbusds + nimbus-jose-jwt + 8.2.1 + + + org.springframework.security + spring-security-oauth2-core + ${spring.security.version} + diff --git a/serving/src/main/java/feast/serving/config/FeastProperties.java b/serving/src/main/java/feast/serving/config/FeastProperties.java index f905f5f5c0..18d9ad7221 100644 --- a/serving/src/main/java/feast/serving/config/FeastProperties.java +++ b/serving/src/main/java/feast/serving/config/FeastProperties.java @@ -25,17 +25,30 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; +import feast.auth.config.SecurityProperties; +import feast.auth.config.SecurityProperties.AuthenticationProperties; +import feast.auth.config.SecurityProperties.AuthorizationProperties; +import feast.auth.credentials.CoreAuthenticationProperties; import feast.proto.core.StoreProto; import java.util.*; import java.util.stream.Collectors; +import javax.annotation.PostConstruct; +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.Positive; import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidHost; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.info.BuildProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; /** Feast Serving properties. */ +@Configuration @ConfigurationProperties(prefix = "feast", ignoreInvalidFields = true) public class FeastProperties { @@ -61,6 +74,41 @@ public FeastProperties() {} /* Feast Core port to connect to. */ @Positive private int coreGrpcPort; + private CoreAuthenticationProperties coreAuthentication; + + public CoreAuthenticationProperties getCoreAuthentication() { + return coreAuthentication; + } + + public void setCoreAuthentication(CoreAuthenticationProperties coreAuthentication) { + this.coreAuthentication = coreAuthentication; + } + + private SecurityProperties security; + + @Bean + SecurityProperties securityProperties() { + return this.getSecurity(); + } + + /** + * Getter for SecurityProperties + * + * @return Returns the {@link SecurityProperties} object. + */ + public SecurityProperties getSecurity() { + return security; + } + + /** + * Setter for SecurityProperties + * + * @param security :input {@link SecurityProperties} object + */ + public void setSecurity(SecurityProperties security) { + this.security = security; + } + /** * Finds and returns the active store * @@ -539,4 +587,41 @@ public void setServiceName(String serviceName) { this.serviceName = serviceName; } } + + /** + * Validates all FeastProperties. This method runs after properties have been initialized and + * individually and conditionally validates each class. + */ + @PostConstruct + public void validate() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + Validator validator = factory.getValidator(); + + // Validate root fields in FeastProperties + Set> violations = validator.validate(this); + if (!violations.isEmpty()) { + throw new ConstraintViolationException(violations); + } + + // Validate CoreAuthenticationProperties + Set> coreAuthenticationPropsViolations = + validator.validate(getCoreAuthentication()); + if (!coreAuthenticationPropsViolations.isEmpty()) { + throw new ConstraintViolationException(coreAuthenticationPropsViolations); + } + + // Validate AuthenticationProperties + Set> authenticationPropsViolations = + validator.validate(getSecurity().getAuthentication()); + if (!authenticationPropsViolations.isEmpty()) { + throw new ConstraintViolationException(authenticationPropsViolations); + } + + // Validate AuthorizationProperties + Set> authorizationPropsViolations = + validator.validate(getSecurity().getAuthorization()); + if (!authorizationPropsViolations.isEmpty()) { + throw new ConstraintViolationException(authorizationPropsViolations); + } + } } diff --git a/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java new file mode 100644 index 0000000000..54753ca6fe --- /dev/null +++ b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java @@ -0,0 +1,94 @@ +/* + * 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.serving.config; + +import feast.auth.credentials.GoogleAuthCredentials; +import feast.auth.credentials.OAuthCredentials; +import io.grpc.CallCredentials; +import java.io.IOException; +import net.devh.boot.grpc.server.security.check.AccessPredicate; +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.ComponentScan; +import org.springframework.context.annotation.Configuration; + +/* + * Copyright 2020 the original author or 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. + */ + +@Configuration +@ComponentScan("feast.auth") +public class ServingSecurityConfig { + + private final FeastProperties feastProperties; + + public ServingSecurityConfig(FeastProperties feastProperties) { + this.feastProperties = feastProperties; + } + + /** + * 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()); + return source; + } + + /** + * Creates a CallCredentials when authentication is enabled on core. This allows serving to + * connect to core with CallCredentials + * + * @return CallCredentials + */ + @Bean + @ConditionalOnProperty(prefix = "feast.core-authentication", name = "enabled") + CallCredentials CoreGrpcAuthenticationCredentials() throws IOException { + switch (feastProperties.getCoreAuthentication().getProvider()) { + case "google": + return new GoogleAuthCredentials(feastProperties.getCoreAuthentication().getOptions()); + case "oauth": + return new OAuthCredentials(feastProperties.getCoreAuthentication().getOptions()); + default: + throw new IllegalArgumentException( + "Please configure an Core Authentication Provider " + + "if you have enabled Authentication on core. " + + "Currently `google` and `oauth` are supported"); + } + } +} diff --git a/serving/src/main/java/feast/serving/config/SpecServiceConfig.java b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java index 0a62557077..75b77a29a0 100644 --- a/serving/src/main/java/feast/serving/config/SpecServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java @@ -21,10 +21,12 @@ import feast.proto.core.StoreProto; import feast.serving.specs.CachedSpecService; import feast.serving.specs.CoreSpecService; +import io.grpc.CallCredentials; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import org.slf4j.Logger; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -58,9 +60,11 @@ public ScheduledExecutorService cachedSpecServiceScheduledExecutorService( } @Bean - public CachedSpecService specService(FeastProperties feastProperties) + public CachedSpecService specService( + FeastProperties feastProperties, ObjectProvider callCredentials) throws InvalidProtocolBufferException, JsonProcessingException { - CoreSpecService coreService = new CoreSpecService(feastCoreHost, feastCorePort); + CoreSpecService coreService = + new CoreSpecService(feastCoreHost, feastCorePort, callCredentials); StoreProto.Store storeProto = feastProperties.getActiveStore().toProto(); CachedSpecService cachedSpecStorage = new CachedSpecService(coreService, storeProto); try { diff --git a/serving/src/main/java/feast/serving/controller/HealthServiceController.java b/serving/src/main/java/feast/serving/controller/HealthServiceController.java index 0810429183..5225a7ea2e 100644 --- a/serving/src/main/java/feast/serving/controller/HealthServiceController.java +++ b/serving/src/main/java/feast/serving/controller/HealthServiceController.java @@ -26,12 +26,12 @@ import io.grpc.health.v1.HealthProto.HealthCheckResponse; import io.grpc.health.v1.HealthProto.HealthCheckResponse.ServingStatus; import io.grpc.stub.StreamObserver; -import org.lognet.springboot.grpc.GRpcService; +import net.devh.boot.grpc.server.service.GrpcService; import org.springframework.beans.factory.annotation.Autowired; // Reference: https://github.com/grpc/grpc/blob/master/doc/health-checking.md -@GRpcService(interceptors = {GrpcMonitoringInterceptor.class}) +@GrpcService(interceptors = {GrpcMonitoringInterceptor.class}) public class HealthServiceController extends HealthImplBase { private CachedSpecService specService; private ServingService servingService; diff --git a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java index 3fae6ae65a..c44737fcc3 100644 --- a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java +++ b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java @@ -16,6 +16,8 @@ */ package feast.serving.controller; +import feast.auth.service.AuthorizationService; +import feast.proto.serving.ServingAPIProto.FeatureReference; import feast.proto.serving.ServingAPIProto.GetBatchFeaturesRequest; import feast.proto.serving.ServingAPIProto.GetBatchFeaturesResponse; import feast.proto.serving.ServingAPIProto.GetFeastServingInfoRequest; @@ -35,11 +37,13 @@ import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.Tracer; -import org.lognet.springboot.grpc.GRpcService; +import java.util.List; +import net.devh.boot.grpc.server.service.GrpcService; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.context.SecurityContextHolder; -@GRpcService(interceptors = {GrpcMonitoringInterceptor.class}) +@GrpcService(interceptors = {GrpcMonitoringInterceptor.class}) public class ServingServiceGRpcController extends ServingServiceImplBase { private static final Logger log = @@ -47,10 +51,15 @@ public class ServingServiceGRpcController extends ServingServiceImplBase { private final ServingService servingService; private final String version; private final Tracer tracer; + private final AuthorizationService authorizationService; @Autowired public ServingServiceGRpcController( - ServingService servingService, FeastProperties feastProperties, Tracer tracer) { + AuthorizationService authorizationService, + ServingService servingService, + FeastProperties feastProperties, + Tracer tracer) { + this.authorizationService = authorizationService; this.servingService = servingService; this.version = feastProperties.getVersion(); this.tracer = tracer; @@ -70,6 +79,13 @@ public void getFeastServingInfo( public void getOnlineFeatures( GetOnlineFeaturesRequest request, StreamObserver responseObserver) { + // authorize for the project in request object. + this.authorizationService.authorizeRequest( + SecurityContextHolder.getContext(), request.getProject()); + // authorize for projects set in feature list, backward compatibility for + // <=v0.5.X + this.checkProjectAccess(request.getFeaturesList()); + Span span = tracer.buildSpan("getOnlineFeatures").start(); try (Scope scope = tracer.scopeManager().activate(span, false)) { RequestHelper.validateOnlineRequest(request); @@ -92,6 +108,7 @@ public void getBatchFeatures( GetBatchFeaturesRequest request, StreamObserver responseObserver) { try { RequestHelper.validateBatchRequest(request); + this.checkProjectAccess(request.getFeaturesList()); GetBatchFeaturesResponse batchFeatures = servingService.getBatchFeatures(request); responseObserver.onNext(batchFeatures); responseObserver.onCompleted(); @@ -116,4 +133,13 @@ public void getJob(GetJobRequest request, StreamObserver respons responseObserver.onError(e); } } + + private void checkProjectAccess(List featureList) { + featureList.stream() + .forEach( + featureRef -> { + this.authorizationService.authorizeRequest( + SecurityContextHolder.getContext(), featureRef.getProject()); + }); + } } diff --git a/serving/src/main/java/feast/serving/specs/CoreSpecService.java b/serving/src/main/java/feast/serving/specs/CoreSpecService.java index e2feaebccb..8dcfd0695e 100644 --- a/serving/src/main/java/feast/serving/specs/CoreSpecService.java +++ b/serving/src/main/java/feast/serving/specs/CoreSpecService.java @@ -24,9 +24,11 @@ import feast.proto.core.CoreServiceProto.UpdateStoreRequest; import feast.proto.core.CoreServiceProto.UpdateStoreResponse; import feast.proto.core.StoreProto.Store; +import io.grpc.CallCredentials; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; import org.slf4j.Logger; +import org.springframework.beans.factory.ObjectProvider; /** Client for interfacing with specs in Feast Core. */ public class CoreSpecService { @@ -34,10 +36,16 @@ public class CoreSpecService { private static final Logger log = org.slf4j.LoggerFactory.getLogger(CoreSpecService.class); private final CoreServiceGrpc.CoreServiceBlockingStub blockingStub; - public CoreSpecService(String feastCoreHost, int feastCorePort) { + public CoreSpecService( + String feastCoreHost, int feastCorePort, ObjectProvider callCredentials) { ManagedChannel channel = ManagedChannelBuilder.forAddress(feastCoreHost, feastCorePort).usePlaintext().build(); - blockingStub = CoreServiceGrpc.newBlockingStub(channel); + CallCredentials creds = callCredentials.getIfAvailable(); + if (creds != null) { + blockingStub = CoreServiceGrpc.newBlockingStub(channel).withCallCredentials(creds); + } else { + blockingStub = CoreServiceGrpc.newBlockingStub(channel); + } } public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest getFeatureSetRequest) { diff --git a/serving/src/main/resources/application.yml b/serving/src/main/resources/application.yml index 2399d132ef..3fef07eb1a 100644 --- a/serving/src/main/resources/application.yml +++ b/serving/src/main/resources/application.yml @@ -3,10 +3,36 @@ feast: # Feast Serving requires connection to Feast Core to retrieve and reload Feast metadata (e.g. FeatureSpecs, Store information) core-host: ${FEAST_CORE_HOST:localhost} core-grpc-port: ${FEAST_CORE_GRPC_PORT:6565} + + core-authentication: + enabled: false # should be set to true if authentication is enabled on core. + provider: google # can be set to `oauth` or `google` + # if google, GOOGLE_APPLICATION_CREDENTIALS environment variable should be set. + options: + #if provider is oauth following properties need to be set, else serving boot up will fail. + oauth_url: https://localhost/oauth/token #oauth token request url + grant_type: client_credentials #oauth grant type + client_id: #oauth client id which will be used for jwt token token request + client_secret: #oauth client secret which will be used for jwt token token request + audience: https://localhost #token audience. + jwkEndpointURI: #jwk enpoint uri, used for caching token till expiry. + # Indicates the active store. Only a single store in the last can be active at one time. In the future this key # will be deprecated in order to allow multiple stores to be served from a single serving instance active_store: online + + security: + authentication: + enabled: false + provider: jwt + options: + jwkEndpointURI: "https://www.googleapis.com/oauth2/v3/certs" + authorization: + enabled: false + provider: none + options: + basePath: http://localhost:3000 # List of store configurations stores: @@ -72,13 +98,15 @@ feast: redis_port: 6379 grpc: - # The port number Feast Serving GRPC service should listen on - # It is set default to 6566 so it does not conflict with the GRPC server on Feast Core - # which defaults to port 6565 - port: ${GRPC_PORT:6566} - # This allows client to discover GRPC endpoints easily - # https://github.com/grpc/grpc-java/blob/master/documentation/server-reflection-tutorial.md - enable-reflection: ${GRPC_ENABLE_REFLECTION:true} + server: + # The port number Feast Serving GRPC service should listen on + # It is set default to 6566 so it does not conflict with the GRPC server on Feast Core + # which defaults to port 6565 + port: ${GRPC_PORT:6566} + security: + enabled: false + certificateChainPath: server.crt + privateKeyPath: server.key server: # The port number on which the Tomcat webserver that serves REST API endpoints should listen diff --git a/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java b/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java index 5c8308daea..bd67dac6e0 100644 --- a/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java +++ b/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java @@ -16,9 +16,20 @@ */ package feast.serving.controller; +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 static org.mockito.MockitoAnnotations.initMocks; import com.google.protobuf.Timestamp; +import feast.auth.authorization.AuthorizationProvider; +import feast.auth.authorization.AuthorizationResult; +import feast.auth.config.SecurityProperties; +import feast.auth.config.SecurityProperties.AuthenticationProperties; +import feast.auth.config.SecurityProperties.AuthorizationProperties; +import feast.auth.service.AuthorizationService; import feast.proto.serving.ServingAPIProto.FeatureReference; import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest; import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow; @@ -34,6 +45,10 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.Mockito; +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; public class ServingServiceGRpcControllerTest { @@ -45,6 +60,10 @@ public class ServingServiceGRpcControllerTest { private ServingServiceGRpcController service; + @Mock private Authentication authentication; + + @Mock private AuthorizationProvider authProvider; + @Before public void setUp() { initMocks(this); @@ -59,23 +78,64 @@ public void setUp() { .putFields("entity1", Value.newBuilder().setInt64Val(1).build()) .putFields("entity2", Value.newBuilder().setInt64Val(1).build())) .build(); + } + private ServingServiceGRpcController getServingServiceGRpcController(boolean enableAuth) { Tracer tracer = Configuration.fromEnv("dummy").getTracer(); FeastProperties feastProperties = new FeastProperties(); - service = new ServingServiceGRpcController(mockServingService, feastProperties, tracer); + + AuthorizationProperties authorizationProps = new AuthorizationProperties(); + authorizationProps.setEnabled(enableAuth); + AuthenticationProperties authenticationProps = new AuthenticationProperties(); + authenticationProps.setEnabled(enableAuth); + SecurityProperties securityProperties = new SecurityProperties(); + securityProperties.setAuthentication(authenticationProps); + securityProperties.setAuthorization(authorizationProps); + feastProperties.setSecurity(securityProperties); + AuthorizationService authorizationservice = + new AuthorizationService(feastProperties.getSecurity(), authProvider); + return new ServingServiceGRpcController( + authorizationservice, mockServingService, feastProperties, tracer); } @Test public void shouldPassValidRequestAsIs() { + service = getServingServiceGRpcController(false); service.getOnlineFeatures(validRequest, mockStreamObserver); Mockito.verify(mockServingService).getOnlineFeatures(validRequest); } @Test public void shouldCallOnErrorIfEntityDatasetIsNotSet() { + service = getServingServiceGRpcController(false); GetOnlineFeaturesRequest missingEntityName = GetOnlineFeaturesRequest.newBuilder(validRequest).clearEntityRows().build(); service.getOnlineFeatures(missingEntityName, mockStreamObserver); Mockito.verify(mockStreamObserver).onError(Mockito.any(StatusRuntimeException.class)); } + + @Test + public void shouldPassValidRequestAsIsIfRequestIsAuthorized() { + service = getServingServiceGRpcController(true); + SecurityContext context = mock(SecurityContext.class); + SecurityContextHolder.setContext(context); + when(context.getAuthentication()).thenReturn(authentication); + doReturn(AuthorizationResult.success()) + .when(authProvider) + .checkAccessToProject(anyString(), any(Authentication.class)); + service.getOnlineFeatures(validRequest, mockStreamObserver); + Mockito.verify(mockServingService).getOnlineFeatures(validRequest); + } + + @Test(expected = AccessDeniedException.class) + public void shouldThrowErrorOnValidRequestIfRequestIsUnauthorized() { + service = getServingServiceGRpcController(true); + SecurityContext context = mock(SecurityContext.class); + SecurityContextHolder.setContext(context); + when(context.getAuthentication()).thenReturn(authentication); + doReturn(AuthorizationResult.failed(null)) + .when(authProvider) + .checkAccessToProject(anyString(), any(Authentication.class)); + service.getOnlineFeatures(validRequest, mockStreamObserver); + } } diff --git a/tests/e2e/redis/basic-ingest-redis-serving.py b/tests/e2e/redis/basic-ingest-redis-serving.py index 54776f33e1..3c9f9fb198 100644 --- a/tests/e2e/redis/basic-ingest-redis-serving.py +++ b/tests/e2e/redis/basic-ingest-redis-serving.py @@ -14,6 +14,8 @@ from google.protobuf.duration_pb2 import Duration from feast.client import Client +from feast.config import Config +from feast.constants import CONFIG_CORE_AUTH_PROVIDER from feast.core import CoreService_pb2 from feast.core.CoreService_pb2 import ApplyFeatureSetResponse, GetFeatureSetResponse from feast.core.CoreService_pb2_grpc import CoreServiceStub @@ -21,6 +23,7 @@ from feast.entity import Entity from feast.feature import Feature from feast.feature_set import FeatureSet, FeatureSetRef +from feast.grpc.auth import get_auth_metadata_plugin from feast.serving.ServingService_pb2 import ( GetOnlineFeaturesRequest, GetOnlineFeaturesResponse, @@ -34,6 +37,7 @@ FLOAT_TOLERANCE = 0.00001 PROJECT_NAME = "basic_" + uuid.uuid4().hex.upper()[0:6] DIR_PATH = os.path.dirname(os.path.realpath(__file__)) +AUTH_PROVIDER = "google" def basic_dataframe(entities, features, ingest_time, n_size, null_features=[]): @@ -103,7 +107,7 @@ def allow_dirty(pytestconfig): @pytest.fixture(scope="module") def enable_auth(pytestconfig): - return pytestconfig.getoption("enable_auth") + return True if pytestconfig.getoption("enable_auth").lower() == "true" else False @pytest.fixture(scope="module") @@ -115,7 +119,7 @@ def client(core_url, serving_url, allow_dirty, enable_auth): core_url=core_url, serving_url=serving_url, core_enable_auth=enable_auth, - core_auth_provider="google", + core_auth_provider=AUTH_PROVIDER, ) client.create_project(PROJECT_NAME) @@ -1197,22 +1201,33 @@ def core_service_stub(self, core_url): core_service_stub = CoreServiceStub(core_channel) return core_service_stub - def apply_feature_set(self, core_service_stub, feature_set_proto): + @pytest.fixture(scope="module") + def auth_meta_data(self, enable_auth): + if not enable_auth: + return None + else: + metadata = {CONFIG_CORE_AUTH_PROVIDER: AUTH_PROVIDER} + metadata_plugin = get_auth_metadata_plugin(config=Config(metadata)) + return metadata_plugin.get_signed_meta() + + def apply_feature_set(self, core_service_stub, feature_set_proto, auth_meta_data): try: apply_fs_response = core_service_stub.ApplyFeatureSet( CoreService_pb2.ApplyFeatureSetRequest(feature_set=feature_set_proto), timeout=self.GRPC_CONNECTION_TIMEOUT, + metadata=auth_meta_data, ) # type: ApplyFeatureSetResponse except grpc.RpcError as e: raise grpc.RpcError(e.details()) return apply_fs_response.feature_set - def get_feature_set(self, core_service_stub, name, project): + def get_feature_set(self, core_service_stub, name, project, auth_meta_data): try: get_feature_set_response = core_service_stub.GetFeatureSet( CoreService_pb2.GetFeatureSetRequest( project=project, name=name.strip(), - ) + ), + metadata=auth_meta_data, ) # type: GetFeatureSetResponse except grpc.RpcError as e: raise grpc.RpcError(e.details()) @@ -1220,17 +1235,17 @@ def get_feature_set(self, core_service_stub, name, project): @pytest.mark.timeout(45) @pytest.mark.run(order=51) - def test_register_feature_set_with_labels(self, core_service_stub): + def test_register_feature_set_with_labels(self, core_service_stub, auth_meta_data): feature_set_name = "test_feature_set_labels" feature_set_proto = FeatureSet( name=feature_set_name, project=PROJECT_NAME, labels={self.LABEL_KEY: self.LABEL_VALUE}, ).to_proto() - self.apply_feature_set(core_service_stub, feature_set_proto) + self.apply_feature_set(core_service_stub, feature_set_proto, auth_meta_data) retrieved_feature_set = self.get_feature_set( - core_service_stub, feature_set_name, PROJECT_NAME + core_service_stub, feature_set_name, PROJECT_NAME, auth_meta_data ) assert self.LABEL_KEY in retrieved_feature_set.spec.labels @@ -1238,7 +1253,7 @@ def test_register_feature_set_with_labels(self, core_service_stub): @pytest.mark.timeout(45) @pytest.mark.run(order=52) - def test_register_feature_with_labels(self, core_service_stub): + def test_register_feature_with_labels(self, core_service_stub, auth_meta_data): feature_set_name = "test_feature_labels" feature_set_proto = FeatureSet( name=feature_set_name, @@ -1251,10 +1266,10 @@ def test_register_feature_with_labels(self, core_service_stub): ) ], ).to_proto() - self.apply_feature_set(core_service_stub, feature_set_proto) + self.apply_feature_set(core_service_stub, feature_set_proto, auth_meta_data) retrieved_feature_set = self.get_feature_set( - core_service_stub, feature_set_name, PROJECT_NAME + core_service_stub, feature_set_name, PROJECT_NAME, auth_meta_data ) retrieved_feature = retrieved_feature_set.spec.features[0] From e8859d57da5632ba4c586a6b4a36f689cfc21221 Mon Sep 17 00:00:00 2001 From: e10112844 Date: Tue, 21 Jul 2020 14:31:28 -0400 Subject: [PATCH 2/8] fix e2e, add metadata plugin in jobs, merge labels, auth failure test, removed unwanted expire time validation from gauth. --- .../credentials/GoogleAuthCredentials.java | 13 ++----- sdk/python/feast/client.py | 36 +++++++++++-------- sdk/python/feast/constants.py | 10 +++--- sdk/python/feast/grpc/auth.py | 16 ++++----- sdk/python/feast/job.py | 27 +++++++++++--- sdk/python/tests/grpc/test_auth.py | 17 ++++----- sdk/python/tests/test_client.py | 16 ++++----- tests/e2e/redis/basic-ingest-redis-serving.py | 15 +++++--- 8 files changed, 85 insertions(+), 65 deletions(-) diff --git a/auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java b/auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java index b5f6a1d786..709b803ce0 100644 --- a/auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java +++ b/auth/src/main/java/feast/auth/credentials/GoogleAuthCredentials.java @@ -24,7 +24,6 @@ import io.grpc.Metadata; import io.grpc.Status; import java.io.IOException; -import java.time.Instant; import java.util.Arrays; import java.util.Map; import java.util.concurrent.Executor; @@ -34,8 +33,6 @@ * Used by CoreSpecService to connect to core. */ public class GoogleAuthCredentials extends CallCredentials { - private String accessToken; - private Instant tokenExpiryTime; private final IdTokenCredentials credentials; private static final String BEARER_TYPE = "Bearer"; private static final Metadata.Key AUTHORIZATION_METADATA_KEY = @@ -62,15 +59,11 @@ public void applyRequestMetadata( appExecutor.execute( () -> { try { - // Fetches new token if it is not available or if token has expired. - if (this.accessToken == null || Instant.now().isAfter(this.tokenExpiryTime)) { - credentials.refreshIfExpired(); - this.accessToken = credentials.getIdToken().getTokenValue(); - this.tokenExpiryTime = credentials.getIdToken().getExpirationTime().toInstant(); - } + credentials.refreshIfExpired(); Metadata headers = new Metadata(); headers.put( - AUTHORIZATION_METADATA_KEY, String.format("%s %s", BEARER_TYPE, this.accessToken)); + AUTHORIZATION_METADATA_KEY, + String.format("%s %s", BEARER_TYPE, credentials.getIdToken().getTokenValue())); applier.apply(headers); } catch (Throwable e) { applier.fail(Status.UNAUTHENTICATED.withCause(e)); diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index c11c01f64f..86b9a2c57f 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -32,10 +32,10 @@ from feast.config import Config from feast.constants import ( - CONFIG_CORE_ENABLE_AUTH_KEY, CONFIG_CORE_ENABLE_SSL_KEY, CONFIG_CORE_SERVER_SSL_CERT_KEY, CONFIG_CORE_URL_KEY, + CONFIG_ENABLE_AUTH_KEY, CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY, CONFIG_PROJECT_KEY, CONFIG_SERVING_ENABLE_SSL_KEY, @@ -112,9 +112,9 @@ 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 – + enable_auth: Enable authentication and authorization + auth_provider: Authentication provider – "google" or "oauth" + if 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: @@ -132,7 +132,7 @@ def __init__(self, options: Optional[Dict[str, str]] = None, **kwargs): self._auth_metadata: Optional[grpc.AuthMetadataPlugin] = None # Configure Auth Metadata Plugin if auth is enabled - if self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY): + if self._config.getboolean(CONFIG_ENABLE_AUTH_KEY): self._auth_metadata = feast_auth.get_auth_metadata_plugin(self._config) @property @@ -146,7 +146,7 @@ def _core_service(self): 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), + enable_auth=self._config.getboolean(CONFIG_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), @@ -165,7 +165,7 @@ def _serving_service(self): channel = create_grpc_channel( url=self._config.get(CONFIG_SERVING_URL_KEY), enable_ssl=self._config.getboolean(CONFIG_SERVING_ENABLE_SSL_KEY), - enable_auth=self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY), + enable_auth=self._config.getboolean(CONFIG_ENABLE_AUTH_KEY), ssl_server_cert_path=self._config.get( CONFIG_SERVING_SERVER_SSL_CERT_KEY ), @@ -523,7 +523,7 @@ def list_features_by_ref( ) feature_protos = self._core_service.ListFeatures( - ListFeaturesRequest(filter=filter) + ListFeaturesRequest(filter=filter), metadata=self._get_grpc_metadata(), ) # type: ListFeaturesResponse features_dict = {} @@ -677,7 +677,11 @@ def get_historical_features( except grpc.RpcError as e: raise grpc.RpcError(e.details()) - return RetrievalJob(response.job, self._serving_service) + return RetrievalJob( + response.job, + self._serving_service, + auth_metadata_plugin=self._auth_metadata, + ) def get_online_features( self, @@ -764,9 +768,9 @@ def list_ingest_jobs( ) request = ListIngestionJobsRequest(filter=list_filter) # make list request & unpack response - response = self._core_service.ListIngestionJobs(request) # type: ignore + response = self._core_service.ListIngestionJobs(request, metadata=self._get_grpc_metadata(),) # type: ignore ingest_jobs = [ - IngestJob(proto, self._core_service) for proto in response.jobs # type: ignore + IngestJob(proto, self._core_service, auth_metadata_plugin=self._auth_metadata) for proto in response.jobs # type: ignore ] return ingest_jobs @@ -783,7 +787,9 @@ def restart_ingest_job(self, job: IngestJob): """ request = RestartIngestionJobRequest(id=job.id) try: - self._core_service.RestartIngestionJob(request) # type: ignore + self._core_service.RestartIngestionJob( + request, metadata=self._get_grpc_metadata(), + ) # type: ignore except grpc.RpcError as e: raise grpc.RpcError(e.details()) @@ -799,7 +805,9 @@ def stop_ingest_job(self, job: IngestJob): """ request = StopIngestionJobRequest(id=job.id) try: - self._core_service.StopIngestionJob(request) # type: ignore + self._core_service.StopIngestionJob( + request, metadata=self._get_grpc_metadata(), + ) # type: ignore except grpc.RpcError as e: raise grpc.RpcError(e.details()) @@ -1025,7 +1033,7 @@ def _get_grpc_metadata(self): Returns: Tuple of metadata to attach to each gRPC call """ - if self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY) and self._auth_metadata: + if self._config.getboolean(CONFIG_ENABLE_AUTH_KEY) and self._auth_metadata: return self._auth_metadata.get_signed_meta() return () diff --git a/sdk/python/feast/constants.py b/sdk/python/feast/constants.py index 911432326a..67f4808010 100644 --- a/sdk/python/feast/constants.py +++ b/sdk/python/feast/constants.py @@ -42,8 +42,8 @@ class AuthProvider(Enum): 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_ENABLE_AUTH_KEY = "enable_auth" +CONFIG_ENABLE_AUTH_TOKEN_KEY = "auth_token" CONFIG_CORE_SERVER_SSL_CERT_KEY = "core_server_ssl_cert" CONFIG_SERVING_URL_KEY = "serving_url" CONFIG_SERVING_ENABLE_SSL_KEY = "serving_enable_ssl" @@ -58,7 +58,7 @@ class AuthProvider(Enum): 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_AUTH_PROVIDER = "auth_provider" CONFIG_TIMEOUT_KEY = "timeout" CONFIG_MAX_WAIT_INTERVAL_KEY = "max_wait_interval" @@ -72,7 +72,7 @@ class AuthProvider(Enum): # 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", + CONFIG_ENABLE_AUTH_KEY: "False", # Path to certificate(s) to secure connection to Feast Core CONFIG_CORE_SERVER_SSL_CERT_KEY: "", # Default Feast Serving URL @@ -91,5 +91,5 @@ class AuthProvider(Enum): CONFIG_TIMEOUT_KEY: "21600", CONFIG_MAX_WAIT_INTERVAL_KEY: "60", # Authentication Provider - Google OpenID/OAuth - CONFIG_CORE_AUTH_PROVIDER: "google", + CONFIG_AUTH_PROVIDER: "google", } diff --git a/sdk/python/feast/grpc/auth.py b/sdk/python/feast/grpc/auth.py index ab1de83631..9680607b8e 100644 --- a/sdk/python/feast/grpc/auth.py +++ b/sdk/python/feast/grpc/auth.py @@ -19,8 +19,8 @@ from feast.config import Config from feast.constants import ( - CONFIG_CORE_AUTH_PROVIDER, - CONFIG_CORE_ENABLE_AUTH_TOKEN_KEY, + CONFIG_AUTH_PROVIDER, + CONFIG_ENABLE_AUTH_TOKEN_KEY, CONFIG_OAUTH_AUDIENCE_KEY, CONFIG_OAUTH_CLIENT_ID_KEY, CONFIG_OAUTH_CLIENT_SECRET_KEY, @@ -44,9 +44,9 @@ def get_auth_metadata_plugin(config: Config) -> grpc.AuthMetadataPlugin: Args: config: Feast Configuration object """ - if AuthProvider(config.get(CONFIG_CORE_AUTH_PROVIDER)) == AuthProvider.GOOGLE: + if AuthProvider(config.get(CONFIG_AUTH_PROVIDER)) == AuthProvider.GOOGLE: return GoogleOpenIDAuthMetadataPlugin(config) - elif AuthProvider(config.get(CONFIG_CORE_AUTH_PROVIDER)) == AuthProvider.OAUTH: + elif AuthProvider(config.get(CONFIG_AUTH_PROVIDER)) == AuthProvider.OAUTH: return OAuthMetadataPlugin(config) else: raise RuntimeError( @@ -75,8 +75,8 @@ def __init__(self, config: Config): 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_ENABLE_AUTH_TOKEN_KEY): + self._static_token = config.get(CONFIG_ENABLE_AUTH_TOKEN_KEY) self._refresh_token(config) elif ( config.exists(CONFIG_OAUTH_GRANT_TYPE_KEY) @@ -171,8 +171,8 @@ def __init__(self, config: Config): 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_ENABLE_AUTH_TOKEN_KEY): + self._static_token = config.get(CONFIG_ENABLE_AUTH_TOKEN_KEY) self._request = requests.Request() self._refresh_token() diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index 25396213e4..cda4b26d30 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -2,6 +2,7 @@ from urllib.parse import urlparse import fastavro +import grpc import pandas as pd from google.protobuf.json_format import MessageToJson @@ -39,15 +40,20 @@ class RetrievalJob: """ def __init__( - self, job_proto: JobProto, serving_stub: ServingServiceStub, + self, + job_proto: JobProto, + serving_stub: ServingServiceStub, + auth_metadata_plugin: grpc.AuthMetadataPlugin = None, ): """ Args: job_proto: Job proto object (wrapped by this job object) serving_stub: Stub for Feast serving service + auth_metadata_plugin: plugin to fetch auth metadata """ self.job_proto = job_proto self.serving_stub = serving_stub + self.auth_metadata = auth_metadata_plugin @property def id(self): @@ -68,7 +74,10 @@ def reload(self): Reload the latest job status Returns: None """ - self.job_proto = self.serving_stub.GetJob(GetJobRequest(job=self.job_proto)).job + self.job_proto = self.serving_stub.GetJob( + GetJobRequest(job=self.job_proto), + metadata=self.auth_metadata.get_signed_meta() if self.auth_metadata else (), + ).job def get_avro_files(self, timeout_sec: int = int(defaults[CONFIG_TIMEOUT_KEY])): """ @@ -218,16 +227,23 @@ class IngestJob: Defines a job for feature ingestion in feast. """ - def __init__(self, job_proto: IngestJobProto, core_stub: CoreServiceStub): + def __init__( + self, + job_proto: IngestJobProto, + core_stub: CoreServiceStub, + auth_metadata_plugin: grpc.AuthMetadataPlugin = None, + ): """ Construct a native ingest job from its protobuf version. Args: job_proto: Job proto object to construct from. core_stub: stub for Feast CoreService + auth_metadata_plugin: plugin to fetch auth metadata """ self.proto = job_proto self.core_svc = core_stub + self.auth_metadata = auth_metadata_plugin def reload(self): """ @@ -235,7 +251,10 @@ def reload(self): """ # pull latest proto from feast core response = self.core_svc.ListIngestionJobs( - ListIngestionJobsRequest(filter=ListIngestionJobsRequest.Filter(id=self.id)) + ListIngestionJobsRequest( + filter=ListIngestionJobsRequest.Filter(id=self.id) + ), + metadata=self.auth_metadata.get_signed_meta() if self.auth_metadata else (), ) self.proto = response.jobs[0] diff --git a/sdk/python/tests/grpc/test_auth.py b/sdk/python/tests/grpc/test_auth.py index 90896ee925..7f023aabcf 100644 --- a/sdk/python/tests/grpc/test_auth.py +++ b/sdk/python/tests/grpc/test_auth.py @@ -76,8 +76,8 @@ def refresh(self, request): def config_oauth(): config_dict = { "core_url": "localhost:50051", - "core_enable_auth": True, - "core_auth_provider": "oauth", + "enable_auth": True, + "auth_provider": "oauth", "oauth_grant_type": "client_credentials", "oauth_client_id": "fakeID", "oauth_client_secret": "fakeSecret", @@ -91,13 +91,8 @@ def config_oauth(): 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, + "enable_auth": True, + "auth_provider": "google", } return Config(config_dict) @@ -106,8 +101,8 @@ def config_google(): def config_with_missing_variable(): config_dict = { "core_url": "localhost:50051", - "core_enable_auth": True, - "core_auth_provider": "oauth", + "enable_auth": True, + "auth_provider": "oauth", "oauth_grant_type": "client_credentials", "oauth_client_id": "fakeID", "oauth_client_secret": "fakeSecret", diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 11b04f8c5d..86f0438e5c 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -109,8 +109,8 @@ def mock_client_with_auth(self): client = Client( core_url=CORE_URL, serving_url=SERVING_URL, - core_enable_auth=True, - core_auth_token=_FAKE_JWT_TOKEN, + enable_auth=True, + auth_token=_FAKE_JWT_TOKEN, ) client._core_url = CORE_URL client._serving_url = SERVING_URL @@ -123,8 +123,8 @@ def secure_mock_client_with_auth(self): serving_url=SERVING_URL, core_enable_ssl=True, serving_enable_ssl=True, - core_enable_auth=True, - core_auth_token=_FAKE_JWT_TOKEN, + enable_auth=True, + auth_token=_FAKE_JWT_TOKEN, ) client._core_url = CORE_URL client._serving_url = SERVING_URL @@ -243,8 +243,8 @@ def secure_core_client_with_auth(self, secure_core_server_with_auth): yield Client( core_url="localhost:50055", core_enable_ssl=True, - core_enable_auth=True, - core_auth_token=_FAKE_JWT_TOKEN, + enable_auth=True, + auth_token=_FAKE_JWT_TOKEN, ) @pytest.fixture @@ -1050,9 +1050,7 @@ 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, + core_url="localhost:50056", enable_auth=True, auth_token=_FAKE_JWT_TOKEN, ) client.list_feature_sets() diff --git a/tests/e2e/redis/basic-ingest-redis-serving.py b/tests/e2e/redis/basic-ingest-redis-serving.py index 3c9f9fb198..341c789f76 100644 --- a/tests/e2e/redis/basic-ingest-redis-serving.py +++ b/tests/e2e/redis/basic-ingest-redis-serving.py @@ -15,7 +15,7 @@ from feast.client import Client from feast.config import Config -from feast.constants import CONFIG_CORE_AUTH_PROVIDER +from feast.constants import CONFIG_AUTH_PROVIDER from feast.core import CoreService_pb2 from feast.core.CoreService_pb2 import ApplyFeatureSetResponse, GetFeatureSetResponse from feast.core.CoreService_pb2_grpc import CoreServiceStub @@ -118,8 +118,8 @@ def client(core_url, serving_url, allow_dirty, enable_auth): client = Client( core_url=core_url, serving_url=serving_url, - core_enable_auth=enable_auth, - core_auth_provider=AUTH_PROVIDER, + enable_auth=enable_auth, + auth_provider=AUTH_PROVIDER, ) client.create_project(PROJECT_NAME) @@ -166,6 +166,13 @@ def test_version_returns_results(client): assert not version_info["serving"] == "not configured" +def test_list_feature_sets_when_auth_enabled_should_raise(enable_auth): + if enable_auth: + client = Client(core_url=core_url, serving_url=serving_url, enable_auth=False) + with pytest.raises(ConnectionError): + client.list_feature_sets() + + @pytest.mark.timeout(45) @pytest.mark.run(order=10) def test_basic_register_feature_set_success(client): @@ -1206,7 +1213,7 @@ def auth_meta_data(self, enable_auth): if not enable_auth: return None else: - metadata = {CONFIG_CORE_AUTH_PROVIDER: AUTH_PROVIDER} + metadata = {CONFIG_AUTH_PROVIDER: AUTH_PROVIDER} metadata_plugin = get_auth_metadata_plugin(config=Config(metadata)) return metadata_plugin.get_signed_meta() From d8ebca5581d933090c4f8755b4f7ae6020c2a182 Mon Sep 17 00:00:00 2001 From: e10112844 Date: Wed, 22 Jul 2020 14:53:25 -0400 Subject: [PATCH 3/8] fix rebase adaption. --- .../auth/service/AuthorizationService.java | 2 +- .../feast/core/config/CoreSecurityConfig.java | 2 +- .../java/feast/core/grpc/CoreServiceImpl.java | 21 +++++++------- .../serving/config/ServingSecurityConfig.java | 2 +- .../ServingServiceGRpcController.java | 28 ++++++++++++++----- .../ServingServiceGRpcControllerTest.java | 4 +-- 6 files changed, 36 insertions(+), 23 deletions(-) diff --git a/auth/src/main/java/feast/auth/service/AuthorizationService.java b/auth/src/main/java/feast/auth/service/AuthorizationService.java index 0d0d4c4934..2494261185 100644 --- a/auth/src/main/java/feast/auth/service/AuthorizationService.java +++ b/auth/src/main/java/feast/auth/service/AuthorizationService.java @@ -57,7 +57,7 @@ public void authorizeRequest(SecurityContext securityContext, String project) { AuthorizationResult result = this.authorizationProvider.checkAccessToProject(project, authentication); if (!result.isAllowed()) { - throw new AccessDeniedException(result.getFailureReason().orElse("AccessDenied")); + throw new AccessDeniedException(result.getFailureReason().orElse("Access Denied")); } } } diff --git a/core/src/main/java/feast/core/config/CoreSecurityConfig.java b/core/src/main/java/feast/core/config/CoreSecurityConfig.java index 90c28dc39f..3e4c2baa9e 100644 --- a/core/src/main/java/feast/core/config/CoreSecurityConfig.java +++ b/core/src/main/java/feast/core/config/CoreSecurityConfig.java @@ -28,7 +28,7 @@ @Configuration @Slf4j -@ComponentScan("feast.auth.config") +@ComponentScan(basePackages = {"feast.auth.config", "feast.auth.service"}) public class CoreSecurityConfig { /** diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java index 558933931e..e920464a50 100644 --- a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java @@ -187,8 +187,7 @@ public void applyFeatureSet( try { projectId = request.getFeatureSet().getSpec().getProject(); - authorizationService.authorizeRequest( - SecurityContextHolder.getContext(), projectId); + authorizationService.authorizeRequest(SecurityContextHolder.getContext(), projectId); ApplyFeatureSetResponse response = specService.applyFeatureSet(request.getFeatureSet()); responseObserver.onNext(response); responseObserver.onCompleted(); @@ -244,10 +243,10 @@ public void createProject( @Override public void archiveProject( ArchiveProjectRequest request, StreamObserver responseObserver) { - String projectId = null; + String projectId = null; try { projectId = request.getName(); - authorizationService.authorizeRequest(SecurityContextHolder.getContext(), projectId); + authorizationService.authorizeRequest(SecurityContextHolder.getContext(), projectId); projectService.archiveProject(projectId); responseObserver.onNext(ArchiveProjectResponse.getDefaultInstance()); responseObserver.onCompleted(); @@ -263,13 +262,13 @@ public void archiveProject( responseObserver.onError( Status.UNIMPLEMENTED.withDescription(e.getMessage()).withCause(e).asRuntimeException()); } catch (AccessDeniedException e) { - log.info(String.format("User prevented from accessing project: %s", projectId)); - responseObserver.onError( - Status.PERMISSION_DENIED - .withDescription(e.getMessage()) - .withCause(e) - .asRuntimeException()); - } catch (Exception e) { + log.info(String.format("User prevented from accessing project: %s", projectId)); + responseObserver.onError( + Status.PERMISSION_DENIED + .withDescription(e.getMessage()) + .withCause(e) + .asRuntimeException()); + } catch (Exception e) { log.error("Exception has occurred in the createProject method: ", e); responseObserver.onError( Status.INTERNAL.withDescription(e.getMessage()).withCause(e).asRuntimeException()); diff --git a/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java index 54753ca6fe..2d0a46763a 100644 --- a/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingSecurityConfig.java @@ -45,7 +45,7 @@ */ @Configuration -@ComponentScan("feast.auth") +@ComponentScan(basePackages = {"feast.auth.config", "feast.auth.service"}) public class ServingSecurityConfig { private final FeastProperties feastProperties; diff --git a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java index c44737fcc3..be0bd411f2 100644 --- a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java +++ b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java @@ -41,6 +41,7 @@ import net.devh.boot.grpc.server.service.GrpcService; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.context.SecurityContextHolder; @GrpcService(interceptors = {GrpcMonitoringInterceptor.class}) @@ -79,15 +80,14 @@ public void getFeastServingInfo( public void getOnlineFeatures( GetOnlineFeaturesRequest request, StreamObserver responseObserver) { - // authorize for the project in request object. - this.authorizationService.authorizeRequest( - SecurityContextHolder.getContext(), request.getProject()); - // authorize for projects set in feature list, backward compatibility for - // <=v0.5.X - this.checkProjectAccess(request.getFeaturesList()); - Span span = tracer.buildSpan("getOnlineFeatures").start(); try (Scope scope = tracer.scopeManager().activate(span, false)) { + // authorize for the project in request object. + this.authorizationService.authorizeRequest( + SecurityContextHolder.getContext(), request.getProject()); + // authorize for projects set in feature list, backward compatibility for + // <=v0.5.X + this.checkProjectAccess(request.getFeaturesList()); RequestHelper.validateOnlineRequest(request); GetOnlineFeaturesResponse onlineFeatures = servingService.getOnlineFeatures(request); responseObserver.onNext(onlineFeatures); @@ -96,6 +96,13 @@ public void getOnlineFeatures( log.error("Failed to retrieve specs in SpecService", e); responseObserver.onError( Status.NOT_FOUND.withDescription(e.getMessage()).withCause(e).asException()); + } catch (AccessDeniedException e) { + log.info(String.format("User prevented from accessing one of the projects in request")); + responseObserver.onError( + Status.PERMISSION_DENIED + .withDescription(e.getMessage()) + .withCause(e) + .asRuntimeException()); } catch (Exception e) { log.warn("Failed to get Online Features", e); responseObserver.onError(e); @@ -116,6 +123,13 @@ public void getBatchFeatures( log.error("Failed to retrieve specs in SpecService", e); responseObserver.onError( Status.NOT_FOUND.withDescription(e.getMessage()).withCause(e).asException()); + } catch (AccessDeniedException e) { + log.info(String.format("User prevented from accessing one of the projects in request")); + responseObserver.onError( + Status.PERMISSION_DENIED + .withDescription(e.getMessage()) + .withCause(e) + .asRuntimeException()); } catch (Exception e) { log.warn("Failed to get Batch Features", e); responseObserver.onError(e); diff --git a/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java b/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java index bd67dac6e0..3577f098c1 100644 --- a/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java +++ b/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java @@ -45,7 +45,6 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.Mockito; -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; @@ -127,7 +126,7 @@ public void shouldPassValidRequestAsIsIfRequestIsAuthorized() { Mockito.verify(mockServingService).getOnlineFeatures(validRequest); } - @Test(expected = AccessDeniedException.class) + @Test public void shouldThrowErrorOnValidRequestIfRequestIsUnauthorized() { service = getServingServiceGRpcController(true); SecurityContext context = mock(SecurityContext.class); @@ -137,5 +136,6 @@ public void shouldThrowErrorOnValidRequestIfRequestIsUnauthorized() { .when(authProvider) .checkAccessToProject(anyString(), any(Authentication.class)); service.getOnlineFeatures(validRequest, mockStreamObserver); + Mockito.verify(mockStreamObserver).onError(Mockito.any(StatusRuntimeException.class)); } } From f5fdb3f7a542738ebb4127ad5c8b384c6ba4432c Mon Sep 17 00:00:00 2001 From: e10112844 Date: Wed, 22 Jul 2020 18:27:18 -0400 Subject: [PATCH 4/8] Fix core integration test. --- .../auth/CoreServiceAuthenticationIT.java | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java b/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java index ee20ce6819..0886bf80b3 100644 --- a/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java +++ b/core/src/test/java/feast/core/auth/CoreServiceAuthenticationIT.java @@ -32,6 +32,7 @@ import io.grpc.CallCredentials; import io.grpc.Channel; import io.grpc.ManagedChannelBuilder; +import io.grpc.StatusRuntimeException; import java.util.*; import org.junit.ClassRule; import org.junit.Rule; @@ -120,24 +121,18 @@ public void shouldGetVersionFromFeastCoreAlways() { assertEquals(feastProperties.getVersion(), feastCoreVersionSecure); } - /** - * For the time being, if authentication is enabled but authorization is disabled, users can still - * connect to Feast Core as anonymous users. They are not forced to authenticate. - */ @Test - public void shouldAllowUnauthenticatedFeatureSetListing() { - FeatureSetProto.FeatureSet expectedFeatureSet = DataGenerator.getDefaultFeatureSet(); - insecureApiClient.simpleApplyFeatureSet(expectedFeatureSet); - - List listFeatureSetsResponse = - insecureApiClient.simpleListFeatureSets("*"); - FeatureSetProto.FeatureSet actualFeatureSet = listFeatureSetsResponse.get(0); - - assert listFeatureSetsResponse.size() == 1; - assertEquals( - actualFeatureSet.getSpec().getProject(), expectedFeatureSet.getSpec().getProject()); - assertEquals( - actualFeatureSet.getSpec().getProject(), expectedFeatureSet.getSpec().getProject()); + public void shouldNotAllowUnauthenticatedFeatureSetListing() { + Exception exception = + assertThrows( + StatusRuntimeException.class, + () -> { + insecureApiClient.simpleListFeatureSets("*"); + }); + + String expectedMessage = "UNAUTHENTICATED: Authentication failed"; + String actualMessage = exception.getMessage(); + assertEquals(actualMessage, expectedMessage); } @Test From 2461ac11ff930308fd572e39d6b8196396f63e0a Mon Sep 17 00:00:00 2001 From: e10112844 Date: Sun, 26 Jul 2020 22:36:00 -0400 Subject: [PATCH 5/8] Authentication integration test. --- serving/pom.xml | 18 ++ .../java/feast/serving/it/AuthTestUtils.java | 211 ++++++++++++++++++ .../java/feast/serving/it/BaseAuthIT.java | 86 +++++++ .../feast/serving/it/CoreSimpleAPIClient.java | 43 ++++ .../ServingServiceOauthAuthenticationIT.java | 117 ++++++++++ .../test/resources/application-it.properties | 18 ++ .../test/resources/docker-compose/core-it.yml | 21 ++ .../docker-compose/docker-compose-it-core.yml | 53 +++++ .../docker-compose-it-hydra.yml | 54 +++++ 9 files changed, 621 insertions(+) create mode 100644 serving/src/test/java/feast/serving/it/AuthTestUtils.java create mode 100644 serving/src/test/java/feast/serving/it/BaseAuthIT.java create mode 100644 serving/src/test/java/feast/serving/it/CoreSimpleAPIClient.java create mode 100644 serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java create mode 100644 serving/src/test/resources/application-it.properties create mode 100644 serving/src/test/resources/docker-compose/core-it.yml create mode 100644 serving/src/test/resources/docker-compose/docker-compose-it-core.yml create mode 100644 serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml diff --git a/serving/pom.xml b/serving/pom.xml index 651050d759..47006e6e14 100644 --- a/serving/pom.xml +++ b/serving/pom.xml @@ -316,6 +316,24 @@ spring-security-oauth2-core ${spring.security.version} + + org.testcontainers + testcontainers + 1.14.3 + test + + + org.testcontainers + junit-jupiter + 1.14.3 + test + + + org.awaitility + awaitility + 3.0.0 + test + diff --git a/serving/src/test/java/feast/serving/it/AuthTestUtils.java b/serving/src/test/java/feast/serving/it/AuthTestUtils.java new file mode 100644 index 0000000000..31ca0ff661 --- /dev/null +++ b/serving/src/test/java/feast/serving/it/AuthTestUtils.java @@ -0,0 +1,211 @@ +/* + * 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.serving.it; + +import static org.awaitility.Awaitility.waitAtMost; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.beans.HasPropertyWithValue.hasProperty; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; +import com.google.protobuf.Timestamp; +import feast.auth.credentials.OAuthCredentials; +import feast.proto.core.CoreServiceGrpc; +import feast.proto.core.FeatureSetProto; +import feast.proto.core.FeatureSetProto.FeatureSetStatus; +import feast.proto.core.SourceProto; +import feast.proto.serving.ServingAPIProto.FeatureReference; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow; +import feast.proto.serving.ServingServiceGrpc; +import feast.proto.types.ValueProto; +import feast.proto.types.ValueProto.Value; +import io.grpc.CallCredentials; +import io.grpc.Channel; +import io.grpc.ManagedChannelBuilder; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import okhttp3.MediaType; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; +import org.apache.commons.lang3.tuple.Pair; +import org.junit.runners.model.InitializationError; + +public class AuthTestUtils { + + static SourceProto.Source defaultSource = + createSource("kafka:9092,localhost:9094", "feast-features"); + + public static SourceProto.Source getDefaultSource() { + return defaultSource; + } + + public static SourceProto.Source createSource(String server, String topic) { + return SourceProto.Source.newBuilder() + .setType(SourceProto.SourceType.KAFKA) + .setKafkaSourceConfig( + SourceProto.KafkaSourceConfig.newBuilder() + .setBootstrapServers(server) + .setTopic(topic) + .build()) + .build(); + } + + public static FeatureSetProto.FeatureSet createFeatureSet( + SourceProto.Source source, + String projectName, + String name, + List> entities, + List> features) { + return FeatureSetProto.FeatureSet.newBuilder() + .setSpec( + FeatureSetProto.FeatureSetSpec.newBuilder() + .setSource(source) + .setName(name) + .setProject(projectName) + .addAllEntities( + entities.stream() + .map( + pair -> + FeatureSetProto.EntitySpec.newBuilder() + .setName(pair.getLeft()) + .setValueType(pair.getRight()) + .build()) + .collect(Collectors.toList())) + .addAllFeatures( + features.stream() + .map( + pair -> + FeatureSetProto.FeatureSpec.newBuilder() + .setName(pair.getLeft()) + .setValueType(pair.getRight()) + .build()) + .collect(Collectors.toList())) + .build()) + .build(); + } + + public static GetOnlineFeaturesRequest createOnlineFeatureRequest( + String projectName, String featureName, String entityId, int entityValue) { + return GetOnlineFeaturesRequest.newBuilder() + .setProject(projectName) + .addFeatures(FeatureReference.newBuilder().setName(featureName).build()) + .addEntityRows( + EntityRow.newBuilder() + .setEntityTimestamp(Timestamp.newBuilder().setSeconds(100)) + .putFields(entityId, Value.newBuilder().setInt64Val(entityValue).build())) + .build(); + } + + public static void applyFeatureSet( + CoreSimpleAPIClient secureApiClient, + String projectName, + String entityId, + String featureName) { + List> entities = new ArrayList<>(); + entities.add(Pair.of(entityId, ValueProto.ValueType.Enum.INT64)); + List> features = new ArrayList<>(); + features.add(Pair.of(featureName, ValueProto.ValueType.Enum.INT64)); + String featureSetName = "test_1"; + FeatureSetProto.FeatureSet expectedFeatureSet = + AuthTestUtils.createFeatureSet( + AuthTestUtils.getDefaultSource(), projectName, featureSetName, entities, features); + secureApiClient.simpleApplyFeatureSet(expectedFeatureSet); + waitAtMost(2, TimeUnit.MINUTES) + .until( + () -> { + return secureApiClient.simpleGetFeatureSet(projectName, featureSetName).getMeta(); + }, + hasProperty("status", equalTo(FeatureSetStatus.STATUS_READY))); + FeatureSetProto.FeatureSet actualFeatureSet = + secureApiClient.simpleGetFeatureSet(projectName, featureSetName); + assertEquals( + expectedFeatureSet.getSpec().getProject(), actualFeatureSet.getSpec().getProject()); + assertEquals(expectedFeatureSet.getSpec().getName(), actualFeatureSet.getSpec().getName()); + assertEquals(expectedFeatureSet.getSpec().getSource(), actualFeatureSet.getSpec().getSource()); + assertEquals(FeatureSetStatus.STATUS_READY, actualFeatureSet.getMeta().getStatus()); + } + + public static CoreSimpleAPIClient getSecureApiClientForCore( + int feastCorePort, Map options) { + CallCredentials callCredentials = null; + callCredentials = new OAuthCredentials(options); + Channel secureChannel = + ManagedChannelBuilder.forAddress("localhost", feastCorePort).usePlaintext().build(); + + CoreServiceGrpc.CoreServiceBlockingStub secureCoreService = + CoreServiceGrpc.newBlockingStub(secureChannel).withCallCredentials(callCredentials); + + return new CoreSimpleAPIClient(secureCoreService); + } + + public static ServingServiceGrpc.ServingServiceBlockingStub getServingServiceStub( + boolean isSecure, int feastServingPort, Map options) { + Channel secureChannel = + ManagedChannelBuilder.forAddress("localhost", feastServingPort).usePlaintext().build(); + + if (isSecure) { + CallCredentials callCredentials = null; + callCredentials = new OAuthCredentials(options); + return ServingServiceGrpc.newBlockingStub(secureChannel).withCallCredentials(callCredentials); + } else { + return ServingServiceGrpc.newBlockingStub(secureChannel); + } + } + + public static void seedHydra( + String hydraExternalUrl, + String clientId, + String clientSecrret, + String audience, + String grantType) + throws IOException, InitializationError { + + OkHttpClient httpClient = new OkHttpClient(); + String createClientEndpoint = String.format("%s/%s", hydraExternalUrl, "clients"); + JsonObject jsonObject = new JsonObject(); + JsonArray audienceArrray = new JsonArray(); + audienceArrray.add(audience); + JsonArray grantTypes = new JsonArray(); + grantTypes.add(grantType); + jsonObject.addProperty("client_id", clientId); + jsonObject.addProperty("client_secret", clientSecrret); + jsonObject.addProperty("token_endpoint_auth_method", "client_secret_post"); + jsonObject.add("audience", audienceArrray); + jsonObject.add("grant_types", grantTypes); + MediaType JSON = MediaType.parse("application/json; charset=utf-8"); + + RequestBody requestBody = RequestBody.create(JSON, jsonObject.toString()); + Request request = + new Request.Builder() + .url(createClientEndpoint) + .addHeader("Content-Type", "application/json") + .post(requestBody) + .build(); + Response response = httpClient.newCall(request).execute(); + if (!response.isSuccessful()) { + throw new InitializationError(response.message()); + } + } +} diff --git a/serving/src/test/java/feast/serving/it/BaseAuthIT.java b/serving/src/test/java/feast/serving/it/BaseAuthIT.java new file mode 100644 index 0000000000..023ee67388 --- /dev/null +++ b/serving/src/test/java/feast/serving/it/BaseAuthIT.java @@ -0,0 +1,86 @@ +/* + * 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.serving.it; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.HashMap; +import java.util.Map; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; + +@ActiveProfiles("it") +@SpringBootTest +public class BaseAuthIT { + + static final String FEATURE_NAME = "feature_1"; + static final String ENTITY_ID = "entity_id"; + static final String PROJECT_NAME = "project_1"; + static final int CORE_START_MAX_WAIT_TIME_IN_MINUTES = 3; + static final String CLIENT_ID = "client_id"; + static final String CLIENT_SECRET = "client_secret"; + static final String TOKEN_URL = "http://localhost:4444/oauth2/token"; + static final String JWK_URI = "http://localhost:4444/.well-known/jwks.json"; + + static final String GRANT_TYPE = "client_credentials"; + + static final String AUDIENCE = "https://localhost"; + + static final String CORE = "core_1"; + + static final String HYDRA = "hydra_1"; + static final Map options = new HashMap<>(); + static final int HYDRA_PORT = 4445; + + static CoreSimpleAPIClient insecureApiClient; + + static final int REDIS_PORT = 6379; + + static final int FEAST_CORE_PORT = 6565; + static final int FEAST_SERVING_PORT = 6566; + + @DynamicPropertySource + static void initialize(DynamicPropertyRegistry registry) throws UnknownHostException { + registry.add("feast.stores[0].name", () -> "online"); + registry.add("feast.stores[0].type", () -> "REDIS"); + // Redis needs to accessible by both core and serving, hence using host address + registry.add( + "feast.stores[0].config.host", + () -> { + try { + return InetAddress.getLocalHost().getHostAddress(); + } catch (UnknownHostException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + return ""; + } + }); + registry.add("feast.stores[0].config.port", () -> REDIS_PORT); + registry.add("feast.stores[0].subscriptions[0].name", () -> "*"); + registry.add("feast.stores[0].subscriptions[0].project", () -> "*"); + + registry.add("feast.core-authentication.options.oauth_url", () -> TOKEN_URL); + registry.add("feast.core-authentication.options.grant_type", () -> GRANT_TYPE); + registry.add("feast.core-authentication.options.client_id", () -> CLIENT_ID); + registry.add("feast.core-authentication.options.client_secret", () -> CLIENT_SECRET); + registry.add("feast.core-authentication.options.audience", () -> AUDIENCE); + registry.add("feast.core-authentication.options.jwkEndpointURI", () -> JWK_URI); + registry.add("feast.security.authentication.options.jwkEndpointURI", () -> JWK_URI); + } +} diff --git a/serving/src/test/java/feast/serving/it/CoreSimpleAPIClient.java b/serving/src/test/java/feast/serving/it/CoreSimpleAPIClient.java new file mode 100644 index 0000000000..7d9313150d --- /dev/null +++ b/serving/src/test/java/feast/serving/it/CoreSimpleAPIClient.java @@ -0,0 +1,43 @@ +/* + * 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.serving.it; + +import feast.proto.core.CoreServiceGrpc; +import feast.proto.core.CoreServiceProto; +import feast.proto.core.FeatureSetProto; + +public class CoreSimpleAPIClient { + private CoreServiceGrpc.CoreServiceBlockingStub stub; + + public CoreSimpleAPIClient(CoreServiceGrpc.CoreServiceBlockingStub stub) { + this.stub = stub; + } + + public void simpleApplyFeatureSet(FeatureSetProto.FeatureSet featureSet) { + stub.applyFeatureSet( + CoreServiceProto.ApplyFeatureSetRequest.newBuilder().setFeatureSet(featureSet).build()); + } + + public FeatureSetProto.FeatureSet simpleGetFeatureSet(String projectName, String name) { + return stub.getFeatureSet( + CoreServiceProto.GetFeatureSetRequest.newBuilder() + .setName(name) + .setProject(projectName) + .build()) + .getFeatureSet(); + } +} diff --git a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java new file mode 100644 index 0000000000..e120550530 --- /dev/null +++ b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java @@ -0,0 +1,117 @@ +/* + * 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.serving.it; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.testcontainers.containers.wait.strategy.Wait.forHttp; + +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesResponse; +import feast.proto.serving.ServingServiceGrpc.ServingServiceBlockingStub; +import feast.proto.types.ValueProto.Value; +import io.grpc.StatusRuntimeException; +import java.io.File; +import java.io.IOException; +import java.time.Duration; +import java.util.Map; +import org.junit.ClassRule; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.runners.model.InitializationError; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; +import org.testcontainers.containers.DockerComposeContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +@ActiveProfiles("it") +@SpringBootTest( + properties = { + "feast.core-authentication.enabled=true", + "feast.core-authentication.provider=oauth", + "feast.security.authentication.enabled=true", + "feast.security.authorization.enabled=false" + }) +@Testcontainers +public class ServingServiceOauthAuthenticationIT extends BaseAuthIT { + + @ClassRule @Container + public static DockerComposeContainer environment = + new DockerComposeContainer( + new File("src/test/resources/docker-compose/docker-compose-it-hydra.yml"), + new File("src/test/resources/docker-compose/docker-compose-it-core.yml")) + .withExposedService(HYDRA, HYDRA_PORT, forHttp("/health/alive").forStatusCode(200)) + .withExposedService( + CORE, + 6565, + Wait.forLogMessage(".*gRPC Server started.*\\n", 1) + .withStartupTimeout(Duration.ofMinutes(CORE_START_MAX_WAIT_TIME_IN_MINUTES))); + + @BeforeAll + static void globalSetup() throws IOException, InitializationError, InterruptedException { + String hydraExternalHost = environment.getServiceHost(HYDRA, HYDRA_PORT); + Integer hydraExternalPort = environment.getServicePort(HYDRA, HYDRA_PORT); + String hydraExternalUrl = String.format("http://%s:%s", hydraExternalHost, hydraExternalPort); + AuthTestUtils.seedHydra(hydraExternalUrl, CLIENT_ID, CLIENT_SECRET, AUDIENCE, GRANT_TYPE); + + // set up options for call credentials + options.put("oauth_url", TOKEN_URL); + options.put(CLIENT_ID, CLIENT_ID); + options.put(CLIENT_SECRET, CLIENT_SECRET); + options.put("jwkEndpointURI", JWK_URI); + options.put("audience", AUDIENCE); + options.put("grant_type", GRANT_TYPE); + } + + @Test + public void shouldNotAllowUnauthenticatedGetOnlineFeatures() { + ServingServiceBlockingStub servingStub = + AuthTestUtils.getServingServiceStub(false, FEAST_SERVING_PORT, null); + GetOnlineFeaturesRequest onlineFeatureRequest = + AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); + Exception exception = + assertThrows( + StatusRuntimeException.class, + () -> { + servingStub.getOnlineFeatures(onlineFeatureRequest); + }); + + String expectedMessage = "UNAUTHENTICATED: Authentication failed"; + String actualMessage = exception.getMessage(); + assertEquals(actualMessage, expectedMessage); + } + + @Test + void canGetOnlineFeaturesIfAuthenticated() { + // apply feature set + CoreSimpleAPIClient coreClient = + AuthTestUtils.getSecureApiClientForCore(FEAST_CORE_PORT, options); + AuthTestUtils.applyFeatureSet(coreClient, PROJECT_NAME, ENTITY_ID, FEATURE_NAME); + ServingServiceBlockingStub servingStub = + AuthTestUtils.getServingServiceStub(true, FEAST_SERVING_PORT, options); + GetOnlineFeaturesRequest onlineFeatureRequest = + AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); + GetOnlineFeaturesResponse featureResponse = servingStub.getOnlineFeatures(onlineFeatureRequest); + assertEquals(1, featureResponse.getFieldValuesCount()); + Map fieldsMap = featureResponse.getFieldValues(0).getFieldsMap(); + assertTrue(fieldsMap.containsKey(ENTITY_ID)); + assertTrue(fieldsMap.containsKey(FEATURE_NAME)); + } +} diff --git a/serving/src/test/resources/application-it.properties b/serving/src/test/resources/application-it.properties new file mode 100644 index 0000000000..000e512a68 --- /dev/null +++ b/serving/src/test/resources/application-it.properties @@ -0,0 +1,18 @@ +# +# Copyright 2018 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. + +feast.core-authentication.enabled=false +feast.security.authentication.enabled=false +feast.security.authorization.enabled=false \ No newline at end of file diff --git a/serving/src/test/resources/docker-compose/core-it.yml b/serving/src/test/resources/docker-compose/core-it.yml new file mode 100644 index 0000000000..35f2ee5463 --- /dev/null +++ b/serving/src/test/resources/docker-compose/core-it.yml @@ -0,0 +1,21 @@ +feast: + jobs: + polling_interval_milliseconds: 30000 + job_update_timeout_seconds: 240 + active_runner: direct + runners: + - name: direct + type: DirectRunner + options: {} + stream: + type: kafka + options: + topic: feast-features + bootstrapServers: "kafka:9092,localhost:9094" + + security: + authentication: + enabled: true + provider: jwt + options: + jwkEndpointURI: http://hydra:4444/.well-known/jwks.json \ No newline at end of file diff --git a/serving/src/test/resources/docker-compose/docker-compose-it-core.yml b/serving/src/test/resources/docker-compose/docker-compose-it-core.yml new file mode 100644 index 0000000000..0a686ffaa4 --- /dev/null +++ b/serving/src/test/resources/docker-compose/docker-compose-it-core.yml @@ -0,0 +1,53 @@ +version: '3' + +services: + core: + image: gcr.io/kf-feast/feast-core:latest + volumes: + - ./core-it.yml:/etc/feast/application.yml + environment: + DB_HOST: db + restart: on-failure + depends_on: + - db + - kafka + ports: + - 6565:6565 + command: + - java + - -jar + - /opt/feast/feast-core.jar + - --spring.config.location=classpath:/application.yml,file:/etc/feast/application.yml + + kafka: + image: confluentinc/cp-kafka:5.2.1 + environment: + KAFKA_ZOOKEEPER_CONNECT: zookeeper:2181 + KAFKA_OFFSETS_TOPIC_REPLICATION_FACTOR: 1 + KAFKA_ADVERTISED_LISTENERS: INSIDE://kafka:9092,OUTSIDE://localhost:9094 + KAFKA_LISTENERS: INSIDE://:9092,OUTSIDE://:9094 + KAFKA_LISTENER_SECURITY_PROTOCOL_MAP: INSIDE:PLAINTEXT,OUTSIDE:PLAINTEXT + KAFKA_INTER_BROKER_LISTENER_NAME: INSIDE + ports: + - "9092:9092" + - "9094:9094" + + depends_on: + - zookeeper + + zookeeper: + image: confluentinc/cp-zookeeper:5.2.1 + environment: + ZOOKEEPER_CLIENT_PORT: 2181 + + db: + image: postgres:12-alpine + environment: + POSTGRES_PASSWORD: password + ports: + - "5432:5432" + + redis: + image: redis:5-alpine + ports: + - "6379:6379" \ No newline at end of file diff --git a/serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml b/serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml new file mode 100644 index 0000000000..42b7c346f2 --- /dev/null +++ b/serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml @@ -0,0 +1,54 @@ +version: '3' + +services: + hydra-migrate: + image: oryd/hydra:v1.6.0 + environment: + - DSN=postgres://hydra:secret@postgresd:5433/hydra?sslmode=disable&max_conns=20&max_idle_conns=4 + command: + migrate sql -e --yes + restart: on-failure + + hydra: + depends_on: + - hydra-migrate + environment: + - DSN=postgres://hydra:secret@postgresd:5433/hydra?sslmode=disable&max_conns=20&max_idle_conns=4 + + postgresd: + image: postgres:9.6 + ports: + - "5433:5433" + environment: + - POSTGRES_USER=hydra + - POSTGRES_PASSWORD=secret + - POSTGRES_DB=hydra + + hydra: + image: oryd/hydra:v1.6.0 + ports: + - "4444:4444" # Public port + - "4445:4445" # Admin port + #- "5555:5555" # Port for hydra token user + command: + serve all --dangerous-force-http + environment: + - URLS_SELF_ISSUER=http://hydra:4444 + - URLS_CONSENT=http://hydra:3000/consent + - URLS_LOGIN=http://hydra:3000/login + - URLS_LOGOUT=http://hydra:3000/logout + - DSN=memory + - SECRETS_SYSTEM=youReallyNeedToChangeThis + - OIDC_SUBJECT_IDENTIFIERS_SUPPORTED_TYPES=public,pairwise + - OIDC_SUBJECT_IDENTIFIERS_PAIRWISE_SALT=youReallyNeedToChangeThis + - OAUTH2_ACCESS_TOKEN_STRATEGY=jwt + - OIDC_SUBJECT_IDENTIFIERS_SUPPORTED_TYPES=public + restart: unless-stopped + + consent: + environment: + - HYDRA_ADMIN_URL=http://hydra:4445 + image: oryd/hydra-login-consent-node:v1.5.2 + ports: + - "3000:3000" + restart: unless-stopped From 59bf780ef34add2d80fec48a2865d619ab0a2d9c Mon Sep 17 00:00:00 2001 From: e10112844 Date: Tue, 28 Jul 2020 12:45:46 -0400 Subject: [PATCH 6/8] Add authorization test and minor refactoring. --- serving/pom.xml | 6 + .../ServingServiceGRpcController.java | 34 ++- serving/src/main/resources/application.yml | 2 +- .../java/feast/serving/it/AuthTestUtils.java | 72 +++++++ .../java/feast/serving/it/BaseAuthIT.java | 6 +- .../ServingServiceOauthAuthenticationIT.java | 3 + .../ServingServiceOauthAuthroizationIT.java | 204 ++++++++++++++++++ .../{core-it.yml => core/application-it.yml} | 0 .../docker-compose/docker-compose-it-core.yml | 2 +- .../docker-compose-it-hydra.yml | 6 +- .../docker-compose/docker-compose-it-keto.yml | 44 ++++ 11 files changed, 358 insertions(+), 21 deletions(-) create mode 100644 serving/src/test/java/feast/serving/it/ServingServiceOauthAuthroizationIT.java rename serving/src/test/resources/docker-compose/{core-it.yml => core/application-it.yml} (100%) create mode 100644 serving/src/test/resources/docker-compose/docker-compose-it-keto.yml diff --git a/serving/pom.xml b/serving/pom.xml index 47006e6e14..70da0e3b04 100644 --- a/serving/pom.xml +++ b/serving/pom.xml @@ -334,6 +334,12 @@ 3.0.0 test + + sh.ory.keto + keto-client + 0.4.4-alpha.1 + test + diff --git a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java index be0bd411f2..ad97747ab1 100644 --- a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java +++ b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java @@ -38,6 +38,8 @@ import io.opentracing.Span; import io.opentracing.Tracer; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import net.devh.boot.grpc.server.service.GrpcService; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; @@ -83,11 +85,15 @@ public void getOnlineFeatures( Span span = tracer.buildSpan("getOnlineFeatures").start(); try (Scope scope = tracer.scopeManager().activate(span, false)) { // authorize for the project in request object. - this.authorizationService.authorizeRequest( - SecurityContextHolder.getContext(), request.getProject()); - // authorize for projects set in feature list, backward compatibility for - // <=v0.5.X - this.checkProjectAccess(request.getFeaturesList()); + if (request.getProject() != null && !request.getProject().isEmpty()) { + // project set at root level overrides the project set at feature set level + this.authorizationService.authorizeRequest( + SecurityContextHolder.getContext(), request.getProject()); + } else { + // authorize for projects set in feature list, backward compatibility for + // <=v0.5.X + this.checkProjectAccess(request.getFeaturesList()); + } RequestHelper.validateOnlineRequest(request); GetOnlineFeaturesResponse onlineFeatures = servingService.getOnlineFeatures(request); responseObserver.onNext(onlineFeatures); @@ -149,11 +155,17 @@ public void getJob(GetJobRequest request, StreamObserver respons } private void checkProjectAccess(List featureList) { - featureList.stream() - .forEach( - featureRef -> { - this.authorizationService.authorizeRequest( - SecurityContextHolder.getContext(), featureRef.getProject()); - }); + Set projectList = + featureList.stream().map(FeatureReference::getProject).collect(Collectors.toSet()); + if (projectList.isEmpty()) { + authorizationService.authorizeRequest(SecurityContextHolder.getContext(), "default"); + } else { + projectList.stream() + .forEach( + project -> { + this.authorizationService.authorizeRequest( + SecurityContextHolder.getContext(), project); + }); + } } } diff --git a/serving/src/main/resources/application.yml b/serving/src/main/resources/application.yml index 3fef07eb1a..08fcdf2874 100644 --- a/serving/src/main/resources/application.yml +++ b/serving/src/main/resources/application.yml @@ -30,7 +30,7 @@ feast: jwkEndpointURI: "https://www.googleapis.com/oauth2/v3/certs" authorization: enabled: false - provider: none + provider: http options: basePath: http://localhost:3000 diff --git a/serving/src/test/java/feast/serving/it/AuthTestUtils.java b/serving/src/test/java/feast/serving/it/AuthTestUtils.java index 31ca0ff661..5ec7298e98 100644 --- a/serving/src/test/java/feast/serving/it/AuthTestUtils.java +++ b/serving/src/test/java/feast/serving/it/AuthTestUtils.java @@ -40,6 +40,8 @@ import io.grpc.ManagedChannelBuilder; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -51,9 +53,17 @@ import okhttp3.Response; import org.apache.commons.lang3.tuple.Pair; import org.junit.runners.model.InitializationError; +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.OryAccessControlPolicy; +import sh.ory.keto.model.OryAccessControlPolicyRole; public class AuthTestUtils { + private static final String DEFAULT_FLAVOR = "glob"; + static SourceProto.Source defaultSource = createSource("kafka:9092,localhost:9094", "feast-features"); @@ -208,4 +218,66 @@ public static void seedHydra( throw new InitializationError(response.message()); } } + + public static void seedKeto(String url, String project, String subjectInProject, String admin) + throws ApiException { + ApiClient ketoClient = Configuration.getDefaultApiClient(); + ketoClient.setBasePath(url); + EnginesApi enginesApi = new EnginesApi(ketoClient); + + // Add policies + OryAccessControlPolicy adminPolicy = getAdminPolicy(); + enginesApi.upsertOryAccessControlPolicy(DEFAULT_FLAVOR, adminPolicy); + + OryAccessControlPolicy projectPolicy = getMyProjectMemberPolicy(project); + enginesApi.upsertOryAccessControlPolicy(DEFAULT_FLAVOR, projectPolicy); + + // Add policy roles + OryAccessControlPolicyRole adminPolicyRole = getAdminPolicyRole(admin); + enginesApi.upsertOryAccessControlPolicyRole(DEFAULT_FLAVOR, adminPolicyRole); + + OryAccessControlPolicyRole myProjectMemberPolicyRole = + getMyProjectMemberPolicyRole(project, subjectInProject); + enginesApi.upsertOryAccessControlPolicyRole(DEFAULT_FLAVOR, myProjectMemberPolicyRole); + } + + private static OryAccessControlPolicyRole getMyProjectMemberPolicyRole( + String project, String subjectInProject) { + OryAccessControlPolicyRole role = new OryAccessControlPolicyRole(); + role.setId(String.format("roles:%s-project-members", project)); + role.setMembers(Collections.singletonList("users:" + subjectInProject)); + return role; + } + + private static OryAccessControlPolicyRole getAdminPolicyRole(String subjectIsAdmin) { + OryAccessControlPolicyRole role = new OryAccessControlPolicyRole(); + role.setId("roles:admin"); + role.setMembers(Collections.singletonList("users:" + subjectIsAdmin)); + return role; + } + + private static OryAccessControlPolicy getAdminPolicy() { + OryAccessControlPolicy policy = new OryAccessControlPolicy(); + policy.setId("policies:admin"); + policy.subjects(Collections.singletonList("roles:admin")); + policy.resources(Collections.singletonList("resources:**")); + policy.actions(Collections.singletonList("actions:**")); + policy.effect("allow"); + policy.conditions(null); + return policy; + } + + private static OryAccessControlPolicy getMyProjectMemberPolicy(String project) { + OryAccessControlPolicy policy = new OryAccessControlPolicy(); + policy.setId(String.format("policies:%s-project-members-policy", project)); + policy.subjects(Collections.singletonList(String.format("roles:%s-project-members", project))); + policy.resources( + Arrays.asList( + String.format("resources:projects:%s", project), + String.format("resources:projects:%s:**", project))); + policy.actions(Collections.singletonList("actions:**")); + policy.effect("allow"); + policy.conditions(null); + return policy; + } } diff --git a/serving/src/test/java/feast/serving/it/BaseAuthIT.java b/serving/src/test/java/feast/serving/it/BaseAuthIT.java index 023ee67388..2d8701cd04 100644 --- a/serving/src/test/java/feast/serving/it/BaseAuthIT.java +++ b/serving/src/test/java/feast/serving/it/BaseAuthIT.java @@ -18,8 +18,6 @@ import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.HashMap; -import java.util.Map; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.DynamicPropertyRegistry; @@ -45,7 +43,6 @@ public class BaseAuthIT { static final String CORE = "core_1"; static final String HYDRA = "hydra_1"; - static final Map options = new HashMap<>(); static final int HYDRA_PORT = 4445; static CoreSimpleAPIClient insecureApiClient; @@ -56,7 +53,7 @@ public class BaseAuthIT { static final int FEAST_SERVING_PORT = 6566; @DynamicPropertySource - static void initialize(DynamicPropertyRegistry registry) throws UnknownHostException { + static void properties(DynamicPropertyRegistry registry) { registry.add("feast.stores[0].name", () -> "online"); registry.add("feast.stores[0].type", () -> "REDIS"); // Redis needs to accessible by both core and serving, hence using host address @@ -66,7 +63,6 @@ static void initialize(DynamicPropertyRegistry registry) throws UnknownHostExcep try { return InetAddress.getLocalHost().getHostAddress(); } catch (UnknownHostException e) { - // TODO Auto-generated catch block e.printStackTrace(); return ""; } diff --git a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java index e120550530..b4a4c4b76e 100644 --- a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java +++ b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java @@ -29,6 +29,7 @@ import java.io.File; import java.io.IOException; import java.time.Duration; +import java.util.HashMap; import java.util.Map; import org.junit.ClassRule; import org.junit.jupiter.api.BeforeAll; @@ -52,6 +53,8 @@ @Testcontainers public class ServingServiceOauthAuthenticationIT extends BaseAuthIT { + static final Map options = new HashMap<>(); + @ClassRule @Container public static DockerComposeContainer environment = new DockerComposeContainer( diff --git a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthroizationIT.java b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthroizationIT.java new file mode 100644 index 0000000000..5ec7767307 --- /dev/null +++ b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthroizationIT.java @@ -0,0 +1,204 @@ +/* + * 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.serving.it; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.testcontainers.containers.wait.strategy.Wait.forHttp; + +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesResponse; +import feast.proto.serving.ServingServiceGrpc.ServingServiceBlockingStub; +import feast.proto.types.ValueProto.Value; +import io.grpc.StatusRuntimeException; +import java.io.File; +import java.io.IOException; +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; +import org.junit.ClassRule; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.runners.model.InitializationError; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.containers.DockerComposeContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import sh.ory.keto.ApiException; + +@ActiveProfiles("it") +@SpringBootTest( + properties = { + "feast.core-authentication.enabled=true", + "feast.core-authentication.provider=oauth", + "feast.security.authentication.enabled=true", + "feast.security.authorization.enabled=true" + }) +@Testcontainers +public class ServingServiceOauthAuthroizationIT extends BaseAuthIT { + + static final Map adminCredentials = new HashMap<>(); + static final Map memberCredentials = new HashMap<>(); + static final String PROJECT_MEMBER_CLIENT_ID = "client_id_1"; + static final String NOT_PROJECT_MEMBER_CLIENT_ID = "client_id_2"; + private static int KETO_PORT = 4466; + private static int KETO_ADAPTOR_PORT = 8080; + static String subjectClaim = "sub"; + static CoreSimpleAPIClient coreClient; + + @ClassRule @Container + public static DockerComposeContainer environment = + new DockerComposeContainer( + new File("src/test/resources/docker-compose/docker-compose-it-hydra.yml"), + new File("src/test/resources/docker-compose/docker-compose-it-core.yml"), + new File("src/test/resources/docker-compose/docker-compose-it-keto.yml")) + .withExposedService(HYDRA, HYDRA_PORT, forHttp("/health/alive").forStatusCode(200)) + .withExposedService( + CORE, + 6565, + Wait.forLogMessage(".*gRPC Server started.*\\n", 1) + .withStartupTimeout(Duration.ofMinutes(CORE_START_MAX_WAIT_TIME_IN_MINUTES))) + .withExposedService("adaptor_1", KETO_ADAPTOR_PORT) + .withExposedService("keto_1", KETO_PORT, forHttp("/health/ready").forStatusCode(200));; + + @DynamicPropertySource + static void initialize(DynamicPropertyRegistry registry) { + + // Seed Keto with data + String ketoExternalHost = environment.getServiceHost("keto_1", KETO_PORT); + Integer ketoExternalPort = environment.getServicePort("keto_1", KETO_PORT); + String ketoExternalUrl = String.format("http://%s:%s", ketoExternalHost, ketoExternalPort); + try { + AuthTestUtils.seedKeto(ketoExternalUrl, PROJECT_NAME, PROJECT_MEMBER_CLIENT_ID, CLIENT_ID); + } catch (ApiException e) { + throw new RuntimeException(String.format("Could not seed Keto store %s", ketoExternalUrl)); + } + + // Get Keto Authorization Server (Adaptor) url + String ketoAdaptorHost = environment.getServiceHost("adaptor_1", KETO_ADAPTOR_PORT); + Integer ketoAdaptorPort = environment.getServicePort("adaptor_1", KETO_ADAPTOR_PORT); + String ketoAdaptorUrl = String.format("http://%s:%s", ketoAdaptorHost, ketoAdaptorPort); + + // Initialize dynamic properties + registry.add("feast.security.authorization.options.subjectClaim", () -> subjectClaim); + registry.add("feast.security.authentication.options.jwkEndpointURI", () -> JWK_URI); + registry.add("feast.security.authorization.options.authorizationUrl", () -> ketoAdaptorUrl); + } + + @BeforeAll + static void globalSetup() throws IOException, InitializationError, InterruptedException { + String hydraExternalHost = environment.getServiceHost(HYDRA, HYDRA_PORT); + Integer hydraExternalPort = environment.getServicePort(HYDRA, HYDRA_PORT); + String hydraExternalUrl = String.format("http://%s:%s", hydraExternalHost, hydraExternalPort); + AuthTestUtils.seedHydra(hydraExternalUrl, CLIENT_ID, CLIENT_SECRET, AUDIENCE, GRANT_TYPE); + AuthTestUtils.seedHydra( + hydraExternalUrl, PROJECT_MEMBER_CLIENT_ID, CLIENT_SECRET, AUDIENCE, GRANT_TYPE); + AuthTestUtils.seedHydra( + hydraExternalUrl, NOT_PROJECT_MEMBER_CLIENT_ID, CLIENT_SECRET, AUDIENCE, GRANT_TYPE); + // set up options for call credentials + adminCredentials.put("oauth_url", TOKEN_URL); + adminCredentials.put(CLIENT_ID, CLIENT_ID); + adminCredentials.put(CLIENT_SECRET, CLIENT_SECRET); + adminCredentials.put("jwkEndpointURI", JWK_URI); + adminCredentials.put("audience", AUDIENCE); + adminCredentials.put("grant_type", GRANT_TYPE); + + coreClient = AuthTestUtils.getSecureApiClientForCore(FEAST_CORE_PORT, adminCredentials); + } + + @BeforeEach + public void setUp() { + // seed core + AuthTestUtils.applyFeatureSet(coreClient, PROJECT_NAME, ENTITY_ID, FEATURE_NAME); + } + + @Test + public void shouldNotAllowUnauthenticatedGetOnlineFeatures() { + ServingServiceBlockingStub servingStub = + AuthTestUtils.getServingServiceStub(false, FEAST_SERVING_PORT, null); + GetOnlineFeaturesRequest onlineFeatureRequest = + AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); + Exception exception = + assertThrows( + StatusRuntimeException.class, + () -> { + servingStub.getOnlineFeatures(onlineFeatureRequest); + }); + + String expectedMessage = "UNAUTHENTICATED: Authentication failed"; + String actualMessage = exception.getMessage(); + assertEquals(actualMessage, expectedMessage); + } + + @Test + void canGetOnlineFeaturesIfAdmin() { + // apply feature set + ServingServiceBlockingStub servingStub = + AuthTestUtils.getServingServiceStub(true, FEAST_SERVING_PORT, adminCredentials); + GetOnlineFeaturesRequest onlineFeatureRequest = + AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); + GetOnlineFeaturesResponse featureResponse = servingStub.getOnlineFeatures(onlineFeatureRequest); + assertEquals(1, featureResponse.getFieldValuesCount()); + Map fieldsMap = featureResponse.getFieldValues(0).getFieldsMap(); + assertTrue(fieldsMap.containsKey(ENTITY_ID)); + assertTrue(fieldsMap.containsKey(FEATURE_NAME)); + } + + @Test + void canGetOnlineFeaturesIfProjectMember() { + Map memberCredsOptions = new HashMap<>(); + memberCredsOptions.putAll(adminCredentials); + memberCredsOptions.put(CLIENT_ID, PROJECT_MEMBER_CLIENT_ID); + ServingServiceBlockingStub servingStub = + AuthTestUtils.getServingServiceStub(true, FEAST_SERVING_PORT, memberCredsOptions); + GetOnlineFeaturesRequest onlineFeatureRequest = + AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); + GetOnlineFeaturesResponse featureResponse = servingStub.getOnlineFeatures(onlineFeatureRequest); + assertEquals(1, featureResponse.getFieldValuesCount()); + Map fieldsMap = featureResponse.getFieldValues(0).getFieldsMap(); + assertTrue(fieldsMap.containsKey(ENTITY_ID)); + assertTrue(fieldsMap.containsKey(FEATURE_NAME)); + } + + @Test + void cantGetOnlineFeaturesIfNotProjectMember() { + Map notMemberCredsOptions = new HashMap<>(); + notMemberCredsOptions.putAll(adminCredentials); + notMemberCredsOptions.put(CLIENT_ID, NOT_PROJECT_MEMBER_CLIENT_ID); + ServingServiceBlockingStub servingStub = + AuthTestUtils.getServingServiceStub(true, FEAST_SERVING_PORT, notMemberCredsOptions); + GetOnlineFeaturesRequest onlineFeatureRequest = + AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); + StatusRuntimeException exception = + assertThrows( + StatusRuntimeException.class, + () -> servingStub.getOnlineFeatures(onlineFeatureRequest)); + + String expectedMessage = + String.format( + "PERMISSION_DENIED: Access denied to project %s for subject %s", + PROJECT_NAME, NOT_PROJECT_MEMBER_CLIENT_ID); + String actualMessage = exception.getMessage(); + assertEquals(actualMessage, expectedMessage); + } +} diff --git a/serving/src/test/resources/docker-compose/core-it.yml b/serving/src/test/resources/docker-compose/core/application-it.yml similarity index 100% rename from serving/src/test/resources/docker-compose/core-it.yml rename to serving/src/test/resources/docker-compose/core/application-it.yml diff --git a/serving/src/test/resources/docker-compose/docker-compose-it-core.yml b/serving/src/test/resources/docker-compose/docker-compose-it-core.yml index 0a686ffaa4..bb7cdce8ab 100644 --- a/serving/src/test/resources/docker-compose/docker-compose-it-core.yml +++ b/serving/src/test/resources/docker-compose/docker-compose-it-core.yml @@ -4,7 +4,7 @@ services: core: image: gcr.io/kf-feast/feast-core:latest volumes: - - ./core-it.yml:/etc/feast/application.yml + - ./core/application-it.yml:/etc/feast/application.yml environment: DB_HOST: db restart: on-failure diff --git a/serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml b/serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml index 42b7c346f2..1c20610cc7 100644 --- a/serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml +++ b/serving/src/test/resources/docker-compose/docker-compose-it-hydra.yml @@ -4,7 +4,7 @@ services: hydra-migrate: image: oryd/hydra:v1.6.0 environment: - - DSN=postgres://hydra:secret@postgresd:5433/hydra?sslmode=disable&max_conns=20&max_idle_conns=4 + - DSN=postgres://hydra:secret@postgresd:5432/hydra?sslmode=disable&max_conns=20&max_idle_conns=4 command: migrate sql -e --yes restart: on-failure @@ -13,12 +13,12 @@ services: depends_on: - hydra-migrate environment: - - DSN=postgres://hydra:secret@postgresd:5433/hydra?sslmode=disable&max_conns=20&max_idle_conns=4 + - DSN=postgres://hydra:secret@postgresd:5432/hydra?sslmode=disable&max_conns=20&max_idle_conns=4 postgresd: image: postgres:9.6 ports: - - "5433:5433" + - "54320:5432" environment: - POSTGRES_USER=hydra - POSTGRES_PASSWORD=secret diff --git a/serving/src/test/resources/docker-compose/docker-compose-it-keto.yml b/serving/src/test/resources/docker-compose/docker-compose-it-keto.yml new file mode 100644 index 0000000000..8ebf7f225e --- /dev/null +++ b/serving/src/test/resources/docker-compose/docker-compose-it-keto.yml @@ -0,0 +1,44 @@ +version: '3' +services: + keto: + depends_on: + - ketodb + - migrations + image: oryd/keto:v0.4.3-alpha.2 + environment: + - DSN=postgres://keto:keto@ketodb:5432/keto?sslmode=disable + command: + - serve + ports: + - 4466 + + ketodb: + image: bitnami/postgresql:9.6 + environment: + - POSTGRESQL_USERNAME=keto + - POSTGRESQL_PASSWORD=keto + - POSTGRESQL_DATABASE=keto + ports: + - "54340:5432" + + migrations: + depends_on: + - ketodb + image: oryd/keto:v0.4.3-alpha.2 + environment: + - DSN=postgres://keto:keto@ketodb:5432/keto?sslmode=disable + command: + - migrate + - sql + - -e + + adaptor: + depends_on: + - keto + image: gcr.io/kf-feast/feast-keto-auth-server:latest + environment: + SERVER_PORT: 8080 + KETO_URL: http://keto:4466 + ports: + - 8080 + restart: on-failure \ No newline at end of file From 4d37a79a09444b0420ceaa33585251d2782ef1f5 Mon Sep 17 00:00:00 2001 From: e10112844 Date: Tue, 28 Jul 2020 15:32:40 -0400 Subject: [PATCH 7/8] fix failing integration test. --- serving/src/test/java/feast/serving/it/BaseAuthIT.java | 1 - .../it/ServingServiceOauthAuthenticationIT.java | 4 ++++ ...IT.java => ServingServiceOauthAuthorizationIT.java} | 10 +++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) rename serving/src/test/java/feast/serving/it/{ServingServiceOauthAuthroizationIT.java => ServingServiceOauthAuthorizationIT.java} (95%) diff --git a/serving/src/test/java/feast/serving/it/BaseAuthIT.java b/serving/src/test/java/feast/serving/it/BaseAuthIT.java index 2d8701cd04..bdbe432ed8 100644 --- a/serving/src/test/java/feast/serving/it/BaseAuthIT.java +++ b/serving/src/test/java/feast/serving/it/BaseAuthIT.java @@ -50,7 +50,6 @@ public class BaseAuthIT { static final int REDIS_PORT = 6379; static final int FEAST_CORE_PORT = 6565; - static final int FEAST_SERVING_PORT = 6566; @DynamicPropertySource static void properties(DynamicPropertyRegistry registry) { diff --git a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java index b4a4c4b76e..edd16c24a8 100644 --- a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java +++ b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthenticationIT.java @@ -25,6 +25,7 @@ import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesResponse; import feast.proto.serving.ServingServiceGrpc.ServingServiceBlockingStub; import feast.proto.types.ValueProto.Value; +import io.grpc.ManagedChannel; import io.grpc.StatusRuntimeException; import java.io.File; import java.io.IOException; @@ -55,6 +56,8 @@ public class ServingServiceOauthAuthenticationIT extends BaseAuthIT { static final Map options = new HashMap<>(); + static final int FEAST_SERVING_PORT = 6566; + @ClassRule @Container public static DockerComposeContainer environment = new DockerComposeContainer( @@ -116,5 +119,6 @@ void canGetOnlineFeaturesIfAuthenticated() { Map fieldsMap = featureResponse.getFieldValues(0).getFieldsMap(); assertTrue(fieldsMap.containsKey(ENTITY_ID)); assertTrue(fieldsMap.containsKey(FEATURE_NAME)); + ((ManagedChannel) servingStub.getChannel()).shutdown(); } } diff --git a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthroizationIT.java b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthorizationIT.java similarity index 95% rename from serving/src/test/java/feast/serving/it/ServingServiceOauthAuthroizationIT.java rename to serving/src/test/java/feast/serving/it/ServingServiceOauthAuthorizationIT.java index 5ec7767307..aaee2321a5 100644 --- a/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthroizationIT.java +++ b/serving/src/test/java/feast/serving/it/ServingServiceOauthAuthorizationIT.java @@ -25,6 +25,7 @@ import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesResponse; import feast.proto.serving.ServingServiceGrpc.ServingServiceBlockingStub; import feast.proto.types.ValueProto.Value; +import io.grpc.ManagedChannel; import io.grpc.StatusRuntimeException; import java.io.File; import java.io.IOException; @@ -55,7 +56,7 @@ "feast.security.authorization.enabled=true" }) @Testcontainers -public class ServingServiceOauthAuthroizationIT extends BaseAuthIT { +public class ServingServiceOauthAuthorizationIT extends BaseAuthIT { static final Map adminCredentials = new HashMap<>(); static final Map memberCredentials = new HashMap<>(); @@ -65,6 +66,7 @@ public class ServingServiceOauthAuthroizationIT extends BaseAuthIT { private static int KETO_ADAPTOR_PORT = 8080; static String subjectClaim = "sub"; static CoreSimpleAPIClient coreClient; + static final int FEAST_SERVING_PORT = 6766; @ClassRule @Container public static DockerComposeContainer environment = @@ -103,6 +105,7 @@ static void initialize(DynamicPropertyRegistry registry) { registry.add("feast.security.authorization.options.subjectClaim", () -> subjectClaim); registry.add("feast.security.authentication.options.jwkEndpointURI", () -> JWK_URI); registry.add("feast.security.authorization.options.authorizationUrl", () -> ketoAdaptorUrl); + registry.add("grpc.server.port", () -> FEAST_SERVING_PORT); } @BeforeAll @@ -136,6 +139,7 @@ public void setUp() { public void shouldNotAllowUnauthenticatedGetOnlineFeatures() { ServingServiceBlockingStub servingStub = AuthTestUtils.getServingServiceStub(false, FEAST_SERVING_PORT, null); + GetOnlineFeaturesRequest onlineFeatureRequest = AuthTestUtils.createOnlineFeatureRequest(PROJECT_NAME, FEATURE_NAME, ENTITY_ID, 1); Exception exception = @@ -148,6 +152,7 @@ public void shouldNotAllowUnauthenticatedGetOnlineFeatures() { String expectedMessage = "UNAUTHENTICATED: Authentication failed"; String actualMessage = exception.getMessage(); assertEquals(actualMessage, expectedMessage); + ((ManagedChannel) servingStub.getChannel()).shutdown(); } @Test @@ -162,6 +167,7 @@ void canGetOnlineFeaturesIfAdmin() { Map fieldsMap = featureResponse.getFieldValues(0).getFieldsMap(); assertTrue(fieldsMap.containsKey(ENTITY_ID)); assertTrue(fieldsMap.containsKey(FEATURE_NAME)); + ((ManagedChannel) servingStub.getChannel()).shutdown(); } @Test @@ -178,6 +184,7 @@ void canGetOnlineFeaturesIfProjectMember() { Map fieldsMap = featureResponse.getFieldValues(0).getFieldsMap(); assertTrue(fieldsMap.containsKey(ENTITY_ID)); assertTrue(fieldsMap.containsKey(FEATURE_NAME)); + ((ManagedChannel) servingStub.getChannel()).shutdown(); } @Test @@ -200,5 +207,6 @@ void cantGetOnlineFeaturesIfNotProjectMember() { PROJECT_NAME, NOT_PROJECT_MEMBER_CLIENT_ID); String actualMessage = exception.getMessage(); assertEquals(actualMessage, expectedMessage); + ((ManagedChannel) servingStub.getChannel()).shutdown(); } } From aae07e5fdf53115db88ed2f3c7d78f9fc140f719 Mon Sep 17 00:00:00 2001 From: e10112844 Date: Wed, 29 Jul 2020 10:01:58 -0400 Subject: [PATCH 8/8] fix lint error. --- sdk/python/tests/test_client.py | 68 ++++++++++++++++----------------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 86f0438e5c..416d4b2dde 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -24,6 +24,7 @@ import pytest from google.protobuf.duration_pb2 import Duration from mock import MagicMock, patch +from pytest_lazyfixture import lazy_fixture from pytz import timezone from feast.client import Client @@ -253,7 +254,7 @@ def client(self, core_server, serving_server): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [lazy_fixture("mock_client"), lazy_fixture("secure_mock_client")], ) def test_version(self, mocked_client, mocker): mocked_client._core_service_stub = Core.CoreServiceStub( @@ -286,10 +287,10 @@ def test_version(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client,auth_metadata", [ - (pytest.lazy_fixture("mock_client"), ()), - (pytest.lazy_fixture("mock_client_with_auth"), (AUTH_METADATA)), - (pytest.lazy_fixture("secure_mock_client"), ()), - (pytest.lazy_fixture("secure_mock_client_with_auth"), (AUTH_METADATA)), + (lazy_fixture("mock_client"), ()), + (lazy_fixture("mock_client_with_auth"), (AUTH_METADATA)), + (lazy_fixture("secure_mock_client"), ()), + (lazy_fixture("secure_mock_client_with_auth"), (AUTH_METADATA)), ], ids=[ "mock_client_without_auth", @@ -371,7 +372,7 @@ def int_val(x): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [lazy_fixture("mock_client"), lazy_fixture("secure_mock_client")], ) def test_get_feature_set(self, mocked_client, mocker): mocked_client._core_service_stub = Core.CoreServiceStub( @@ -435,7 +436,7 @@ def test_get_feature_set(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [lazy_fixture("mock_client"), lazy_fixture("secure_mock_client")], ) def test_list_feature_sets(self, mocked_client, mocker): mocker.patch.object( @@ -496,7 +497,7 @@ def test_list_feature_sets(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [lazy_fixture("mock_client"), lazy_fixture("secure_mock_client")], ) def test_list_features(self, mocked_client, mocker): mocker.patch.object( @@ -542,7 +543,7 @@ def test_list_features(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [lazy_fixture("mock_client"), lazy_fixture("secure_mock_client")], ) def test_list_ingest_jobs(self, mocked_client, mocker): mocker.patch.object( @@ -598,7 +599,7 @@ def test_list_ingest_jobs(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [lazy_fixture("mock_client"), lazy_fixture("secure_mock_client")], ) def test_restart_ingest_job(self, mocked_client, mocker): mocker.patch.object( @@ -621,7 +622,7 @@ def test_restart_ingest_job(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client", - [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + [lazy_fixture("mock_client"), lazy_fixture("secure_mock_client")], ) def test_stop_ingest_job(self, mocked_client, mocker): mocker.patch.object( @@ -645,10 +646,10 @@ def test_stop_ingest_job(self, mocked_client, mocker): @pytest.mark.parametrize( "mocked_client", [ - pytest.lazy_fixture("mock_client"), - pytest.lazy_fixture("mock_client_with_auth"), - pytest.lazy_fixture("secure_mock_client"), - pytest.lazy_fixture("secure_mock_client_with_auth"), + lazy_fixture("mock_client"), + lazy_fixture("mock_client_with_auth"), + lazy_fixture("secure_mock_client"), + lazy_fixture("secure_mock_client_with_auth"), ], ) def test_get_historical_features(self, mocked_client, mocker): @@ -768,8 +769,7 @@ def test_get_historical_features(self, mocked_client, mocker): assert actual_dataframe[["driver_id"]].equals(expected_dataframe[["driver_id"]]) @pytest.mark.parametrize( - "test_client", - [pytest.lazy_fixture("client"), pytest.lazy_fixture("secure_client")], + "test_client", [lazy_fixture("client"), lazy_fixture("secure_client")], ) def test_apply_feature_set_success(self, test_client): @@ -813,8 +813,8 @@ def test_apply_feature_set_success(self, test_client): @pytest.mark.parametrize( "dataframe,test_client", [ - (dataframes.GOOD, pytest.lazy_fixture("client")), - (dataframes.GOOD, pytest.lazy_fixture("secure_client")), + (dataframes.GOOD, lazy_fixture("client")), + (dataframes.GOOD, lazy_fixture("secure_client")), ], ) def test_feature_set_ingest_success(self, dataframe, test_client, mocker): @@ -845,7 +845,7 @@ def test_feature_set_ingest_success(self, dataframe, test_client, mocker): @pytest.mark.parametrize( "dataframe,test_client,exception", - [(dataframes.GOOD, pytest.lazy_fixture("client"), Exception)], + [(dataframes.GOOD, lazy_fixture("client"), Exception)], ) def test_feature_set_ingest_throws_exception_if_kafka_down( self, dataframe, test_client, exception, mocker @@ -878,8 +878,8 @@ def test_feature_set_ingest_throws_exception_if_kafka_down( @pytest.mark.parametrize( "dataframe,exception,test_client", [ - (dataframes.GOOD, TimeoutError, pytest.lazy_fixture("client")), - (dataframes.GOOD, TimeoutError, pytest.lazy_fixture("secure_client")), + (dataframes.GOOD, TimeoutError, lazy_fixture("client")), + (dataframes.GOOD, TimeoutError, lazy_fixture("secure_client")), ], ) def test_feature_set_ingest_fail_if_pending( @@ -915,26 +915,22 @@ def test_feature_set_ingest_fail_if_pending( @pytest.mark.parametrize( "dataframe,exception,test_client", [ - (dataframes.BAD_NO_DATETIME, Exception, pytest.lazy_fixture("client")), + (dataframes.BAD_NO_DATETIME, Exception, lazy_fixture("client")), ( dataframes.BAD_INCORRECT_DATETIME_TYPE, Exception, - pytest.lazy_fixture("client"), - ), - (dataframes.BAD_NO_ENTITY, Exception, pytest.lazy_fixture("client")), - (dataframes.NO_FEATURES, Exception, pytest.lazy_fixture("client")), - ( - dataframes.BAD_NO_DATETIME, - Exception, - pytest.lazy_fixture("secure_client"), + lazy_fixture("client"), ), + (dataframes.BAD_NO_ENTITY, Exception, lazy_fixture("client")), + (dataframes.NO_FEATURES, Exception, lazy_fixture("client")), + (dataframes.BAD_NO_DATETIME, Exception, lazy_fixture("secure_client"),), ( dataframes.BAD_INCORRECT_DATETIME_TYPE, Exception, - pytest.lazy_fixture("secure_client"), + lazy_fixture("secure_client"), ), - (dataframes.BAD_NO_ENTITY, Exception, pytest.lazy_fixture("secure_client")), - (dataframes.NO_FEATURES, Exception, pytest.lazy_fixture("secure_client")), + (dataframes.BAD_NO_ENTITY, Exception, lazy_fixture("secure_client")), + (dataframes.NO_FEATURES, Exception, lazy_fixture("secure_client")), ], ) def test_feature_set_ingest_failure(self, test_client, dataframe, exception): @@ -954,8 +950,8 @@ def test_feature_set_ingest_failure(self, test_client, dataframe, exception): @pytest.mark.parametrize( "dataframe,test_client", [ - (dataframes.ALL_TYPES, pytest.lazy_fixture("client")), - (dataframes.ALL_TYPES, pytest.lazy_fixture("secure_client")), + (dataframes.ALL_TYPES, lazy_fixture("client")), + (dataframes.ALL_TYPES, lazy_fixture("secure_client")), ], ) def test_feature_set_types_success(self, test_client, dataframe, mocker):