diff --git a/gradle.properties b/gradle.properties index 7385599a3..9bfc28da7 100644 --- a/gradle.properties +++ b/gradle.properties @@ -3,3 +3,5 @@ org.gradle.parallel=true spinnakerGradleVersion=8.32.1 targetJava11=true includeRuntimes=actuator,core,eureka,retrofit,secrets-aws,secrets-gcp,stackdriver,swagger,tomcat,web + +org.gradle.jvmargs=-Xmx2g -Xms2g diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfiguration.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfiguration.java index e792d0c13..14ebcc73d 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfiguration.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfiguration.java @@ -19,6 +19,7 @@ import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreGetter; import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreStorer; import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreURIBuilder; +import com.netflix.spinnaker.security.UserPermissionEvaluator; import java.net.URI; import java.util.Optional; import lombok.extern.log4j.Log4j2; @@ -27,7 +28,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.security.access.PermissionEvaluator; import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; @@ -57,18 +57,18 @@ public ArtifactStoreStorer artifactStoreStorer( @Bean public ArtifactStoreGetter artifactStoreGetter( - Optional permissionEvaluator, + Optional userPermissionEvaluator, ArtifactStoreConfigurationProperties properties, @Qualifier("artifactS3Client") S3Client s3Client) { - if (permissionEvaluator.isEmpty()) { + if (userPermissionEvaluator.isEmpty()) { log.warn( - "PermissionEvaluator is not present. This means anyone will be able to access any artifact in the store."); + "UserPermissionEvaluator is not present. This means anyone will be able to access any artifact in the store."); } String bucket = properties.getS3().getBucket(); - return new S3ArtifactStoreGetter(s3Client, permissionEvaluator.orElse(null), bucket); + return new S3ArtifactStoreGetter(s3Client, userPermissionEvaluator.orElse(null), bucket); } @Bean diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetter.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetter.java index 778c46528..5c31b6b43 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetter.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetter.java @@ -23,13 +23,11 @@ import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreGetter; import com.netflix.spinnaker.kork.artifacts.model.Artifact; import com.netflix.spinnaker.security.AuthenticatedRequest; +import com.netflix.spinnaker.security.UserPermissionEvaluator; import java.util.Base64; import java.util.NoSuchElementException; import lombok.extern.log4j.Log4j2; -import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.authentication.AuthenticationServiceException; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; import software.amazon.awssdk.core.ResponseBytes; import software.amazon.awssdk.services.s3.S3Client; import software.amazon.awssdk.services.s3.model.GetObjectRequest; @@ -42,14 +40,14 @@ @Log4j2 public class S3ArtifactStoreGetter implements ArtifactStoreGetter { private final S3Client s3Client; - private final PermissionEvaluator permissionEvaluator; + private final UserPermissionEvaluator userPermissionEvaluator; private final String bucket; public S3ArtifactStoreGetter( - S3Client s3Client, PermissionEvaluator permissionEvaluator, String bucket) { + S3Client s3Client, UserPermissionEvaluator userPermissionEvaluator, String bucket) { this.s3Client = s3Client; this.bucket = bucket; - this.permissionEvaluator = permissionEvaluator; + this.userPermissionEvaluator = userPermissionEvaluator; } /** @@ -99,11 +97,11 @@ private void hasAuthorization(ArtifactReferenceURI uri, String userId) { .filter(t -> t.key().equals(ENFORCE_PERMS_KEY)) .findFirst() .orElse(null); - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); if (tag == null - || (permissionEvaluator != null - && !permissionEvaluator.hasPermission(auth, tag.value(), "application", "READ"))) { + || (userPermissionEvaluator != null + && !userPermissionEvaluator.hasPermission( + userId, tag.value(), "application", "READ"))) { log.error( "Could not authenticate to retrieve artifact user={} applicationOfStoredArtifact={}", userId, diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetterTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetterTest.java index 06bd43ee1..848f7498a 100644 --- a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetterTest.java +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetterTest.java @@ -20,7 +20,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -29,9 +28,9 @@ import com.netflix.spinnaker.kork.artifacts.model.Artifact; import com.netflix.spinnaker.kork.common.Header; import com.netflix.spinnaker.security.AuthenticatedRequest; +import com.netflix.spinnaker.security.UserPermissionEvaluator; import java.util.List; import org.junit.jupiter.api.Test; -import org.springframework.security.access.PermissionEvaluator; import software.amazon.awssdk.core.ResponseBytes; import software.amazon.awssdk.services.s3.S3Client; import software.amazon.awssdk.services.s3.model.GetObjectRequest; @@ -46,16 +45,11 @@ public class S3ArtifactStoreGetterTest { public void testGetAuthenticatedWithUser() { // Verify that having a user set is sufficient to authenticate against that // user. - // - // Currently, S3ArtifactStoreGetter.hasAuthorization uses - // SecurityContextHolder.getContext().getAuthentication(). In this test - // that's null. In orca at least, there are some calls to - // get/hasAuthorization where this is also true, but there is a user - // available in AuthenticatedRequest. // given: String application = "my-application"; - AuthenticatedRequest.set(Header.USER, "my-user"); + String user = "my-user"; + AuthenticatedRequest.set(Header.USER, user); S3Client client = mock(S3Client.class); @@ -72,20 +66,16 @@ public void testGetAuthenticatedWithUser() { when(client.getObjectTagging(any(GetObjectTaggingRequest.class))) .thenReturn(getObjectTaggingResponse); - PermissionEvaluator permissionEvaluator = mock(PermissionEvaluator.class); + UserPermissionEvaluator userPermissionEvaluator = mock(UserPermissionEvaluator.class); - // FIXME: The current behavior is to call permissionEvaluator.hasPermission - // with a null Authentication object. The correct behavior is to - // authenticate against AuthenticatedRequest.getSpinnakerUser(). - // // It's arbitrary whether to give permission or not (i.e. return true or // false). Choose true since there are then no exceptions to deal with. - when(permissionEvaluator.hasPermission( - isNull(), eq(application), eq("application"), eq("READ"))) + when(userPermissionEvaluator.hasPermission( + eq(user), eq(application), eq("application"), eq("READ"))) .thenReturn(true); S3ArtifactStoreGetter artifactStoreGetter = - new S3ArtifactStoreGetter(client, permissionEvaluator, "my-bucket"); + new S3ArtifactStoreGetter(client, userPermissionEvaluator, "my-bucket"); ArtifactReferenceURI uri = mock(ArtifactReferenceURI.class); @@ -95,9 +85,7 @@ public void testGetAuthenticatedWithUser() { // then assertThat(artifact).isNotNull(); - // FIXME: Again, the correct behavior is to authenticate against - // AuthenticatedRequest.getSpinnakerUser(). - verify(permissionEvaluator) - .hasPermission(isNull(), eq(application), eq("application"), eq("READ")); + verify(userPermissionEvaluator) + .hasPermission(eq(user), eq(application), eq("application"), eq("READ")); } }