From 62883b1e2153281f9eeb82a35cac773c24dd24c1 Mon Sep 17 00:00:00 2001 From: suresh Date: Fri, 12 Jun 2026 13:59:15 -0700 Subject: [PATCH 1/2] Switch from granting broad public access to CloudFront access to Objects in the content bucket. Co-authored-by: Claude --- cicd/3-app/javabuilder/template.yml.erb | 30 ++++++++++++++++++++----- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 7ece8203..6e9b5c7a 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/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,8 +553,10 @@ 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] From f17046d8c29fe16d67814d2ab151db33bfeb46aa Mon Sep 17 00:00:00 2001 From: Molly Moen Date: Fri, 31 Jul 2026 15:28:37 -0700 Subject: [PATCH 2/2] proposed fix for theater --- cicd/3-app/javabuilder/template.yml.erb | 7 +++++-- .../org/code/javabuilder/AWSContentManager.java | 5 ++++- .../code/javabuilder/AWSContentManagerTest.java | 16 ++++++++-------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 6e9b5c7a..7d6290fe 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -559,10 +559,13 @@ Resources: 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 | -%> diff --git a/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/AWSContentManager.java b/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/AWSContentManager.java index 83139590..61f57d2e 100644 --- a/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/AWSContentManager.java +++ b/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. diff --git a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java b/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java index cf77efe3..235de9eb 100644 --- a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java +++ b/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)))