From 7826d22d301b1a6accef2b7686700231ffba7e29 Mon Sep 17 00:00:00 2001 From: Shri Javadekar Date: Fri, 26 Aug 2016 16:03:54 -0700 Subject: [PATCH] JCLOUDS-1161: Make AWSS3BlobRequestSignerV4 the default signer. Added new live tests and fixed some unit tests accordingly. --- .../integration/S3BlobSignerLiveTest.java | 8 ++- .../internal/BaseBlobSignerLiveTest.java | 20 +++++-- .../config/AWSS3BlobStoreContextModule.java | 4 +- .../jclouds/aws/s3/AWSS3ClientLiveTest.java | 55 +++++++++++++++++++ .../blobstore/AWSS3BlobSignerExpectTest.java | 21 +++++-- 5 files changed, 94 insertions(+), 14 deletions(-) diff --git a/apis/s3/src/test/java/org/jclouds/s3/blobstore/integration/S3BlobSignerLiveTest.java b/apis/s3/src/test/java/org/jclouds/s3/blobstore/integration/S3BlobSignerLiveTest.java index 872e2d9cbe..4270b97b4e 100644 --- a/apis/s3/src/test/java/org/jclouds/s3/blobstore/integration/S3BlobSignerLiveTest.java +++ b/apis/s3/src/test/java/org/jclouds/s3/blobstore/integration/S3BlobSignerLiveTest.java @@ -52,7 +52,9 @@ public class S3BlobSignerLiveTest extends BaseBlobSignerLiveTest { @Test public void testSignGetUrlWithTimeExpired() throws InterruptedException, IOException { try { - super.testSignGetUrlWithTimeExpired(); + // Intentionally try with a timeout of 0. AWS signature v4 throws an error if + // the timeout is negative. + super.testSignGetUrlWithTime(/*timeout=*/ 0); if (!supportsUrlWithTime()) { fail(); } @@ -76,7 +78,9 @@ public class S3BlobSignerLiveTest extends BaseBlobSignerLiveTest { @Test public void testSignPutUrlWithTimeExpired() throws Exception { try { - super.testSignPutUrlWithTimeExpired(); + // Intentionally try with a timeout of 0. AWS signature v4 throws an error if + // the timeout is negative. + super.testSignPutUrlWithTime(/*timeout=*/ 0); if (!supportsUrlWithTime()) { fail(); } diff --git a/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobSignerLiveTest.java b/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobSignerLiveTest.java index 8d5f5ea2a4..8ec0434d17 100644 --- a/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobSignerLiveTest.java +++ b/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobSignerLiveTest.java @@ -98,8 +98,7 @@ public class BaseBlobSignerLiveTest extends BaseBlobStoreIntegrationTest { } } - @Test - public void testSignGetUrlWithTimeExpired() throws InterruptedException, IOException { + public void testSignGetUrlWithTime(final long timeout) throws InterruptedException, IOException { String name = "hello"; String text = "fooooooooooooooooooooooo"; @@ -108,7 +107,7 @@ public class BaseBlobSignerLiveTest extends BaseBlobStoreIntegrationTest { try { view.getBlobStore().putBlob(container, blob); assertConsistencyAwareContainerSize(container, 1); - HttpRequest request = view.getSigner().signGetBlob(container, name, -getSignedUrlTimeout()); + HttpRequest request = view.getSigner().signGetBlob(container, name, timeout); assertEquals(request.getFilters().size(), 0); try { @@ -121,6 +120,11 @@ public class BaseBlobSignerLiveTest extends BaseBlobStoreIntegrationTest { } } + @Test + public void testSignGetUrlWithTimeExpired() throws InterruptedException, IOException { + testSignGetUrlWithTime(-getSignedUrlTimeout()); + } + @Test public void testSignPutUrl() throws Exception { String name = "hello"; @@ -159,15 +163,14 @@ public class BaseBlobSignerLiveTest extends BaseBlobStoreIntegrationTest { } } - @Test - public void testSignPutUrlWithTimeExpired() throws Exception { + public void testSignPutUrlWithTime(final long timeout) throws InterruptedException, IOException { String name = "hello"; String text = "fooooooooooooooooooooooo"; Blob blob = view.getBlobStore().blobBuilder(name).payload(text).contentType("text/plain").build(); String container = getContainerName(); try { - HttpRequest request = view.getSigner().signPutBlob(container, blob, -getSignedUrlTimeout()); + HttpRequest request = view.getSigner().signPutBlob(container, blob, 0); assertEquals(request.getFilters().size(), 0); // Strip Expect: 100-continue to make actual responses visible, since @@ -185,6 +188,11 @@ public class BaseBlobSignerLiveTest extends BaseBlobStoreIntegrationTest { } } + @Test + public void testSignPutUrlWithTimeExpired() throws Exception { + testSignPutUrlWithTime(-getSignedUrlTimeout()); + } + @Test public void testSignRemoveUrl() throws Exception { String name = "hello"; diff --git a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/config/AWSS3BlobStoreContextModule.java b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/config/AWSS3BlobStoreContextModule.java index 6c551d5d71..79ea8c70e5 100644 --- a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/config/AWSS3BlobStoreContextModule.java +++ b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/config/AWSS3BlobStoreContextModule.java @@ -16,7 +16,7 @@ */ package org.jclouds.aws.s3.blobstore.config; -import org.jclouds.aws.s3.blobstore.AWSS3BlobRequestSigner; +import org.jclouds.aws.s3.blobstore.AWSS3BlobRequestSignerV4; import org.jclouds.aws.s3.blobstore.AWSS3BlobStore; import org.jclouds.blobstore.BlobRequestSigner; import org.jclouds.s3.blobstore.S3BlobStore; @@ -34,6 +34,6 @@ public class AWSS3BlobStoreContextModule extends S3BlobStoreContextModule { @Override protected void bindRequestSigner() { - bind(BlobRequestSigner.class).to(AWSS3BlobRequestSigner.class); + bind(BlobRequestSigner.class).to(AWSS3BlobRequestSignerV4.class); } } diff --git a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java index 8c898f609b..ecdcdab8b1 100644 --- a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java +++ b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java @@ -22,12 +22,17 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.fail; +import com.google.common.collect.Iterables; import org.jclouds.aws.AWSResponseException; import org.jclouds.aws.domain.Region; import org.jclouds.blobstore.BlobStore; import org.jclouds.blobstore.domain.Blob; import org.jclouds.blobstore.domain.StorageMetadata; import org.jclouds.domain.Location; +import org.jclouds.http.HttpRequest; +import org.jclouds.http.HttpResponse; +import org.jclouds.location.predicates.LocationPredicates; +import org.jclouds.rest.HttpClient; import org.jclouds.s3.S3Client; import org.jclouds.s3.S3ClientLiveTest; import org.jclouds.s3.domain.ListBucketResponse; @@ -154,4 +159,54 @@ public class AWSS3ClientLiveTest extends S3ClientLiveTest { returnContainer(containerName); } } + + /** + * Test signed get/put operations using signature v4. This is done by explicitly + * using the "eu-central-1" region which only support signature v4. + */ + public void testV4SignatureOps() throws InterruptedException { + String containerName = getContainerName(); + try { + BlobStore blobStore = view.getBlobStore(); + Location location = Iterables.tryFind(blobStore.listAssignableLocations(), + LocationPredicates.idEquals(Region.EU_CENTRAL_1)).orNull(); + assertNotNull(location); + blobStore.createContainerInLocation(location, containerName); + + final HttpClient client = view.utils().http(); + String blobName = "test-blob"; + Blob blob = blobStore.blobBuilder(blobName).payload("something").build(); + + // Signed put, no timeout. + HttpRequest request = view.getSigner().signPutBlob(containerName, blob); + assertNotNull(request); + HttpResponse response = client.invoke(request); + assertEquals(response.getStatusCode(), 200); + + // Signed get, no timeout. + request = view.getSigner().signGetBlob(containerName, blobName); + assertNotNull(request); + response = client.invoke(request); + assertEquals(response.getStatusCode(), 200); + + blobStore.removeBlob(containerName, blobName); + + // Signed put with timeout. + request = view.getSigner().signPutBlob(containerName, blob, /*seconds=*/ 60); + assertNotNull(request); + response = client.invoke(request); + assertEquals(response.getStatusCode(), 200); + + // Signed get with timeout. + request = view.getSigner().signGetBlob(containerName, blobName, /*seconds=*/ 60); + assertNotNull(request); + response = client.invoke(request); + assertEquals(response.getStatusCode(), 200); + + // Cleanup the container. + blobStore.removeBlob(containerName, blobName); + } finally { + returnContainer(containerName); + } + } } diff --git a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java index dfe4c7a01b..6f417845f4 100644 --- a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java +++ b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java @@ -18,6 +18,8 @@ package org.jclouds.aws.s3.blobstore; import static org.testng.Assert.assertEquals; +import java.util.Map; + import org.jclouds.aws.s3.config.AWSS3HttpApiModule; import org.jclouds.aws.s3.filters.AWSRequestAuthorizeSignature; import org.jclouds.blobstore.BlobStore; @@ -29,6 +31,7 @@ import org.jclouds.s3.blobstore.S3BlobSignerExpectTest; import org.jclouds.s3.filters.RequestAuthorizeSignature; import org.testng.annotations.Test; +import com.google.common.base.Splitter; import com.google.common.base.Supplier; import com.google.inject.Module; import com.google.inject.Scopes; @@ -72,12 +75,21 @@ public class AWSS3BlobSignerExpectTest extends S3BlobSignerExpectTest { .addHeader("Authorization", "AWS identity:0uvBv1wEskuhFHYJF/L6kEV9A7o=").build(); } + private void compareRequestComponents(final HttpRequest request, final HttpRequest compare) { + assertEquals(request.getMethod(), compare.getMethod()); + String query = request.getEndpoint().toString().split("\\?")[1]; + final Map params = Splitter.on('&').trimResults().withKeyValueSeparator("=").split(query); + assertEquals(params.get("X-Amz-Algorithm"), "AWS4-HMAC-SHA256"); + assertEquals(params.get("X-Amz-Expires"), "3"); + assertEquals(params.get("X-Amz-SignedHeaders"), "host"); + } + @Test public void testSignGetBlobWithTime() { BlobStore getBlobWithTime = requestsSendResponses(init()); HttpRequest compare = getBlobWithTime(); - assertEquals(getBlobWithTime.getContext().getSigner().signGetBlob(container, name, 3l /* seconds */), - compare); + HttpRequest request = getBlobWithTime.getContext().getSigner().signGetBlob(container, name, 3l /* seconds */); + compareRequestComponents(request, compare); } @Override @@ -118,8 +130,9 @@ public class AWSS3BlobSignerExpectTest extends S3BlobSignerExpectTest { Blob blob = signPutBloblWithTime.blobBuilder(name).payload(text).contentType("text/plain").build(); HttpRequest compare = putBlobWithTime(); compare.setPayload(blob.getPayload()); - assertEquals(signPutBloblWithTime.getContext().getSigner().signPutBlob(container, blob, 3l /* seconds */), - compare); + HttpRequest request = signPutBloblWithTime.getContext().getSigner().signPutBlob(container, blob, 3l /* seconds */); + compareRequestComponents(request, compare); + assertEquals(request.getPayload(), compare.getPayload()); } @Override