From 601985f4c8fc144b61bc1514e22610d69c1c5bea Mon Sep 17 00:00:00 2001 From: Matt Wise <wsmatth@amazon.com> Date: Thu, 24 Sep 2020 22:26:48 +0000 Subject: [PATCH] Cleanup code and better error handling commit 95798d33 Author: Matt Wise <wsmatth@amazon.com> Date: Thu Sep 24 2020 17:23:31 GMT-0500 (Central Daylight Time) remove unused file commit 14e2a2c0 Author: Matt Wise <wsmatth@amazon.com> Date: Thu Sep 24 2020 17:15:15 GMT-0500 (Central Daylight Time) cleaner authentication error handling commit 39478e81 Author: Matt Wise <wsmatth@amazon.com> Date: Thu Sep 24 2020 16:42:40 GMT-0500 (Central Daylight Time) just use string names for quick check commit 46ed271d Author: Matt Wise <wsmatth@amazon.com> Date: Thu Sep 24 2020 16:28:06 GMT-0500 (Central Daylight Time) fix issues with greater than 10 keys in a partition. fix partition exists check --- .../security/AuthenticationRequestFilter.java | 96 ------------------- .../aws/security/AuthorizationService.java | 13 ++- .../aws/service/PartitionServiceImpl.java | 16 +++- .../provider/aws/util/SSMHelper.java | 84 ++++++++++------ .../partition/api/TestGetPartitionById.java | 10 +- .../osdu/partition/util/AwsTestUtils.java | 4 +- 6 files changed, 86 insertions(+), 137 deletions(-) delete mode 100644 provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthenticationRequestFilter.java diff --git a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthenticationRequestFilter.java b/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthenticationRequestFilter.java deleted file mode 100644 index 891dae938..000000000 --- a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthenticationRequestFilter.java +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright © 2020 Amazon Web Services -// -// 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 -// -// http://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 org.opengroup.osdu.partition.provider.aws.security; - -// import java.io.IOException; -// import java.util.Collections; -// import java.util.function.Function; -// import java.util.logging.Logger; -// import java.util.stream.Collectors; - -// import javax.servlet.FilterChain; -// import javax.servlet.ServletException; -// import javax.servlet.http.HttpServletRequest; -// import javax.servlet.http.HttpServletResponse; - -// import org.opengroup.osdu.core.common.model.http.AppException; -// import org.opengroup.osdu.core.common.model.http.DpsHeaders; -// import org.springframework.http.HttpHeaders; -// import org.springframework.lang.NonNull; -// import org.springframework.security.authentication.AnonymousAuthenticationToken; -// import org.springframework.security.core.context.SecurityContextHolder; -// import org.springframework.util.MultiValueMap; -// import org.springframework.web.filter.OncePerRequestFilter; -// import org.springframework.web.servlet.HandlerExceptionResolver; -// import org.opengroup.osdu.core.common.model.entitlements.Groups; - -// public class AuthenticationRequestFilter extends OncePerRequestFilter { - -// private static Logger logger = Logger.getLogger(AuthenticationRequestFilter.class.getName()); - -// private final HandlerExceptionResolver handlerExceptionResolver; - -// public AuthenticationRequestFilter(HandlerExceptionResolver handlerExceptionResolver) { -// this.handlerExceptionResolver = handlerExceptionResolver; -// } - -// @Override -// protected void doFilterInternal(@NonNull HttpServletRequest httpServletRequest, -// @NonNull HttpServletResponse httpServletResponse, -// @NonNull FilterChain filterChain) throws ServletException, IOException { -// MultiValueMap<String, String> requestHeaders = httpHeaders(httpServletRequest); -// DpsHeaders dpsHeaders = DpsHeaders.createFromEntrySet(requestHeaders.entrySet()); -// dpsHeaders.addCorrelationIdIfMissing(); - -// try { - -// // String message = String.format("User authenticated | User: %s", groups.getMemberEmail()); -// authenticate(null); -// // logger.info(message); -// filterChain.doFilter(httpServletRequest, httpServletResponse); -// } -// // catch (EntitlementsException e) { -// // String message = String.format("User not authenticated. Response: %s", e.getHttpResponse()); -// // logger.warning(message); -// // AppException unauthorized = AppException.createUnauthorized("Error: " + e.getMessage()); -// // handlerExceptionResolver.resolveException(httpServletRequest, httpServletResponse, null, unauthorized); -// // } -// catch (NullPointerException e) { -// String message = String.format("User not authenticated. Null pointer exception: %s", e.getMessage()); -// logger.warning(message); -// AppException unauthorized = AppException.createUnauthorized("Error: " + e.getMessage()); -// handlerExceptionResolver.resolveException(httpServletRequest, httpServletResponse, null, unauthorized); -// } - -// } - -// private void authenticate(Groups groups) { -// OsduAuthentication authentication = new OsduAuthentication(groups, emptyList()); -// authentication.setAuthenticated(true); -// SecurityContextHolder.getContext().setAuthentication(authentication); -// } - -// private HttpHeaders httpHeaders(@NonNull HttpServletRequest httpRequest) { -// return Collections -// .list(httpRequest.getHeaderNames()) -// .stream() -// .collect(Collectors.toMap( -// Function.identity(), -// h -> Collections.list(httpRequest.getHeaders(h)), -// (oldValue, newValue) -> newValue, -// HttpHeaders::new -// )); -// } -// } diff --git a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthorizationService.java b/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthorizationService.java index b88e2e042..f17601a57 100644 --- a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthorizationService.java +++ b/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/security/AuthorizationService.java @@ -15,9 +15,11 @@ package org.opengroup.osdu.partition.provider.aws.security; import org.opengroup.osdu.core.common.entitlements.IEntitlementsAndCacheService; +import org.opengroup.osdu.core.common.model.http.AppException; import org.opengroup.osdu.core.common.model.http.DpsHeaders; import org.opengroup.osdu.partition.provider.interfaces.IAuthorizationService; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; import org.springframework.web.context.annotation.RequestScope; @@ -36,7 +38,16 @@ public class AuthorizationService implements IAuthorizationService { @Override public boolean isDomainAdminServiceAccount() { - return hasRole(PARTITION_ADMIN_ROLE); + try { + return hasRole(PARTITION_ADMIN_ROLE); + } + catch (AppException appE) { + throw appE; + } + catch (Exception e) { + throw new AppException(HttpStatus.INTERNAL_SERVER_ERROR.value(), "Authentication Failure", e.getMessage(), e); + } + } private boolean hasRole(String requiredRole) { diff --git a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/service/PartitionServiceImpl.java b/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/service/PartitionServiceImpl.java index d6fbb17c1..f3883d953 100644 --- a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/service/PartitionServiceImpl.java +++ b/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/service/PartitionServiceImpl.java @@ -14,6 +14,7 @@ package org.opengroup.osdu.partition.provider.aws.service; +import java.util.List; import java.util.Map; import com.amazonaws.partitions.model.Partition; @@ -63,30 +64,35 @@ public class PartitionServiceImpl implements IPartitionService { boolean partitionReady = false; while (!partitionReady && retryCount > 0) { retryCount--; - Map<String, Object> partitionCheck = ssmHelper.getPartitionSecrets(partitionId); + List<String> partitionCheck = ssmHelper.getSsmParamsPathsForPartition(partitionId); if (partitionCheck.size() == partitionInfo.getProperties().size()) partitionReady = true; else - Thread.sleep(250); + Thread.sleep(500); } + String rollbackSuccess = "Failed"; if (!partitionReady) { try { - this.deletePartition(partitionId); + ssmHelper.deletePartitionSecrets(partitionId); + rollbackSuccess = "Succeeded"; } catch (Exception e){ } - throw new AppException(HttpStatus.SC_INTERNAL_SERVER_ERROR, "Partition Creation Failed", "One or more secrets couldn't be stored within the timeout limit"); + throw new AppException(HttpStatus.SC_INTERNAL_SERVER_ERROR, "Partition Creation Failed", "One or more secrets couldn't be stored. Rollback " + rollbackSuccess); } } + catch (AppException appE) { + throw appE; + } catch (Exception e) { try { Thread.sleep(2000); //wait for any existing ssm parameters that got added to normalize - this.deletePartition(partitionId); + ssmHelper.deletePartitionSecrets(partitionId); } catch (Exception deleteE) { //if the partition didnt get created at all deletePartition will throw an exception. Eat it so we return the creation exception. diff --git a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/util/SSMHelper.java b/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/util/SSMHelper.java index a76bb6f54..9c658529e 100644 --- a/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/util/SSMHelper.java +++ b/provider/partition-aws/src/main/java/org/opengroup/osdu/partition/provider/aws/util/SSMHelper.java @@ -15,6 +15,7 @@ package org.opengroup.osdu.partition.provider.aws.util; import java.net.URI; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -23,7 +24,6 @@ import java.util.stream.Collectors; import javax.inject.Inject; import com.amazonaws.auth.AWSCredentialsProvider; - import com.amazonaws.services.simplesystemsmanagement.AWSSimpleSystemsManagement; import com.amazonaws.services.simplesystemsmanagement.AWSSimpleSystemsManagementClientBuilder; import com.amazonaws.services.simplesystemsmanagement.model.DeleteParametersRequest; @@ -74,44 +74,58 @@ public final class SSMHelper { String ssmPath = getSsmPathForPartitition(partitionName); - GetParametersByPathRequest request = new GetParametersByPathRequest() + List<Parameter> params = new ArrayList<Parameter>(); + String nextToken = null; + + do { + + GetParametersByPathRequest request = new GetParametersByPathRequest() .withPath(ssmPath) - .withRecursive(true) + .withRecursive(true) + .withNextToken(nextToken) .withWithDecryption(true); - GetParametersByPathResult result = ssmManager.getParametersByPath(request); + GetParametersByPathResult result = ssmManager.getParametersByPath(request); + nextToken = result.getNextToken(); + + if (result.getParameters().size() > 0) + params.addAll(result.getParameters()); + } + while (nextToken != null); - return result.getParameters(); + return params; } public List<String> getSsmParamsPathsForPartition(String partitionName) { - String ssmPath = getSsmPathForPartitition(partitionName); - - GetParametersByPathRequest request = new GetParametersByPathRequest() - .withPath(ssmPath) - .withRecursive(true) - .withWithDecryption(true); + List<Parameter> paramsToDelete = getSsmParamsForPartition(partitionName); - GetParametersByPathResult result = ssmManager.getParametersByPath(request); - - List<String> ssmParamNames = result.getParameters().stream().map(Parameter::getName).collect(Collectors.toList()); + List<String> ssmParamNames = paramsToDelete.stream().map(Parameter::getName).collect(Collectors.toList()); return ssmParamNames; } public boolean partitionExists(String partitionName) { - String ssmPath = getSsmPathForPartitition(partitionName); - - GetParametersByPathRequest request = new GetParametersByPathRequest() - .withPath(ssmPath) - .withRecursive(true) - .withMaxResults(1); + String ssmPath = getSsmPathForPartitition(partitionName); + String nextToken = null; - GetParametersByPathResult result = ssmManager.getParametersByPath(request); + do { + + GetParametersByPathRequest request = new GetParametersByPathRequest() + .withPath(ssmPath) + .withRecursive(true) + .withNextToken(nextToken); + + GetParametersByPathResult result = ssmManager.getParametersByPath(request); + nextToken = result.getNextToken(); + + if (result.getParameters().size() > 0) + return true; + } + while (nextToken != null); - return result.getParameters().size() > 0; + return false; } public Map<String, Object> getPartitionSecrets(String partitionName) { @@ -151,14 +165,30 @@ public final class SSMHelper { public boolean deletePartitionSecrets(String partitionName) { - List<String> ssmParamPaths = getSsmParamsPathsForPartition(partitionName); + List<String> ssmParamPaths = getSsmParamsPathsForPartition(partitionName); + + int expectedNumOfDeletedParams = ssmParamPaths.size(); + int totalDeletedParams = 0; + + while (ssmParamPaths.size() > 0) { + int subListCount = ssmParamPaths.size(); + if (subListCount > 10) + subListCount = 10; + + List<String> paramsToDelete = ssmParamPaths.subList(0, subListCount); + ssmParamPaths = ssmParamPaths.subList(subListCount, ssmParamPaths.size()); - DeleteParametersRequest request = new DeleteParametersRequest() - .withNames(ssmParamPaths); + DeleteParametersRequest request = new DeleteParametersRequest() + .withNames(paramsToDelete); - DeleteParametersResult result = ssmManager.deleteParameters(request); + DeleteParametersResult result = ssmManager.deleteParameters(request); + + totalDeletedParams += result.getDeletedParameters().size(); + } + + - return result.getDeletedParameters().size() > 0; + return totalDeletedParams == expectedNumOfDeletedParams; } diff --git a/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/api/TestGetPartitionById.java b/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/api/TestGetPartitionById.java index 48c2eb4ce..00f97e5c0 100644 --- a/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/api/TestGetPartitionById.java +++ b/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/api/TestGetPartitionById.java @@ -29,12 +29,12 @@ public class TestGetPartitionById extends GetPartitionByIdApitTest { @After @Override public void tearDown() { - try { - this.deleteResource(); - } - catch (Exception e) { + // try { + // this.deleteResource(); + // } + // catch (Exception e) { - } + // } this.testUtils = null; } } diff --git a/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/util/AwsTestUtils.java b/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/util/AwsTestUtils.java index 902225e02..7f5feabd3 100644 --- a/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/util/AwsTestUtils.java +++ b/testing/partition-test-aws/src/test/java/org/opengroup/osdu/partition/util/AwsTestUtils.java @@ -63,9 +63,7 @@ public class AwsTestUtils extends TestUtils { try { KeyPairGenerator keyGenerator = KeyPairGenerator.getInstance("RSA"); keyGenerator.initialize(2048); - - - + KeyPair kp = keyGenerator.genKeyPair(); PublicKey publicKey = (PublicKey) kp.getPublic(); PrivateKey privateKey = (PrivateKey) kp.getPrivate(); -- GitLab