refactor: presigner generates querystring signed requests (#388)

kggilmer · web-flow · commit 5d5746924b6b · 2021-11-10T11:58:06.000-08:00
diff --git a/aws-runtime/aws-signing/common/src/aws/sdk/kotlin/runtime/auth/signing/Presigner.kt b/aws-runtime/aws-signing/common/src/aws/sdk/kotlin/runtime/auth/signing/Presigner.kt
@@ -59,7 +59,7 @@ public enum class SigningLocation {
  * @property path HTTP path of the presigned request
  * @property queryString the HTTP querystring of the presigned request
  * @property durationSeconds Number of seconds that the request will be valid for after being signed.
- * @property hasBody Specifies if the request contains a body
+ * @property signBody Specifies if the request body should be signed
  * @property signingLocation Specifies where the signing information should be placed in the presigned request
  * @property additionalHeaders Custom headers that should be signed as part of the request
  */
@@ -68,7 +68,7 @@ public data class PresignedRequestConfig(
     public val path: String,
     public val queryString: QueryParameters = QueryParameters.Empty,
     public val durationSeconds: Long,
-    public val hasBody: Boolean = false,
+    public val signBody: Boolean = false,
     public val signingLocation: SigningLocation,
     public val additionalHeaders: Headers = Headers.Empty
 )
@@ -90,7 +90,7 @@ public suspend fun createPresignedRequest(serviceConfig: ServicePresignConfig, r
         credentials = crtCredentials
         signatureType = if (requestConfig.signingLocation == SigningLocation.HEADER) AwsSignatureType.HTTP_REQUEST_VIA_HEADERS else AwsSignatureType.HTTP_REQUEST_VIA_QUERY_PARAMS
         signedBodyHeader = AwsSignedBodyHeaderType.X_AMZ_CONTENT_SHA256
-        signedBodyValue = if (requestConfig.hasBody) AwsSignedBodyValue.UNSIGNED_PAYLOAD else null
+        signedBodyValue = if (requestConfig.signBody) null else AwsSignedBodyValue.UNSIGNED_PAYLOAD
         expirationInSeconds = requestConfig.durationSeconds
     }
 
diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/PresignerGenerator.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/PresignerGenerator.kt
@@ -2,7 +2,6 @@ package aws.sdk.kotlin.codegen
 
 import aws.sdk.kotlin.codegen.model.traits.Presignable
 import aws.sdk.kotlin.codegen.protocols.core.AwsEndpointResolverGenerator
-import aws.sdk.kotlin.codegen.protocols.core.QueryBindingResolver
 import aws.sdk.kotlin.codegen.protocols.middleware.AwsSignatureVersion4
 import software.amazon.smithy.aws.traits.auth.SigV4Trait
 import software.amazon.smithy.aws.traits.protocols.AwsQueryTrait
@@ -31,8 +30,6 @@ import software.amazon.smithy.kotlin.codegen.rendering.ClientConfigProperty
 import software.amazon.smithy.kotlin.codegen.rendering.ClientConfigPropertyType
 import software.amazon.smithy.kotlin.codegen.rendering.protocol.HttpBindingProtocolGenerator
 import software.amazon.smithy.kotlin.codegen.rendering.protocol.HttpBindingResolver
-import software.amazon.smithy.kotlin.codegen.rendering.protocol.HttpTraitResolver
-import software.amazon.smithy.kotlin.codegen.rendering.protocol.hasHttpBody
 import software.amazon.smithy.kotlin.codegen.rendering.serde.serializerName
 import software.amazon.smithy.kotlin.codegen.utils.dq
 import software.amazon.smithy.model.shapes.OperationShape
@@ -46,16 +43,13 @@ import software.amazon.smithy.model.traits.TimestampFormatTrait
  *
  * @property serviceId ID of service presigning applies to
  * @property operationId Operation capable of presigning
- * @property presignedParameterId (Optional) parameter in which presigned URL should be passed in the request
- * @property hasBody true if operation will pass an unsigned body with the request
+ * @property signBody true if the body is to be read and signed, otherwise body specified as unsigned.
  *
  */
 data class PresignableOperation(
     val serviceId: String,
     val operationId: String,
-    // TODO ~ Implementation of embedded presigned URLs is TBD
-    val presignedParameterId: String?,
-    val hasBody: Boolean,
+    val signBody: Boolean,
 )
 
 /**
@@ -103,9 +97,10 @@ class PresignerGenerator : KotlinIntegration {
             .filter { operationShape -> operationShape.hasTrait(Presignable.ID) }
             .map { operationShape ->
                 check(AwsSignatureVersion4.hasSigV4AuthScheme(ctx.model, service, operationShape)) { "Operation does not have valid auth trait" }
-                val resolver: HttpBindingResolver = getProtocolHttpBindingResolver(ctx, service)
-                val hasBody = resolver.hasHttpBody(operationShape)
-                PresignableOperation(service.id.toString(), operationShape.id.toString(), null, hasBody)
+                val protocol = requireNotNull(ctx.protocolGenerator).protocol.name
+                val shouldSignBody = signBody(protocol)
+
+                PresignableOperation(service.id.toString(), operationShape.id.toString(), shouldSignBody)
             }
 
         // If presignable operations found for this service, generate a Presigner file
@@ -116,6 +111,16 @@ class PresignerGenerator : KotlinIntegration {
         }
     }
 
+    // Determine if body should be read and signed by CRT.  If body is to be signed by CRT, null is passed to signer
+    // for signedBodyValue parameter. This causes CRT to read the body and compute the signature.
+    // Otherwise, AwsSignedBodyValue.UNSIGNED_PAYLOAD is passed specifying that the body will be ignored and CRT
+    // will not take the body into account when signing the request.
+    private fun signBody(protocol: String) =
+        when (protocol) {
+            "awsQuery" -> true // Query protocol always contains a body
+            else -> false
+        }
+
     private fun renderPresigner(
         writer: KotlinWriter,
         ctx: CodegenContext,
@@ -302,7 +307,7 @@ class PresignerGenerator : KotlinIntegration {
                     write("httpRequestBuilder.url.path,")
                     presignConfigFnVisitor.renderQueryParameters(writer)
                     write("durationSeconds.toLong(),")
-                    write("${presignableOp.hasBody},")
+                    write("${presignableOp.signBody},")
                     write("SigningLocation.HEADER")
                 }
             }
@@ -345,12 +350,4 @@ class PresignerGenerator : KotlinIntegration {
             write("return createPresignedRequest(presignConfig, $requestConfigFnName(this, durationSeconds))")
         }
     }
-
-    private fun getProtocolHttpBindingResolver(ctx: CodegenContext, service: ServiceShape): HttpBindingResolver =
-        when (requireNotNull(ctx.protocolGenerator).protocol) {
-            AwsQueryTrait.ID -> QueryBindingResolver(ctx.model, service)
-            RestJson1Trait.ID -> HttpTraitResolver(ctx.model, service, "application/json")
-            RestXmlTrait.ID -> HttpTraitResolver(ctx.model, service, "application/xml")
-            else -> throw CodegenException("Unable to create HttpBindingResolver for unhandled protocol ${ctx.protocolGenerator?.protocol}")
-        }
 }
diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/polly/PollyPresigner.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/polly/PollyPresigner.kt
@@ -62,7 +62,7 @@ class PollyPresigner : KotlinIntegration {
                 httpRequestBuilder.url.path,
                 queryStringBuilder.build(),
                 durationSeconds.toLong(),
-                false,
+                true,
                 SigningLocation.QUERY_STRING
             )
             """.trimIndent()
diff --git a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt
@@ -176,7 +176,7 @@ class PresignerGeneratorTest {
                     httpRequestBuilder.url.path,
                     httpRequestBuilder.url.parameters.build(),
                     durationSeconds.toLong(),
-                    true,
+                    false,
                     SigningLocation.HEADER
                 )
             }
@@ -214,7 +214,7 @@ class PresignerGeneratorTest {
                     httpRequestBuilder.url.path,
                     httpRequestBuilder.url.parameters.build(),
                     durationSeconds.toLong(),
-                    true,
+                    false,
                     SigningLocation.HEADER
                 )
             }
@@ -233,7 +233,7 @@ class PresignerGeneratorTest {
                 companion object {
                     @JvmStatic
                     fun fluentBuilder(): FluentBuilder = BuilderImpl()
-
+            
                     operator fun invoke(block: DslBuilder.() -> kotlin.Unit): ServicePresignConfig = BuilderImpl().apply(block).build()
                 }
             
diff --git a/services/polly/e2eTest/PollyPresignerTest.kt b/services/polly/e2eTest/PollyPresignerTest.kt
@@ -0,0 +1,40 @@
+package aws.sdk.kotlin.services.polly
+
+import aws.sdk.kotlin.runtime.http.engine.crt.CrtHttpEngine
+import aws.sdk.kotlin.services.polly.model.OutputFormat
+import aws.sdk.kotlin.services.polly.model.SynthesizeSpeechRequest
+import aws.sdk.kotlin.services.polly.model.VoiceId
+import aws.smithy.kotlin.runtime.http.response.complete
+import aws.smithy.kotlin.runtime.http.sdkHttpClient
+import aws.smithy.kotlin.runtime.testing.runSuspendTest
+import org.junit.jupiter.api.TestInstance
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+/**
+ * Tests for presigner
+ */
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class PollyPresignerTest {
+
+    @Test
+    fun clientBasedPresign() = runSuspendTest {
+        val request = SynthesizeSpeechRequest {
+            voiceId = VoiceId.Salli
+            outputFormat = OutputFormat.Pcm
+            text = "hello world"
+        }
+
+        val client = PollyClient { region = "us-east-1" }
+        val presignedRequest = request.presign(client.config, 10)
+
+        CrtHttpEngine().use { engine ->
+            val httpClient = sdkHttpClient(engine)
+
+            val call = httpClient.call(presignedRequest)
+            call.complete()
+
+            assertEquals(200, call.response.status.value)
+        }
+    }
+}
diff --git a/services/s3/e2eTest/S3IntegrationTest.kt b/services/s3/e2eTest/S3IntegrationTest.kt
@@ -6,7 +6,7 @@ package aws.sdk.kotlin.e2etest
 
 import aws.sdk.kotlin.runtime.testing.runSuspendTest
 import aws.sdk.kotlin.services.s3.S3Client
-import aws.sdk.kotlin.services.s3.model.*
+import aws.sdk.kotlin.services.s3.model.GetObjectRequest
 import aws.smithy.kotlin.runtime.content.ByteStream
 import aws.smithy.kotlin.runtime.content.decodeToString
 import aws.smithy.kotlin.runtime.content.fromFile
@@ -22,19 +22,19 @@ import kotlin.time.Duration
 import kotlin.time.ExperimentalTime
 
 /**
- * Tests for bucket operations
+ * Tests for bucket operations and presigner
  */
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class S3BucketOpsIntegrationTest {
     companion object {
         const val DEFAULT_REGION = "us-east-2"
     }
 
-    val client = S3Client {
+    private val client = S3Client {
         region = DEFAULT_REGION
     }
 
-    lateinit var testBucket: String
+    private lateinit var testBucket: String
 
     @BeforeAll
     private fun createResources(): Unit = runBlocking {
diff --git a/services/s3/e2eTest/S3PresignerTest.kt b/services/s3/e2eTest/S3PresignerTest.kt
@@ -0,0 +1,91 @@
+package aws.sdk.kotlin.e2etest
+
+import aws.sdk.kotlin.runtime.http.engine.crt.CrtHttpEngine
+import aws.sdk.kotlin.runtime.testing.runSuspendTest
+import aws.sdk.kotlin.services.s3.S3Client
+import aws.sdk.kotlin.services.s3.model.GetObjectRequest
+import aws.sdk.kotlin.services.s3.model.PutObjectRequest
+import aws.sdk.kotlin.services.s3.presign
+import aws.smithy.kotlin.runtime.content.ByteStream
+import aws.smithy.kotlin.runtime.content.decodeToString
+import aws.smithy.kotlin.runtime.http.response.complete
+import aws.smithy.kotlin.runtime.http.sdkHttpClient
+import aws.smithy.kotlin.runtime.http.toByteStream
+import kotlinx.coroutines.runBlocking
+import org.junit.jupiter.api.AfterAll
+import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.TestInstance
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class S3PresignerTest {
+    companion object {
+        const val DEFAULT_REGION = "us-east-2"
+    }
+
+    private val client = S3Client {
+        region = DEFAULT_REGION
+    }
+
+    private lateinit var testBucket: String
+
+    @BeforeAll
+    private fun createResources(): Unit = runBlocking {
+        testBucket = S3TestUtils.getTestBucket(client)
+    }
+
+    @AfterAll
+    private fun cleanup() = runBlocking {
+        S3TestUtils.deleteBucketAndAllContents(client, testBucket)
+    }
+
+    @Test
+    fun testPutObjectPresigner() = runSuspendTest {
+        val contents = "presign-test"
+        val keyName = "put-obj-from-memory-presigned.txt"
+
+        val presignedRequest = PutObjectRequest {
+            bucket = testBucket
+            key = keyName
+        }.presign(client.config, 60)
+
+        S3TestUtils.responseCodeFromPut(presignedRequest, contents)
+
+        val req = GetObjectRequest {
+            bucket = testBucket
+            key = keyName
+        }
+        val roundTrippedContents = client.getObject(req) { it.body?.decodeToString() }
+
+        assertEquals(contents, roundTrippedContents)
+    }
+
+    @Test
+    fun testGetObjectPresigner() = runSuspendTest {
+        val contents = "presign-test"
+        val keyName = "put-obj-from-memory-presigned.txt"
+
+        client.putObject {
+            bucket = testBucket
+            key = keyName
+            body = ByteStream.fromString(contents)
+        }
+
+        val presignedRequest = GetObjectRequest {
+            bucket = testBucket
+            key = keyName
+        }.presign(client.config, 60)
+
+        CrtHttpEngine().use { engine ->
+            val httpClient = sdkHttpClient(engine)
+
+            val call = httpClient.call(presignedRequest)
+            call.complete()
+
+            assertEquals(200, call.response.status.value)
+            val body = call.response.body.toByteStream()?.decodeToString()
+            assertEquals(contents, body)
+        }
+    }
+}
diff --git a/services/s3/e2eTest/S3TestUtils.kt b/services/s3/e2eTest/S3TestUtils.kt
@@ -5,13 +5,20 @@
 package aws.sdk.kotlin.e2etest
 
 import aws.sdk.kotlin.services.s3.S3Client
-import aws.sdk.kotlin.services.s3.model.*
+import aws.sdk.kotlin.services.s3.model.BucketLocationConstraint
+import aws.sdk.kotlin.services.s3.model.ExpirationStatus
+import aws.sdk.kotlin.services.s3.model.LifecycleRule
+import aws.sdk.kotlin.services.s3.model.LifecycleRuleFilter
+import aws.sdk.kotlin.services.s3.model.NotFound
+import aws.smithy.kotlin.runtime.http.request.HttpRequest
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.withTimeout
+import java.io.OutputStreamWriter
+import java.net.URL
 import java.util.*
+import javax.net.ssl.HttpsURLConnection
 import kotlin.time.Duration
 import kotlin.time.ExperimentalTime
-import kotlin.time.seconds
 
 object S3TestUtils {
 
@@ -94,4 +101,24 @@ object S3TestUtils {
             throw ex
         }
     }
+
+    fun responseCodeFromPut(presignedRequest: HttpRequest, content: String): Int {
+        val url = URL(presignedRequest.url.toString())
+        val connection: HttpsURLConnection = url.openConnection() as HttpsURLConnection
+        presignedRequest.headers.forEach { key, values ->
+            connection.setRequestProperty(key, values.first())
+        }
+
+        connection.doOutput = true
+        connection.requestMethod = "PUT"
+        val out = OutputStreamWriter(connection.outputStream)
+        out.write(content)
+        out.close()
+
+        if (connection.errorStream != null) {
+            error("request failed: ${connection.errorStream?.bufferedReader()?.readText()}")
+        }
+
+        return connection.responseCode
+    }
 }
diff --git a/services/sts/e2eTest/STSPresignerTest.kt b/services/sts/e2eTest/STSPresignerTest.kt
@@ -0,0 +1,33 @@
+package aws.sdk.kotlin.services.sts
+
+import aws.sdk.kotlin.runtime.http.engine.crt.CrtHttpEngine
+import aws.sdk.kotlin.runtime.testing.runSuspendTest
+import aws.sdk.kotlin.services.sts.model.GetCallerIdentityRequest
+import aws.smithy.kotlin.runtime.http.response.complete
+import aws.smithy.kotlin.runtime.http.sdkHttpClient
+import org.junit.jupiter.api.TestInstance
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+/**
+ * Tests for presigner
+ */
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class StsPresignerTest {
+
+    @Test
+    fun testGetCallerIdentityPresigner() = runSuspendTest {
+        val c = StsClient { region = "us-east-2" }
+        val req = GetCallerIdentityRequest { }
+        val presignedRequest = req.presign(c.config, 60)
+
+        CrtHttpEngine().use { engine ->
+            val httpClient = sdkHttpClient(engine)
+
+            val call = httpClient.call(presignedRequest)
+            call.complete()
+
+            assertEquals(200, call.response.status.value)
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ class PollyPresigner : KotlinIntegration {`
`62`	`62`	`httpRequestBuilder.url.path,`
`63`	`63`	`queryStringBuilder.build(),`
`64`	`64`	`durationSeconds.toLong(),`
`65`		`- false,`
	`65`	`+ true,`
`66`	`66`	`SigningLocation.QUERY_STRING`
`67`	`67`	`)`
`68`	`68`	`""".trimIndent()`
Original file line number	Diff line number	Diff line change
`@@ -176,7 +176,7 @@ class PresignerGeneratorTest {`
`176`	`176`	`httpRequestBuilder.url.path,`
`177`	`177`	`httpRequestBuilder.url.parameters.build(),`
`178`	`178`	`durationSeconds.toLong(),`
`179`		`- true,`
	`179`	`+ false,`
`180`	`180`	`SigningLocation.HEADER`
`181`	`181`	`)`
`182`	`182`	`}`
`@@ -214,7 +214,7 @@ class PresignerGeneratorTest {`
`214`	`214`	`httpRequestBuilder.url.path,`
`215`	`215`	`httpRequestBuilder.url.parameters.build(),`
`216`	`216`	`durationSeconds.toLong(),`
`217`		`- true,`
	`217`	`+ false,`
`218`	`218`	`SigningLocation.HEADER`
`219`	`219`	`)`
`220`	`220`	`}`
`@@ -233,7 +233,7 @@ class PresignerGeneratorTest {`
`233`	`233`	`companion object {`
`234`	`234`	`@JvmStatic`
`235`	`235`	`fun fluentBuilder(): FluentBuilder = BuilderImpl()`
`236`		`-`
	`236`	`+`
`237`	`237`	`operator fun invoke(block: DslBuilder.() -> kotlin.Unit): ServicePresignConfig = BuilderImpl().apply(block).build()`
`238`	`238`	`}`
`239`	`239`