-
Notifications
You must be signed in to change notification settings - Fork 3
feat: sigva #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: sigva #30
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,4 +19,4 @@ kotestVersion=4.6.2 | |
| kotlinxCliVersion=0.3.2 | ||
|
|
||
| # JVM | ||
| crtJavaVersion=0.14.2 | ||
| crtJavaVersion=0.14.7 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package aws.sdk.kotlin.crt.auth.signing | ||
|
|
||
| import aws.sdk.kotlin.crt.http.HttpRequest | ||
|
|
||
| /** | ||
| * Wrapper that holds signing-related output. Depending on the signing configuration, not all members may be | ||
| * assigned and some members, like signature, may have a variable format. | ||
| */ | ||
| public data class AwsSigningResult( | ||
| /** | ||
| * The signed HTTP request from the result, may be null if an http request was not signed | ||
| */ | ||
| val signedRequest: HttpRequest?, | ||
|
|
||
| /** | ||
| * Gets the signature value from the result. Depending on the requested signature type and algorithm, this value | ||
| * will be in one of the following formats: | ||
| * | ||
| * (1) HTTP_REQUEST_VIA_HEADERS - hex encoding of the binary signature value | ||
| * (2) HTTP_REQUEST_VIA_QUERY_PARAMS - hex encoding of the binary signature value | ||
| * (3) HTTP_REQUEST_CHUNK/SIGV4 - hex encoding of the binary signature value | ||
| * (4) HTTP_REQUEST_CHUNK/SIGV4_ASYMMETRIC - '*'-padded hex encoding of the binary signature value | ||
| * (5) HTTP_REQUEST_EVENT - binary signature value (NYI) | ||
| * | ||
| * @return the signature value from the signing process | ||
| */ | ||
| val signature: ByteArray | ||
| ) { | ||
|
|
||
| override fun equals(other: Any?): Boolean { | ||
| if (this === other) return true | ||
| if (other == null || this::class != other::class) return false | ||
|
|
||
| other as AwsSigningResult | ||
|
|
||
| if (signedRequest != other.signedRequest) return false | ||
| if (!signature.contentEquals(other.signature)) return false | ||
|
|
||
| return true | ||
| } | ||
|
|
||
| override fun hashCode(): Int { | ||
| var result = signedRequest?.hashCode() ?: 0 | ||
| result = 31 * result + signature.contentHashCode() | ||
| return result | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,15 +19,35 @@ import software.amazon.awssdk.crt.auth.signing.AwsSigningConfig.AwsSignatureType | |
| import software.amazon.awssdk.crt.auth.signing.AwsSigningConfig.AwsSignedBodyHeaderType as AwsSignedBodyHeaderTypeJni | ||
| import software.amazon.awssdk.crt.auth.signing.AwsSigningConfig.AwsSigningAlgorithm as AwsSigningAlgorithmJni | ||
|
|
||
| /** | ||
| * Static class for a variety of AWS signing APIs. | ||
| */ | ||
| public actual object AwsSigner { | ||
| public actual suspend fun signRequest(request: HttpRequest, config: AwsSigningConfig): HttpRequest { | ||
|
|
||
| /** | ||
| * Signs an http request according to the supplied signing configuration | ||
| * @param request http request to sign | ||
| * @param config signing configuration | ||
| * @return signed request | ||
| */ | ||
| public actual suspend fun signRequest(request: HttpRequest, config: AwsSigningConfig): HttpRequest = | ||
| checkNotNull(sign(request, config).signedRequest) { "AwsSigningResult request must not be null" } | ||
|
Comment on lines
+33
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: What circumstances could lead to the signing result's request being null? Looking through the code I can't see how that would occur.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on the comments from crt-java: I think it's due to the flexibility in the signing implementation but in practice it shouldn't be null the way we use it. Or at least that's my understanding. |
||
|
|
||
| /** | ||
| * Signs an http request according to the supplied signing configuration | ||
| * @param request http request to sign | ||
| * @param config signing configuration | ||
| * @return signing result, which provides access to all signing-related result properties | ||
| */ | ||
| public actual suspend fun sign(request: HttpRequest, config: AwsSigningConfig): AwsSigningResult { | ||
| // FIXME - this would be a good area where talking directly to JNI would be convenient so we don't have to | ||
| // do [KotlinHttpReq -> CrtJava -> Native] and back | ||
| val jniReq = request.into() | ||
| return asyncCrtJniCall { | ||
| val reqFuture = AwsSignerJni.signRequest(jniReq, config.into()) | ||
| val signedJniReq = reqFuture.await() | ||
| HttpRequest.from(signedJniReq) | ||
| val reqFuture = AwsSignerJni.sign(jniReq, config.into()) | ||
| val jniResult = reqFuture.await() | ||
| val signedRequest = HttpRequest.from(jniResult.signedRequest) | ||
| AwsSigningResult(signedRequest, jniResult.signature) | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Why are these custom
equals/hashCodeimplementations necessary? They seem identical to what I'd expect from the default implementations in a data class.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh because Kotlin's data classes don't handle arrays as expected. Nevermind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^ yup that's why