Switch Content S3 Bucket to Use CloudFront Origin Access Control v2

cicd/3-app/javabuilder/template.yml.erb

@@ -487,16 +487,32 @@ Resources:
             Status: Enabled
             ExpirationInDays: 1
 
+  ContentOriginAccessControl:
+    Type: AWS::CloudFront::OriginAccessControl
+    Properties:
+      OriginAccessControlConfig:
+        Name: !Sub "${SubdomainName}-${BaseDomainName}-content-oac"
+        OriginAccessControlOriginType: s3
+        SigningBehavior: always
+        SigningProtocol: sigv4
+
   ContentBucketPolicy:
     Type: AWS::S3::BucketPolicy
     Properties:
       Bucket: !Ref ContentBucket
       PolicyDocument:
+        Version: '2012-10-17'
         Statement:
-        - Action: ['s3:GetObject']
-          Effect: Allow
-          Resource: !Sub "arn:aws:s3:::${ContentBucket}/*"
-          Principal: '*'
+          - Sid: AllowCloudFrontRead
+            Effect: Allow
+            Principal:
+              Service: cloudfront.amazonaws.com
+            Action:
+              - s3:GetObject
+            Resource: !Sub "arn:aws:s3:::${ContentBucket}/*"
+            Condition:
+              StringEquals:
+                AWS:SourceArn: !Sub "arn:aws:cloudfront::${AWS::AccountId}:distribution/${ContentCDN}"
 
   ContentApiCertificate:
     Type: AWS::CertificateManager::Certificate
@@ -537,14 +553,19 @@ Resources:
         #   Prefix: !Sub "${SubdomainName}-content.${BaseDomainName}"
         Origins:
           - Id: ContentBucket
-            DomainName: !GetAtt ContentBucket.DomainName
-            S3OriginConfig: {}
+            DomainName: !GetAtt ContentBucket.RegionalDomainName
+            OriginAccessControlId: !GetAtt ContentOriginAccessControl.Id
+            S3OriginConfig:
+              OriginAccessIdentity: ""
         DefaultCacheBehavior:
           TargetOriginId: ContentBucket
-          AllowedMethods: [DELETE, GET, HEAD, OPTIONS, PATCH, POST, PUT]
+          # Only reads go through the CDN; prompter uploads use presigned URLs directly to S3.
+          AllowedMethods: [GET, HEAD, OPTIONS]
           Compress: true
           DefaultTTL: 0
-          ForwardedValues: {QueryString: true}
+          # Don't forward query strings: clients append cache-bust suffixes that would otherwise
+          # be included in the origin access control's SigV4 signature of the origin request.
+          ForwardedValues: {QueryString: false}
           ViewerProtocolPolicy: redirect-to-https
 
 <%JAVALAB_APP_TYPES.each do | name | -%>

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/AWSContentManager.java

@@ -118,7 +118,10 @@ public String generateAssetUploadUrl(String filename) throws JavabuilderExceptio
       this.uploads++;
       // Add the GET url for this file to the asset map so it can be referenced later.
       this.projectData.addNewAssetUrl(filename, this.contentBucketUrl + "/" + key);
-      return this.contentBucketUrl + presignedUrl.getFile();
+      // Return the raw S3 presigned URL. Uploads must go directly to S3 rather than through
+      // CloudFront, because the CloudFront origin access control signs origin requests and S3
+      // rejects requests that carry both that signature and presigned URL auth parameters.
+      return presignedUrl.toString();
     } catch (AbortedException e) {
       // this is most likely because the end user interrupted program execution. We can safely
       // ignore this.

org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java

@@ -69,19 +69,19 @@ void writesToS3() throws JavabuilderException {
   }
 
   @Test
-  public void testGetUploadUrlReturnsGeneratedUrl() throws JavabuilderException {
+  public void testGetUploadUrlReturnsGeneratedUrl() throws Exception {
     final String fileName = "file1";
     final String key = FAKE_SESSION_ID + "/" + fileName;
-    final String urlFileName = "/file/path?queryParams";
-    final URL presignedUrl = mock(URL.class);
-    when(presignedUrl.getFile()).thenReturn(urlFileName);
+    final URL presignedUrl =
+        new URL("https://" + FAKE_BUCKET_NAME + ".s3.amazonaws.com/" + key + "?queryParams");
     when(context.getRemainingTimeInMillis()).thenReturn(1000);
     when(s3ClientMock.generatePresignedUrl(
             eq(FAKE_BUCKET_NAME), eq(key), any(Date.class), eq(HttpMethod.PUT)))
         .thenReturn(presignedUrl);
 
     final String uploadUrl = contentManager.generateAssetUploadUrl(fileName);
-    assertEquals(FAKE_OUTPUT_URL + urlFileName, uploadUrl);
+    // The upload URL is the raw S3 presigned URL, not a CloudFront URL.
+    assertEquals(presignedUrl.toString(), uploadUrl);
     verify(s3ClientMock)
         .generatePresignedUrl(eq(FAKE_BUCKET_NAME), eq(key), any(Date.class), eq(HttpMethod.PUT));
     // Verify that the URL was added to the project data's asset map
@@ -90,9 +90,9 @@ public void testGetUploadUrlReturnsGeneratedUrl() throws JavabuilderException {
   }
 
   @Test
-  public void testGetUploadUrlThrowsExceptionForTooManyUploads() throws JavabuilderException {
-    final URL presignedUrl = mock(URL.class);
-    when(presignedUrl.getFile()).thenReturn("/file/path?queryParams");
+  public void testGetUploadUrlThrowsExceptionForTooManyUploads() throws Exception {
+    final URL presignedUrl =
+        new URL("https://" + FAKE_BUCKET_NAME + ".s3.amazonaws.com/file/path?queryParams");
     when(context.getRemainingTimeInMillis()).thenReturn(1000);
     when(s3ClientMock.generatePresignedUrl(
             eq(FAKE_BUCKET_NAME), anyString(), any(Date.class), eq(HttpMethod.PUT)))