Skip to content
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

chore: Add an integration test for SigV4A signing region #1852

Merged
merged 2 commits into from
Dec 27, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

import AWSS3
import AWSSDKHTTPAuth
import ClientRuntime
import Smithy
import SmithyHTTPAPI
import SmithyHTTPAuthAPI
import SmithyTestUtil
import XCTest

// Checks that S3's rules-based SigV4A operations resolve signing region to "*" using PutObject with MRAP ARN.
class SigV4ASigningRegionTest: XCTestCase {
private var sigv4aClient: S3Client!
private var sigv4aConfig: S3Client.S3ClientConfiguration!

override func setUp() async throws {
sigv4aConfig = try await S3Client.S3ClientConfiguration(region: "dummy-region")
sigv4aConfig.authSchemes = [SigV4AAuthScheme()]
sigv4aConfig.httpClientEngine = ProtocolTestClient() // Mock HTTP client that doesn't actually send a request
sigv4aConfig.addInterceptorProvider(SigningRegionAssertInterceptorProvider())
sigv4aClient = S3Client(config: sigv4aConfig)
}

func testResolvedSigningRegionIsAsterisk() async throws {
do {
// Providing input.bucket value in the format of MRAP ARN causes endpoint to resolve to MRAP endpoint with
// auth schemes property that has SigV4A.
// Then, when signing properties are resolved by the auth scheme, signing region resolved by the service's
// AuthSchemeResolver, saved in AuthOption, supersedes the one provided in config, saved in context.
_ = try await sigv4aClient.putObject(input: PutObjectInput(
body: ByteStream.data(Data()),
bucket: "arn:aws:s3::123456789012:accesspoint/mfzwi23gnjvgw.mrap", // Dummy ARN for MRAP
key: "dummy-key"
))
} catch is TestCheckError {
// This is the expected error thrown by Mock HTTP client; no-op.
}
}
}

class SigningRegionAssertInterceptor<InputType, OutputType>: Interceptor {
typealias RequestType = HTTPRequest
typealias ResponseType = HTTPResponse

func readBeforeSigning(context: some AfterSerialization<InputType, RequestType>) async throws {
let resolvedSigningRegion = context.getAttributes().selectedAuthScheme?.signingProperties?.get(
key: SigningPropertyKeys.signingRegion
)
XCTAssertNotNil(resolvedSigningRegion)
XCTAssertEqual(resolvedSigningRegion!, "*") // Assert that the resolved signing region is "*", as expected.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we either (or both) need to 1) re-write this test to use a real endpoint that resolves to 2 different regions and then make requests to both regions (as in the sev2 test code) 2) change this to a unit test since we don't actually need to make a real request to check if the resolvedSigningRegion is *

Copy link
Contributor Author

@sichanyoo sichanyoo Dec 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. There's no real endpoint that resolves to 2 different regions atm. Every endpoint for every service that uses SigV4A uses * for signingRegionSet value, including SESv2 that caused the Sev2.
  2. I think using integration test using the actual S3 operation flow is best for testing this, given doing so actually uses the typical auth flow for rules-based auth scheme resolution. Also, since it doesn't make actual network calls, there isn't a downside either for having the test.

Copy link
Contributor Author

@sichanyoo sichanyoo Dec 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion offline we concluded that maybe I could just use SESv2's multiregion (global) endpoint to test it in real-service scenario, but that was called off because all (all two of them) SESv2 API using global endpoints (sendEmail and sendBulkEmail) require involved email verification process that either requires manual verification via clicking a button in the actual email sent to the provided email address, or providing a key pair for a domain name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After offline discussion, having the test separated out like this vs. including it into S3 MRAP test is pending tie-breaker from Josh.

}
}

class SigningRegionAssertInterceptorProvider: HttpInterceptorProvider {
func create<InputType, OutputType>() -> any Interceptor<InputType, OutputType, HTTPRequest, HTTPResponse> {
return SigningRegionAssertInterceptor()
}
}
Loading